test: refuse to test fs_errors on root environments#364
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes the fs_errors integration test safer and more reliable by refusing to run under root privileges (where permission errors behave differently), and adds an opt-out compile-time flag to skip the test entirely.
Changes:
- Add an effective-UID check (
libc::geteuid() == 0) to fail fast with guidance when the test is run as root. - Add
#[cfg(not(pdu_test_skip_fs_errors))]to allow skipping the test viaRUSTFLAGS='--cfg pdu_test_skip_fs_errors'. - Add
libcas a dev-dependency to support the Unix UID check.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/cli_errors.rs | Adds root-environment guard + optional cfg-based skip for fs_errors. |
| Cargo.toml | Adds libc under dev-dependencies for UID detection. |
| Cargo.lock | Locks the new libc dependency. |
Performance Regression Reportscommit: 67c2b72 There are no regressions. |
Root (uid=0) bypasses Unix discretionary access controls, so removing read permissions with chmod has no effect. This causes the fs_errors test to fail because read_dir() succeeds instead of returning Permission denied. Skip the test with an explanatory message when euid is 0. https://claude.ai/code/session_01Ki7wbrgQXWs2GXf8h2hybQ
Replace the `unsafe { libc::geteuid() }` check with a direct probe:
after chmod -r, try read_dir and skip if it still succeeds. This
removes the `unsafe` block and the `libc` dev-dependency while being
more semantically correct — it tests the actual condition we care
about rather than a proxy.
https://claude.ai/code/session_01Ki7wbrgQXWs2GXf8h2hybQ
This reverts commit bb8e8cc.
Instead of silently skipping, panic with an actionable error message when running as root. The test can be excluded entirely via `RUSTFLAGS='--cfg pdu_test_skip_fs_errors'` for environments like Claude Code Web where root is unavoidable. https://claude.ai/code/session_01Ki7wbrgQXWs2GXf8h2hybQ
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Suppress the -D unexpected-cfgs error by adding #[allow(unexpected_cfgs)] before the custom cfg gate. https://claude.ai/code/session_01Ki7wbrgQXWs2GXf8h2hybQ
The #[allow(unexpected_cfgs)] approach doesn't override -D warnings used in CI release builds. Register the custom cfg in Cargo.toml's [lints.rust] section instead, which properly declares it as a known cfg to the compiler. https://claude.ai/code/session_01Ki7wbrgQXWs2GXf8h2hybQ
KSXGitHub
force-pushed
the
claude/debug-fs-errors-tests-ZosXe
branch
from
837f345 to
0de4461
Compare
…rors Gate the Unix-specific imports and fs_permission helper with cfg(all(unix, not(pdu_test_skip_fs_errors))) so they don't produce dead code warnings when the test is skipped. https://claude.ai/code/session_01Ki7wbrgQXWs2GXf8h2hybQ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Add a root environment check to the
fs_errorstest to prevent it from running with elevated privileges, which would compromise test accuracy. The test now panics with helpful guidance if executed as root, and can be skipped via a configuration flag.Changes
fs_errorstest usinglibc::geteuid()RUSTFLAGS='--cfg pdu_test_skip_fs_errors'to skip the test#[cfg(not(pdu_test_skip_fs_errors))]attribute to allow conditional compilation of the testlibcas a dev dependency to access thegeteuid()functionImplementation Details
The test requires non-root execution because filesystem permission errors behave differently when running with elevated privileges, which would invalidate the test's assertions about permission-denied scenarios.
https://claude.ai/code/session_01Ki7wbrgQXWs2GXf8h2hybQ