Skip to content

seclog: strip trailing whitespace from audit netlink message payload - #17384

Merged
sergiocazzolato merged 1 commit into
canonical:masterfrom
bboozzoo:bboozzoo/seclog-trailing-audit-newline
Jul 24, 2026
Merged

seclog: strip trailing whitespace from audit netlink message payload#17384
sergiocazzolato merged 1 commit into
canonical:masterfrom
bboozzoo:bboozzoo/seclog-trailing-audit-newline

Conversation

@bboozzoo

Copy link
Copy Markdown
Contributor

Should the payload be constructed using a logger that appends a newline (e.g. slog), the newline would be embedded verbatim in the netlink message and appear in journald output as a trailing newline inside the quoted message field (LP: #2160691).

Strip all trailing whitespace in AuditWriter.Write before building the netlink message. The returned byte count still reflects the original input length to satisfy the io.Writer contract.

Related: SNAPDENG-37246
Fixes: LP#2160691

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 updates seclog’s Linux audit netlink writer to strip trailing whitespace from the payload before constructing/sending the AUDIT_TRUSTED_APP message, preventing embedded newlines (e.g. from newline-appending loggers) from showing up in journald output.

Changes:

  • Trim trailing \t\r\n from AuditWriter.Write payloads while still returning the original input length to satisfy io.Writer expectations for transformed writers.
  • Update AuditWriter.Write doc comment to clarify message expectations and trimming behavior.
  • Add a unit test covering trailing-whitespace stripping behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
seclog/audit_linux.go Strips trailing whitespace from the payload before building/sending the netlink audit message, while preserving the reported byte count.
seclog/audit_linux_test.go Adds coverage to ensure Write removes trailing whitespace and still reports the original input length.

Comment on lines +261 to +285
func (s *AuditSuite) TestWriteStripsTrailingWhitespace(c *C) {
for _, tc := range []struct {
input string
want string
}{
{"{\"foo\":\"bar\"}\n", "{\"foo\":\"bar\"}"},
{"{\"foo\":\"bar\"} \t\r\n", "{\"foo\":\"bar\"}"},
{"{\"foo\":\"bar\"}", "{\"foo\":\"bar\"}"},
} {
mock := &mockSyscallOps{socketFD: 7}
restore := seclog.MockSyscallOps(mock)
defer restore()

writer, err := seclog.OpenAuditWriter()
c.Assert(err, IsNil)

n, err := writer.Write([]byte(tc.input))
c.Assert(err, IsNil)
c.Check(n, Equals, len(tc.input))

payload := mock.sendtoData[syscall.SizeofNlMsghdr:]
c.Check(string(payload[:len(tc.want)]), Equals, tc.want)
c.Check(payload[len(tc.want)], Equals, byte(0))
}
}

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.

I think copilot's suggestion makes sense

@maykathm maykathm 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!

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.83%. Comparing base (17deb50) to head (5e3f1ea).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #17384      +/-   ##
==========================================
- Coverage   78.93%   78.83%   -0.10%     
==========================================
  Files        1390     1408      +18     
  Lines      196047   196784     +737     
  Branches     2462     2462              
==========================================
+ Hits       154740   155143     +403     
- Misses      32024    32298     +274     
- Partials     9283     9343      +60     
Flag Coverage Δ
unittests 78.83% <100.00%> (-0.10%) ⬇️

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

☔ View full report in Codecov by Harness.
📢 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.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

Fri Jul 24 14:36:42 UTC 2026
The following results are from: https://github.com/canonical/snapd/actions/runs/30080062935

Failures:

Preparing:

  • openstack:opensuse-tumbleweed-64:
  • openstack:opensuse-tumbleweed-64:
  • openstack:opensuse-tumbleweed-64:
  • openstack:opensuse-tumbleweed-64:
  • openstack-arm:ubuntu-24.04-arm-64:tests/main/snap-seccomp-blocks-certain-mknod
  • openstack-arm:ubuntu-24.04-arm-64:tests/main/snap-seccomp-blocks-certain-creat
  • openstack:ubuntu-26.10-64:tests/main/lxd-postrm-purge
  • openstack:ubuntu-26.10-64:tests/main/interfaces-posix-mq

Executing:

  • openstack:ubuntu-core-24-64:tests/main/snap-debug-raa
  • openstack:ubuntu-26.10-64:tests/main/lxd
  • openstack:ubuntu-26.10-64:tests/main/bad-interfaces-warn
  • openstack:ubuntu-26.10-64:tests/main/xdg-settings
  • openstack:ubuntu-26.10-64:tests/main/broken-seeding
  • openstack:ubuntu-26.10-64:tests/main/apparmor-prompting-integration-tests:write_read_multiple_actioned_by_other_pid_deny_allow
  • openstack:ubuntu-26.10-64:tests/main/apparmor-prompting-integration-tests:write_read_multiple_actioned_by_other_pid_allow_deny

Restoring:

  • openstack:opensuse-tumbleweed-64:
  • openstack:opensuse-tumbleweed-64:
  • openstack:opensuse-tumbleweed-64:
  • openstack:opensuse-tumbleweed-64:
  • openstack:ubuntu-core-24-64:tests/main/snap-debug-raa
  • openstack:ubuntu-26.10-64:tests/main/apparmor-prompting-integration-tests:write_read_multiple_actioned_by_other_pid_deny_allow
  • openstack:ubuntu-26.10-64:tests/main/
  • openstack:ubuntu-26.10-64:
  • openstack:ubuntu-26.10-64:tests/main/apparmor-prompting-integration-tests:write_read_multiple_actioned_by_other_pid_allow_deny
  • openstack:ubuntu-26.10-64:tests/main/
  • openstack:ubuntu-26.10-64:

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 of the below tests you wish to run (unskip plus test list must be valid yaml)

  • openstack-arm:ubuntu-24.04-arm-64:tests/main/i18n
  • openstack-arm:ubuntu-core-24-arm-64:tests/main/i18n
  • openstack:debian-sid-64:tests/main/interfaces-network-status-classic
  • openstack:debian-sid-64:tests/main/interfaces-xdg-portal-permission-store
  • openstack:ubuntu-24.04-64:tests/main/apparmor-prompting-flag-restart
  • openstack:ubuntu-24.04-64:tests/main/apparmor-prompting-integration-tests
  • openstack:ubuntu-24.04-64:tests/main/apparmor-prompting-prompt-restoration
  • openstack:ubuntu-24.04-64:tests/main/apparmor-prompting-smoke
  • openstack:ubuntu-24.04-64:tests/main/apparmor-prompting-snapd-startup
  • openstack:ubuntu-24.04-64:tests/main/apparmor-prompting-support
  • openstack:ubuntu-24.04-64:tests/main/i18n
  • openstack:ubuntu-24.04-64:tests/main/interfaces-requests-activates-handlers
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-flag-restart
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-integration-tests
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-prompt-restoration
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-smoke
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-snapd-startup
  • openstack:ubuntu-26.04-64:tests/main/apparmor-prompting-support
  • openstack:ubuntu-26.04-64:tests/main/i18n
  • openstack:ubuntu-26.04-64:tests/main/interfaces-requests-activates-handlers

@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.

thx

@bboozzoo bboozzoo added the Auto rerun spread Auto reruns spread up to 4 times in non-draft PRs w/ >=1 approval and <20 fails in any fund. system label Jul 20, 2026
@sergiocazzolato
sergiocazzolato force-pushed the bboozzoo/seclog-trailing-audit-newline branch from e7c1610 to 7904eb2 Compare July 21, 2026 15:56
@bboozzoo bboozzoo added the cross-distro Runs all spread systems in parallel label Jul 23, 2026
@bboozzoo bboozzoo closed this Jul 23, 2026
@bboozzoo bboozzoo reopened this Jul 23, 2026

@zyga zyga 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

Should the payload be constructed using a logger that appends a newline
(e.g. slog), the newline would be embedded verbatim in the netlink
message and appear in journald output as a trailing newline inside the
quoted message field (LP: #2160691).

Strip all trailing whitespace in AuditWriter.Write before building the
netlink message. The returned byte count still reflects the original
input length to satisfy the io.Writer contract.

Related: SNAPDENG-37246
Fixes: LP#2160691

Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
@sergiocazzolato
sergiocazzolato force-pushed the bboozzoo/seclog-trailing-audit-newline branch from 7904eb2 to 5e3f1ea Compare July 24, 2026 01:36
@bboozzoo bboozzoo closed this Jul 24, 2026
@bboozzoo bboozzoo reopened this Jul 24, 2026
@sergiocazzolato

Copy link
Copy Markdown
Contributor

Just openstack:ubuntu-core-24-64:tests/main/snap-debug-raa failing on required systems but this is also failing on othe prs.

@sergiocazzolato
sergiocazzolato merged commit b373311 into canonical:master Jul 24, 2026
898 of 958 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto rerun spread Auto reruns spread up to 4 times in non-draft PRs w/ >=1 approval and <20 fails in any fund. system cross-distro Runs all spread systems in parallel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants