diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 5acc22d01..ac0324985 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1734,7 +1734,7 @@ outcome for a selected record, rather than inferring a write from exit 0. | `manifest_not_found` | remove, repair, rollback, vex, apply (only when the manifest vanishes between apply's existence probe and its locked read) (not `list` since v5.0: a missing manifest is an empty list) | `.socket/manifest.json` doesn't exist. For `vex` (and `scan --vex`) it fires only when, in addition, NOTHING else names a patch — no vendor-ledger entry, no lockfile reference (hosted or vendored) — and the message says so (exit 2 standalone; `apply`/`vendor --vex` treat it as their calm no-op). v3.5: `repair` proceeds anyway (vendored phase only) when a vendor ledger or vendor-path lockfile references exist, and exits 0 with a `redirect_only_project` skip (not this error) when the project's only patch state is hosted pins in its lockfiles (v5.0; or a pre-v5 `redirect-state.json`). `list` likewise no longer fires this on a hosted-only project: v5.0 lists every hosted pin the lockfiles wire (exit 0, labeled `details.mode: "hosted"` + `details.lockfiles: []` — no `details.ledger`, since hosted mode keeps none; when the manifest exists too, both are shown, purl-sorted with the manifest entry first on a tie). A pin carries its uuid and empty details unless a pre-v5 redirect ledger records the same purl and uuid (read for migration only: its record supplies the vulnerabilities / tier / description); a pre-v5 ledger record whose pin is in no lockfile is not listed. v5.0: `list` reads the vendor ledger the same way — a vendored-only project (every `scan`/`get --mode vendored` project) lists its ledger entries' embedded records labeled `Mode: vendored (recorded in .socket/vendor/state.json)` in human mode — the twin of the hosted `Mode: hosted (wired in )` line — (`details.mode: "vendored"` + `details.ledger: ".socket/vendor/state.json"` in JSON), exit 0. A standalone-`vendor` entry's fallback `record` lists the same way once no manifest entry covers it (by ledger key or base purl) — the copy manifest-less `vex` attests from, so `list` never reports `manifest_not_found` for a tree whose VEX document attests a patch; while the manifest covers it, only the manifest entry is listed. All sources always come from the SAME project (see the `--manifest-path` flag row), so `--manifest-path` into another project reads that project's state, never the local one. The error still fires when NONE of the three sources has a patch, and a present-but-broken manifest still reports `manifest_invalid`/`manifest_unreadable` regardless (corruption is never masked). A malformed pre-v5 redirect ledger degrades to "nothing to consult" with a stderr warning, muted by `--silent` (the pins still list); `list --json` carries it in the run-level `warnings[]` as `redirect_ledger_corrupt` instead of on stderr. v5.0: `rollback` likewise proceeds manifest-less when the vendor ledger or the lockfiles' hosted pins hold work (its error is the legacy `{status: "error", error: {code: "manifest_not_found", message: "Manifest not found"}, path}` shape); only the truly-empty project — no manifest, no vendor ledger, no hosted pin (a lone pre-v5 redirect ledger is deleted, exit 0) — keeps the exit-1 error, and a project whose lockfiles still reference `.socket/vendor/` artifacts with NO vendor ledger gets a distinct error naming `socket-patch repair`. `remove` (v5.0) proceeds manifest-less whenever a vendor ledger file exists or the lockfiles pin a hosted patch (an existence probe and the read-only hosted-pin discovery before the lock; the vendor ledger loads under it): ANY vendor-ledger entry matching the identifier — detached or not — is removed through the ledger path (`--preserve-state` and drift-keeps behave exactly as on the manifest path), a hosted-only match restores its upstream registry entry, and when that state exists but holds nothing for the identifier the error is `not_found` (exit 1), not this code — `manifest_not_found` fires from `remove` only when all three sources are empty. Manifest entries are removed in sorted purl order. | | `ambiguous_target` | remove, get, rollback | The identifier is a package name whose last-segment rule selects several packages across the manifest, the vendor ledger and the hosted pins (`core` → `@angular/core` and `@babel/core`). Nothing is changed (exit 1); the message names each package. Use the full name or a purl. See **Target grammar**. | | `manifest_invalid` | list, remove, apply (incl. `--check`), repair, vendor (incl. `--check`), vex, rollback, get, scan (agent mode) | Manifest exists but is unparseable (malformed JSON or a schema violation). **v5.0, MAJOR (#931)**: one mapping on every command — `apply` used to say `apply_failed`, `repair` `repair_failed`, `vendor` the undocumented `invalid_manifest`, and `apply --check` / `vendor --check` / `vex` / `get` / `rollback` `manifest_unreadable`. Exit codes unchanged (`vex`: 2, every other command 1). Agent-mode `scan` fails with it (exit 1); the other scan modes, which don't read the manifest, add a warning with the same code and go on. | -| `manifest_unreadable` | list, remove, apply (incl. `--check`), repair, vendor (incl. `--check`), vex, rollback, get, scan (agent mode) | Manifest exists but cannot be read (an I/O error: permission denied, a directory at the path, …). `vex`: exit 2. Other scan modes warn, as for `manifest_invalid`. | +| `manifest_unreadable` | list, remove, apply (incl. `--check`), repair, vendor (incl. `--check`), vex, rollback, get, scan (agent mode) | Manifest exists but cannot be read (an I/O error: permission denied, a directory at the path, …). Only a missing file is absence: a path that cannot even be stated (an unsearchable `.socket/`, `.socket` as a plain file, a symlink loop) is this code too, never `noManifest` or `manifest_not_found` (#998), and the verdict is the same whether the path is the default, spelled with `--manifest-path` or set through `SOCKET_MANIFEST_PATH` (#1123). `vex`: exit 2. Other scan modes warn, as for `manifest_invalid`, and so does a `scan --prune` GC pass that finds the manifest unloadable under its lock (#1063). | | `manifest_write_failed` | get | The manifest write after a download failed; the blobs that run wrote are removed again. | | `patch_fetch_failed` | get | The patch search or view request failed. | | `patch_no_applicable_files` | get | The fetched patch has no file it could record; nothing written. | @@ -2071,7 +2071,7 @@ Exit `1` when `status` is `partialFailure` (any `events[*].action == "failed"`) |---|---| | `0` | Success | | `1` | Error (missing/invalid manifest, fetch failed, apply failed, selection cancelled in non-JSON mode, an invalid or ambiguous socket.yml on `scan` (v5.0), etc.) | -| `2` | Usage error: clap parse failures (unknown flag/value, missing required arg, an unknown subcommand such as the removed `setup`) and the conflicts the commands enforce themselves — `scan`'s cross-mode conflict (`--sync` combined with a `--mode` other than `agent`, rejected in `resolve_mode_flags`) and `--mode hosted\|vendored` with `--global`/`--global-prefix` (same enforcement point; `get` refuses the same combination) or with a `--manifest-path` in another project (v5.0, `manifest_path_foreign_project`; `get` and `vendor` other than `--check` likewise, as do `apply --vex` and any other `scan --vex`); a `scan`/`get` with no `--mode` on a project holding both agent and vendored state (`mode_ambiguous`, v5.0); in hosted/vendored `scan` (bare `scan` included), a PATH that is not a directory, a PATH glob matching no directory, and `--json` with more than one project directory (`run_project_dirs`); on every project command (all but `--update`), a `--cwd` / `SOCKET_CWD` or `--global-prefix` / `SOCKET_GLOBAL_PREFIX` that is not an existing directory, and a non-default `--manifest-path` / `SOCKET_MANIFEST_PATH` that names a directory or sits in a project directory that does not exist (v5.0; checked once before the command runs, whether or not the command uses the flag — previously these read as an empty project and exited 0, and `remove` / `rollback` exited 1; a missing manifest FILE in an existing project stays legal, and `get` / `scan`, which create the manifest, still create a missing manifest directory); `remove --preserve-state --skip-rollback` (the no-op quadrant; flag- or env-sourced alike), an unparseable path glob on `scan`/`rollback`, a `scan` PATH outside the repository root and a malformed `SOCKET_MIN_SEVERITY` or `SOCKET_MAX_NEW_PATCHES` (v5.0), `repair --offline --download-only`. Under `--json`, every usage error a command enforces itself prints the coded error on stdout (v5.0, MAJOR: `scan`, `remove` and `rollback` printed nothing there): a full envelope for the envelope commands, `{status: "error", error: {code, message}}` for `scan`, `get` and `rollback`; clap's own parse errors, and the path-flag check that runs before every command (`--cwd` / `--global-prefix` / `--manifest-path` naming nothing), still print nothing on stdout. `vex` also exits `2` on hard errors before document generation (see its tri-state table below). v5.0: `get`'s self-enforced conflicts exit `2` too (`--id`/`--cve`/`--ghsa`/`--package` multi-select, `--mode hosted\|vendored --save-only`, a malformed identifier for a forced `--id`/`--cve`/`--ghsa`) — previously `1` (MAJOR). The never-implemented `get --one-off` / `rollback --one-off` (and `SOCKET_ONE_OFF`) are removed in v5.0; `--one-off` is now an ordinary unknown-flag clap error. | +| `2` | Usage error: clap parse failures (unknown flag/value, missing required arg, an unknown subcommand such as the removed `setup`) and the conflicts the commands enforce themselves — `scan`'s cross-mode conflict (`--sync` combined with a `--mode` other than `agent`, rejected in `resolve_mode_flags`) and `--mode hosted\|vendored` with `--global`/`--global-prefix` (same enforcement point; `get` refuses the same combination) or with a `--manifest-path` in another project (v5.0, `manifest_path_foreign_project`; `get` and `vendor` other than `--check` likewise, as do `apply --vex` and any other `scan --vex`); a `scan`/`get` with no `--mode` on a project holding both agent and vendored state (`mode_ambiguous`, v5.0); in hosted/vendored `scan` (bare `scan` included), a PATH that is not a directory, a PATH glob matching no directory, and `--json` with more than one project directory (`run_project_dirs`); on every project command (all but `--update`), a `--cwd` / `SOCKET_CWD` or `--global-prefix` / `SOCKET_GLOBAL_PREFIX` that is not an existing directory, and a non-default `--manifest-path` / `SOCKET_MANIFEST_PATH` that sits in a project directory that does not exist (v5.0; checked once before the command runs, whether or not the command uses the flag — previously these read as an empty project and exited 0, and `remove` / `rollback` exited 1; a missing manifest FILE in an existing project stays legal, and `get` / `scan`, which create the manifest, still create a missing manifest directory; one that names a directory is an unreadable manifest, `manifest_unreadable` exit 1 like the default path, not a usage error, #1123); `remove --preserve-state --skip-rollback` (the no-op quadrant; flag- or env-sourced alike), an unparseable path glob on `scan`/`rollback`, a `scan` PATH outside the repository root and a malformed `SOCKET_MIN_SEVERITY` or `SOCKET_MAX_NEW_PATCHES` (v5.0), `repair --offline --download-only`. Under `--json`, every usage error a command enforces itself prints the coded error on stdout (v5.0, MAJOR: `scan`, `remove` and `rollback` printed nothing there): a full envelope for the envelope commands, `{status: "error", error: {code, message}}` for `scan`, `get` and `rollback`; clap's own parse errors, and the path-flag check that runs before every command (`--cwd` / `--global-prefix` / `--manifest-path` naming nothing), still print nothing on stdout. `vex` also exits `2` on hard errors before document generation (see its tri-state table below). v5.0: `get`'s self-enforced conflicts exit `2` too (`--id`/`--cve`/`--ghsa`/`--package` multi-select, `--mode hosted\|vendored --save-only`, a malformed identifier for a forced `--id`/`--cve`/`--ghsa`) — previously `1` (MAJOR). The never-implemented `get --one-off` / `rollback --one-off` (and `SOCKET_ONE_OFF`) are removed in v5.0; `--one-off` is now an ordinary unknown-flag clap error. | `list` returns **`0`** for every project it can read, empty or not (**v5.0, BREAKING**: a project with no manifest and no ledger record — normal for hosted mode, which writes no manifest — used to exit `1` with `manifest_not_found`; it is now an empty list: `No patches in this project. Run \`socket-patch scan\`.` on stdout, and under `--json` the success envelope with `events: []`). Only an unreadable or invalid manifest (`manifest_unreadable` / `manifest_invalid`) exits `1`. Every lock-taking subcommand — including `scan`/`get --mode hosted` as of v5.0 — returns **`1`** with `error.code: lock_held` when another live socket-patch process holds `<.socket>/apply.lock`. diff --git a/crates/socket-patch-cli/src/args.rs b/crates/socket-patch-cli/src/args.rs index 2a8b5d6f6..5321d118b 100644 --- a/crates/socket-patch-cli/src/args.rs +++ b/crates/socket-patch-cli/src/args.rs @@ -357,8 +357,8 @@ pub struct GlobalArgs { impl GlobalArgs { /// Reject path flags that name nothing: `--cwd` and `--global-prefix` /// (flag or env) must be existing directories, and a `--manifest-path` - /// other than the default must sit in an existing project directory and - /// must not itself be a directory. `main` maps `Err` to the usage exit + /// other than the default must sit in an existing project directory. + /// `main` maps `Err` to the usage exit /// (2), the same exit a hosted/vendored `scan` PATH that is not a /// directory gets. /// @@ -399,16 +399,15 @@ impl GlobalArgs { )); } } + // A manifest path that names a directory is not refused here: that + // is a corrupt project, not a bad flag, and `read_manifest` refuses + // it as `manifest_unreadable` under every spelling of the path. A + // check keyed on the flag's text gave the explicit spellings exit 2 + // with no `--json` document instead (#1123). if self.manifest_path != DEFAULT_PATCH_MANIFEST_PATH { - let manifest = self.resolved_manifest_path(); - if manifest.is_dir() { - return Err(not_dir( - "--manifest-path", - "SOCKET_MANIFEST_PATH", - &manifest, - "is a directory, not a manifest file", - )); - } + // `.`-free for the message: `--cwd .` joined onto + // `./.socket/manifest.json` would print `././.socket/…`. + let manifest: PathBuf = self.resolved_manifest_path().components().collect(); // The directory that must exist: the project of a `.socket/` // manifest (`.socket/` itself is created on demand), else the // manifest file's own directory. diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs index 428ec63cb..5c16e2423 100644 --- a/crates/socket-patch-cli/src/commands/apply.rs +++ b/crates/socket-patch-cli/src/commands/apply.rs @@ -6,7 +6,7 @@ use socket_patch_core::crawlers::ruby_crawler::{ use socket_patch_core::crawlers::{ bun_uses_global_store, detect_npm_pkg_manager, Ecosystem, NpmPkgManager, RubyCrawler, }; -use socket_patch_core::manifest::operations::read_manifest; +use socket_patch_core::manifest::operations::{manifest_exists, read_manifest}; use socket_patch_core::manifest::schema::{PatchFileInfo, PatchManifest, PatchRecord}; use socket_patch_core::patch::apply::{ apply_package_patch, verify_file_patch, ApplyResult, MismatchPolicy, PatchSources, VerifyStatus, @@ -318,25 +318,7 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 { } return 0; } - Err(e) => { - let msg = format!( - "Patch check could not read the manifest ({e}); \ - treating it as drift (fail-closed)." - ); - if args.common.json { - let mut env = Envelope::new(Command::Apply); - env.dry_run = args.common.dry_run; - let mut err = crate::json_envelope::manifest_load_error(manifest_path, &e); - err.message = msg; - env.mark_error(err); - println!("{}", env.to_pretty_json()); - } else { - // Errors print even under --silent ("errors only", never - // "nothing"): exit 1 with no message would be undiagnosable. - eprintln!("Error: {msg}"); - } - return 1; - } + Err(e) => return refuse_unreadable_manifest(args, manifest_path, &e), }; // (purl_or_name, reason_code, detail) for each drift. @@ -868,6 +850,32 @@ fn manifest_targets_npm(manifest: &PatchManifest) -> bool { /// The yarn-berry PnP refusal's envelope error text. const YARN_PNP_UNSUPPORTED: &str = "yarn-berry Plug'n'Play layout is not supported by socket-patch (packages live inside .yarn/cache zips). Use `yarn patch ` instead."; +/// A manifest that exists but cannot be stated, refused before the API +/// client, the lock or telemetry are touched: the same envelope and exit +/// as the load failure further down (#931, #998). +/// `--check` says so as the drift it is treated as, whether the manifest +/// failed to stat here or to load in [`run_check`]. +fn refuse_unreadable_manifest(args: &ApplyArgs, manifest_path: &Path, e: &std::io::Error) -> i32 { + let mut err = crate::json_envelope::manifest_load_error(manifest_path, e); + if args.check { + err.message = format!( + "Patch check could not read the manifest ({e}); \ + treating it as drift (fail-closed)." + ); + } + if args.common.json { + let mut env = Envelope::new(Command::Apply); + env.dry_run = args.common.dry_run; + env.mark_error(err); + println!("{}", env.to_pretty_json()); + } else { + // Errors print even under --silent ("errors only", never + // "nothing"): exit 1 with no message would be undiagnosable. + eprintln!("Error: {}", crate::ui::sentence_case(&err.message)); + } + 1 +} + fn refuse_yarn_pnp(args: &ApplyArgs) -> i32 { refuse_yarn_pnp_with_skips(args, &[]) } @@ -921,7 +929,14 @@ pub async fn run(args: ApplyArgs) -> i32 { // every install). // Nothing below this gate is touched — no API client (its config read, // stderr advisory and org-slug round-trip), no lock, no `.socket/`. - if tokio::fs::metadata(&manifest_path).await.is_err() { + // Only a missing file is "no manifest": one apply cannot even stat + // (an unsearchable `.socket/`, `.socket` as a file, a symlink loop) + // fails closed, or every patch would be skipped with exit 0 (#998). + let manifest_present = match manifest_exists(&manifest_path).await { + Ok(present) => present, + Err(e) => return refuse_unreadable_manifest(&args, &manifest_path, &e), + }; + if !manifest_present { // A yarn-PnP layout refuses loudly even with no manifest: scan // cannot discover PnP packages (they live inside .yarn/cache zips), // so it never writes one — without this hoisted check the layout diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index 3965a9ee2..16a70cbcb 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -5,7 +5,7 @@ use socket_patch_core::ledgers::hosted_pins_matching; use socket_patch_core::manifest::cleanup_blobs::{ format_bytes, format_cleanup_result_for, ArtifactReferences, }; -use socket_patch_core::manifest::operations::{read_manifest, write_manifest}; +use socket_patch_core::manifest::operations::{manifest_exists, read_manifest, write_manifest}; use socket_patch_core::manifest::schema::PatchManifest; use socket_patch_core::patch::redirect::upstream::HostedPin; use socket_patch_core::telemetry::{track_patch_remove_failed, track_patch_removed, TelemetryAuth}; @@ -365,7 +365,21 @@ pub async fn run(args: RemoveArgs) -> i32 { // remove restores the global copies and drops their manifest records // only. let project_state = crate::commands::project_state_in_scope(&args.common); - let manifest_missing = tokio::fs::metadata(&manifest_path).await.is_err(); + // Only a missing file is "no manifest"; one remove cannot even stat is + // unreadable, not a bare project (#998). + let manifest_missing = match manifest_exists(&manifest_path).await { + Ok(present) => !present, + Err(e) => { + let err = crate::json_envelope::manifest_load_error(&manifest_path, &e); + emit_error_envelope( + args.common.json, + args.common.dry_run, + &err.code, + err.message, + ); + return 1; + } + }; let hosted_inventory = if project_state { crate::commands::hosted_inventory(&args.common, &args.common.project_root()).await } else { diff --git a/crates/socket-patch-cli/src/commands/repair.rs b/crates/socket-patch-cli/src/commands/repair.rs index 57dbf0ddd..929987f43 100644 --- a/crates/socket-patch-cli/src/commands/repair.rs +++ b/crates/socket-patch-cli/src/commands/repair.rs @@ -7,7 +7,7 @@ use socket_patch_core::api::client::{get_api_client_with_overrides, ApiClient}; use socket_patch_core::manifest::cleanup_blobs::{ format_all_in_use, format_cleanup_result_for, ArtifactReferences, CleanupResult, }; -use socket_patch_core::manifest::operations::read_manifest; +use socket_patch_core::manifest::operations::{manifest_exists, read_manifest}; use socket_patch_core::telemetry::{ track_patch_repair_failed, track_patch_repaired, TelemetryAuth, }; @@ -65,7 +65,31 @@ pub async fn run(args: RepairArgs) -> i32 { // for a ledger-less project, and that result is reused under the lock. let mut vendor_references: Option> = None; - if tokio::fs::metadata(&manifest_path).await.is_err() { + // Only a missing file is "no manifest"; one repair cannot even stat is + // unreadable, not a project with nothing to repair (#998). + let manifest_present = match manifest_exists(&manifest_path).await { + Ok(present) => present, + Err(e) => { + let err = crate::json_envelope::manifest_load_error(&manifest_path, &e); + // Recorded like the load failure in `repair_inner` (no client + // is built before this gate). + track_patch_repair_failed(&err.message, &TelemetryAuth::from_credentials(None, None)) + .await; + if args.common.json { + let env = error_envelope( + Command::Repair, + args.common.dry_run, + &err.code, + &err.message, + ); + println!("{}", env.to_pretty_json()); + } else { + eprintln!("Error: {}", crate::ui::sentence_case(&err.message)); + } + return 1; + } + }; + if !manifest_present { // Hosted mode leaves no local artifacts to repair: the lockfiles // point at patch.socket.dev URLs, not `.socket/vendor/...`, and // there is no manifest or vendor ledger. A project whose only trace diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 6f98e9957..fdda420c6 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -4,7 +4,7 @@ use socket_patch_core::api::client::{get_api_client_with_overrides, ApiClient}; use socket_patch_core::crawlers::{CrawlerOptions, Ecosystem}; use socket_patch_core::manifest::cleanup_blobs::ArtifactReferences; use socket_patch_core::manifest::operations::{ - get_before_hash_blobs, read_manifest, write_manifest, + get_before_hash_blobs, manifest_exists, read_manifest, write_manifest, }; use socket_patch_core::manifest::schema::{PatchFileInfo, PatchManifest, PatchRecord}; use socket_patch_core::patch::apply::select_installed_variants; @@ -1397,7 +1397,22 @@ pub async fn run(args: RollbackArgs) -> i32 { // not this run's to unwind (see `project_state_in_scope`): a global // rollback restores the global copies and their manifest records only. let project_state = crate::commands::project_state_in_scope(&args.common); - let manifest_missing = tokio::fs::metadata(&manifest_path).await.is_err(); + // Only a missing file is "no manifest"; one rollback cannot even stat + // fails like the load below, not as "Manifest not found" (#998). + let manifest_missing = match manifest_exists(&manifest_path).await { + Ok(present) => !present, + Err(e) => { + track_patch_rollback_failed(&e.to_string(), &telemetry).await; + let err = crate::json_envelope::manifest_load_error(&manifest_path, &e); + emit_rollback_error( + args.common.json, + args.common.dry_run, + &err.code, + &err.message, + ); + return 1; + } + }; let vendor_ledger_exists = project_state && tokio::fs::metadata(ledger_root.join(".socket/vendor/state.json")) .await diff --git a/crates/socket-patch-cli/src/commands/scan/gc.rs b/crates/socket-patch-cli/src/commands/scan/gc.rs index ab5eabcd7..a013aac58 100644 --- a/crates/socket-patch-cli/src/commands/scan/gc.rs +++ b/crates/socket-patch-cli/src/commands/scan/gc.rs @@ -3,7 +3,7 @@ //! read-only (preview) forms. use socket_patch_core::manifest::cleanup_blobs::{ArtifactReferences, CleanupResult}; -use socket_patch_core::manifest::operations::{read_manifest, write_manifest}; +use socket_patch_core::manifest::operations::{manifest_exists, read_manifest, write_manifest}; use socket_patch_core::manifest::schema::PatchManifest; use socket_patch_core::utils::purl_key::PurlKey; use socket_patch_core::vendor::{purl_keys_cover, VENDOR_STATE_REL}; @@ -56,7 +56,9 @@ pub(super) struct GcSummary { /// mutations already happened on disk, so the stale record is reported, /// not the pass failed. Also the vendored reverts' reinstall advisories /// (`vendor_bun_reinstall_required`, `vendor_vlt_reinstall_required`) - /// and no other revert warning. Serialized as additive + /// and no other revert warning. And `manifest_invalid` / + /// `manifest_unreadable` when the manifest half could not load the + /// manifest (#1063). Serialized as additive /// `warnings[]` on the apply shape only. warnings: Vec<(&'static str, String)>, } @@ -74,6 +76,19 @@ impl GcSummary { gc } + /// The vendored half alone, for a pass whose manifest exists but could + /// not be loaded: the manifest half did not run, and the load error + /// (`manifest_invalid` / `manifest_unreadable`) joins `warnings` + /// instead of the pass reading as "no manifest, nothing to prune". + fn manifest_unloadable(v: VendorGcSummary, manifest_path: &Path, e: &std::io::Error) -> Self { + let mut gc = GcSummary::vendor_only(v); + gc.warnings.push(( + crate::json_envelope::manifest_load_error_code(e), + crate::ui::manifest_error_message(manifest_path, e), + )); + gc + } + /// Fold a vendored-state GC pass into this summary: purl lists sorted, /// its failed post-revert rewrites join `warnings`. (`skipped` is this /// pass's alone — the vendored half runs under this pass's lock and @@ -231,9 +246,10 @@ pub(super) async fn run_apply_gc( // pristine checkout with neither a manifest nor a ledger must not gain // one. Either store alone is enough to proceed; the authoritative reads // happen under the lock. - let has_manifest = tokio::fs::metadata(manifest_path) - .await - .is_ok_and(|m| m.is_file()); + // A manifest that cannot be stated is not absent (#998): the pass goes + // on and reports it, as the locked read's load error or, when + // `.socket/` itself is unusable, as the lock failure. + let has_manifest = manifest_exists(manifest_path).await.unwrap_or(true); let has_ledger = tokio::fs::metadata(common.cwd.join(VENDOR_STATE_REL)) .await .is_ok_and(|m| m.is_file()); @@ -261,11 +277,13 @@ pub(super) async fn run_apply_gc( // Vendored-state GC FIRST (see the fn doc), under this guard. let vendor_gc = run_vendor_gc(common, manifest_path, /*dry_run=*/ false).await; - // Authoritative read under the lock. Missing or unreadable ⇒ nothing - // to prune, and the blob sweep has no referenced-set to work from. + // Authoritative read under the lock. Missing ⇒ nothing to prune, and + // the blob sweep has no referenced-set to work from; unreadable ⇒ the + // same, but said (#1063). let mut manifest = match read_manifest(manifest_path).await { Ok(Some(m)) => m, - _ => return GcSummary::vendor_only(vendor_gc), + Ok(None) => return GcSummary::vendor_only(vendor_gc), + Err(e) => return GcSummary::manifest_unloadable(vendor_gc, manifest_path, &e), }; let mut prunable = detect_prunable(&manifest, scanned_purls, vendored); let kept = keep_patched_cargo_copies(common, &manifest, socket_dir, &mut prunable).await; @@ -384,7 +402,8 @@ async fn preview_apply_gc( let mut manifest = match read_manifest(manifest_path).await { Ok(Some(m)) => m, - _ => return GcSummary::vendor_only(vendor_gc), + Ok(None) => return GcSummary::vendor_only(vendor_gc), + Err(e) => return GcSummary::manifest_unloadable(vendor_gc, manifest_path, &e), }; // Mirror the wet pass, which drops an unused vendored entry's manifest // keys before the blob sweep, or the preview under-reports orphans. @@ -939,6 +958,65 @@ mod tests { (manifest_path, socket_dir, blob_path) } + /// #1063: a manifest that exists but cannot be loaded is not "no + /// manifest". Both passes skip the manifest half (nothing is pruned or + /// swept without a referenced set) and say why with the shared load + /// code, instead of a clean summary that reads as nothing to prune. + #[tokio::test] + async fn apply_gc_reports_an_unloadable_manifest() { + for (seed, code) in [ + ( + (|p: &std::path::Path| std::fs::write(p, r#"{"patches": {"#).unwrap()) + as fn(&std::path::Path), + "manifest_invalid", + ), + ( + |p: &std::path::Path| std::fs::create_dir_all(p).unwrap(), + "manifest_unreadable", + ), + ] { + for dry_run in [true, false] { + let tmp = tempfile::tempdir().unwrap(); + let socket_dir = tmp.path().join(".socket"); + let blob = socket_dir.join("blobs").join("a".repeat(64)); + std::fs::create_dir_all(blob.parent().unwrap()).unwrap(); + std::fs::write(&blob, b"orphan?").unwrap(); + let manifest_path = socket_dir.join("manifest.json"); + seed(&manifest_path); + + let common = gc_common(tmp.path()); + let gc = if dry_run { + preview_apply_gc( + &common, + &manifest_path, + &socket_dir, + &HashSet::new(), + &no_vendored(), + ) + .await + } else { + run_apply_gc( + &common, + &manifest_path, + &socket_dir, + &HashSet::new(), + &no_vendored(), + ) + .await + }; + let codes: Vec<&str> = gc.warnings.iter().map(|(c, _)| *c).collect(); + assert_eq!(codes, vec![code], "dry_run={dry_run}: {gc:?}"); + assert!(gc.skipped.is_none(), "dry_run={dry_run}: {gc:?}"); + assert!(gc.pruned.is_empty(), "dry_run={dry_run}: {gc:?}"); + assert_eq!(gc.blobs.blobs_removed, 0, "dry_run={dry_run}: {gc:?}"); + assert!( + blob.exists(), + "dry_run={dry_run}: no sweep without a manifest" + ); + } + } + } + #[tokio::test] async fn preview_apply_gc_reports_blobs_of_prunable_entry() { // The package is not installed (empty scan), so its entry is prunable diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index bd85b661f..7effb167a 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -24,7 +24,7 @@ use futures_util::StreamExt; use socket_patch_core::api::client::get_api_client_with_overrides; use socket_patch_core::constants::SOCKET_DIR; use socket_patch_core::crawlers::{CrawlerOptions, Ecosystem}; -use socket_patch_core::manifest::operations::{read_manifest, write_manifest}; +use socket_patch_core::manifest::operations::{manifest_exists, read_manifest, write_manifest}; use socket_patch_core::manifest::schema::{PatchManifest, PatchRecord}; use socket_patch_core::patch::apply::{verify_file_patch, PatchSources}; use socket_patch_core::patch::redirect::upstream::HostedPin; @@ -954,6 +954,21 @@ pub async fn run(args: VendorArgs) -> i32 { let manifest_path = args.common.resolved_manifest_path(); let socket_dir = crate::args::socket_dir_of(&manifest_path, &args.common.cwd); + // Only a missing file is "no manifest": one vendor cannot even stat + // is refused, never reported as a calm `noManifest` exit 0 (#998). + // Revert needs only the vendor ledger, so it never asks. + let manifest_present = if args.revert { + false + } else { + match manifest_exists(&manifest_path).await { + Ok(present) => present, + Err(e) => { + let err = crate::json_envelope::manifest_load_error(&manifest_path, &e); + return emit_eject_refusal(&args.common, &err.code, &err.message); + } + } + }; + // Hosted pins are the input, even when an agent manifest also exists. // The raw lockfile inventory does not require VEX eligibility or an // installed copy of a patch. Revert needs only the vendor ledger. @@ -966,8 +981,7 @@ pub async fn run(args: VendorArgs) -> i32 { let refusal = inventory.contested_refusal(); let pins = hosted_pins_in_scope(&args.common, inventory.pins); if let Some(refusal) = refusal { - let has_manifest = tokio::fs::metadata(&manifest_path).await.is_ok(); - if !has_manifest || !pins.is_empty() { + if !manifest_present || !pins.is_empty() { return emit_eject_refusal(&args.common, "hosted_wiring_contested", &refusal); } } @@ -993,7 +1007,7 @@ pub async fn run(args: VendorArgs) -> i32 { // Preserve legacy manifest-only vendoring when there are no hosted // pins. A missing manifest is a no-op; detached vendored projects keep // their records in the ledger instead. - if !args.revert && tokio::fs::metadata(&manifest_path).await.is_err() { + if !args.revert && !manifest_present { // A requested `--vex` still attests what the `.socket/vendor` // ledgers and lockfiles already wire. Same contract as `apply --vex` // with no manifest: nothing referenced anywhere keeps exit 0; any diff --git a/crates/socket-patch-cli/tests/apply/apply_invariants.rs b/crates/socket-patch-cli/tests/apply/apply_invariants.rs index b2d8ca66b..ac613dc4f 100644 --- a/crates/socket-patch-cli/tests/apply/apply_invariants.rs +++ b/crates/socket-patch-cli/tests/apply/apply_invariants.rs @@ -491,10 +491,6 @@ fn apply_with_no_socket_dir_silent_emits_nothing() { /// which the install hook and CI both read as success. Fail closed. #[cfg(unix)] #[test] -#[ignore = "RED: apply's manifest probe is `tokio::fs::metadata(..).is_err()`, so \ - an UNREADABLE manifest is reported as the clean `noManifest` no-op \ - (exit 0) exactly like a missing one — the install hook and CI both \ - read that as success. The fail-closed fix was not part of this change."] fn apply_with_unreadable_socket_dir_fails_closed() { use std::os::unix::fs::PermissionsExt; diff --git a/crates/socket-patch-cli/tests/cli_parse_list.rs b/crates/socket-patch-cli/tests/cli_parse_list.rs index 741db07d7..e5c5d7682 100644 --- a/crates/socket-patch-cli/tests/cli_parse_list.rs +++ b/crates/socket-patch-cli/tests/cli_parse_list.rs @@ -380,14 +380,12 @@ fn missing_manifest_under_valid_cwd_is_not_an_error_via_binary() { } #[test] -fn manifest_path_is_existing_directory_is_a_usage_error_via_binary() { - // A `--manifest-path` naming a directory is never a manifest: v5.0 - // rejects it before the command runs, as a usage error (exit 2, the - // `Error:` line naming the flag), on every project command - // (`GlobalArgs::validate_paths`). Before, `list` reached `read_manifest` - // and failed with `manifest_unreadable` (exit 1); the I/O - // classification itself (a non-absence error is never - // `manifest_not_found`) is pinned on `read_manifest` in core. +fn manifest_path_is_existing_directory_is_manifest_unreadable_via_binary() { + // A `--manifest-path` naming a directory is a corrupt project, not a + // bad flag: `read_manifest` refuses it as `manifest_unreadable` (exit 1, + // with the `--json` envelope), the same verdict the default path gets. + // Refusing it in `GlobalArgs::validate_paths` made the verdict depend + // on whether the path was spelled out (#1123). let tmp = tempfile::tempdir().unwrap(); let manifest_path = tmp.path().join("manifest-is-a-dir"); std::fs::create_dir(&manifest_path).unwrap(); @@ -396,12 +394,12 @@ fn manifest_path_is_existing_directory_is_a_usage_error_via_binary() { tmp.path(), &["--json", "--manifest-path", manifest_path.to_str().unwrap()], ); + let stdout = String::from_utf8_lossy(&out.stdout); let stderr = String::from_utf8_lossy(&out.stderr); - assert_eq!(out.status.code(), Some(2), "stderr={stderr}"); - assert!( - stderr.contains("--manifest-path") && stderr.contains("is a directory"), - "{stderr}" - ); + assert_eq!(out.status.code(), Some(1), "stderr={stderr}"); + let v: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("stdout must be one envelope ({e}): {stdout}")); + assert_eq!(v["error"]["code"], "manifest_unreadable", "envelope: {v}"); } // --------------------------------------------------------------------------- diff --git a/crates/socket-patch-cli/tests/cli_path_flags_validated.rs b/crates/socket-patch-cli/tests/cli_path_flags_validated.rs index 2e01d22e7..df2c29420 100644 --- a/crates/socket-patch-cli/tests/cli_path_flags_validated.rs +++ b/crates/socket-patch-cli/tests/cli_path_flags_validated.rs @@ -121,8 +121,8 @@ fn nonexistent_global_prefix_is_a_usage_error_on_every_command() { } /// A `--manifest-path` in a directory that does not exist fails every -/// command with exit 2 (`list` used to say "No patches" and exit 0); one -/// naming a directory fails too. +/// command with exit 2 (`list` used to say "No patches" and exit 0). One +/// naming a directory is a manifest that cannot be read, not a bad flag. #[test] fn manifest_path_in_a_missing_directory_is_a_usage_error_on_every_command() { let tmp = tempfile::tempdir().unwrap(); @@ -142,13 +142,16 @@ fn manifest_path_in_a_missing_directory_is_a_usage_error_on_every_command() { ); assert!(stderr.contains("--manifest-path"), "{command:?}: {stderr}"); } + // A directory is an existing (unreadable) manifest, not a bad flag: + // `list` runs and refuses it (exit 1), as it does under the default + // path (#1123). let (code, _stdout, stderr) = run( tmp.path(), &["list"], &["--manifest-path", tmp.path().to_str().unwrap()], ); - assert_eq!(code, 2, "a directory is no manifest file: {stderr}"); - assert!(stderr.contains("is a directory"), "{stderr}"); + assert_eq!(code, 1, "a directory is an unreadable manifest: {stderr}"); + assert!(!stderr.contains("--manifest-path"), "{stderr}"); } /// Anti-vacuous controls: real directories still run, and a missing @@ -191,7 +194,9 @@ fn existing_paths_and_a_missing_manifest_file_still_run() { /// The commands that create the manifest keep creating its directory: a /// fresh checkout's `get --manifest-path state/patches.json` is no usage -/// error, and neither is `scan`. A directory in that position still is. +/// error, and neither is `scan`. A directory in that position is no usage +/// error either, but the run still fails: it is an unreadable manifest +/// (#1123). #[test] fn manifest_creating_commands_accept_a_manifest_path_in_a_new_directory() { let tmp = tempfile::tempdir().unwrap(); @@ -212,10 +217,11 @@ fn manifest_creating_commands_accept_a_manifest_path_in_a_new_directory() { command, &["--manifest-path", tmp.path().to_str().unwrap()], ); - assert_eq!( - code, 2, + assert_ne!( + code, 0, "{command:?}: a directory is no manifest file: {stderr}" ); + assert!(!stderr.contains("--manifest-path"), "{command:?}: {stderr}"); } } diff --git a/crates/socket-patch-cli/tests/manifest_load_error_matrix.rs b/crates/socket-patch-cli/tests/manifest_load_error_matrix.rs index 79f48b63f..fe912e751 100644 --- a/crates/socket-patch-cli/tests/manifest_load_error_matrix.rs +++ b/crates/socket-patch-cli/tests/manifest_load_error_matrix.rs @@ -7,6 +7,11 @@ //! different codes (`apply_failed`, `repair_failed`, `invalid_manifest`, //! `manifest_unreadable`, `manifest_invalid`). Nothing here reaches the //! network. +//! +//! #998: the commands that probe for a manifest before loading it must +//! read a stat error (`.socket` as a plain file, a symlink loop) as +//! unreadable, not as "no manifest". #1123: the verdict must not depend on +//! how the manifest path is spelled. use std::path::Path; @@ -15,13 +20,14 @@ use socket_patch_cli::args::{GLOBAL_ARG_ENV_VARS, LOCAL_ARG_ENV_VARS}; #[path = "common/hermetic.rs"] mod hermetic; -fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) { +fn run(cwd: &Path, args: &[&str], env: &[(&str, &str)]) -> (i32, String, String) { let mut cmd = hermetic::binary_command(); hermetic::scrub_extra(&mut cmd, &[hermetic::Extra::Venv]); cmd.args(args).current_dir(cwd); for var in GLOBAL_ARG_ENV_VARS.iter().chain(LOCAL_ARG_ENV_VARS.iter()) { cmd.env_remove(var); } + cmd.envs(env.iter().copied()); cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); // Unroutable: a case that reached the network would fail differently. cmd.env("SOCKET_API_URL", "http://127.0.0.1:1"); @@ -48,6 +54,7 @@ const MATRIX: &[(&[&str], &str, i32)] = &[ (&["apply", "--json", "--offline"], "apply", 1), (&["apply", "--check", "--json"], "apply", 1), (&["repair", "--json", "--offline"], "repair", 1), + (&["rollback", "--json", "--offline"], "rollback", 1), (&["vendor", "--json", "--offline"], "vendor", 1), (&["vendor", "--check", "--json"], "vendor", 1), (&["vex", "--json", "--output", "out.json"], "vex", 2), @@ -55,29 +62,69 @@ const MATRIX: &[(&[&str], &str, i32)] = &[ /// A project holding `package.json` plus whatever `seed` puts at /// `.socket/manifest.json`. -fn project(seed: impl Fn(&Path)) -> tempfile::TempDir { +fn project(seed: &dyn Fn(&Path)) -> tempfile::TempDir { + project_root(&|root| { + let socket = root.join(".socket"); + std::fs::create_dir_all(&socket).unwrap(); + seed(&socket.join("manifest.json")); + }) +} + +/// A project holding `package.json` plus whatever `seed` puts in its root. +fn project_root(seed: &dyn Fn(&Path)) -> tempfile::TempDir { let tmp = tempfile::tempdir().unwrap(); std::fs::write( tmp.path().join("package.json"), r#"{"name":"proj","version":"1.0.0"}"#, ) .unwrap(); - let socket = tmp.path().join(".socket"); - std::fs::create_dir_all(&socket).unwrap(); - seed(&socket.join("manifest.json")); + seed(tmp.path()); tmp } +/// How a run names the manifest (#1123): the default, or spelled out +/// through the flag (relative or absolute) or the environment. +#[derive(Clone, Copy, Debug)] +enum Spelling { + Default, + FlagRelative, + FlagAbsolute, + Env, +} + fn assert_matrix(seed: impl Fn(&Path), code: &str) { + assert_matrix_in(project, &seed, &[code], Spelling::Default); +} + +fn assert_matrix_in( + make: fn(&dyn Fn(&Path)) -> tempfile::TempDir, + seed: &dyn Fn(&Path), + codes: &[&str], + spelling: Spelling, +) { for (argv, command, exit) in MATRIX { // A fresh project per command: nothing one run leaves behind (an // apply.lock, a VEX file) can steer the next. - let tmp = project(&seed); - let (got_exit, stdout, stderr) = run(tmp.path(), argv); + let tmp = make(seed); + let absolute = tmp.path().join(".socket").join("manifest.json"); + let absolute = absolute.to_str().unwrap(); + let mut args: Vec<&str> = argv.to_vec(); + let mut env: Vec<(&str, &str)> = Vec::new(); + match spelling { + Spelling::Default => {} + Spelling::FlagRelative => args.extend(["--manifest-path", "./.socket/manifest.json"]), + Spelling::FlagAbsolute => args.extend(["--manifest-path", absolute]), + Spelling::Env => env.push(("SOCKET_MANIFEST_PATH", "./.socket/manifest.json")), + } + let argv = (&args, spelling); + let (got_exit, stdout, stderr) = run(tmp.path(), &args, &env); let v: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|e| { panic!("{argv:?}: stdout must be one JSON envelope ({e}): {stdout:?}\nstderr={stderr}") }); - assert_eq!(v["error"]["code"], code, "{argv:?}: {v}"); + assert!( + codes.iter().any(|c| v["error"]["code"] == *c), + "{argv:?}: expected one of {codes:?}: {v}" + ); assert_eq!(got_exit, *exit, "{argv:?}: {v}\nstderr={stderr}"); // A full envelope, not a bare `{status, error}`. assert_eq!(v["command"], *command, "{argv:?}: {v}"); @@ -112,3 +159,50 @@ fn unreadable_manifest_is_manifest_unreadable_on_every_command() { "manifest_unreadable", ); } + +/// #998: `.socket` is a plain file, so stating the manifest fails with +/// ENOTDIR. That is unreadable, not missing: `apply` and `vendor` used to +/// report `noManifest` and exit 0, the others `manifest_not_found`. Runs +/// as root, unlike a `chmod 000` fixture. Unix only: Windows reports a +/// path under a file as not found, so there it is no manifest. +#[cfg(unix)] +#[test] +fn socket_dir_as_file_is_manifest_unreadable_on_every_command() { + assert_matrix_in( + project_root, + &|root| std::fs::write(root.join(".socket"), "not a directory").unwrap(), + // `vendor --check` reads the vendor ledger under the same `.socket` + // first, and refuses that store just as closed. + &["manifest_unreadable", "vendor_state_unreadable"], + Spelling::Default, + ); +} + +/// #998: a manifest symlinked to itself fails stat with ELOOP. +#[cfg(unix)] +#[test] +fn manifest_symlink_loop_is_manifest_unreadable_on_every_command() { + assert_matrix( + |path| std::os::unix::fs::symlink("manifest.json", path).unwrap(), + "manifest_unreadable", + ); +} + +/// #1123: a directory at the manifest path gets the same verdict however +/// the path is spelled. A spelled-out path used to exit 2 from argument +/// validation with no `--json` document. +#[test] +fn directory_manifest_verdict_does_not_depend_on_spelling() { + for spelling in [ + Spelling::FlagRelative, + Spelling::FlagAbsolute, + Spelling::Env, + ] { + assert_matrix_in( + project, + &|path| std::fs::create_dir_all(path).unwrap(), + &["manifest_unreadable"], + spelling, + ); + } +} diff --git a/crates/socket-patch-core/src/manifest/operations.rs b/crates/socket-patch-core/src/manifest/operations.rs index 4e00a7ac9..74a680e2f 100644 --- a/crates/socket-patch-core/src/manifest/operations.rs +++ b/crates/socket-patch-core/src/manifest/operations.rs @@ -68,6 +68,19 @@ pub async fn read_manifest( parse_manifest(json).map(Some) } +/// Whether a manifest exists at `path`, by [`read_manifest`]'s rule: only +/// `NotFound` is absence. Any other stat error (an unsearchable `.socket/`, +/// `.socket` as a plain file, a symlink loop) is returned, so a caller's +/// "no manifest" path never runs for a manifest it merely cannot read. +/// Something that is not a regular file still counts as present: +/// [`read_manifest`] then refuses it, whatever the path's spelling. +/// +/// Windows reports a path under a plain file as not found, so there (as in +/// [`read_manifest`]) `.socket` as a file reads as no manifest. +pub async fn manifest_exists(path: impl AsRef) -> Result { + tokio::fs::try_exists(path.as_ref()).await +} + /// Write a manifest to the filesystem with pretty-printed JSON. /// /// The write is atomic: the JSON is staged in a sibling temp file, fsync'd, @@ -471,6 +484,51 @@ mod tests { ); } + // #998: `manifest_exists` keeps `read_manifest`'s rule. Only a missing + // file is absent; a stat error is returned, never read as "no manifest". + #[tokio::test] + async fn test_manifest_exists_only_not_found_is_absent() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("manifest.json"); + assert!(!manifest_exists(&path).await.unwrap()); + + write_manifest(&path, &create_test_manifest()) + .await + .unwrap(); + assert!(manifest_exists(&path).await.unwrap()); + + // A directory is present; `read_manifest` refuses it. + let as_dir = dir.path().join("dir").join("manifest.json"); + std::fs::create_dir_all(&as_dir).unwrap(); + assert!(manifest_exists(&as_dir).await.unwrap()); + assert!(read_manifest(&as_dir).await.is_err()); + } + + // `.socket` as a plain file: ENOTDIR, not NotFound (Windows reports + // such a path as not found, for both functions alike). + #[cfg(unix)] + #[tokio::test] + async fn test_manifest_exists_under_a_file_is_error() { + let dir = tempfile::tempdir().unwrap(); + let socket_file = dir.path().join("socket-file"); + std::fs::write(&socket_file, "x").unwrap(); + let under_file = socket_file.join("manifest.json"); + let err = manifest_exists(&under_file).await.unwrap_err(); + assert_ne!(err.kind(), std::io::ErrorKind::NotFound); + assert!(read_manifest(&under_file).await.is_err()); + } + + #[cfg(unix)] + #[tokio::test] + async fn test_manifest_exists_symlink_loop_is_error() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("manifest.json"); + std::os::unix::fs::symlink("manifest.json", &path).unwrap(); + let err = manifest_exists(&path).await.unwrap_err(); + assert_ne!(err.kind(), std::io::ErrorKind::NotFound); + assert!(read_manifest(&path).await.is_err()); + } + // Regression: write_manifest -> read_manifest must preserve the full record, // not merely the patch count. Guards against a serializer that drops nested // fields (file hashes, vulnerabilities) while still round-tripping the keys.