audit-core: C19/C21 filed (#727, #728), new C42 (#726)

Assisted-by: Claude Code:claude-opus-5-5

Author: claude

doc/07-infra-agent.md

@@ -2,7 +2,7 @@
 
 ## Part 7: Core infrastructure and agent (in-place) mode
 
-_Last checked against main @ 045d7ec on 2026-10-03 by audit-core. Owner: audit-core._ Only the timeout, blob/diff body, zip-read, process-spawning, API-pacing, URL-builder, retry, batching, hashing and UUID passages have been re-checked; the rest is as of `2463257`.
+_Last checked against main @ 045d7ec on 2026-10-03 by audit-core. Owner: audit-core._ Only the timeout, blob/diff body, zip-read, process-spawning, API-pacing, URL-builder, retry, batching, hashing, UUID, env/home-dir and atomic-write passages have been re-checked; the rest is as of `2463257`.
 
 > Scope: `api/*`, `manifest/*`, `ledgers.rs`, `constants.rs`, `patch/` (excluding `redirect/`), `policy/*`, `rollout*`, `update/*`, the CLI `update_notifier.rs`/`update.rs`, `telemetry.rs`, and the generic `utils/*` and `hash/*`.
 
@@ -67,13 +67,13 @@ The vendor policy also has three separate hand-written retry loops, plus a first
 - **UUID checks:** five grammars. `client.rs` has one, with a near byte-identical copy in CLI `lib.rs`; `path_safety.rs` accepts lowercase only; `apply.rs` accepts any alphanumeric plus `-` and `_`; `utils/python_script.rs` uses `uuid::Uuid::parse_str`, which also takes simple, braced and `urn:uuid:` forms. {{C18}}
 - **Line endings:** `utils/line_endings.rs` has 7 users, but `python_lock`, `vendor/common::detect_eol` (which contradicts `LineEndings::Mixed`), `redirect::crlf_to_lf` and `poetry_lock` each implement their own rules.
 - **Purls:** `utils/purl.rs` has two builder families (unvalidated `build_*` and validated `*_purl`). `vex/product.rs` hand-rolls a third. 48 production `format!("pkg:…")` sites and 58 `starts_with("pkg:<type>/")` checks bypass `Ecosystem::from_purl`.
-- **Env truthiness:** three vocabularies.
-  - `"1"|"true"` in `env_compat.rs`;
+- **Env truthiness:** three vocabularies, plus a fourth rule in `update_notifier::in_ci`. {{C19}}
+  - `"1"|"true"` in `env_compat.rs` (`SOCKET_DEBUG`, `SOCKET_OFFLINE`);
   - `"1"|"true"` separately in `telemetry.rs`;
-  - `1|true|yes|on|y|t` in `socket_cli_config::env_truthy`, which `update_notifier` imports.
+  - `1|true|yes|on|y|t` in `socket_cli_config::env_truthy`, which `update_notifier` imports, and the same set in the CLI's clap `parse_bool_flag`.
 
-  There are also 37 inline "empty means unset" reads in 22 files and at least four home-directory resolvers.
-- **Atomic writes:** `utils/fs.rs` has six writers. `atomic_write_sync` re-implements `stage_and_rename` + `commit_stage` in blocking form. Separate stage+rename code also exists in `blob_fetcher`, `update/download.rs` and `update/swap.rs`.
+  The narrow core match stays correct only because `apply_env_toggles` rewrites every truthy flag back into the env as `"1"`; all ten command entry points call it on `045d7ec`. "Empty means unset" is one private helper (`socket_cli_config::env_non_empty`) plus about 29 inline copies in 16 files. In this area there are at least six home-directory resolvers, and they disagree: `policy::home_dir` reads only `USERPROFILE` on Windows, while `utils::fs::home_dir` prefers `HOME`. The crawlers keep further variants.
+- **Atomic writes:** `utils/fs.rs` has six writers, which are four boolean policies (capture, fsync, keep mode, durability record) spelled as separate functions. `atomic_write_sync` re-implements `stage_and_rename` + `create_stage` + `commit_stage` in blocking form, with no drift yet. `blob_fetcher::write_cache_entry_atomic` is a third, deliberately non-fsyncing stage+rename. The self-update stage (`update/download.rs`, `update/swap.rs`) is legitimately separate. Correction: the artifact writers inside `utils::fs` don't capture into a group commit either, so bypassing `utils::fs` isn't itself a group-commit escape. {{C21}} `get` writes `.socket/blobs/<hash>` with neither: it uses an in-place `fs::write` and doesn't check the content's hash ({{C42}}).
 - **Process spawning** is well centralized in `process.rs::resolve_tool`/`command_for`, with one exception: `vendor/pypi_hatch.rs:118` runs `Command::new("hatch").current_dir(root)` (verified by execution on `045d7ec`). {{C04}} That is exactly the planted-binary pattern `process.rs:23-33` documents as unsafe: "a bare `Command::new("git")` would execute a `git` planted in the repository being scanned".
 
 ### 7.4 Agent mode
@@ -157,6 +157,7 @@ Maven sidecars are not handled at all. This code exists only for in-place mode.
 - {{C37}} Patch blob and diff downloads (`fetch_binary`) buffer the whole body with `resp.bytes()`, while the vendor and self-update downloads use the shared `read_capped` (256 MiB). On `1169ae6` a 600 MiB blob response was buffered in full.
 - {{C38}} API pacing has one policy (`utils::concurrent`: proxy cap 4, `SOCKET_API_CONCURRENCY` override, fd-limit rule), and every CLI window uses it, except the client's own public-proxy per-package fallback. That fallback keeps a private `PROXY_BATCH_PATH_CONCURRENCY = 10`, and on `045d7ec` it ran 10 GETs in flight with `SOCKET_API_CONCURRENCY=1`. `registry_concurrency()` has no caller.
 - {{C41}} Hash case policy is decided per site. Blob download compares case-insensitively on purpose (`blob_hash_matches`), and the blob-name validators accept uppercase, but agent-mode apply and rollback verify with exact `==` against the lowercase computed hash; vendored verify sites are split the same way. On `045d7ec` an uppercased manifest hash downloaded and passed `is_valid_blob_hash`, then failed both apply and rollback verification with `HashMismatch`.
+- {{C42}} `.socket/blobs/<hash>` has two writers. The fetch path verifies the git-sha256 and stages+renames, because `get_missing_blobs` trusts presence. `get::write_blob_entry` stores a patch view's inline `blobContent` under its claimed hash without checking it, truncate-writes in place, and overwrites an existing verified blob; it also hand-rolls base64 although the crate is a dependency. On `045d7ec` a verified `blobs/<H>` was replaced with bytes that don't hash to `H`.
 
 ---
 _Generated by [Claude Code](https://claude.ai/code)_

entries/audit-core/20261003T215159Z.md

@@ -0,0 +1,24 @@
+[agent] 2026-10-03: architecture audit (CLI and core)
+
+**main @ `045d7ec`**, unchanged since the previous run. There were no new maintainer comments in the discussion, and no handovers addressed to `audit-core`.
+
+**Reconciled:** all register rows hold their statuses: C03 #568, C04 in PR #617, C05 #615, C07 #648, C08 #649, C09/C39 #647, C14 #704, C15 #676/#677, C16 #675, C17 #706, C18 #705, C37 in PR #607, C38 #614, C40 #678 and C41 #707. `main` hasn't moved, so I didn't re-check open issues against it again.
+
+**Backlog verified and decomposed:**
+- **C19 → [#727](https://github.com/SocketDev/socket-patch/issues/727)** (refactor). There are three truthiness vocabularies, plus `in_ci`'s own rule. Core's `"1"|"true"` gates (`env_compat`, and a copy in telemetry) are correct only because `apply_env_toggles` rewrites truthy flags back into the env as `"1"`. I checked that all ten command entry points call it, so there is no user-visible drift today. "Empty means unset" is one private helper plus about 29 inline copies in 16 files. There are at least six home-directory resolvers in this area: `policy::home_dir` reads only `USERPROFILE` on Windows, while `utils::fs::home_dir` prefers `HOME`. Target: `utils::env::{non_empty, truthy, parse_bool, home_dir}`. This unblocks deleting `apply_env_toggles` under C10.
+- **C21 → [#728](https://github.com/SocketDev/socket-patch/issues/728)** (refactor, mechanical). The six `utils::fs` writers are four boolean policies; `atomic_write_sync` is a blocking copy of the stage code (no drift yet); and `blob_fetcher::write_cache_entry_atomic` is a third stage+rename. Target: one `WriteOpts` core, keeping the public names as wrappers. Two corrections to the review: the "bypassing `utils::fs` escapes group commit" claim is wrong, because the `utils::fs` artifact writers don't capture either; and `update/` staging is legitimately separate, because it needs exec bits.
+
+**New finding: C42 → [#726](https://github.com/SocketDev/socket-patch/issues/726)** (bug). `.socket/blobs/<hash>` has two writers. The fetch path verifies the git-sha256 and stages+renames, and its doc comment explains that `get_missing_blobs` trusts presence forever. `get::write_blob_entry` stores a patch view's inline `blobContent` with only a 64-hex shape check, truncate-writes in place, and overwrites an existing blob. A temporary unit test (run twice, not committed) seeded a verified `blobs/<H>` and passed it different bytes named `H`: the result was `Ok(false)`, and the file no longer hashes to its name. `get` also hand-rolls base64 although the `base64` crate is a CLI dependency, and an existing test pins the missing check (`"patched\n"` under `1111…`).
+
+**Searched without filing:**
+- Raw `fs::write` / `fs::rename` outside `utils::fs` in production code. `vex.rs` writes a user-chosen output file, which is fine. `vendor/npm_dir.rs`, `vendor/gem.rs` and `vendor/redownload.rs` are in the sibling's area, and their stage dirs are already covered by vendored takeover work, so no handover. `oracle_support.rs` is test support.
+- Telemetry home redaction with an empty `HOME`: it is already guarded through `fs::home_dir`. The macOS npm-crawler `HOME` read checks `is_empty`.
+
+**False positives ruled out:** core's narrow truthiness is not a live bug, because the clap vocabulary is a superset and every entry point mirrors it. `--offline=false` with `SOCKET_OFFLINE=1` can't occur, because the bool flags are `SetTrue`.
+
+**Living document:** Part 7.3's env and atomic-write passages are rewritten with {{C19}}, {{C21}} and {{C42}}, and a C42 bullet is added under new findings. The check line now covers the env/home-dir and atomic-write passages. The §0 numbers were already refreshed today.
+
+**Next backlog rows:** C10 (`RunCtx` tracking; C19 is now its first child), C20 (purl builders), C13 (typed error codes; after #704), C23 (dead code), C22 (telemetry `track(Event)`).
+
+---
+_Generated by [Claude Code](https://claude.ai/code)_

register/20-audit-core.md

@@ -1,5 +1,5 @@
 ### CLI layer, core infrastructure, agent mode, tests and docs (`audit-core`)
-_Last updated 2026-10-03T15:50Z · main @ 045d7ec_
+_Last updated 2026-10-03T21:51Z · main @ 045d7ec_
 
 | ID | P | Problem | Source | Issues | Status |
 |---|:-:|---|---|---|---|
@@ -21,9 +21,9 @@ _Last updated 2026-10-03T15:50Z · main @ 045d7ec_
 | C16 | 2 | Batch limits are split across crates. The CLI owns 500 / 100 / 256 KiB, and `search_patches_batch` documents a maximum of 500 without enforcing it. The in-memory engine keeps a third copy (default 100, no body cap). | 7.2 | #675 | filed #675 |
 | C17 | 2 | Digest helpers are duplicated: ~30 inline `hex::encode(Sha256::digest(..))` sites, and `sha256_hex` copies that *compute* beside a `utils::digest::sha256_hex` that *validates*. `sha1_hex` exists twice, and SRI formatting is inlined three times. | 4.4; 7.3 | #706 | filed #706 |
 | C18 | 2 | There are four UUID grammars. `client.rs` has one, CLI `lib.rs` a byte-identical copy, `path_safety.rs` accepts lowercase only, and `apply.rs` accepts any alphanumeric plus `-`/`_`. | 7.3 | #705 | filed #705; a fifth grammar (`Uuid::parse_str` in `python_script.rs`) |
-| C19 | 2 | Env truthiness has three vocabularies, and there are 37 inline "empty means unset" reads and four home-directory resolvers. | 7.3 | | to verify |
+| C19 | 2 | Env truthiness has three vocabularies (core's `"1"`/`"true"` match kept correct only by `apply_env_toggles`), and there are ~29 inline "empty means unset" reads and six or more home-directory resolvers that disagree on Windows. | 7.3 | #727 | filed #727 |
 | C20 | 2 | Purls have two builder families in `utils/purl.rs`, plus 78 hand-built `pkg:` strings and 58 `starts_with("pkg:<type>/")` checks that bypass `Ecosystem::from_purl`. | 6.4; 7.3 | | to verify |
-| C21 | 3 | `utils/fs.rs` has six atomic writers, and separate stage + rename code lives in `blob_fetcher` and `update/`. Writes that bypass `utils::fs` (`blob_fetcher.rs`) escape group commit. | 5.7; 7.3 | | to verify |
+| C21 | 3 | `utils/fs.rs` has six atomic writers (four boolean policies) and a blocking copy of the stage code; `blob_fetcher` has a third stage+rename. (The group-commit escape claim was wrong; `update/` is legitimately separate.) | 5.7; 7.3 | #728 | filed #728 |
 | C22 | 2 | Telemetry has 17 near-identical `track_*` wrappers, builds a new HTTP client for every event, and threads the token and org through 125 signatures. Target: one `track(Event)` with a shared client. | 7.5; R15 | | to verify |
 | C23 | 2 | Dead code: `PatchSources::mem_blobs` is never `Some`. `save_redirect_state` and its group-commit entry journal a file that nothing writes. The `switched_off("group_commit")` oracle path, the `Pypi`/`LauncherCache` update channels and `--vendor-source` (one value) also remain. | 7.4; 7.6 #3; 5.6 | | to verify |
 | C24 | 2 | Apply and rollback are mirror images: the verify types are identical, and `fold_copy_result`, the pnpm peer fan-out and the sidecar boundary are each written twice. Target: one engine. | 7.4; 7.6 #4 | | to verify |
@@ -44,6 +44,7 @@ _Last updated 2026-10-03T15:50Z · main @ 045d7ec_
 | C39 | 2 | The 401/403 proxy fallback is missing beyond `get` search: `apply`, `rollback` and `repair` blob/diff downloads and `vendor` eject view fetches fail on a stale token, although the contract promises eject `get`'s fallback. Fix: the fallback moves into `ApiClient`. | new finding | #647 | filed #647 |
 | C40 | 3 | `SOCKET_API_CONCURRENCY` and `SOCKET_WALK_THREADS` are read by core but documented nowhere; only clap-bound env vars have a guard test. | new finding | #678 | filed #678 |
 | C41 | 2 | Hash case policy is per site: blob download compares case-insensitively and the validators accept uppercase, but agent-mode apply/rollback verify with exact `==`, so an uppercase manifest hash never verifies. Vendored verify sites are split the same way. | new finding | #707 | filed #707 |
+| C42 | 2 | `get` writes a patch view's inline blobs to `.socket/blobs/<hash>` without verifying the hash, in place, overwriting existing verified blobs; the fetch path verifies and stages. It also hand-rolls base64. | new finding | #726 | filed #726 |
 
 **Handed off** (to the CI janitor): report-only coverage and LTO `docker-base` off PRs; e2e from 148 to ~50 legs; a reusable compat workflow; no per-leg compiles; dead CI path filters (review 8.2, 8.5 B/C/E).