fix(p0-7): run_validation default-safe output + symlink-resistant containment#433
fix(p0-7): run_validation default-safe output + symlink-resistant containment#433
Conversation
…po reports/ (F009 S4)
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 56 minutes and 6 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Addresses a high-impact safety issue in scripts/e2b/run_validation.py where --output could lead to destructive deletion of an existing directory, by making output handling default-safe and constraining deletion to a repo-controlled reports area.
Changes:
- Writes each run into a new
<output>/run-<timestamp>/directory by default (no deletion of existing output). - Adds
--clean-outputas an explicit opt-in to delete the base output directory. - Restricts
--clean-outputto output paths contained within the repo’sreports/root, with additional symlink-resistant resolution checks, and adds tests for these behaviors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
scripts/e2b/run_validation.py |
Implements default-safe per-run subdirs, adds --clean-output, and enforces containment within repo reports/ with resolved-path checks. |
tests/scripts/test_e2b_run_validation_output.py |
Adds coverage for non-deletion by default, opt-in deletion, containment enforcement, and symlink escape scenarios. |
|
@copilot please address the blocking signals on this PR and push a fix commit. Trigger: ci-failure Follow the PR's |
Summary
P0 fix for
reports/autosearch-p0-fix-plan.md§7 —scripts/e2b/run_validation.py --output <dir>previouslyrmtree'd the existing output directory at startup. Pointing--outputat a sensitive dir (e.g. home) destroyed data without warning.Changes
<output>/run-<timestamp>/subdir. Existing files never deleted.--clean-outputflag is required to allow rmtree. Without it, no delete.--clean-outputonly allowed when<output>is inside the repo'sreports/root. Reports root derived viaPath(__file__).resolve().parents[N] / "reports"(not hardcoded — repo-relative).<output>andreports_rootgo through.resolve()beforeis_relative_to(...)comparison. Tests cover symlink escape (target outside reports_root) and symlink-as-reports-root.Plan
docs/exec-plans/active/autosearch-0426-p0-fix-plan-execution.md§ F009 (S1-S5).Commits
Test plan
pytest tests/scripts/test_e2b_run_validation_output.py— 5 passedpytest tests/scripts/— 31 passedpytest tests/unit/ -m "not real_llm and not slow and not network"— 673 passed, 3 skipped🤖 Generated with Claude Code