Repository navigation
fix(container): prevent path traversal in untar test helper - #172
Conversation
Scope all filesystem writes in the untar test helper to an os.Root on the destination directory, so archive entries escaping it are rejected. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9241b633a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🟢 Approval recommended
Static review identified no blocking issues in the focused test-helper changes; tests were not executed.
No open issues 🎉
What changed in this PR
This PR hardens the container tests’ untar helper against path traversal by confining archive extraction to the destination directory.
Changes:
- Uses
os.Rootfor directory creation and file writes during extraction. - Adds regression tests for parent-directory traversal and absolute paths.
- Closes the output file if copying archive contents fails.
| File | Description |
|---|---|
container/files_test.go |
Confines extraction to the destination, adds traversal tests, and improves file cleanup. |
Review effort: Balanced
os.Root.OpenFile rejects permission bits outside 0o777, so archives with setuid, setgid or sticky bits could no longer be extracted. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
What does this PR do?
It makes the
untartest helper incontainer/files_test.gowrite through anos.Rootscoped to the destination dir, so entries like../evilor/evilfail instead of escaping it. It also addsTestUntar_rejectsPathTraversalto cover those cases.Why is it important?
Sonar flags it as Zip Slip. It's only test code, but I'd like to have it clean before the next release.
Related issues
🤖 Generated with Claude Code
🤖 Written assisted by my Claude