fix(security): validate --output-dir against path traversal (#30) - #34
Open
seojcarlos wants to merge 1 commit into
Open
fix(security): validate --output-dir against path traversal (#30)#34seojcarlos wants to merge 1 commit into
seojcarlos wants to merge 1 commit into
Conversation
…niel#30) run_batch() already strips path components from per-job filenames but did not validate the --output-dir argument itself, so a path that resolves outside CWD (e.g. "/tmp/../../etc/cron.d") would be created and written into. This is defense in depth — exploitation requires the script to be invoked by an orchestrator that forwards user-influenced arguments (e.g. an agent), not a direct shell user who already has write access. Adds _validate_output_dir() that resolves the path and rejects anything outside the working directory. Set CLAUDE_ADS_OUTPUT_BASE to override the base when an absolute path outside CWD is intentional. Includes regression tests using stdlib unittest (no new dependencies).
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
1 participant
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.
Summary
run_batch()strips path components from per-jobfilenames but does not validate
--output-diritself,so a path that resolves outside CWD (e.g.
/tmp/../../etc/cron.d) gets created and written into.This is defense in depth, not RCE — exploitation
requires the script to be invoked by an orchestrator
forwarding user-influenced arguments (e.g. an agent or
wrapper). A direct shell user can already write wherever
their account allows.
Type of Change
What this PR does
_validate_output_dir(output_dir)that resolvesthe path and rejects anything outside the working
directory.
CLAUDE_ADS_OUTPUT_BASEenv var as the escape hatchwhen an absolute path outside CWD is intentional.
Path.resolve()+relative_to()(matches youroption 2 in the issue).
run_batchswapsPath(output_dir)for the validatedpath; nothing else in the function is touched.
Tests
7 regression tests in
tests/test_output_dir_validation.pyusing stdlibunittest— no new deps. Covers the reporter's exactreproduction, relative traversal, absolute paths outside
CWD, dot-path acceptance, the env override, and the env
override not allowing escape above the configured base.
CI doesn't run tests today (only
py_compile/pip-audit). Happy to wire
python -m unittest discoverinto CI in a follow-up.
Checklist
keep working)
code touched)
Reproducer (pre-fix)
prompts.json --output-dir /tmp/../../etc/cron.d
# Pre-fix: creates /etc/cron.d and writes images
there.
# Post-fix: errors out with clear message and exits
1.
Closes #30