GeezerVison is showing github.com — Skip permission-denial unit tests when run as root by mikolalysenko · Pull Request #2058 · SocketDev/socket-patch
Conversation
Problem
CI is healthy this run. The merge queue is empty, every merge_group run since 11-10 passed, main's latest push run and today's nightly (42967443938) are green, and nothing is queued or in progress. The only red PR run in the last day (42916092136, #2052) was a real command_module_layering failure from that PR's own change. So this run takes a priority-4 robustness target in the same class as #2054–#2057: unit tests that pass only because of how CI's environment is set up.
#2057 lists 4 tests that fail on a clean origin/main checkout in every env row. I reproduced them on c09cb0e with cargo test -p socket-patch-core --lib as uid 0, the default user in dev containers and cloud agent sandboxes:
test result: FAILED. 7688 passed; 4 failed
patch::copy_tree::tests::relax_loop_must_not_traverse_symlinked_root
patch::redirect::vlt_heal::tests::an_unremovable_hidden_lock_keeps_every_store_entry
vendor::pypi_poetry::tests::wire_write_failure_maps_error_and_leaves_lock_untouched
vendor::pypi_requirements::tests::wire_failure_rolls_back_already_written_files
Root cause
Each test makes a directory 0o555 and asserts that a write or removal inside it fails. Root ignores permission bits, so the operation succeeds.
Fix
Each of the 4 tests now returns early under libc::geteuid() == 0, the same guard the permission tests in pnpm_lock, bun_lock and composer_lock already use. No assertion changes. 12 added lines, test code only.
Proof
- As root, after the change:
cargo test -p socket-patch-core --lib gives 7692 passed, 0 failed.
cargo clippy --locked -p socket-patch-core --all-features -- -D warnings (CI's command) is clean. --tests reports no lint in a touched file. Its existing lints are in other targets, such as crawler_ruby_e2e.- rustfmt
--check on the 4 touched files is clean.
Where the tests still run
GitHub-hosted and Depot runners run as non-root. That's why these tests pass in CI today, and every assertion still runs there on every PR, in the merge queue and on main. Only root runs skip them, and under root the assertions can't hold anyway.
Generated by Claude Code
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Four socket-patch-core unit tests make a directory read-only and
assert that a write or removal fails. Root ignores permission bits,
so under uid 0 (dev containers, cloud agent sandboxes, root CI
images) the operation succeeds and the tests fail on a clean
checkout:
patch::copy_tree::relax_loop_must_not_traverse_symlinked_root
patch::redirect::vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry
vendor::pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched
vendor::pypi_requirements::wire_failure_rolls_back_already_written_files
Give them the same geteuid() == 0 early return that the pnpm_lock,
bun_lock and composer_lock permission tests already use. GitHub-hosted
and Depot runners are non-root, so CI still runs every assertion.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011f9aeSKziH186vNriv2dbS
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
commented
Nov 11, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
PR Summary
Low Risk
Test-only early returns; no production code or assertion logic changes.
Overview
Four Unix-only permission-denial tests in copy_tree, vlt_heal, pypi_poetry, and pypi_requirements now return immediately when the effective UID is root (libc::geteuid() == 0), with a short comment that root bypasses permission bits so the scenario cannot be exercised.
Assertions and test setup are unchanged. The guard matches the pattern already used elsewhere in this crate (e.g. lockfile tests). CI and normal non-root runs still execute these tests; only root environments (common dev containers / sandboxes) skip them, fixing false failures without weakening checks where 0o555 actually blocks I/O.
Reviewed by Cursor Bugbot for commit 05a92bc. Configure here.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 05a92bc. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Ready for review at 05a92bc.
- CI: 50/50 checks done, none failed (31 success incl.
ci-ok and clippy, 19 skipped by path filters). No main-wide failures.
- Bugbot: reviewed
05a92bc, no findings.
- Look at: the four
geteuid() == 0 early returns in copy_tree, vlt_heal, pypi_poetry and pypi_requirements tests. They're test-only and match the existing guards in pnpm_lock/bun_lock/composer_lock.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Final review brief
What it does: Four unit tests make a directory 0o555 and assert that a write or removal inside it fails. Root ignores permission bits, so they fail when run as uid 0 (dev containers, agent sandboxes). Each test now returns early under geteuid() == 0. Test code only, 12 lines.
Risk: low. No production code changes. The guard is the same one pnpm_lock, bun_lock and composer_lock tests already use, and CI runners are non-root, so every assertion still runs in CI.
Look here:
Verified:
- All 4 tests are
#[cfg(unix)], and libc is a unix-only dependency of socket-patch-core, so Windows builds are unaffected.
- Ran the 4 tests as root on this head: 4 passed. The PR body reports that they fail as root on main.
- CI:
ci-ok and clippy are green, and no check failed. Bugbot reviewed this head and left no open threads. CHANGELOG.md is untouched.
Changes I made: none.
Open questions: none.
Auto-merge is armed, so approving sends this PR straight to the merge queue.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
At least 1 approving review is required to merge this pull request.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
ci-janitor
Opened by the CI janitor routine (flakes, redundant tests, CI perf)
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|