Skip to content

fix(wren): validate import paths before force cleanup#2580

Open
GiacomoZGW wants to merge 1 commit into
Canner:mainfrom
GiacomoZGW:fix/force-import-preflight
Open

fix(wren): validate import paths before force cleanup#2580
GiacomoZGW wants to merge 1 commit into
Canner:mainfrom
GiacomoZGW:fix/force-import-preflight

Conversation

@GiacomoZGW

@GiacomoZGW GiacomoZGW commented Jul 25, 2026

Copy link
Copy Markdown

What

Validate all project output paths before write_project_files() performs
--force cleanup.

Also reject paths that resolve to the project root itself, such as an empty
path, ., or models/...

Why

The previous implementation removed Wren-managed project files before
validating every ProjectFile.relative_path.

A malformed import could therefore fail with invalid output path after
the existing project had already been partially deleted.

Paths resolving exactly to the project root also passed the containment
check and failed during write_text() only after cleanup.

Changes

  • preflight all resolved output paths before filesystem mutation
  • reject targets outside the project directory
  • reject targets equal to the project root
  • reuse validated resolved paths during conflict detection and writing
  • add regression tests proving existing files remain unchanged

Validation

  • uv run --no-sync pytest tests/unit/test_convert_mdl.py -v
    • 30 passed
  • uvx ruff format --check src/
    • 80 files already formatted
  • uvx ruff check src/
    • All checks passed!

Additional local stress testing covered both force=True and force=False
with valid entries preceding invalid targets.

Disclosure

This issue was initially reported through the project's coordinated
disclosure process. The maintainers reproduced it, classified it as a
low-severity local data-integrity issue, and authorized proceeding through
the normal public contribution process.

Scope

This change prevents filesystem mutation when output-path preflight fails.
It does not implement transactional rollback for disk errors or process
interruption during the later cleanup/write phase.

Summary by CodeRabbit

  • Bug Fixes
    • Improved validation of generated output paths to prevent files from being written outside the intended project directory.
    • Added preflight checks so invalid paths are detected before any files or directories are created.
    • Preserved existing files when invalid output paths are rejected, including when force overwrite is enabled.
    • Improved conflict detection for existing output files.

@github-actions github-actions Bot added python Pull requests that update Python code core labels Jul 25, 2026
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6708a257-9f4f-4684-a3a3-395994da2120

📥 Commits

Reviewing files that changed from the base of the PR and between d472877 and 3d0c86e.

📒 Files selected for processing (2)
  • core/wren/src/wren/context.py
  • core/wren/tests/unit/test_convert_mdl.py

Walkthrough

write_project_files() now pre-resolves and validates all output paths within the project root before conflict checks or writes. Tests cover escaped paths, root-target paths, force mode, existing files, and prevention of partial output creation.

Changes

Safe output path handling

Layer / File(s) Summary
Preflight validation and resolved writes
core/wren/src/wren/context.py
Output targets are resolved and validated before conflict detection; validated paths are then used for conflict checks and file writes.
Path validation coverage
core/wren/tests/unit/test_convert_mdl.py
Tests verify rejection of escaped or root-target paths, preservation of existing files, and absence of partial writes after validation failures.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: goldmedal

Poem

A rabbit checks each path with care,
No wandering files escape the lair.
First paths are proved, then writes begin,
Old sentinel files stay safe within.
“Preflight!” cries Bun, with a cheerful spin.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: validating import paths before force cleanup in write_project_files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core python Pull requests that update Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant