Stop recording absolute paths in the audit trail - #400
Merged
vpetersson merged 3 commits intoSep 1, 2026
Merged
Conversation
The audit trail is a compliance artifact: it exists to be handed to someone other than whoever generated it. The absolute path of the input lockfile tells that reader nothing they can use, while publishing the generating machine's directory layout and username: # Input: /private/tmp/claude-501/-Users-<username>-PycharmProjects-sbomify/.../requirements.txt Record a path under the working directory relative to it, and keep only the file name for anything outside. os.path.relpath is the wrong tool for the second case -- "../../../Users/<username>/..." leaks exactly what we are removing. Values that are not paths (docker:..., the additional-packages-only sentinel) are not absolute, so they pass through untouched. Both emitters are covered: the audit_trail.txt file and the copy printed to stdout for attestation. Follow-up to sbomify#393, which ignored the file but deliberately left the question of what it records open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the audit trail emitters to avoid recording absolute filesystem paths for Input/Output, reducing accidental disclosure of local usernames and directory layouts in compliance artifacts (audit_trail.txt and the attestation stdout copy).
Changes:
- Add
_audit_path()to sanitize absolute paths: paths under the current working directory become relative; paths outside it are reduced to the basename; non-absolute values pass through unchanged. - Apply sanitization consistently in both
audit_trail.txtoutput and the attestation stdout output. - Add tests covering inside-cwd relativization, outside-cwd basename stripping (ensuring temp dir paths don’t leak), and pass-through behavior; update README to document the rule.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
sbomify_action/console.py |
Introduces _audit_path() and applies it to audit trail file and attestation stdout emitters. |
tests/test_console.py |
Adds unit tests validating path sanitization behavior for both file output and stdout attestation. |
README.md |
Documents how Input/Output are recorded (relative-to-cwd or basename outside cwd). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
# Conflicts: # README.md
_audit_path reached the filesystem through Path.cwd(), which raises when the working directory has been deleted or is unreadable. The attestation copy of the trail is printed unguarded on the success path -- unlike the file write, which _finalize_run wraps -- so that would have turned a completed run into a crash at the very last step. Before this change the line could not raise at all. Fall back to the file name when the filesystem cannot answer. That errs toward less disclosure, never more, which is the direction this helper exists to enforce. Raised by Copilot on sbomify#400. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vpetersson
approved these changes
Sep 1, 2026
vpetersson-bot
added a commit
to vpetersson-bot/sbomify-action
that referenced
this pull request
Sep 1, 2026
Master moved four commits while this PR was open. sbomify#400 added `_audit_path` to console.py, which uses `os` -- and this branch had removed that import, because after the platform refactor nothing in its version of the file used it. Neither side conflicts textually, so git merged them cleanly into a module that calls os.path with no os in scope. That is why CI went from green to five failing jobs on a branch whose own tests passed: every job that imports the package died at import time, and ruff's F821 was the first thing to say so. Import restored. Whole suite green against the merged tree, run with GITHUB_ACTIONS and GITHUB_WORKSPACE set as CI has them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The audit trail is a compliance artifact — it exists to be handed to someone other than whoever generated it. The absolute path of the input lockfile tells that reader nothing they can use, while publishing the generating machine's directory layout and username:
This is the question #393 deliberately left open when it ignored
audit_trail.txtrather than changing what the file records.What changes. A path under the working directory is recorded relative to it (
src/requirements.txt); a path anywhere else keeps only its file name (requirements.txt).os.path.relpathis the wrong tool for the second case —../../../Users/<username>/...leaks exactly what we are removing. Values that are not paths at all (docker:nginx:latest, theadditional-packages-onlysentinel) are not absolute, so they pass through untouched.Both emitters are covered: the
audit_trail.txtfile and the copy printed to stdout for attestation.No flag to opt back in. A flag whose only function is to re-enable the leak adds config surface for no gain — anyone who wants the full local path already has it in the run logs. Easy to add if you disagree.
The README's
audit_trail.txtexample already showed relative paths, so nothing there was inconsistent; it gains one sentence stating the rule.Tests: relative-inside-cwd, name-only-outside-cwd (asserting the temp directory string appears nowhere in the output), pass-through for non-paths, and the same sanitization on the attestation stdout copy. Full suite green locally — 3566 passed, coverage 80.34%.