[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: discussion #560 register.
Kind: bug. Source: new finding; register C56. A #[ignore = "RED: …"] test added in #708 already pins this bug, but no issue tracks it and no CI job runs it.
Problem (main @ 9c43dfc)
Core's read_manifest treats only ErrorKind::NotFound as "no manifest", and its own regression test says any other I/O error must surface as Err. Five CLI commands run their own existence probe before calling it, and that probe treats any stat error as "missing":
if tokio::fs::metadata(&manifest_path).await.is_err() { /* no manifest */ }
list and vendor --check go through read_manifest and report the contract's manifest_unreadable correctly. The contract (CLI_CONTRACT.md#L1291-L1293) defines manifest_not_found as "doesn't exist" and manifest_unreadable as an I/O error reading the manifest.
Proof by execution. I used a debug build at 9c43dfc, ran every command under env -i … --json, and ran the whole set twice with identical results. There were two fixtures: .socket as a regular file (ENOTDIR), and .socket/manifest.json as a symlink to itself (ELOOP).
| Command |
Result |
apply |
status: noManifest, exit 0 |
apply --check |
status: noManifest, exit 0 |
vendor |
status: noManifest, exit 0 |
repair, remove <purl> |
manifest_not_found, exit 1 |
rollback |
{"error":"Manifest not found"}, exit 1 |
list, vendor --check |
manifest_unreadable (Too many levels of symbolic links), exit 1 |
The pinned test, apply_with_unreadable_socket_dir_fails_closed,`` uses a chmod 000 .socket/ (EACCES) fixture. It self-skips as root, which is why I reproduced the bug with ENOTDIR and ELOOP instead.
Symptoms
No open issue. The test's own comment states the impact: install hooks and CI steps run apply --silent and read exit 0 as "patched".
Impact
apply and vendor fail open. Every patch in a project whose .socket/ can't be traversed is silently left unapplied with a success exit; the triggers are a root-owned or ACL-restricted .socket/ on a CI runner, a symlink loop, or .socket checked in as a file. The three other commands fail closed but with the wrong code, and their remedy text points at a missing file. Small fix, real CI consequence.
Proposed change
- Add one core probe next to
read_manifest, for example manifest::probe(path) -> Result<Presence, io::Error> (or reuse read_manifest's Ok(None) directly), with the same NotFound-only rule.
- Delete the five
metadata(&manifest_path).await.is_err() probes and route those commands through it:
- a non-NotFound error becomes
manifest_unreadable (exit 1) on apply, apply --check, vendor, repair, remove and rollback;
- a real NotFound keeps today's behavior:
noManifest on apply/vendor, the hosted/vendored-trace fallbacks on repair/remove/rollback.
- Un-ignore
apply_with_unreadable_socket_dir_fails_closed, and add a root-proof variant (ENOTDIR or ELOOP) so the test runs in CI containers.
Size and scope
About 40 production lines across apply.rs, vendor.rs, repair.rs, remove.rs, rollback.rs and manifest/operations.rs, plus about 80 test lines. Out of scope:
Acceptance criteria
Dependencies
None blocking. It pairs with #931 (one manifest-load error mapping); whichever lands second reuses the other's helper.
[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: discussion #560 register.
Kind: bug. Source: new finding; register C56. A
#[ignore = "RED: …"]test added in #708 already pins this bug, but no issue tracks it and no CI job runs it.Problem (main @
9c43dfc)Core's
read_manifesttreats onlyErrorKind::NotFoundas "no manifest", and its own regression test says any other I/O error must surface asErr. Five CLI commands run their own existence probe before calling it, and that probe treats any stat error as "missing":apply.rs#L835: the cleannoManifestno-op, exit 0.vendor.rs#L756:noManifest, exit 0.repair.rs#L67,remove.rs#L353androllback.rs#L1044:manifest_not_found, or a bare "Manifest not found" string from rollback.listandvendor --checkgo throughread_manifestand report the contract'smanifest_unreadablecorrectly. The contract (CLI_CONTRACT.md#L1291-L1293) definesmanifest_not_foundas "doesn't exist" andmanifest_unreadableas an I/O error reading the manifest.Proof by execution. I used a debug build at
9c43dfc, ran every command underenv -i … --json, and ran the whole set twice with identical results. There were two fixtures:.socketas a regular file (ENOTDIR), and.socket/manifest.jsonas a symlink to itself (ELOOP).applystatus: noManifest, exit 0apply --checkstatus: noManifest, exit 0vendorstatus: noManifest, exit 0repair,remove <purl>manifest_not_found, exit 1rollback{"error":"Manifest not found"}, exit 1list,vendor --checkmanifest_unreadable(Too many levels of symbolic links), exit 1The pinned test,
apply_with_unreadable_socket_dir_fails_closed,`` uses achmod 000 .socket/(EACCES) fixture. It self-skips as root, which is why I reproduced the bug with ENOTDIR and ELOOP instead.Symptoms
No open issue. The test's own comment states the impact: install hooks and CI steps run
apply --silentand read exit 0 as "patched".Impact
applyandvendorfail open. Every patch in a project whose.socket/can't be traversed is silently left unapplied with a success exit; the triggers are a root-owned or ACL-restricted.socket/on a CI runner, a symlink loop, or.socketchecked in as a file. The three other commands fail closed but with the wrong code, and their remedy text points at a missing file. Small fix, real CI consequence.Proposed change
read_manifest, for examplemanifest::probe(path) -> Result<Presence, io::Error>(or reuseread_manifest'sOk(None)directly), with the same NotFound-only rule.metadata(&manifest_path).await.is_err()probes and route those commands through it:manifest_unreadable(exit 1) onapply,apply --check,vendor,repair,removeandrollback;noManifestonapply/vendor, the hosted/vendored-trace fallbacks onrepair/remove/rollback.apply_with_unreadable_socket_dir_fails_closed, and add a root-proof variant (ENOTDIR or ELOOP) so the test runs in CI containers.Size and scope
About 40 production lines across
apply.rs,vendor.rs,repair.rs,remove.rs,rollback.rsandmanifest/operations.rs, plus about 80 test lines. Out of scope:rollback's bare-string envelope, which is Decide: one shape for the--jsontop-levelerror(scan and get emit both a string and a {code, message} object) #704.Acceptance criteria
grep -rn "metadata(&manifest_path).await.is_err()" crates/socket-patch-cli/srcfinds nothing..socketas a file, and with a self-referencingmanifest.jsonsymlink,apply,apply --check,vendor,repair,removeandrollbackall exit 1 withmanifest_unreadableunder--json.apply_with_unreadable_socket_dir_fails_closedis no longer#[ignore]d, and an ENOTDIR twin runs as root.applynoManifestexit 0 and its human line;repair's hosted-only skip;remove/rollbackledger-only and hosted-only paths), along with core'sread_manifestNotFound tests.Dependencies
None blocking. It pairs with #931 (one manifest-load error mapping); whichever lands second reuses the other's helper.