Skip to content

fix: log the full request target in Rust audit and unlink the socket when chown fails (#55) - #72

Merged
abienkowski merged 2 commits into
mainfrom
fix/rust-audit-socket-55
Oct 11, 2026
Merged

abienkowski merged 2 commits into
mainfrom
fix/rust-audit-socket-55

Conversation

@abienkowski

Copy link
Copy Markdown
Collaborator

Description

Part of #55 (Task 4 of the plan: the Rust audit uri and the socket left behind after a failed chown).

What changes

  • Rust audit uri is the raw request target (path plus query, escapes kept), for allowed and denied requests. It used to be the path only, so ?force=1 or ?all=1 was missing from the audit trail. Go (r.RequestURI) and TS (req.url) already logged the full target; routing still uses the path alone.
  • Rust unlinks the socket when chown or chmod fails. bind_unix_listener left the freshly bound socket on disk (mode 0600) when setting its group or mode failed. Go (l.Close()) and TS (server.close()) already removed it. The chown and chmod steps move into set_socket_ownership_and_mode; on error, Rust drops the listener, removes the file (ignoring a removal error), and returns the unchanged error.
  • Tests, in all three:
    • TestHandlerAuditURIIsRequestTarget / test_handler_audit_uri_is_request_target / "logs the raw request target as the audit uri": GET /containers/json?all=1 (ALLOW) and DELETE /containers/a%2Fb?force=1 (DENY); asserts the exact uri and decision of each line.
    • Each chown-EPERM test now also asserts the socket path is gone after the error.

Review notes

  • Release: merging cuts v0.3.5 (patch). Simulated squash: bump=patch tag=v0.3.5.
  • Audit logs now include query strings in Rust, as in Go and TS. The Docker API carries no credentials in query strings.
  • Only the chown failure is tested; a chmod failure on a just-bound socket is not practical to induce, and it runs through the same cleanup branch.
  • Counts in AGENTS.md/README.md are updated in Task 6, as planned.

Verification

  • Test-first, at 56540e6 (tests, no fix): Rust failed both new assertions, Go and TS passed (pinning their existing behaviour):
    test handler::tests::test_handler_audit_uri_is_request_target ... FAILED
    test tests::test_bind_chown_eperm_names_group ... FAILED
    socket left on disk after chown failed: symlink_metadata = Ok(Metadata { … mode: 0o140600 … })
    test result: FAILED. 145 passed; 2 failed
    
  • This branch: make test-all: Go ok (109 tests), Rust 147 passed, TS 164 tests, 0 fail, 1 skipped. make lint-all passes. Rust integration: ALL 45 TESTS PASSED.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Implementation(s) changed

  • Go (tests only)
  • Rust
  • TypeScript (tests only)
  • Quint specification
  • CI / infrastructure

Testing

  • Unit tests pass (make test-all)
  • Integration tests pass (make test-integration-rs: 45; Go and TS code unchanged, CI runs all three)
  • Quint verification: not affected
  • New tests added for the change

Checklist

  • I have read CONTRIBUTING.md
  • My code follows the project's coding style
  • I have updated documentation as needed

@abienkowski abienkowski added Priority: P3 Added to issues and PRs relating to a low severity bugs. Type: Bug Added to issues and PRs if they are addressing a bug labels Oct 11, 2026
@abienkowski
abienkowski merged commit 32c8463 into main Oct 11, 2026
7 checks passed
@abienkowski
abienkowski deleted the fix/rust-audit-socket-55 branch October 11, 2026 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: P3 Added to issues and PRs relating to a low severity bugs. Type: Bug Added to issues and PRs if they are addressing a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant