fix(pf-vdisplay): the capability-hint test asserted the environment, not the code
apple / swift (pull_request) Successful in 1m33s
apple / screenshots (pull_request) Skipped
android / android (pull_request) Successful in 5m0s
ci / bun-nix (pull_request) Successful in 42s
ci / docs-site (pull_request) Successful in 1m21s
ci / web (pull_request) Successful in 1m48s
ci / rust-arm64 (pull_request) Successful in 2m47s
ci / rust (pull_request) Successful in 6m57s
apple / swift (pull_request) Successful in 1m33s
apple / screenshots (pull_request) Skipped
android / android (pull_request) Successful in 5m0s
ci / bun-nix (pull_request) Successful in 42s
ci / docs-site (pull_request) Successful in 1m21s
ci / web (pull_request) Successful in 1m48s
ci / rust-arm64 (pull_request) Successful in 2m47s
ci / rust (pull_request) Successful in 6m57s
`silent_without_capabilities` called the real `capability_denial_hint()` and
asserted it returns "", on the strength of a doc comment that read "The test
process has no capabilities."
That is true on a dev box and false in CI, where the runner container is root
with a full permitted set. main went red on 0f79587d with:
left: " — NOTE: this process carries capabilities (CapPrm=0x000001ffffffffff) …"
right: ""
Nothing was wrong: the hint fired correctly, on a process that really did hold
every capability. The test was reading the ambient environment and calling it a
property of the code.
`permitted_caps_from_status` had already been split out for exactly this reason
— "so that shape is testable without a capability-carrying process to point at"
— but only the PARSE half. The message half still went to /proc/self/status.
This finishes the split: `capability_denial_hint_for(Option<u64>)` holds the
formatting and takes the mask, `capability_denial_hint()` reads /proc and
delegates. Both keep their callers, so neither is dead code.
Also adds `names_the_mask_and_the_repair_when_capped`. Without it the silent
case passes just as well against a function that returns "" unconditionally —
which is the failure mode this repo has been bitten by before, and the reason
every decode fix carries a counterfactual.
No behaviour change: the three error paths call the same function and get the
same string.
⚠ Verification is CI. `kwin.rs` is `#[cfg(target_os = "linux")]`, so it does not
compile on the macOS host this was written from; `cargo fmt --all --check` is
clean and a Linux container check was attempted but the stock rust image has no
cmake for audiopus_sys, so it never reached the test. ci.yml going green on main
is the proof — and unlike the case it replaces, this test now fails or passes
for reasons that have nothing to do with the machine running it.
Does not touch the v0.26.0 tag: ci.yml runs on `push: branches: [main]` and
`pull_request` only, and no tag leg runs cargo test.
This commit is contained in:
@@ -1093,6 +1093,19 @@ fn capability_denial_hint() -> String {
|
||||
let permitted = std::fs::read_to_string("/proc/self/status")
|
||||
.ok()
|
||||
.and_then(|status| permitted_caps_from_status(&status));
|
||||
capability_denial_hint_for(permitted)
|
||||
}
|
||||
|
||||
/// The message half of [`capability_denial_hint`], split from the `/proc/self/status` read so it is
|
||||
/// testable against a *given* mask instead of whatever the test process happens to hold.
|
||||
///
|
||||
/// That distinction is not academic: the first version of this asserted the empty case by calling
|
||||
/// the real thing and trusting the test process to be uncapped. That holds on a dev box and is
|
||||
/// false in CI, where the runner container is root with a full permitted set
|
||||
/// (`CapPrm=0x000001ffffffffff`) — so the hint fired, correctly, and the test failed on a machine
|
||||
/// where nothing was wrong. A check whose answer depends on the ambient environment tests the
|
||||
/// environment, not the code.
|
||||
fn capability_denial_hint_for(permitted: Option<u64>) -> String {
|
||||
match permitted {
|
||||
Some(caps) if caps != 0 => format!(
|
||||
" — NOTE: this process carries capabilities (CapPrm={caps:#018x}), which is enough on \
|
||||
@@ -1135,10 +1148,30 @@ mod capability_hint_tests {
|
||||
|
||||
/// A capability-free host must not append the hint — the message it decorates is also printed
|
||||
/// on genuinely missing `.desktop` files, and a spurious "you have capabilities" line would
|
||||
/// send the reader chasing a setcap that was never there. The test process has no capabilities.
|
||||
/// send the reader chasing a setcap that was never there.
|
||||
///
|
||||
/// Driven off an explicit mask rather than the test process's own: see
|
||||
/// [`capability_denial_hint_for`] for why calling the real reader here fails in CI.
|
||||
#[test]
|
||||
fn silent_without_capabilities() {
|
||||
assert_eq!(capability_denial_hint(), "");
|
||||
assert_eq!(
|
||||
capability_denial_hint_for(permitted_caps_from_status(CLEAN)),
|
||||
""
|
||||
);
|
||||
// Absent or unparseable field: also silent, never a panic and never a spurious hint.
|
||||
assert_eq!(capability_denial_hint_for(None), "");
|
||||
}
|
||||
|
||||
/// ...and the case that matters actually speaks, naming the mask and the repair. Without this
|
||||
/// the test above passes just as well against a function that returns `""` unconditionally.
|
||||
#[test]
|
||||
fn names_the_mask_and_the_repair_when_capped() {
|
||||
let hint = capability_denial_hint_for(permitted_caps_from_status(CAPPED));
|
||||
assert!(
|
||||
hint.contains("0x0000000000800000"),
|
||||
"names the mask: {hint}"
|
||||
);
|
||||
assert!(hint.contains("setcap -r"), "names the repair: {hint}");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user