#1882 fix: harden the metafile check and exempt remote sources - #1883
#1882 fix: harden the metafile check and exempt remote sources#1883RAprogramm wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe metafile checker now handles malformed ChangesMetafile validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/python/check_dots.py`:
- Around line 65-68: Update the source classification logic in the body/table
loop so the remote flag is set only when source has a valid supported remote
value, not merely when the key exists. Validate both component-level and
table-level source values, reject malformed values such as numeric source
entries with the checker’s existing invalid-source error path, and retain local
source_root containment and path checks for non-remote entries.
- Around line 41-46: Update inside to catch RuntimeError from Path.resolve()
alongside the existing OSError and ValueError exceptions, returning False when
symlink-loop resolution fails.
In `@tests/README.md`:
- Line 25: Update the test_dots.sh description in tests/README.md to state that
local entries undergo checkout-root and source-path validation, while entries
with a remote source are exempt from all such validation because their files
come from a downloaded archive.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a77d25ad-9106-4431-bd4b-d767cba7b692
📒 Files selected for processing (2)
tests/README.mdtests/python/check_dots.py
Fixes #1882
Three fixes to the metafile check, all reproduced before and after.
Crash on a malformed
paths.paths = 42raisedTypeError: 'int' object is not iterable. Non-list values are wrapped, so the type check reports them.Paths escaping
source_root. Each declared path now resolves against the resolved source root and has to stay under it, checked before the glob exemption.Remote-source entries. A dot declaring
sourcepulls its files from an archive the installer downloads, so itssource_rootresolves inside the extracted tree. Existence checks are skipped for those; type checks still apply. This is what #1874 needs.The first two were raised by the reviewer on #1881 after it merged.
Type of change
Checklist
Testing
Eight cases through a scratch metafile:
paths = 42int, no crash../../etc/passwd/etc/passwdsourcewith a path outside the checkoutsourcewithpaths = 7intsh tests/run.shpasses, 8 cases.Summary by CodeRabbit
Tests
source_rootand that non-globsource/pathsexist and stay within the expected repository boundaries.pathsdeclarations are malformed, preventing crashes and enforcing correct types.Documentation