{{ message }}
Repository navigation
fix(security): confine tree_optimize --out/--log writes to allowed directories - #602
Open
rtmalikian wants to merge 1 commit into
Open
rtmalikian wants to merge 1 commit into
rtmalikian wants to merge 1 commit into
Conversation
…rectories --out and --log were passed straight to open(path, 'w'), so a typo or an untrusted invocation could silently overwrite any user-writable file (e.g. --out /etc/cron.daily/x.json). _confine_output_path() resolves the path (symlinks/.. normalized) and rejects anything outside the current working directory or the input structure's directory; relative paths still resolve under the CWD exactly as before. Fixes VectifyAI#584. Signed-off-by: Raphael Malikian <rtmalikian@gmail.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. |
This branch has not been deployed
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
--outand--loginpageindex/tree_optimize.pywere passed straight toopen(path, "w")with no validation, so an operator typo or an untrusted invocation could silently overwrite any user-writable file (e.g.--out /etc/cron.daily/x.json,--log ~/.ssh/authorized_keys).The fix adds
_confine_output_path(): it expands~, resolves relative paths against the CWD (exactly as before), fully resolves the path (symlinks and..normalized), and raisesValueErrorunless the result sits inside the current working directory or the input--structuredirectory.main()catches the error, prints a clean message to stderr, and exits non-zero (2). All previously legitimate flows are preserved: relative paths resolve under the CWD, the default<structure>.optimized.jsonlands next to the input, and absolute paths under either allowed directory still work.Fixes #584
Changes
pageindex/tree_optimize.py_confine_output_path()guard;main()validates--outand--logbefore any write, clean stderr error + exit 2 on escapetests/test_tree_optimize_paths.py..escape rejection, symlink escape rejection, union of CWD + structure dir, default out-path from--structureVerification
python -m pytest tests/test_tree_optimize_paths.py -q— 7/7 pass (venv: PyPDF2, pyyaml, pytest, python-dotenv, regex, sortedcontainers).tree_optimize.py(viagit stash) fail at collection (_confine_output_pathdoes not exist), i.e. the guard is absent pre-fix._confine_output_path("/tmp/canary.txt", allowed_dirs=[cwd, struct_dir])raisesValueError: --out: refusing to write outside ..., the canary file is untouched, and the default<structure>.optimized.jsonpath still resolves fine.python -m pytest tests/test_tree_format.py -q— 30/30 pass (added pypdfium2 + litellm to the venv for collection); no regressions.py_compileclean on both touched files. Repo CI runs plainpytest(no ruff/black config inpyproject.toml).Author
Raphael Malikian — rtmalikian@gmail.com
If you would like similar help with your own project or codebase, I am open to collaborate — feel free to reach out.
This contribution was prepared with AI assistance.