Skip to content

seclog: add seclog with nop implementation (part 1) - #16987

Merged
ernestl merged 3 commits into
canonical:masterfrom
ernestl:ernestl/seclog-part1
Apr 30, 2026
Merged

seclog: add seclog with nop implementation (part 1)#16987
ernestl merged 3 commits into
canonical:masterfrom
ernestl:ernestl/seclog-part1

Conversation

@ernestl

@ernestl ernestl commented Apr 28, 2026

Copy link
Copy Markdown
Member

Implement security logging: Part 1

This PR introduces a dedicated security/audit logging subsystem:

  • Stable schema enforced by the package +slog implementation. Can add event version when required without breaking compat.
  • Generic interface between seclog API and implementations that can easily be used with typical slog alternatives.
  • Snapd emits security audit events into the system audit pipeline rather than implementing persistence/rotation policy (handled by auditd/journald configuration). Events are sent fire-and-forget (no netlink ACK). We can implement ACK in the future.
  • Deals seemlessly with absence of slog in go < 1.21, both with implementation and tests. Tests uses very mock implementation to avoid tying to slog specifically.

See complete implementation for reference: https://github.com/ernestl/snapd/pull/5/changes

Jira: https://warthogs.atlassian.net/browse/SNAPDENG-35756
Spec: https://docs.google.com/document/d/1fl7Gy3GJVu8VwUtT1_LWwZ7DEAW9sUMAzF3XGrcZ_d4/edit?tab=t.0

@github-actions github-actions Bot added the Run only one system Only runs spread tests on one system label Apr 28, 2026
@ernestl ernestl changed the title add seclog with nop implementation seclog: add seclog with nop implementation (part 1) Apr 28, 2026
@ernestl ernestl added this to the 2.76 milestone Apr 28, 2026
@ernestl
ernestl requested review from bboozzoo and Copilot April 28, 2026 07:31
@ernestl ernestl added the Needs Samuele review Needs a review from Samuele before it can land label Apr 28, 2026
@ernestl
ernestl requested a review from pedronis April 28, 2026 07:31
@ernestl ernestl closed this Apr 28, 2026
@ernestl ernestl reopened this Apr 28, 2026
@github-actions

github-actions Bot commented Apr 28, 2026

Copy link
Copy Markdown

Wed Apr 29 20:57:22 UTC 2026
The following results are from: https://github.com/canonical/snapd/actions/runs/25129136338

Failures:

Preparing:

  • openstack:ubuntu-20.04-64:tests/main/nss-modules:winbind
  • openstack:ubuntu-24.04-64:tests/main/interfaces-block-devices-zfs

Executing:

  • openstack:debian-12-64:tests/main/cgroup-devices-v2
  • openstack:ubuntu-24.04-64:tests/main/layout-change

Restoring:

  • openstack:ubuntu-24.04-64:tests/main/interfaces-block-devices-zfs
  • openstack:ubuntu-24.04-64:tests/main/
  • openstack:ubuntu-24.04-64:
  • openstack:ubuntu-24.04-64:tests/main/nss-modules:winbind

Skipped tests from snapd-testing-skip

If you wish to have any of the below tests run in your PR, in your PR description, add 'unskip:' followed by a copy-and-pasted list (without variants) of the below tests you wish to run (unskip plus test list must be valid yaml)

  • openstack:ubuntu-24.04-64:tests/main/i18n

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces the initial seclog (security/audit logging) package with a global logger API, a no-op default implementation, and test helpers to validate emitted events without depending on slog/JSON. This fits into snapd as a foundational subsystem for structured security event emission, intended to later plug into the system audit pipeline.

Changes:

  • Add seclog core types and global logging entrypoints (levels, events, attrs, setup, and a few initial AUTHN/SYS events).
  • Add a no-op SecurityLogger implementation as the default backend.
  • Add test helpers (seclogtest) plus initial unit tests for formatting and logger replacement behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
seclog/seclog.go Core security logging API: level formatting, event/attr types, global logger setup, and initial event emitters.
seclog/nop.go Default no-op SecurityLogger implementation.
seclog/seclog_test.go Unit tests for level/user/reason formatting and basic global logger setup/replacement behavior.
seclog/nop_test.go Unit tests ensuring the no-op logger accepts calls without error.
seclog/seclogtest/seclogtest.go Test helper SecurityLogger that records events to a buffer.

Comment thread seclog/seclogtest/seclogtest.go Outdated
Comment thread seclog/seclog.go Outdated
Comment thread seclog/seclog.go
@ernestl
ernestl marked this pull request as ready for review April 28, 2026 07:35
@github-actions github-actions Bot removed the Run only one system Only runs spread tests on one system label Apr 28, 2026
@codecov

codecov Bot commented Apr 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.13861% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.11%. Comparing base (c42c483) to head (a2956c6).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
seclog/seclogtest/seclogtest.go 0.00% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #16987      +/-   ##
==========================================
+ Coverage   79.05%   79.11%   +0.06%     
==========================================
  Files        1371     1362       -9     
  Lines      190706   190551     -155     
  Branches     2465     2465              
==========================================
+ Hits       150757   150759       +2     
+ Misses      30864    30723     -141     
+ Partials     9085     9069      -16     
Flag Coverage Δ
unittests 79.11% <86.13%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread seclog/nop.go
Comment thread seclog/seclog.go Outdated
Comment thread seclog/seclog.go Outdated
Comment thread seclog/seclog.go
Comment thread seclog/seclog.go
Comment thread seclog/seclog.go
@ernestl
ernestl requested a review from bboozzoo April 28, 2026 13:02

@bboozzoo bboozzoo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bboozzoo
bboozzoo requested a review from miguelpires April 28, 2026 13:07

@miguelpires miguelpires left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some questions/small notes

Comment thread seclog/seclog.go Outdated
Comment thread seclog/seclog.go
Comment thread seclog/seclog.go
Comment thread seclog/seclog_test.go Outdated
Comment thread seclog/seclogtest/seclogtest.go Outdated

@pedronis pedronis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bunch of comments and questions

Comment thread seclog/nop.go
Comment thread seclog/seclogtest/seclogtest.go Outdated
Comment thread seclog/seclog.go Outdated
Comment thread seclog/seclog.go
Comment thread seclog/seclog.go Outdated
Comment thread seclog/seclog.go
@ernestl
ernestl requested a review from pedronis April 29, 2026 19:23

@pedronis pedronis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@ernestl
ernestl merged commit f7187e4 into canonical:master Apr 30, 2026
87 of 92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Samuele review Needs a review from Samuele before it can land

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants