Conversation
os.symlink raises WinError 1314 on unprivileged Windows, so these two escape-rejection tests error in setup. Skip only that case; re-raise other OSErrors so real failures aren't masked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughTwo symlink escape tests now skip on Windows when symlink creation fails with privilege error 1314. Other symlink creation errors still fail the tests. ChangesWindows symlink test handling
Suggested reviewers: Priority: ⬇️ Low Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to The security tests now skip only where Windows lacks symlink privileges and continue validating supported environments, with no product-code risk introduced. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
What
Two path-containment security tests in
lib/crewai-toolscreate a symlink insetup with no guard, so they error out on Windows for any user who is not
elevated and does not have Developer Mode on:
tests/utilities/test_safe_path.py—test_rejects_symlink_escapetests/tools/test_file_writer_tool.py—test_blocks_symlink_escapeThese are exactly the tests that guard escape-via-symlink rejection (the
behaviour hardened in #6248 / #6249), so today an ordinary Windows contributor
can't run the suite that protects that path — it goes red in setup.
Fix
Wrap each
os.symlink()call: if it fails withWinError 1314,pytest.skipthat one test; re-raise any other
OSErrorso a genuine failure never turnsinto a silent skip. Where symlinks work — every current CI job, and any elevated
or Developer-Mode machine — behaviour is unchanged and the escape assertions
still run. The file-writer test also cleans up its
mkdtemp()dir on the skippath so nothing leaks.
Guard is inlined at the two call sites (13 lines total) rather than adding a
shared helper, to keep it a two-file, dependency-free diff — happy to factor it
into a shared helper if you'd prefer.
Why CI doesn't catch it
The workflow matrix runs
ubuntu-latestandmacos-*; no job runs the suite onWindows. GitHub's Windows runners can create symlinks anyway, so even adding a
Windows job wouldn't surface this — the gap is the privilege, not just the OS.
Product code is unaffected
This is tests-only.
validate_file_path(safe_path.py) usesos.path.realpath()containment itself is fine.
Notes
Fixes #7431
Verified locally by
py_compileand code inspection; I don't have anunprivileged-Windows box wired into CI to execute the skip path, so a maintainer
confirming the skip fires there (and still runs everywhere else) would be
welcome.
🤖 Generated with Claude Code