fix: add cwd to sandbox read allowlist - #835
Conversation
tarsafe calls os.getcwd() during archive extraction to bound path-traversal checks. The kernel-level sandbox never granted READ on the current working directory, so on platforms where the sandbox denies it (macOS Seatbelt) the PermissionError surfaced through tarfile as a misleading 'not a gzip file' ReadError. Grant READ on the cwd like the other common paths.
|
lgtm, adding a quick Codex review in case I missed anything then I'll merge, thanks for the contribution! |
|
@codex review |
![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: 1ebb5d2b6d
ℹ️ 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".
| # extraction to bound path-traversal checks. Without READ access here | ||
| # that call fails with PermissionError, which tarfile misreports as | ||
| # "not a gzip file". | ||
| candidates.append(os.getcwd()) |
There was a problem hiding this comment.
When GuardDog is launched from a sensitive directory such as the user's home or a repository root, allow_path grants recursive READ access to that entire tree in both apply_sandbox and extract_sandboxed. This defeats the documented filesystem isolation for archive/parser exploits and can expose credentials or source files unrelated to the scan; satisfy tarsafe's getcwd() requirement by changing into an already-allowed temporary/extraction directory rather than globally allowlisting the caller's cwd.
Useful? React with 👍 / 👎.
| # extraction to bound path-traversal checks. Without READ access here | ||
| # that call fails with PermissionError, which tarfile misreports as | ||
| # "not a gzip file". | ||
| candidates.append(os.getcwd()) |
There was a problem hiding this comment.
When the process's working directory has been renamed or deleted after startup, os.getcwd() raises FileNotFoundError, so every sandboxed scan now fails while constructing the common allowlist—even remote scans whose inputs and temporary directories remain valid. Catch OSError here and omit the cwd, or obtain it only in the archive path that actually requires it.
Useful? React with 👍 / 👎.
2 participants
Summary
Fix local archive scans failing under the kernel-level sandbox with a misleading "not a gzip file" ReadError.
tarsafe calls os.getcwd() during archive extraction to bound path-traversal checks. The sandbox allowlist in guarddog/sandbox.py (_get_common_read_paths) never included the current working directory, so on platforms where the sandbox denies access to it (macOS Seatbelt), that call raises PermissionError. tarfile wraps it and reports "not a gzip file".
Changes
Test plan
Notes
Fixes #818