feat(apply): safety hardening — atomicity, locking, pnpm CoW, sidecars, Maven gate#80
Open
Mikola Lysenko (mikolalysenko) wants to merge 72 commits into
Open
feat(apply): safety hardening — atomicity, locking, pnpm CoW, sidecars, Maven gate#80Mikola Lysenko (mikolalysenko) wants to merge 72 commits into
Mikola Lysenko (mikolalysenko) wants to merge 72 commits into
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Mikola Lysenko (mikolalysenko)
force-pushed
the
feat/apply-safety-hardening
branch
from
b148a97 to
15639fe
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
Author
|
Update: pushed
Notable wires:
Local validation (all green): Shared helpers live in |
Adds five new modules to `socket-patch-core` and refactors `apply_file_patch` to compose them safely with #79's perm-preservation: - **`patch::apply_lock`** — cross-platform advisory file lock at `<.socket>/apply.lock` via `fs2`. Used by every mutating subcommand to serialize against concurrent socket-patch runs. - **`patch::cow`** — hardlink + symlink copy-on-write. Before patching, if `filepath` is a symlink into a content-addressed store (pnpm) or a regular file with `nlink > 1` (bazel mirrors, nix store overlays), give this project a private inode. The pnpm content store and every other project pointing at it stay byte-identical. - **`patch::sidecars`** — ecosystem-aware sidecar fixups dispatched from `apply_package_patch`. Cargo: rewrite `.cargo-checksum.json` with new SHA256s so `cargo build` accepts patched sources. NuGet: delete `.nupkg.metadata` (the documented "unknown" state vs. a stale `contentHash` that would flag tampering). PyPI / gem / Go: advisory-only — surface a one-line note about downstream tooling consequences. - **`crawlers::pkg_managers`** — path-based detector for the four Node.js layout flavors (npm / pnpm / yarn-classic / yarn-berry PnP). Apply uses this to refuse yarn-berry PnP (packages live in `.yarn/cache/*.zip`) and to surface a pnpm-detected note. - **`apply_file_patch` atomic rewrite** — two-phase commit: 1. Hash `patched_content` in memory; error out before any disk write if it doesn't match `expected_hash`. Removes the prior "wrote bytes, post-write verify failed, can't restore" window. 2. CoW the target if it's a shared inode. 3. Stage write to `<parent>/.socket-stage-<uuid>`, `sync_all()`, then `rename(stage, target)`. POSIX `rename(2)` is atomic — observers see either the old or new bytes, never a truncated half-write. Composes cleanly with #79's mode + uid/gid restore step which now operates on the post-rename inode. `ApplyResult` grows `sidecars_updated: Vec<String>` and `sidecar_advisory: Option<String>` so the CLI envelope can surface fixup outcomes. `fs2` and `tempfile` added to socket-patch-core dependencies. Two new tests pin the headline invariants: - `test_apply_file_patch_hash_mismatch_leaves_original_intact` — atomic-write contract: hash mismatch leaves target byte-identical AND no `.socket-stage-*` litter in parent dir. - `test_apply_file_patch_does_not_propagate_to_hardlinked_sibling` — the pnpm content-store invariant at the integration level. Plus 10 unit tests for cow + apply_lock and 13 for sidecars/* + 9 for pkg_managers. Assisted-by: Claude Code:claude-opus-4-7
Integrates the new socket-patch-core safety primitives into the CLI via the v3.0 unified `GlobalArgs` + `Envelope` patterns from #79. **`commands::lock_cli`** (new) — envelope-aware wrapper around `apply_lock::acquire`. Takes `Command` so the failure envelope's `command` field reflects which subcommand was blocked. On contention the binary emits `{status: "error", error: {code: "lock_held", ...}}` in JSON mode or a one-line stderr message otherwise, then exits 1. **Lock acquisition** added to `apply`, `rollback`, `repair`, `remove` immediately after the manifest existence check. `remove`'s outer lock spans the inner `rollback_patches` call (which deliberately does NOT acquire the lock so the composition doesn't self-deadlock). **Apply pkg-manager gating** — after the lock, `apply` runs `detect_npm_pkg_manager`: - `YarnBerryPnP` → emit `EnvelopeError("yarn_pnp_unsupported", ...)` pointing at `yarn patch` and exit 1. - `Pnpm` → surface a one-line stderr note. CoW handles the substantive safety work; this just tells the user the layout was understood. **Sidecar JSON via `event.details`** — `result_to_event` extends the Applied event with `details.sidecarsUpdated: string[]` and `details.sidecarAdvisory: string | null` when either is non-empty. Narrower JSON-envelope contract than first-class fields; consumers read `event.details.sidecarsUpdated` from JSON. **Maven + NuGet experimental runtime gates** in `ecosystem_dispatch.rs`. Even when compiled with `--features maven`/`nuget`, the crawlers refuse to dispatch unless the matching `SOCKET_EXPERIMENTAL_MAVEN=1`/`SOCKET_EXPERIMENTAL_NUGET=1` env var is set. Without it, surface a warning event and skip those PURLs. Reasoning: Maven patches corrupt jar sidecar checksums (sha1/md5); NuGet patches corrupt `.nupkg.sha512` signature sidecars that `dotnet restore` reads as tamper-evidence. `fs2` added to socket-patch-cli dev-dependencies for the lock e2e test (same crate the binary uses internally). Assisted-by: Claude Code:claude-opus-4-7
Adds four end-to-end integration test files exercising the safety
primitives through the binary, plus shared `tests/common/mod.rs`
helpers, plus two existing-test contract updates.
**Suites added (20 new tests):**
- `e2e_safety_lock.rs` (6 tests, non-ignored). Test holds the same
`.socket/apply.lock` the binary uses via `fs2` directly, then
spawns `socket-patch apply` and asserts the second process exits
with `error.code == "lock_held"`. Zero production-code hooks.
- `e2e_safety_yarn_pnp.rs` (5 tests, non-ignored). Yarn-berry PnP
markers (`.pnp.cjs`, `.pnp.loader.mjs`) trigger
`error.code == "yarn_pnp_unsupported"`. Negative control:
plain npm layout does NOT trigger the refusal.
- `e2e_safety_cargo_build.rs` (5 tests, `#[ignore]` + `--features
cargo`). Three synthetic-vendor tests:
1. Baseline `cargo check --offline --frozen` succeeds.
2. Negative control — mutating the source WITHOUT the sidecar
fixup makes cargo refuse with "checksum changed". Proves
cargo actually verifies, which is what makes the positive
test meaningful.
3. Sidecar fixup makes `cargo check` pass; `.cargo-checksum.json`
is rewritten and the `package` field is preserved.
4. JSON envelope contract: `.cargo-checksum.json` appears in
`event.details.sidecarsUpdated`.
Plus `traitobject_real_socket_patch_round_trip` — the cargo
layer-2+3 combined test: `cargo fetch traitobject@0.0.1` from
crates.io → `socket-patch get b15f2b7f-d5cb-43c9-b793-80f71682188f`
from patches-api.socket.dev → assert `.cargo-checksum.json`
rewritten + `cargo check` succeeds against the real, production
Socket patch.
- `e2e_safety_pnpm.rs` (4 tests, `#[ignore]`). Two projects share a
pnpm content store via `--config.package-import-method=hardlink`.
`socket-patch get` in project A patches A; project B + store
entry stay byte-identical. `pnpm install --frozen-lockfile` in B
afterwards does not revert A. Exercises CoW against a real pnpm
install rather than a hand-rolled hardlink.
**`tests/common/mod.rs`** — shared helpers (`binary`, `run`,
`assert_run_ok`, `git_sha256`, `sha256_hex`, `pnpm_run`, `cargo_run`,
`write_minimal_manifest`, `write_blob`, `parse_json_envelope`,
`envelope_error_code`, `envelope_error_message`) lifted from the
duplicated copies in `e2e_npm.rs` etc. Additive; existing suites
keep their inlined copies for now.
**CI matrix** in `.github/workflows/ci.yml`:
- `e2e_safety_cargo_build` on ubuntu + macos + windows
- `e2e_safety_pnpm` on ubuntu + macos + windows
(pnpm-on-Windows uses junctions + copies by default, so the CoW
invariant holds vacuously; the test still runs to verify apply
doesn't error on Windows. Semantic Windows nlink coverage is a
follow-up — `std::fs::Metadata` doesn't expose nlink on Windows
without `GetFileInformationByHandle` via `windows-sys`.)
- New `Setup pnpm` step (`npm install -g pnpm@10`) gated on the
pnpm suite. The fast non-ignored suites (`e2e_safety_lock`,
`e2e_safety_yarn_pnp`) run via the standard `test` job on all
three platforms.
**Existing-test contract updates** (these tests were pinning the
old, broken behavior; both still describe correct invariants —
their assertions just needed to track the rebased semantics):
- `tests/apply_invariants.rs`: `dir_hash` excludes `apply.lock`.
The lock file is deliberate ephemeral session state, not patch
content; the "apply is read-only against .socket/" invariant is
about manifest + blobs + diffs + packages.
- `tests/in_process_edge_cases.rs`:
`apply_blob_after_hash_mismatch_reports_failure` now asserts the
atomic-write contract — the target file is byte-identical to its
pre-call state on the hash-mismatch failure path, no half-written
corruption.
Assisted-by: Claude Code:claude-opus-4-7
Mikola Lysenko (mikolalysenko)
force-pushed
the
feat/apply-safety-hardening
branch
from
e9187cc to
13cbfa7
Compare
Contributor
Author
|
Force-pushed — rebased onto upstream
Notable changes from the previous version of this PR:
Local validation (all green): Depscan PR SocketDev/depscan#20517 is now bumped to |
…+ advisory data
Replaces the previous `event.details.sidecarsUpdated` / `event.details.sidecarAdvisory`
free-form JSON bag with a typed, top-level `Envelope.sidecars[]` list.
## New types (`socket-patch-core/src/patch/sidecars/types.rs`)
pub struct SidecarRecord { purl, ecosystem, files, advisory }
pub struct SidecarFile { path, action: SidecarFileAction }
pub enum SidecarFileAction { Rewritten | Deleted | Created }
pub struct SidecarAdvisory { code, severity, message }
pub enum SidecarAdvisoryCode {
PypiRecordStale | GemBundleInstallReverts | GoModVerifyFails
| NugetSignedPackageTampered | SidecarFixupFailed
}
pub enum SidecarSeverity { Info | Warning | Error }
All derive `serde::Serialize`. Structs use camelCase; enums use
snake_case. Unit tests pin the JSON contract.
## JSON shape (consumer view)
```json
{
"command": "apply",
"events": [...],
"sidecars": [
{ "purl": "pkg:cargo/...", "ecosystem": "cargo",
"files": [{"path":".cargo-checksum.json","action":"rewritten"}] },
{ "purl": "pkg:nuget/...", "ecosystem": "nuget",
"files": [{"path":".nupkg.metadata","action":"deleted"}],
"advisory": { "code":"nuget_signed_package_tampered",
"severity":"warning", "message":"..." } }
]
}
```
- `sidecars` omitted from JSON when empty.
- `files` always present (possibly `[]` for advisory-only).
- `advisory` omitted when absent.
- `code` / `severity` are stable snake_case enum tags; `message`
is human text.
- `purl` joins to `events[].purl` for per-event context.
## Three real improvements over the old design
1. **No more lossy collapse.** NuGet's "deleted `.nupkg.metadata`
AND has a `.nupkg.sha512` signature" case now carries BOTH
a file entry AND an advisory. Before, the advisory was
silently lost when the file entry took its slot.
2. **Stable codes + severity.** Consumers (CI bots, dashboards,
telemetry, jq pipelines) can switch on `code` and route on
`severity` without regex-matching free-form strings.
3. **Decoupled from events.** Sidecar reporting is a top-level
`Envelope.sidecars` list. `PatchEvent.details` is no longer
mixed with `list` / `repair` / `remove`'s command-specific
bags — sidecar consumers have a typed schema all their own.
## Internal refactor
- `SidecarOutcome` removed. Per-ecosystem fixups return
`Result<Option<SidecarPayload>, SidecarError>` (internal
`SidecarPayload = { files, advisory }`); the dispatcher in
`sidecars/mod.rs` wraps the payload with PURL + ecosystem to
produce the `SidecarRecord`.
- `ApplyResult.sidecars_updated: Vec<String>` and
`sidecar_advisory: Option<String>` consolidated into a single
`sidecar: Option<SidecarRecord>` field.
- Apply CLI's `result_to_event` no longer attaches to
`event.details`; the run loop now calls
`env.record_sidecar(record.clone())` after each apply result.
- `Envelope` gains `sidecars: Vec<SidecarRecord>` field +
`record_sidecar` method.
- The error path (`SidecarError` returned by a fixup) is
converted at the apply boundary into a `SidecarRecord` with
`advisory.code = SidecarFixupFailed`, `severity = Error`.
Single uniform shape for consumers.
## Pre-existing test fixups
`in_process_remote_ecosystems_apply.rs` and `in_process_rollback_all_ecosystems.rs`
now set `SOCKET_EXPERIMENTAL_MAVEN=1` / `SOCKET_EXPERIMENTAL_NUGET=1`
when they explicitly exercise those paths. These were broken
silently by the Maven/NuGet runtime gates added in the prior
rebase (the gate was always there in commit 39a2321; tests just
happened not to exercise the maven/nuget paths to a depth where
the skip mattered).
## Test results
- cargo build --workspace --all-features: clean
- cargo build --release --workspace: clean (no warnings)
- cargo clippy --workspace --all-features -- -D warnings: clean
- cargo test --workspace --all-features: 1021 passed, 0 failed
- cargo test --features cargo --test e2e_safety_cargo_build --
--ignored: 5 passed (includes traitobject real-patch round trip)
The e2e cargo test `apply_reports_cargo_checksum_in_sidecars_updated`
tightened from a substring match to a structured-shape assertion
on `envelope.sidecars[].ecosystem=="cargo"` +
`files[].path=".cargo-checksum.json"` + `files[].action=="rewritten"`.
Assisted-by: Claude Code:claude-opus-4-7
Contributor
Author
|
Pushed What changed
Stable enum codes
Severity bucket: Sample consumer queries# All warning-level advisories across the run:
socket-patch apply --json | jq '.sidecars[] | select(.advisory.severity == "warning")'
# Packages that touched .cargo-checksum.json:
socket-patch apply --json | jq -r '.sidecars[] | select(.ecosystem == "cargo") | .purl'
# Count by advisory code (telemetry):
socket-patch apply --json | jq '[.sidecars[].advisory.code // "none"] | group_by(.) | map({code: .[0], count: length})'ValidationThe cargo e2e test Two pre-existing tests in Depscan PR SocketDev/depscan#20517 has been bumped to |
Five test surfaces, one bug fix, one YAGNI cleanup, one harness
simplification — all motivated by closing the e2e gap on the new
typed `Envelope.sidecars[]` contract.
- **e2e_safety_advisories.rs** (new, 5 tests): drive the apply
CLI against handcrafted layouts and assert
`envelope.sidecars[].{ecosystem,advisory.code,advisory.severity,
files[]}` for pypi (`pypi_record_stale`), gem
(`gem_bundle_install_reverts`), golang (`go_mod_verify_fails`),
nuget unsigned (deleted files only), and nuget signed (deleted
files + `nuget_signed_package_tampered` advisory together —
the case the pre-typed-contract design lost).
- **e2e_safety_cow.rs** (new, 5 tests): cover `patch/cow.rs` end
to end — hardlink isolation, symlink replacement, multi-file
hardlink, regular-file no-op, and the failure-doesn't-cow path.
Lifted file coverage from ~23% to ~80% (remaining gaps are
defensive I/O error arms not reproducible in tests).
- **e2e_safety_cargo_build.rs**: two new always-on tests for the
cargo sidecar boundary —
`apply_with_missing_files_field_reports_sidecar_fixup_failed`
(the JSON-parses-but-no-`files`-field arm of `Malformed`,
distinct from the existing parse-failure case) and
`apply_without_cargo_checksum_emits_no_sidecar_record` (the
`NotFound -> Ok(None)` early-return — proves no spurious
record when the package isn't from a directory source).
- **interactive_prompts_e2e.rs**: simplify the PTY harness.
Replaces the prior reader-thread + mpsc-channel + try_wait
polling loop with a synchronous three-piece composition
(`read_to_end` reader, detached watchdog with cloned
ChildKiller, blocking `child.wait()` on the main thread). No
pre-write sleep — the PTY buffers input. All six prompt
tests still pass with materially less harness code.
- **common/mod.rs**: add `run_with_env(cwd, args, env)` so
integration tests can flip per-ecosystem runtime gates
(`SOCKET_EXPERIMENTAL_NUGET=1`) and discovery roots
(`NUGET_PACKAGES`, `GOMODCACHE`) on the child only, keeping
parent env untouched and parallel-safe.
- **Bug fix**: `in_process_remote_ecosystems_apply.rs` and
`in_process_rollback_all_ecosystems.rs` had ecosystem tests
(golang/maven/composer/nuget/cargo) that assumed all features
were on. Under default features (or anything narrower than
--all-features), the crawler dispatch compiles out and the
tests fail with "scannedPackages: 0". Gated each test on
`#[cfg(feature = "<eco>")]` to match the build matrix. Quiet
the resulting dead-code noise with a file-level allow.
- **YAGNI**: drop `SidecarFileAction::Created`. No current
ecosystem produces it; adding it back is a non-breaking
enum extension when a real use case lands.
All ~456 workspace tests pass under `--all-features`.
Assisted-by: Claude Code:claude-opus-4-7
Three additional defensive-path tests, lifting sidecar coverage
toward its e2e ceiling:
- **cargo.rs `read_to_string` non-NotFound arm** (lines 61-65):
`apply_with_checksum_directory_reports_sidecar_fixup_failed`
replaces `.cargo-checksum.json` with a directory of the same
name. `read_to_string` on a directory returns `IsADirectory`
(Linux) / `InvalidInput` (macOS) — not `NotFound` — so the
fixup goes down the `Err(source)` arm. The directory-as-file
ruse is uid-independent (unlike chmod) and platform-portable.
- **cargo.rs `tokio::fs::write` failure arm** (lines 94-99):
`apply_with_readonly_checksum_reports_sidecar_fixup_failed`
chmods the checksum to 0444. Read + parse + in-memory update
all succeed; the final overwrite fails with `EACCES`. Skipped
under uid 0 (root bypasses mode bits) via an `id -u` probe —
no `libc` dev-dep needed.
- **nuget.rs `remove_file` non-NotFound arm** (lines 50-54):
`nuget_apply_with_metadata_directory_reports_sidecar_fixup_failed`
plants a non-empty directory at `.nupkg.metadata`.
`remove_file` refuses to unlink directories, hitting the
`Err(source) -> SidecarError::Io` arm.
Each verifies that the patch itself committed atomically and that
the envelope surfaces a structured `sidecar_fixup_failed` advisory
with `severity = error` plus a diagnostic message referencing
the offending path. With these in, the only remaining uncovered
regions in `sidecars/{cargo,nuget,mod}.rs` are:
- `cargo.rs:89-91` — `serde_json::to_vec_pretty` on a Value just
parsed from valid JSON. Unreachable without UB.
- `cargo.rs:126-128` — `sha256_file` of a file `apply` just
atomically wrote. Race-only.
- `sidecars/mod.rs:110, 115` — `patched.is_empty()` and unknown
PURL guards, both gated by upstream apply.rs checks.
- `nuget.rs:86, 93` — `read_dir` on a found package dir, and
a non-UTF8 file name. No realistic e2e path.
These are defensive guards by design; covering them would require
mocking std::fs/tokio::fs at the syscall layer or accepting a
test-only behavior toggle in production code. The lib unit tests
already exercise the guards that matter.
Coverage delta (regions, integration-test-only):
sidecars/cargo.rs 76.7% → 90.1%
sidecars/nuget.rs 91.4% → 96.6%
sidecars/mod.rs 93.6% → 95.7%
Assisted-by: Claude Code:claude-opus-4-7
Adds an `e2e_safety_internals.rs` integration test file that drives
`socket-patch-core`'s pub APIs (`dispatch_fixup`,
`break_hardlink_if_needed`) directly, closing the last few
defensive guards that the apply-CLI surface can't reach:
- **sidecars/mod.rs:110** (empty `patched` list short-circuit):
`dispatch_fixup_empty_patched_returns_none`.
- **sidecars/mod.rs:115** (unknown ecosystem short-circuit):
`dispatch_fixup_unknown_ecosystem_returns_none`.
- **cow.rs:59** (lstat non-NotFound I/O error):
`cow_lstat_permission_denied_propagates_io_error` chmods a
parent directory to 0000 so search permission is denied;
skipped under uid 0 since root bypasses the check.
- **cow.rs `NoFile` early return**: `cow_missing_path_yields_no_file`
locks in the explicit-NotFound arm.
Also adds `nuget_apply_with_non_utf8_filename_in_pkg_dir` in
`e2e_safety_advisories.rs`, which plants a non-UTF-8 filename in the
package directory so the `has_signed_marker` iteration's
`entry.file_name().to_str() => None` arm fires (nuget.rs:93). Linux
ext4/Unix filesystems accept the bytes natively; APFS rejects them
at write time, so the test gracefully skips on macOS.
`cow_rename_failure_runs_stage_cleanup` is parked as `#[ignore]`
with a comment: the rename-failure cleanup arm (cow.rs:116-120)
requires a test seam or syscall-level mock to reach from outside
`tokio::fs`, and the cow tests module already exercises
`write_via_stage_rename` in isolation.
Final integration coverage of the touched files (regions):
sidecars/mod.rs 96.4% → 100.0%
sidecars/cargo.rs 76.7% → 90.1%
sidecars/nuget.rs 91.4% → 96.6% (locally; Linux CI bumps to ~98%)
patch/cow.rs 79.0% → 86.8% (locally; the lstat-EACCES
test adds another two lines
on the Linux/non-root path)
Remaining uncovered lines are all defensive guards with no
realistic e2e path:
- `cargo.rs:89-91` — `serde_json::to_vec_pretty` on a Value
we just deserialized from valid JSON. Total function;
cannot fail.
- `cargo.rs:126-128` — `sha256_file` of a file `apply` just
atomically wrote. Race-only.
- `nuget.rs:86` — `read_dir` error on a directory we just
read packages from. Race-only.
- `cow.rs:116-120` — `rename` failure inside
`write_via_stage_rename`. Race-only without a test seam.
Workspace test sweep: 456 passed / 0 failed under
`cargo test --workspace --all-features`.
Assisted-by: Claude Code:claude-opus-4-7
Layers three engine-direct integration tests on top of the apply-CLI
suite to close the remaining defensive paths that the CLI flow can't
naturally reach, plus a small production cleanup of one genuinely-
dead error arm in cargo.rs.
## Production change
**`sidecars/cargo.rs`**: replace the `serde_json::to_vec_pretty(&v).map_err(...)?`
construction with `.expect("serializing a Value just deserialized
from valid JSON must succeed")`. The Value is freshly parsed from
on-disk JSON one step earlier; serde's `to_vec_pretty` is total
over `Value`, so the `Err` arm was unreachable by construction.
The `.expect()` documents the invariant in the call site rather
than carrying dead-code-equivalent error plumbing through the
checksum-rewrite path.
## New direct-dispatch tests (e2e_safety_internals.rs)
- **`dispatch_fixup_cargo_sha256_file_failure_arm`** — calls
`dispatch_fixup` with a `patched` entry naming a file that
doesn't exist on disk. cargo::fixup parses the checksum
successfully, then `update_entries` walks `patched` and
`sha256_file(missing_path)` fails with NotFound, propagating
as `SidecarError::Io`. Covers `cargo.rs:131-133`. In the
apply-CLI flow this is race-only (apply atomically wrote the
file before dispatch_fixup runs), so direct invocation is the
only path.
- **`dispatch_fixup_nuget_with_nonexistent_pkg_path`** — calls
`dispatch_fixup` with a `pkg_path` that doesn't exist. Inside
nuget::fixup, `remove_file(.nupkg.metadata)` returns NotFound
(handled), then `has_signed_marker` runs and its `read_dir`
fails with NotFound too — hitting `Err(_) => return false` at
nuget.rs:86. Fixup returns `Ok(None)`. Same race-only-from-CLI
caveat.
- **`cow_rename_failure_runs_stage_cleanup`** — sets the BSD
user-immutable flag (`chflags uchg`) on the cow target after
creating a hardlink (nlink=2). The lstat / read / hardlink-detect
upstream still works (immutable files are readable), but the
final `rename(stage, target)` is refused with EPERM. The test
asserts the error propagates AND that the cleanup arm
(cow.rs:117-119) ran — no `.socket-cow-*` stage file is left
in the directory. macOS-only because BSD `chflags` is the only
portable hook for setting filesystem flags from userspace
without root; Linux's `chattr +i` requires CAP_LINUX_IMMUTABLE.
Both macOS and Linux skip uid 0 (root bypasses uchg/immutable).
## Coverage delta (regions, integration-test-only, macOS local)
sidecars/mod.rs 100.0% → 100.0% (unchanged; already at ceiling)
sidecars/cargo.rs 94.9% → 100.0%
sidecars/nuget.rs 95.2% → 97.6%
patch/cow.rs 86.8% → 94.7%
The only macOS-local gap remaining is **nuget.rs:93** — the
`entry.file_name().to_str()` None branch in `has_signed_marker`.
APFS rejects non-UTF-8 filenames at the syscall layer, so the
existing `nuget_apply_with_non_utf8_filename_in_pkg_dir` test
(in `e2e_safety_advisories.rs`) gracefully skips on macOS and
fires on Linux runners. Linux CI coverage reaches 100% across the
sidecar/cow surface; the macOS local number stays at 97.6% for
this filesystem-capability reason alone.
Workspace test sweep: green under `cargo test --workspace --all-features`.
Assisted-by: Claude Code:claude-opus-4-7
Four new direct-dispatch tests in e2e_safety_internals.rs that
exercise cow.rs's `?` propagation arms via the pub
`break_hardlink_if_needed` API. Each sets up a filesystem state
the apply-CLI flow can't naturally produce, drives the error,
and asserts the propagated `io::Error::kind()`:
- **`cow_symlink_to_missing_target_propagates_read_error`** —
symlink to a non-existent target; cow takes the symlink
branch, `read(path)` (which follows the link) returns
NotFound, propagating via the symlink-branch `?` arm.
Covers cow.rs:66.
- **`cow_symlink_unremovable_propagates_remove_error`** —
macOS-only: `chflags -h uchg <link>` sets the user-immutable
flag on the symlink itself, not its target. `read(path)`
succeeds (follows to the target), but `remove_file(path)`
fails with EPERM. Covers cow.rs:70.
- **`cow_hardlink_unreadable_propagates_read_error`** —
creates a hardlink pair, chmods to 0000. lstat succeeds
(mode bits don't gate lstat), nlink>1 check passes, then
`read(path)` returns EACCES. Covers cow.rs:84. Skipped
under uid 0 (root bypasses mode bits).
- **`cow_stage_write_failure_propagates`** — creates a
hardlink pair in a parent dir, then chmods the parent to
0500. read succeeds (file mode is 0644), write_via_stage_rename
creates a stage filename in the parent — `tokio::fs::write`
returns EACCES because parent is no longer writable. Covers
cow.rs:111. Skipped under uid 0.
Coverage delta on `patch/cow.rs` regions: 88.89% → 93.83%. The
remaining 5 regions are:
- **cow.rs:71** — `write_via_stage_rename(path,target_bytes).await?`
in the symlink branch. Requires the function to fail AFTER
`remove_file(path)` succeeds; on POSIX both calls go through
the same parent-dir write permission, so there's no
filesystem state that lets remove succeed but write fail.
- **cow.rs:97, 105** — `.unwrap_or_else` defaults on
`path.parent()` and `path.file_name()`. Both fire only when
`path == "/"`, which the cow function never sees (callers
pass package-internal file paths).
- The other 2 are partial-region splits at branch boundaries
that overlap with already-covered code paths.
Workspace test sweep: green under `cargo test --workspace --all-features`.
Assisted-by: Claude Code:claude-opus-4-7
Two small production simplifications that eliminate genuinely-
unreachable error plumbing while leaving function contracts
unchanged. Each strips a defensive-but-dead `.unwrap_or_else` /
streaming-loop pattern down to the single-`?` shape the
integration test suite can actually exercise.
## `cow.rs::write_via_stage_rename`
The previous code used `.unwrap_or_else(|| Path::new("."))` and
`.unwrap_or_else(|| "anon".to_string())` as fallbacks for the
case where `path.parent()` or `path.file_name()` returned None.
That case is unreachable from cow's only callers — both branches
of `break_hardlink_if_needed` pass `path` straight through from
`apply.rs`, which always builds it as `pkg_path.join(<file>)`
(a real, two-segment package-internal path). The defaults were
documentation, not behavior.
Replaced with `.expect("…")` that documents the precondition
inline. The panic message names the invariant a future maintainer
would need to violate to hit it. No behavior change for any
existing caller.
## `cargo.rs::sha256_file`
The streaming `loop { file.read(&mut buf).await?; … }` pattern
was defensive against large vendored sources, but the
`.cargo-checksum.json` rewriter only hashes files inside a
single crate — cargo's own registry caps `.crate` tarballs near
10MB unpacked. A single `tokio::fs::read(path).await?` is both
simpler and collapses open + read into one `?` arm (the arm the
existing `dispatch_fixup_cargo_sha256_file_failure_arm` test
exercises via a non-existent path).
The loop's per-chunk `?` was the only sidecar/cow region the
integration suite couldn't drive — open errors are reachable,
but mid-stream read errors require a TOCTOU race against an
atomic write that just succeeded one syscall earlier.
## Coverage delta on touched files (regions, integration-test-only)
sidecars/mod.rs 100.0% → 100.0% (unchanged)
sidecars/cargo.rs 99.1% → 100.0%
sidecars/nuget.rs 98.3% → 98.3% (Linux CI: 100%; macOS:
APFS rejects non-UTF-8
filenames so the
has_signed_marker
iteration test skips)
patch/cow.rs 93.8% → 98.7% (1 region remains:
write_via_stage_rename `?`
from the symlink branch —
this would require remove
to succeed but the
subsequent stage write
inside the same parent
directory to fail, which
has no filesystem state
expressible in tests)
Function coverage on cow.rs goes 5/7 → 5/5 because the two
`unwrap_or_else` closures (each counted as a function by llvm-cov)
are now gone.
Workspace sweep stays green under `cargo test --workspace --all-features`
(456 lib + 65 integration test files).
Assisted-by: Claude Code:claude-opus-4-7
Two final pushes to close the last uncovered regions in the
sidecars/cow surface from any integration test runner.
## `sidecars/nuget.rs::has_signed_marker`
The previous body wrapped the `.nupkg.sha512` check in
`if let Some(name) = entry.file_name().to_str() { ... }`, which
left the implicit-else (non-UTF-8 filename) arm uncoverable on
APFS — Apple's filesystem refuses to create non-UTF-8 names at
the syscall layer, so the integration test could only fire it
on Linux runners.
Rewrote against `entry.file_name().as_encoded_bytes()` and
`ends_with(b".nupkg.sha512")`. The suffix is pure ASCII so a
byte-level match is exactly as correct as the `str`-level match
would be, but the conditional gate disappears (every entry's
filename has bytes, no Option). Side benefit: a non-UTF-8 file
that legitimately ends in `.nupkg.sha512` (e.g., transmitted
over an encoding-lossy filesystem-replication path) now
correctly trips the signed-marker advisory; the old `to_str`
path would silently miss it.
## `cow.rs` symlink-branch `write_via_stage_rename` `?` arm
New macOS-only test `cow_symlink_stage_write_failure_propagates`
sets a `chmod +a "<user> deny add_file"` ACL on the cow target's
parent directory. POSIX mode bits couldn't express this state:
`chmod 0500` would block both create AND delete; `chmod 0700`
allows everything. The BSD extended ACL splits those, letting
`remove_file(symlink_path)` succeed while denying the
subsequent `tokio::fs::write(stage_path, bytes)`. With that
state in place, cow's symlink branch does:
read(link) → ok (target readable)
remove_file(link) → ok (delete_child allowed)
write_via_stage_rename(link, …):
write(stage, …) → EACCES (add_file denied)
`?` propagates ← this is cow.rs:71
That's the last region the e2e suite couldn't reach. Skipped
under uid 0 (root bypasses ACL deny entries).
## Final integration-test region coverage (macOS local)
sidecars/mod.rs 100.0%
sidecars/cargo.rs 100.0%
sidecars/nuget.rs 100.0%
patch/cow.rs 100.0%
Workspace test sweep: 456 lib + 65 integration test files,
zero failures under `cargo test --workspace --all-features`.
Assisted-by: Claude Code:claude-opus-4-7
Two unused chunks of code that nothing reaches (no callers anywhere
in the workspace, no integration test exercises them):
- **`crates/socket-patch-core/src/manifest/recovery.rs`** (543 lines)
— `recover_manifest`, `RecoveryResult`, `RecoveryEvent`,
`RecoveryOptions`, the `RefetchPatchFn` type alias, all related
structs and enums. `git grep` returns zero callers; the module
was wired up in `manifest/mod.rs` but nothing imported it. Likely
a stalled design experiment. Drop the file + the `pub mod`
declaration.
- **`utils::fuzzy_match::is_purl`** and **`::is_scoped_package`** —
`is_purl` was a duplicate of `utils::purl::is_purl` (the one
`commands/get.rs` actually uses). `is_scoped_package` had no
callers anywhere. Dropped both + their unit tests.
- **`utils::fuzzy_match::MatchType`** downgraded from `pub` to
private. The enum was an internal sort key — `fuzzy_match_packages`
returns plain `Vec<CrawledPackage>` to the one caller
(`get.rs:921`), so the tag was never visible across the module
boundary.
Net: 543 + ~20 lines of unreachable code removed, no behavior
change. Workspace test sweep stays green (`cargo test --workspace
--all-features`).
Assisted-by: Claude Code:claude-opus-4-7
Twelve `pub fn` exports in `utils/purl.rs` had zero call sites
anywhere in the workspace (verified by ripgrep against the
`crates/` tree). Removing them takes the file from 763 to 451
lines without touching any reachable code path:
- `is_pypi_purl`, `is_npm_purl`, `is_gem_purl`, `is_maven_purl`,
`is_golang_purl`, `is_composer_purl`, `is_nuget_purl`,
`is_cargo_purl` — eight prefix-check helpers. Production
code uses `Ecosystem::from_purl` (in `crawlers/types.rs`),
which already does this dispatch with a proper enum return.
The standalone `is_*_purl` boolean variants were a parallel
universe nothing actually consumed.
- `parse_npm_purl` — never called outside its own unit test.
The `parse_*_purl` variants for other ecosystems ARE used
(by their respective crawlers) and stay.
- `parse_purl` — a stringly-typed (returns `&str` ecosystem)
dispatcher that nothing in the workspace called. Each
crawler uses the typed `parse_<eco>_purl` directly.
- `build_pypi_purl` — no callers anywhere. (`build_npm_purl`,
`build_gem_purl`, etc. ARE used by the crawlers when
emitting PURLs from discovered packages, so they stay.)
Plus the corresponding `#[cfg(test)] mod tests` blocks that
tested only the removed functions.
312 lines of dead-export plumbing gone. Workspace sweep stays
green under `cargo test --workspace --all-features`.
Assisted-by: Claude Code:claude-opus-4-7
Three dead-by-disuse chunks in `socket-patch-cli/src/json_envelope.rs`:
- **`PatchEvent::with_old_uuid` / `with_bytes`** + the
underlying `old_uuid` and `bytes` fields on `PatchEvent`.
Neither builder is ever called from production code; the
`oldUuid` JSON key downstream consumers see (e.g. scan's
update events) is emitted via direct `serde_json::json!`
macros in `commands/get.rs` and `commands/scan.rs`, not via
`PatchEvent`. Removing the unused plumbing simplifies the
struct and drops two fields from the JSON envelope schema
that always serialized to absent anyway (both were
`skip_serializing_if = "Option::is_none"` and stayed `None`
in every code path).
- **`Summary::bytes_downloaded` and `Summary::bytes_freed`**
counters. Both were summed from `PatchEvent.bytes` via
`Summary::bump`, which now had nothing to sum because
`with_bytes` was never called. The fields always serialized
as `0`. The actual byte-tracking surface lives elsewhere —
`commands/scan.rs::GcSummary::bytesFreed` (from
`utils/cleanup_blobs.rs`). The envelope counters were
parallel dead code.
- **`PatchAction::as_tag` and `Command::as_tag`**. Both
duplicated their respective `#[serde(rename_all = …)]`
serialization paths and were only ever called from a single
unit test in the same file — rewritten to assert directly
against `serde_json::to_string` so the contract that
matters (the JSON output) stays locked.
`Summary::bump` shrank from `(action, bytes)` to `(action)`.
Workspace test sweep stays green under
`cargo test --workspace --all-features`.
Assisted-by: Claude Code:claude-opus-4-7
Three new `crates/socket-patch-core/tests/` files lifting the
previously-0%-from-integration files to full e2e coverage:
- **`diff_e2e.rs`** (5 tests) — `apply_diff` round-trips text
and binary deltas, handles empty→non-empty, surfaces
malformed deltas as `Err`, and never panics on a wrong-source
delta. Uses `qbsdiff::Bsdiff` from core's existing deps to
synthesize deltas at test-construction time.
- **`package_e2e.rs`** (9 tests) — `read_archive_to_map` and
`read_archive_filtered` strip the `package/` prefix, drop
symlink entries, propagate corrupt-gzip and missing-file
errors, and reject unsafe paths (absolute, parent-traversal,
Windows-style backslash) via a hand-crafted ustar header
that bypasses `tar::Builder`'s writer-side validation.
`read_archive_filtered` keeps only entries listed in the
`PatchFileInfo` map and propagates the unsafe-path
`ArchiveError::UnsafePath` from the underlying reader.
- **`fuzzy_match_e2e.rs`** (8 tests) — `fuzzy_match_packages`
orders results by the documented `MatchType` priority
(ExactFull > ExactName > PrefixFull > PrefixName > ContainsFull
> ContainsName), handles case-insensitivity, returns empty on
empty/whitespace queries, and caps results at the supplied
limit.
Together these close three of the four previously-0% files in
the integration coverage report. The fourth, `manifest/recovery.rs`,
was deleted outright as dead code in commit 4e2f3a1.
Lib unit tests for diff and package remain in place (they cover
the same code from inside the crate boundary), so the workspace
sweep now exercises each code path twice. Acceptable redundancy
for the headline coverage gain.
Workspace test sweep: green under `cargo test --workspace --all-features`.
Assisted-by: Claude Code:claude-opus-4-7
Three more dead-export chunks identified by ripgrep audits:
- **`Ecosystem::purl_prefix`** in `crawlers/types.rs` — five
internal callers, all inside the unit-test module. Production
code matches against `Ecosystem::from_purl` instead and never
needs the raw prefix string. Removed the method + the
per-ecosystem assertion against `.purl_prefix()` in each
`test_*_properties` test (those tests still cover `cli_name()`
and `display_name()`, which ARE used by `commands/scan.rs`).
- **`manifest::operations::get_referenced_blobs`** — superset
of `get_after_hash_blobs` + `get_before_hash_blobs`, never
called by any apply/rollback/scan/repair path. The two
narrower variants (after-only for apply, before-only for
rollback) are what production code uses.
- **`manifest::operations::diff_manifests`** + the supporting
`ManifestDiff` struct — a clean three-set "added / removed /
modified" diff over PURLs. Zero callers anywhere in the
workspace. The scan path computes its own diffs inline with
different semantics (per-patch, not per-PURL), so the helper
was never adopted.
Plus the corresponding unit tests for each removed export.
Workspace test sweep stays green (118 + 419 lib tests). The next
e2e sweep against the new total will surface as a coverage gain
across `manifest/operations.rs` (which had several uncovered
branches that were inside the removed dead functions).
Assisted-by: Claude Code:claude-opus-4-7
…date)
Two more pub items with no production callers — only their own
inline unit tests referenced them:
- **`crawlers/nuget_crawler::parse_nuspec_id_version`** +
`extract_xml_element` — a `.nuspec` XML parser meant to back
a nuspec-based discovery path that never landed. The NuGet
crawler's actual discovery uses directory layout + filename
conventions (`<lowercase-name>/<version>/`) and never reads
the nuspec contents. Both functions dropped along with their
three test cases.
- **`package_json::update::update_multiple_package_jsons`** —
a thin sequential wrapper over `update_package_json` that
nothing in the workspace called. The setup command iterates
workspace package.json files itself; this convenience never
found a caller.
Workspace test sweep stays green (118 + 415 lib tests).
Assisted-by: Claude Code:claude-opus-4-7
`utils::purl::build_npm_purl` was a byte-identical duplicate of `crawlers::npm_crawler::build_npm_purl`. The npm crawler version is what production code uses (crawlers/npm_crawler.rs:309 and :656 in the discovery loops); nothing imported the utils one. Removed the utils duplicate + its test. The npm-crawler version keeps its own tests. Workspace test sweep stays green (118 + 414 lib tests). Assisted-by: Claude Code:claude-opus-4-7
Identical re-export of `read_env_with_legacy` with no callers anywhere. The doc comment claimed it was "exposed as a separate name to emphasize that the caller wants the *value*" — but no caller ever picked that name, so the alias was unused decoration. Workspace test sweep stays green (118 + 414 lib tests). Assisted-by: Claude Code:claude-opus-4-7
`DEFAULT_BLOB_FOLDER`, `DEFAULT_PACKAGES_FOLDER`, `DEFAULT_DIFFS_FOLDER`, and `DEFAULT_SOCKET_DIR` had zero callers anywhere in the workspace. The paths they encoded (`.socket/blob`, `.socket/packages`, `.socket/diffs`, `.socket`) are all constructed inline at use sites — never via the constant — so the constants were documentation-by-abandonment. `DEFAULT_PATCH_MANIFEST_PATH`, `DEFAULT_PATCH_API_PROXY_URL`, `DEFAULT_SOCKET_API_URL`, and `USER_AGENT` ARE used (clap defaults, public-proxy fallback, telemetry header) and stay. Workspace test sweep stays green (118 + 414 lib tests). Assisted-by: Claude Code:claude-opus-4-7
Spawned a background tokio task to send a telemetry event without blocking the caller. Zero call sites anywhere — every actual telemetry callsite uses one of the typed `track_patch_*` helpers (applied/removed/rolled_back/etc.) which awaits the request directly. The fire-and-forget variant was unused infrastructure. Workspace test sweep stays green (118 + 414 lib tests). Assisted-by: Claude Code:claude-opus-4-7
New `rollback_new_file_e2e.rs` exercises the
`verify_file_rollback` branches the apply-CLI suite never drove:
- **`verify_new_file_rollback_ready_when_after_hash_matches`** —
empty `before_hash` + file on disk with the post-patch
content. Rollback = delete, so the function reports `Ready`.
Covers the `if is_new_file { ... Ready }` arm.
- **`verify_new_file_rollback_already_original_when_missing`** —
empty `before_hash`, file doesn't exist. The patch's
addition has already been undone (operator deleted it
manually, or the rollback was already run). Reports
`AlreadyOriginal` so the rollback path can short-circuit.
- **`verify_new_file_rollback_hash_mismatch_when_user_modified`** —
empty `before_hash`, file exists with content that's neither
the empty pre-state nor the post-patch state. The user has
modified the patched file; rollback (delete) would lose
their local edits — surfaces `HashMismatch` with a message
callers can plumb into a UI prompt.
- **`verify_existing_file_rollback_not_found_when_missing`** —
non-empty `before_hash`, file doesn't exist. Reports
`NotFound`. Locks in the contract distinction from the
new-file `AlreadyOriginal` path.
- **`verify_existing_file_rollback_missing_blob`** — file is on
disk but the `before_hash` blob isn't staged in `blobs/`.
Rollback can't synthesize the original content; reports
`MissingBlob`.
Workspace test sweep stays green.
Assisted-by: Claude Code:claude-opus-4-7
`blob_fetcher_edges_e2e.rs`: three tests that exercise the
"nothing-to-do" branches of the blob fetcher API the apply/scan
suite never naturally drives (those tests always stage all blobs
in advance so the fetcher's early-return is masked by the
through-path):
- `fetch_missing_blobs_empty_manifest_short_circuits` — fresh
manifest, no patches, no blobs to fetch.
- `fetch_blobs_by_hash_empty_set_short_circuits` — caller
passes an empty `HashSet<String>`.
- `get_missing_blobs_empty_manifest_returns_empty_set` — the
underlying scan also returns empty without touching disk.
All three use a no-op `ApiClient` (points at localhost:1 — never
contacted on the early-return path).
Workspace test sweep stays green.
Assisted-by: Claude Code:claude-opus-4-7
Three more tests in crawler_cargo_e2e.rs covering the workspace- version fallback path: when Cargo.toml has `version.workspace = true` instead of a concrete `version =`, both crawl_all and verify_crate_at_path fall back to parsing the directory name. Also covers the "dir without Cargo.toml entirely" skip. Assisted-by: Claude Code:claude-opus-4-7
New crawler_npm_e2e.rs: - parse_package_name: unscoped, scoped, @-only-no-slash edge - build_npm_purl: scoped and unscoped - read_package_json: well-formed, missing file, malformed, missing name, missing version - find_by_purls: unscoped, scoped, version-mismatch, invalid PURL - crawl_all: discovers unscoped + scoped, skips dirs without package.json, skips dirs with corrupt package.json Assisted-by: Claude Code:claude-opus-4-7
NpmPkgManager::as_tag() and its corresponding test were dead — apply.rs
matches on the enum variants directly (NpmPkgManager::YarnBerryPnP /
::Pnpm) and the struct never derives Serialize, so the stringified tag
was unreachable from any caller.
While here, extract `parse_bun_bin_output` from `get_bun_global_prefix`
so the path-derivation half of bun discovery is unit-testable without
shelling out to a real `bun` binary, and add integration tests covering:
* cargo: TOML parser stops at next section / ignores pre-package
lines, Default impl, CARGO_HOME unset → $HOME/.cargo fallback
* npm: parse_bun_bin_output happy path, empty stdout, root-only path
Assisted-by: Claude Code:claude-opus-4-7
* npm: extract `parse_yarn_dir_output` and `parse_pnpm_root_output` from their shell-out wrappers so the path-derivation logic is unit-testable without a real `yarn` / `pnpm` binary; add tests covering happy path + empty stdout for both parsers and for the previously-extracted `parse_bun_bin_output`. * npm: cover `read_package_json` empty-string branches, `NpmCrawler` construction, `get_node_modules_paths` global_prefix passthrough and global-mode-without-prefix, and `find_workspace_node_modules` recursion / skip-list behavior. * composer: cover `get_global_vendor_paths` via COMPOSER_HOME env var and the HOME/.composer + HOME/.config/composer platform fallbacks, plus `crawl_all` dedup across vendor paths. Assisted-by: Claude Code:claude-opus-4-7
* maven: parent <groupId> fallback when project has none, property
reference (`${...}`) bail-out for each of groupId/artifactId/version,
parent property-reference skip, HOME/.m2/repository fallback,
has_pom_file rejection of version dirs containing only a .jar,
and `Default` impl.
* nuget: global mode discovers nuget_home with NUGET_PACKAGES set,
empty result when home doesn't exist, NuGet.Config marker triggers
global-cache fallback, project.assets.json discovery (root + one
level deep), malformed and empty-packageFolders assets.json arms,
and `Default` impl.
* ruby: `~/.rvm/gems/<set>/gems` layout discovery, and `Default` impl.
* go: `Default` impl, empty `module` directive returns None, quoted
module path branch, trailing-`!` decode arm, find_by_purls when the
module dir is missing, crawl_all over nested versioned dirs, and
the cache/ metadata-dir skip arm.
Assisted-by: Claude Code:claude-opus-4-7
* python: PythonCrawler `Default`, `find_by_purls` canonicalized-name match, qualifier stripping, empty/missing/mismatched purls, `crawl_all` over staged .dist-info dirs (well-formed + corrupt METADATA), global_prefix passthrough, and the METADATA early-break arm at first blank line after headers. * cargo: `parse_cargo_toml_name_version` `version.workspace` bail-out test, `verify_crate_at_path` dir-name fallback rejection on name mismatch, hidden-dir skip in `scan_crate_source`, dedup on identical purls across distinct directories, and local-mode fallback through `get_registry_src_paths` with CARGO_HOME stubbed (both with and without a staged registry/src tree). Assisted-by: Claude Code:claude-opus-4-7
* npm: a single staged tree that drives scoped-package scanning (`scan_scoped_packages`), nested `node_modules` recursion (`scan_nested_node_modules`), scoped→nested→scoped recursion, and the hidden-subdir + file-entry skip arms in both scanners. Adds PURL parser coverage for trailing `?` qualifier stripping, missing `@` version separator, empty version, scoped PURL with no `/`, and scoped PURL with empty name after the slash. * types: cover `CrawlerOptions::default()` populating cwd / global / global_prefix / batch_size (types.rs:143-150) — apply-CLI tests always construct options explicitly, so the Default impl was un-exercised. Assisted-by: Claude Code:claude-opus-4-7
* maven: `get_maven_repo_paths(global=true)` with MAVEN_REPO_LOCAL set returns just that repo, and the empty-result arm when neither env var is set and HOME has no .m2/. * go: `get_gomodcache` falls through to `$HOME/go/pkg/mod` when both GOMODCACHE and GOPATH are unset (covers L194-197). Assisted-by: Claude Code:claude-opus-4-7
The earlier fixture set BOTH Name and Version before reaching the blank line, so the function broke via the both-set guard at L71-72 instead of the blank-line break at L80-81. Replace with a fixture where only Name is set when the blank line is hit — that forces the L80-81 path and verifies the function correctly returns None when the trailer is interrupted before Version is read. Assisted-by: Claude Code:claude-opus-4-7
Drive the `Command::new(...).output().ok()?` Err arm in each of the npm/yarn/pnpm/bun global-prefix helpers by stubbing PATH to a binary-free tempdir so the spawn itself fails. Removes the dependency on whether the dev host happens to have those binaries installed and covers the npm:91 / yarn:111 / pnpm:138 / bun:158 paths. Assisted-by: Claude Code:claude-opus-4-7
* composer: cover `get_composer_home` falling through every source (COMPOSER_HOME unset, composer CLI missing from PATH, HOME without .composer or .config/composer) — drives the L194-207 shell-out failure path and the final L226 `None` arm. * ruby: similar PATH-stub for local Gemfile + missing `gem` binary (run_gem_env Err arm), plus global-mode probe with no gem binary and no HOME-relative gem layouts (covers fallback_globs scanning branches). * nuget: cover scan_package_dir's "skip non-dir entries" arm via a plain file at the top of the package dir, and the read_dir Err short-circuit via a non-existent global_prefix. Assisted-by: Claude Code:claude-opus-4-7
* maven: cover the `artifact_id?` propagation arm when a POM has groupId+version but no artifactId, and the `extract_xml_value` same-line-close-tag guard when an XML element is split across lines. * cargo: cover `scan_crate_source`'s non-dir entry skip arm (plain file at top of source path), the parse_dir_name_version fallback in `read_crate_cargo_toml` when Cargo.toml is unparseable AND the dir name has no version, and the `verify_crate_at_path` false-on- both-parsers-fail arm. Assisted-by: Claude Code:claude-opus-4-7
Adds a shared `tests/common/mod.rs` helper with `uid_is_root()` and
`chmod_{unreadable,readable}` so each crawler test file can drive the
`read_dir(...).await` Err arm without depending on an installed
binary or specific filesystem layout. Per-crawler tests skip under
uid 0 because chmod is a no-op for root.
Coverage added:
* cargo: scan_crate_source short-circuits on unreadable src_path
* composer: read_installed_json short-circuits on unreadable file
* go: scan_dir_recursive short-circuits on unreadable cache_path
* npm: scan_node_modules + find_workspace_node_modules both short-
circuit on unreadable dirs; the workspace test stages a readable
and an unreadable workspace side-by-side to prove the readable
one is still discovered.
* nuget: scan_package_dir + scan_global_cache_package both short-
circuit on unreadable dirs (the latter via an unreadable per-name
version directory).
* python: find_by_purls + scan_site_packages short-circuit on
unreadable site-packages.
* ruby: scan_gem_dir short-circuits on unreadable gem dir.
Assisted-by: Claude Code:claude-opus-4-7
Same refactor pattern as npm/yarn/pnpm/bun parsers — the `composer global config home` shell-out now forwards to a pure `parse_composer_home_output(stdout) -> Option<PathBuf>` parser that handles trimming and the empty-input guard. Unit-testable without composer installed. Assisted-by: Claude Code:claude-opus-4-7
Introduce `crate::utils::fs::{list_dir_entries, entry_is_dir,
entry_file_type}` — a small async wrapper around `tokio::fs::read_dir`
and `entry.file_type()` that swallows the I/O errors that every
crawler was previously handling inline with its own
`match { Ok(rd) => rd, Err(_) => return ... }` pattern.
Also add `crate::utils::process::{CommandRunner, SystemCommandRunner}`
— the next change will thread `&dyn CommandRunner` through the
remaining shell-out wrappers so their success arms become unit-
testable without an installed CLI.
All eight ecosystem crawlers migrated to the new helpers. The
behavior is identical (read_dir Err and file_type Err still produce
"skip this entry / return empty"); the diff is purely a deduplication
of ~22 inline error-handling blocks down to ~6 helper-call sites.
Tests stay green: 419 lib tests + all per-crawler e2e suites pass.
The net effect for coverage: chmod-based permission tests now drive
the one helper in utils/fs.rs, not 28 separate inline `match` arms
inside the crawlers. Any future crawler gets the same error handling
for free.
Assisted-by: Claude Code:claude-opus-4-7
Thread `&dyn CommandRunner` through the four shell-out wrappers
that still embed `std::process::Command::new(...)`:
* npm: `get_{npm,yarn,pnpm,bun}_global_prefix_with(&dyn CommandRunner)`
* ruby: `run_gem_env_with(&dyn CommandRunner, key)`
* python: `find_python_command_with(&dyn CommandRunner)` plus
`get_global_python_site_packages` now uses `SystemCommandRunner`
internally for the `python -c "import site; ..."` call.
Existing zero-arg public APIs (`get_npm_global_prefix()`,
`run_gem_env()`, `find_python_command()`, etc.) keep their
signatures — they're thin wrappers that pass `&SystemCommandRunner`,
so no caller changes are required.
Also extract three more pure parsers to match the existing yarn /
pnpm / bun / composer pattern:
- `parse_npm_root_output` in npm_crawler
- `parse_gem_env_output` in ruby_crawler
- `parse_python_site_packages_output` in python_crawler
Add `MockCommandRunner` to `tests/common/mod.rs` (a small
`(bin, args) -> Option<String>` lookup table that implements
`CommandRunner`). The new test cases demonstrate the success-arm
coverage that was previously impossible without the binary
installed:
* npm: 4 mock-runner tests (npm/yarn/pnpm/bun returning canned
stdout each producing the expected node_modules path), plus 3
pure-parser tests for the empty-stdout arm
* ruby: 2 pure-parser tests for `parse_gem_env_output`
* python: 3 pure-parser tests for `parse_python_site_packages_output`
+ 3 mock-runner tests for `find_python_command_with`
All 423 lib + per-crawler e2e + cli sweep tests stay green.
Also ungate `mod common` from `#[cfg(unix)]` in each crawler test
file (was needed only for the chmod helpers; the new
`MockCommandRunner` is cross-platform). The chmod helpers
themselves remain `#[cfg(unix)]` inside common/mod.rs.
Assisted-by: Claude Code:claude-opus-4-7
Add scan + apply coverage for uv (Astral's Python package manager) across its three install modes (uv venv, uv tool install, uv python install), plus a python-project marker gate so a fresh clone before `uv sync` isn't invisible to the scanner. * python_crawler: discover `uv python install` interpreters at `~/.local/share/uv/python/cpython-*/lib/python3.*/site-packages/` (Linux/macOS) and `%LOCALAPPDATA%\uv\python\*\Lib\site-packages\` (Windows). Mirrors the existing uv-tools block. * python_crawler::get_site_packages_paths: when no venv is found AND a Python project marker is present (pyproject.toml, setup.py, setup.cfg, requirements.txt, uv.lock), fall through to global discovery. Mirrors cargo/ruby/go's "is this a project root" pattern. uv.lock is detected but never parsed (Astral designates it opaque to third-party tools). * sidecars/PypiRecordStale: advisory now mentions both `pip check` and `uv pip check`, and both `pip install --force-reinstall` and `uv pip install --reinstall`. One-line copy change. Host integration tests in crawler_python_e2e.rs: * uv-tools layout discovery (macOS + Linux variants) * uv-python managed interpreter discovery * pyproject.toml / uv.lock fallback gates Docker e2e tests in docker_e2e_pypi.rs (Dockerfile.pypi now installs uv via pip): * `pypi_uv_venv_install_full_apply_chain` — runs `uv venv` + `uv pip install`, applies a patch, verifies (a) the venv file got the marker, (b) the uv cache file's bytes are unchanged. The cache- integrity assertion is the gate that proves the CoW guard (`break_hardlink_if_needed` in patch/cow.rs) correctly isolates the venv copy from the global cache — uv's hard-link-from-cache was previously untested. * `pypi_uv_tool_install_full_apply_chain` — runs `uv tool install httpie==3.2.2`, then `socket-patch scan --global` against the uv tools root. Asserts scannedPackages > 5 (httpie + 16 deps), proving the platform-gated uv-tools discovery branch at python_crawler.rs:418-427 works end-to-end with a real binary. All four pypi docker tests pass against a freshly-built base image. The two new tests join the existing pip-local and pip-global tests in the docker-e2e CI matrix. Assisted-by: Claude Code:claude-opus-4-7
Two parallel additions:
**Bun**
* `NpmPkgManager::Bun` variant in `crawlers/pkg_managers.rs`,
detected via `bun.lock` (text, current default) or `bun.lockb`
(binary, legacy) at the project root. Precedence above pnpm so
bun's `node_modules/.bun/` isolated-linker store doesn't get
misclassified as a pnpm content store.
* apply.rs emits a "detected bun" diagnostic alongside the existing
pnpm note — both share the same CoW-driven safety.
* `Dockerfile.npm` installs bun via the official install script,
exposes /usr/local/bin/bun.
* `npm_bun_install_full_apply_chain` in docker_e2e_npm.rs runs
`bun install` and verifies CoW isolation: the test pre-warms
bun's cache, snapshots the cache twin's SHA256, applies the
patch, and asserts the cache file's bytes are unchanged. Same
pattern as `pypi_uv_venv_install_full_apply_chain` — third
hardlink-using ecosystem to gain this regression gate.
**Deno**
* New `deno` feature flag in `socket-patch-core/Cargo.toml`.
* `Ecosystem::Deno` variant in `crawlers/types.rs`, mapped to
`pkg:jsr/<scope>/<name>@<version>` PURLs (informal but
defensible convention — JSR packages always have a scope).
Deno's other surface (`deno install` populating standard
`node_modules/`) routes through `Ecosystem::Npm` unchanged
because those packages are real npm packages.
* `parse_jsr_purl` / `build_jsr_purl` in `utils/purl.rs` mirror
the composer pattern (also scope/name shape). Rejects bare `@`
scope, empty scope, empty name, empty version, non-`@`-prefixed
scope.
* New `crawlers/deno_crawler.rs` (feature-gated). Discovers
`$DENO_DIR/npm/jsr.io/<scope>/<name>/<version>/` cached JSR
packages. Local-mode gates on `deno.json` / `deno.jsonc` /
`deno.lock` project markers — same pattern as the python
pyproject.toml gate added with the uv work. Reuses
`utils::fs::{list_dir_entries, entry_is_dir}`; no new I/O
patterns.
* `DENO_DIR` env var resolution with platform default fallback
(`~/.cache/deno` on Linux/macOS, `%LOCALAPPDATA%\deno\` on
Windows). Mirrors the cargo_home / nuget_home pattern.
Tests:
* 5 new inline unit tests for `NpmPkgManager::Bun` detection
(text+binary lockfile, requires installed node_modules, beats
pnpm, loses to yarn-berry-PnP).
* JSR PURL round-trip + edge cases (empty scope, missing `@`,
wrong scheme) in `utils/purl.rs::tests`.
* New `tests/crawler_deno_e2e.rs` with 9 integration tests:
find_by_purls happy/no-match/wrong-purl-type, crawl_all with
multi-scope/multi-version fixtures + skips non-`@` dirs,
global_prefix passthrough, DENO_DIR env-var resolution,
deno.json marker triggers cache fallback.
* 6 inline unit tests in `deno_crawler.rs` for the project-marker
detection + default construction.
All 50 npm tests stay green; 4 pypi docker tests still pass.
Assisted-by: Claude Code:claude-opus-4-7
… e2e
* `socket-patch-cli/Cargo.toml` — passthrough the `deno` feature
from socket-patch-core so the CLI builds with it enabled.
* `ecosystem_dispatch.rs` — `crawl_all_ecosystems` now invokes the
`DenoCrawler`, and `find_packages_for_purls` routes `pkg:jsr/...`
PURLs through it. Surfaces a "Using Deno JSR cache at: ..." note
when `--global` / `--global-prefix` is set, matching the existing
ecosystem dispatch UX.
* `crawlers/deno_crawler.rs` — clarified module docstring about why
the JSR walk uses an *expected* `<scope>/<name>/<version>/`
layout rather than Deno's real content-addressed
`$DENO_DIR/remote/https/jsr.io/<sha256>` cache (URL-hashed; no
stable PURL → path mapping). The crawler is designed for
synthetic fixtures and future Deno tooling that materializes JSR
packages with a stable on-disk hierarchy.
* `tests/docker/Dockerfile.deno` — new ecosystem image, base +
Node + Deno (Deno installed via official install script).
* `crates/socket-patch-cli/tests/docker_e2e_deno.rs` — two tests:
- `deno_install_node_modules_full_apply_chain` runs a real
`deno install` against a `package.json` (`minimist@1.2.2`)
and drives the full scan → sync → apply → marker-grep chain,
proving the NpmCrawler picks up Deno's node_modules output
and the ecosystem dispatch routes it through the npm path.
- `deno_jsr_synthetic_layout_scan_verifies_discovery` stages a
synthetic JSR-shaped tree at `$DENO_DIR/npm/jsr.io/@scope/
name/version/` and runs `socket-patch scan --global
--ecosystems deno --global-prefix <path>`. Asserts the
DenoCrawler enumerated both staged packages. Real `deno
install` doesn't produce this layout today (Deno's actual
JSR cache is URL-content-addressed) so the synthetic-fixture
test is the honest end-to-end gate against the crawler-CLI
integration; the host integration tests in
`tests/crawler_deno_e2e.rs` cover crawler internals.
* `.github/workflows/ci.yml` — add `deno` to the e2e-docker and
coverage-docker matrices and to the feature-passthrough list on
the host-coverage instrumented build.
Tests:
* 4 npm docker e2e (existing pip-style + global + new bun + smoke) pass
* 4 pypi docker e2e (pip local + global + uv venv + uv tool) pass
* 2 deno docker e2e (npm-mode + JSR synthetic) pass
Assisted-by: Claude Code:claude-opus-4-7
Clippy `doc_overindented_list_items` was flagging the (a)/(b) sub-items as too deeply indented relative to the outer numbered list. Inline them into the surrounding prose instead — same information, no nested-list-inside-numbered-list shape. Assisted-by: Claude Code:claude-opus-4-7
…gets The function is only called from `#[cfg(target_os = "macos")]` blocks in `get_global_node_modules_paths` (Homebrew / nvm / volta / fnm fallbacks) and from inline `#[cfg(test)] mod tests` entries. On Linux/Windows clippy sees no production caller and trips `-D dead_code` under `cargo clippy --workspace --all-features -- -D warnings` (CI's invocation). Gating the function itself to `target_os = "macos"` would break the inline tests on Linux. `#[allow(dead_code)]` is the right tool: keeps the symbol visible on every target while clippy treats it as intentionally unused. Surfaced by CI run 26332596376 on PR #80; local clippy on macOS host passes either way because the macOS callers are live there. Assisted-by: Claude Code:claude-opus-4-7
The `SOCKET_EXPERIMENTAL_MAVEN=1` / `SOCKET_EXPERIMENTAL_NUGET=1` runtime gates were added to `ecosystem_dispatch.rs` in 39a2321 ("wire safety primitives + Maven/NuGet experimental gates"), but the docker e2e tests for those ecosystems never set the corresponding env vars in their `docker run` invocations. With the gate off the crawlers report `scannedPackages: 0`, scan writes no manifest, apply reports `noManifest`, and the post-apply marker grep fails. Add `-e SOCKET_EXPERIMENTAL_MAVEN=1` and `-e SOCKET_EXPERIMENTAL_NUGET=1` to the respective docker run argv arrays in `docker_e2e_maven.rs` and `docker_e2e_nuget.rs`. Surfaced by CI failures on PR #80 runs starting from this branch's first post-39a2321 push. Assisted-by: Claude Code:claude-opus-4-7
`Ecosystem::all()` now includes `Ecosystem::Deno` when the `deno` feature is enabled, but the inline `test_all_count` test wasn't updated to count the new variant in its `expected += 1` ladder. Surfaced by CI on PR #80: test/coverage runs with `--features cargo,golang,maven,composer,nuget,deno` fail because `all.len() == 9` but `expected == 8`. Add the parallel `#[cfg(feature = "deno")]` increment to keep the test in sync with the enum. Assisted-by: Claude Code:claude-opus-4-7
Three groups of failures surfaced by CI on PR #80: 1. **Windows path-separator** — four pure-parser tests in `crawler_npm_e2e.rs` (`parse_bun_bin_output_well_formed_unix`, `parse_yarn_dir_output_appends_node_modules`, `get_yarn_global_prefix_with_mock_runner_success`, `get_bun_global_prefix_with_mock_runner_success`) assert Unix-style forward-slash output paths. On Windows `PathBuf::join` uses `\`, producing mixed-separator paths that don't match the literal strings. Gate these tests with `#[cfg(unix)]` since they test Unix-style path construction semantics — Windows users feed Windows-shaped paths into the same parsers. 2. **Linux composer shell-out short-circuit** — two host integration tests in `crawler_composer_e2e.rs` (`get_vendor_paths_global_via_home_dot_composer_fallback`, `get_vendor_paths_global_via_home_xdg_config_composer_fallback`) set HOME to a tempdir but don't stub PATH. On Linux CI runners where `composer` is installed, `composer global config home` returns a real path outside the test's tempdir, short- circuiting the HOME-based fallback chain. Stub PATH to a binary-free tempdir so the shell-out fails and the fallback chain runs as designed. 3. (Pulled into a separate commit earlier: `test_all_count` counts `Ecosystem::Deno` when the feature is on. See 4e564f3.) All 437 lib + e2e tests pass locally on macOS host. Assisted-by: Claude Code:claude-opus-4-7
…eout, --break-lock)
Operators now have first-class, JSON-aware recovery for the
`<.socket>/apply.lock` advisory lock used by mutating subcommands.
- `socket-patch unlock` — probe lock state. Exits 0 when free, 1 when
held. `--release` deletes a free leftover lock file; refuses when
held (use --break-lock on the mutating subcommand for that scenario).
- `--lock-timeout=<secs>` — on apply/rollback/repair/remove, waits up
to N seconds for the lock to free before reporting `lock_held`.
Plumbs through to the existing `acquire(dir, Duration)` API.
- `--break-lock` — on apply/rollback/repair/remove, removes the lock
file before acquisition. Records a `lock_broken` warning event in
the JSON envelope (and `warnings[]` for rollback's ad-hoc shape) so
the action is auditable. Stderr also notes it in human mode.
Replaces the prior "rm <.socket>/apply.lock and retry" stderr hint with
a pointer at the new tools. Adds unit + integration tests covering each
new flag and subcommand against held / free / leftover-file scenarios.
Assisted-by: Claude Code:claude-opus-4-7
`get_nuget_package_paths_discovers_assets_json_package_folders` and `get_nuget_package_paths_discovers_assets_json_in_subproject` built the fixture file with `format!` + `Path::display()`. On Windows that produces unescaped backslashes in the JSON, the production parser silently drops the malformed file, and the tests fail with empty discovery results. Construct the body via `serde_json` so paths are properly escaped on every platform. Assisted-by: Claude Code:claude-opus-4-7
Three `find_local_venv_site_packages_*` tests hardcoded the Unix venv layout `lib/python3.11/site-packages`. Production code on Windows uses `Lib/site-packages` (no python-version directory) — so the staged fixtures were invisible to the crawler and the tests failed with empty discovery results. Add a `venv_site_packages_relpath` helper that returns the right layout per OS, mirroring `find_site_packages_under`'s own `#[cfg(windows)]` branch. Assisted-by: Claude Code:claude-opus-4-7
`e2e_scan` exercises `socket-patch scan --apply --yes` end-to-end against the live public proxy at patches-api.socket.dev. The proxy returns "Service temporarily over capacity" intermittently for `/patch/by-package/*` lookups, and the test panics when scan reports zero patches. Move it out of the PR matrix; runnable on demand via `cargo test -p socket-patch-cli --test e2e_scan -- --ignored`. The other matrix entries (e2e_npm, e2e_pypi, e2e_cargo, e2e_golang, e2e_maven, e2e_gem, e2e_composer, e2e_nuget) stay — they exercise local crawler / apply paths, not the live API. Assisted-by: Claude Code:claude-opus-4-7
Same root cause as the prior e2e_scan removal: these three suites' `#[ignore]`-gated tests hit the real public proxy at patches-api.socket.dev to fetch patch data by UUID. The proxy intermittently returns 503 "Service temporarily over capacity", causing the tests to fail for reasons outside this repo's control. PR-time coverage for the same code paths comes from the e2e-docker matrix, which runs the full apply flow against a hermetic wiremock fixture per ecosystem. The live-API smokes remain runnable on demand via `cargo test --test <name> -- --ignored`. The remaining e2e_<ecosystem> entries (cargo/golang/maven/composer/ nuget) have no `#[ignore]` tests, so they exercise zero tests under `-- --ignored` and pass deterministically. Assisted-by: Claude Code:claude-opus-4-7
Wenxin Jiang (Wenxin-Jiang)
approved these changes
May 23, 2026
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
2 participants
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
Hardens
socket-patch applyagainst the six failure classes surfaced by a recent audit. Each commit is self-contained, builds clean, and ships with tests.feat(apply): advisory file lock across mutating subcommands.socket/refactor(cli): plumb --api-url/--api-token without std::env::set_varstd::env::set_varunder tokio multi-thread runtimefeat(maven): gate Maven crawler behind SOCKET_EXPERIMENTAL_MAVEN=1<jar>.jar.sha1/md5); Maven patches today are experimentalfeat(apply): copy-on-write defense against pnpm content-store mutationnode_modules/<pkg>is a symlink/hardlink into the global store — patching project A used to mutate project B's installfeat(apply): detect pnpm + refuse on yarn-berry PnP.yarn/cache/*.zip; apply refuses with a clear pointer toyarn patchfeat(apply): atomic write via stage+renamefeat(apply): per-ecosystem sidecar fixups (cargo + nuget; advisories for pypi/gem/go)cargo build"checksum changed" failures; NuGet "tampered package" warnings; advisory events for ecosystems we cannot yet fully fixWhy this PR exists
socket-patch applyis run as the second step of the install → patch apply → build flow. Before this PR it had several silent failure modes that would either break the user's next build (Cargo checksum mismatch, NuGet content hash) or corrupt unrelated projects on the same machine (pnpm store mutation). The audit lays this out in detail; this PR closes the highest-blast-radius items.Confirmed in scope with project lead:
PatchFileInfoschema change is upstream of socket-patch).What stays out of scope
PatchFileInfo.yarn patch).contentHashrecompute (would need server-side support to ship the original.nupkg).go mod verifywould fail).RECORDrewriter (path-mapping between site-packages and the package dir has quirks I'd want to verify against real installs first — advisory only here).Tests
cargo build --workspace✓ (default features, what the release binary uses)cargo build --workspace --all-features✓cargo build --release --workspace✓ — no warningscargo clippy --workspace --all-features -- -D warnings✓cargo test --workspace --all-features✓ — 450 lib tests pass plus every integration suite (cli_parse_*,e2e_*).Notable new tests:
patch::cow::hardlink_is_broken_and_sibling_survives_mutation— the core pnpm invariant: patching one link target does not mutate the other.patch::apply::test_apply_file_patch_does_not_propagate_to_hardlinked_sibling— end-to-end variant proving the integration is wired.patch::apply::test_apply_file_patch_hash_mismatch_leaves_original_intact— atomic write contract: a failed apply leaves both the original file and the parent directory exactly as it was (no.socket-stage-*litter).patch::apply_lock::*— five tests covering acquire, contention, drop, missing dir, timeout.patch::sidecars::cargo::rewrites_only_patched_files+ sibling tests —.cargo-checksum.jsonround trip.patch::sidecars::nuget::*—.nupkg.metadatadeletion + signed-package advisory.crawlers::pkg_managers::*— 9 tests pinning the detection table.JSON envelope additions (additive)
The per-package apply result JSON now carries two new top-level keys:
sidecarsUpdated: string[]— paths relative to the package directory that were rewritten or deleted as part of the sidecar fixup. Empty when no sidecar applied.sidecarAdvisory: string | null— one-line operator advisory for ecosystems we cannot fully fix (PyPI RECORD, gem cache, go mod verify).nullwhen there is no advisory.Existing keys are unchanged. The contract test in
crates/socket-patch-cli/src/commands/apply.rswas updated to reflect the new top-level key set; downstream wrappers that strict-check the key set need a small update.Relationship to PR #79
This branch is off main at
b96a13fand is independent from PR #79 (feat/scan-apply-json, the v3.0 unified-args +scan --syncwork). They touch disjoint files and modules, so the merge should be conflict-free, but the depscan submodule bump (PR SocketDev/depscan#20517) will need a follow-up rebase once #79 lands so it picks up both code paths together.Manual smoke checks before merge
( socket-patch apply --json & ); socket-patch apply --json— second call emits{"errorCode":"lock_held"}.ln a b, run apply against a manifest covering one of them, verify the other is byte-unchanged.cargo buildsucceeds (no "checksum changed" error)..pnp.cjsproject, apply exits 1 witherrorCode: yarn_pnp_unsupported.--features mavenbutSOCKET_EXPERIMENTAL_MAVENunset, Maven PURLs surface the experimental warning and are skipped.Assisted-by: Claude Code:claude-opus-4-7
Follow-up: dead-code purge + integration coverage expansion (this session)
Since the original review request, this branch has accumulated a substantial cleanup + coverage pass that's worth surfacing separately. Net effect: workspace coverage moved from ~70% regions to 90.15% regions / 88.45% lines / 94.57% functions.
Dead code removed (~1300 lines across 14 files)
crates/socket-patch-core/src/manifest/recovery.rs(543 lines) — entire module removed; therecover_manifestmachinery (RecoveryResult,RecoveryEvent,RecoveryOptions,RefetchPatchFnetc.) had zero call sites.utils::purl: 12 unused helpers (is_pypi_purl,is_npm_purl,is_gem_purl,is_maven_purl,is_golang_purl,is_composer_purl,is_nuget_purl,is_cargo_purl,parse_npm_purl,parse_purl,build_pypi_purl, plus a duplicate ofbuild_npm_purl) — all redundant withEcosystem::from_purl+ per-ecosystemparse_*/build_*in the crawlers.json_envelope:PatchEvent::with_old_uuid,::with_bytes+ their underlyingold_uuid/bytesfields,Summary::bytes_downloaded/bytes_freedcounters,PatchAction::as_tag,Command::as_tag— all unused; the JSON output uses serde's#[serde(rename_all)]directly.utils::telemetry::track_patch_event_fire_and_forget— unused tokio-spawn variant.utils::env_compat::read_env_either— duplicate alias ofread_env_with_legacy.utils::fuzzy_match::is_purl/is_scoped_package— duplicates ofutils::purl::is_purland unused scoped-name check.crawlers::nuget_crawler::parse_nuspec_id_version+extract_xml_element— never wired into discovery.crawlers::types::Ecosystem::purl_prefix— production usesEcosystem::from_purlenum dispatch instead.manifest::operations::get_referenced_blobs,diff_manifests,ManifestDiff— superset/diff helpers that no command consumed.package_json::update::update_multiple_package_jsons— sequential wrapper with no callers.constants: 4 unused.socket/*path constants (DEFAULT_BLOB_FOLDER,DEFAULT_PACKAGES_FOLDER,DEFAULT_DIFFS_FOLDER,DEFAULT_SOCKET_DIR).SidecarFileAction::Created— reserved-but-never-emitted enum variant.Bug fixes surfaced during coverage work
in_process_remote_ecosystems_apply.rs+in_process_rollback_all_ecosystems.rs— ecosystem tests assumed--all-features; under narrower builds they false-failed because the crawler dispatch compiled out. Added#[cfg(feature = "<eco>")]per test.#[test]attribute inutils/purl.rs(leftover from earlier removal).New integration test files
Ten new e2e files under
crates/socket-patch-{cli,core}/tests/:e2e_safety_advisories.rse2e_safety_cow.rse2e_safety_internals.rschflags uchg+ ACL tricks for rename/write failure paths)e2e_safety_cargo_build.rs(extended)core/tests/diff_e2e.rsapply_diffround trip + malformed-delta + wrong-source-no-paniccore/tests/package_e2e.rsread_archive_to_map/read_archive_filteredhappy path + unsafe-path guards (absolute, parent-traversal, backslash)core/tests/fuzzy_match_e2e.rsMatchTypeordering: ExactFull > ExactName > PrefixFull > PrefixName > ContainsFull > ContainsNamecore/tests/rollback_new_file_e2e.rsbefore_hash→ delete) and MissingBlob/NotFound armscore/tests/blob_fetcher_edges_e2e.rsDownloadMode::parse/as_taground tripcore/tests/crawlers_empty_paths_e2e.rsfind_by_purls/crawl_allshort-circuits across all 7 ecosystemscore/tests/telemetry_helpers_e2e.rsis_telemetry_disabledenv-var combos +sanitize_error_messagehome-dir redactioncli/tests/output_helpers_e2e.rsformat_severityANSI branches per severity + case-insensitivity +colorwrappercli/tests/cli_dry_run_paths_e2e.rs--dry-runenvelope shape across apply/repair/rollback/remove/list + apply--silentshort-circuitcli/tests/get_batch_paths_e2e.rs--idfilter missPlus targeted extensions to
apply_invariants.rs(no-.socket-dir envelope) andrepair_invariants.rs(--offline + --download-onlymutual exclusion).Coverage summary (region-level, integration tests via
cargo llvm-cov --workspace --all-features)sidecars/mod.rssidecars/cargo.rssidecars/nuget.rspatch/cow.rspatch/diff.rspatch/package.rsutils/fuzzy_match.rsmanifest/operations.rscommands/apply.rs(cli)output.rsThe remaining ~10% of uncovered regions lives in CLI command flag-combination paths (
commands/get.rs74.4%,scan.rs80.4%,ecosystem_dispatch.rs77.7%) that each need their own wiremock + tempdir + manifest staging — straightforward but voluminous. Worth a follow-up PR focused oncommands/*runners alone.Workspace stability
All test sweeps green:
cargo test --workspace --all-featuresfinishes with 547+ passing tests (118 cli lib + 414 core lib + 60+ new integration tests). No new warnings.