GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(get,scan): retain patch-added new files; guard empty patch records by mikolalysenko · Pull Request #158 · SocketDev/socket-patch

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Aug 12, 2026 •
edited by cursor Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

Problem

fn files_with_both_hashes(patch) in crates/socket-patch-cli/src/commands/get.rs kept only patch files carrying BOTH beforeHash+afterHash, silently dropping every net-new file a patch ADDS (a file with afterHash but no beforeHash). It backed the scan/download/vendor record builders, while the by-uuid apply path save_and_apply_patch already TOLERATED new files via an empty-beforeHash sentinel.

So get <uuid> patched correctly, but scan / apply / vendor dropped added files:

  • P0 (real prod): the only free cargo patch, cf2e6f58 for pkg:cargo/[email protected], is a whole-crate export where ALL 9 files lack beforeHash. The manifest recorded files:{}, yet scan --mode vendored, apply, and vendor all printed applied:1 / success while writing NOTHING — telling the user to commit a .socket/vendor/ directory that never got created.
  • P1 (real prod): gem patch 2535d43d for [email protected] ships a genuinely-new lib/rubygems_plugin.rb (a CVE-2022-21831 runtime guard) with no beforeHash → dropped by agent-mode and --vendor-source build, silently omitting a protection file the patch author intended.

Repro (traitobject): fresh cargo new with traitobject="=0.1.1", isolated CARGO_HOME, SOCKET_NO_CONFIG=true, scan --json --yes --mode vendored → observed applied:1 with nothing written; expected either the files recorded/vendored, or a loud failure.

Fix

Two parts, both in crates/socket-patch-cli/src/commands/get.rs:

  1. Retain new-file entries. New shared helper files_for_manifest (get.rs:384) keeps every file with an afterHash, recording missing beforeHash as the empty-string sentinel — the exact convention save_and_apply_patch already relied on. All record builders now use it: record_from_patch_response (:405), download_patch_records (:938), download_and_apply_patches (:1226), and save_and_apply_patch's inline loop (:1928, refactored to the shared helper). files_with_both_hashes (:353) is retained ONLY for installed-variant matching in filter_to_installed_releases (:800), where new files are non-discriminating anyway (select_installed_variants filters on a non-empty beforeHash).

  2. Guardrail. If a built record's files map ends up EMPTY, it is now surfaced as a failure instead of a success: download_and_apply_patches and download_patch_records push a failed action and increment the failed count (degrading status/exit via run_outcome), and save_and_apply_patch reports a loud error and returns exit 1. A defective all-files-dropped patch can never again be counted as applied:1.

Test

Three hermetic unit tests in get.rs's #[cfg(test)] module:

  • files_for_manifest_retains_new_file_without_before_hash — a patch adding lib/rubygems_plugin.rb (afterHash, no beforeHash) is retained with an empty-beforeHash sentinel; the strict helper still drops it.
  • files_for_manifest_keeps_all_new_file_whole_crate_export — the P0 cargo case: 9 whole-crate-export files all retained (old rule produced files:{}).
  • build_patch_record_from_new_files_is_not_empty — record_from_patch_response retains new files (RED before the fix), and a no-afterHash patch yields the empty map the guardrail rejects.

Verified RED→GREEN: reverting record_from_patch_response to files_with_both_hashes makes build_patch_record_from_new_files_is_not_empty fail; with the fix all 56 commands::get::tests pass. cargo build -p socket-patch-cli is clean.

Scope

Focused on the CLI record-builder + guardrail. Does not touch the core apply engine (already new-file-tolerant) or blob download. The other half of the gem-new-file finding (service-side export) is out of scope. Kills sweep findings: cargo-scan-apply-vendor-silent-noop, scan-vendored-false-applied-nothing-written, download-flow-drops-new-files, gem-new-file-dropped-build-weaker-than-service (CLI-drop half), and the K4 axios "works via get, breaks via scan" class.

🤖 Generated with Claude Code


Note

Medium Risk
Changes how scan/apply/vendor build manifest records and exit/status for defective patches—user-visible behavior improves but wrong patches now fail instead of silent no-ops; core apply engine unchanged.

Overview
Aligns scan, apply, and vendor with get <uuid> by introducing shared files_for_manifest, which records every file with an afterHash—including net-new files that only have an afterHash—using an empty-string beforeHash sentinel. The previous files_with_both_hashes rule dropped those files, so whole-crate cargo exports and gem runtime-guard additions could end up with files:{} while JSON still reported applied:1 and nothing was written.

files_with_both_hashes is only used for installed-release variant matching in filter_to_installed_releases; all manifest record paths now call files_for_manifest (record_from_patch_response, download/apply, vendor download, and save_and_apply_patch).

Guardrail: if a patch yields no recordable files, flows treat it as failed (not skipped/applied)—vendor download, download_and_apply_patches, and save_and_apply_patch error out or increment failure counts so empty patches cannot masquerade as successful protection.

Unit tests cover new-file retention, whole-crate export (9 files), and non-empty records from record_from_patch_response.

Reviewed by Cursor Bugbot for commit 32bb21d. Configure here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mikolalysenko @claude
fix(get,scan): retain patch-added new files; guard empty patch records
32bb21d
The scan/download/vendor record builder used `files_with_both_hashes`,
which kept only files carrying BOTH before+after hashes and silently
dropped every net-new file a patch ADDS (afterHash, no beforeHash). The
by-uuid apply path already tolerated new files via an empty-beforeHash
sentinel, so `get <uuid>` patched correctly while `scan`/`apply`/`vendor`
dropped added files.

Real-prod impact: the only free cargo patch ([email protected]) is a
whole-crate export where ALL files lack beforeHash, so the manifest
recorded `files:{}` yet reported `applied:1` while writing nothing; and a
gem patch's genuinely-new `lib/rubygems_plugin.rb` CVE guard was dropped.

Fix (two parts):
1. New shared `files_for_manifest` helper keeps every file with an
   afterHash using the empty-beforeHash sentinel; the record builders
   (`record_from_patch_response`, `download_patch_records`,
   `download_and_apply_patches`, `save_and_apply_patch`) now use it.
   `files_with_both_hashes` is retained ONLY for installed-variant
   matching, where new files are non-discriminating anyway.
2. Guardrail: a record whose resulting files map is EMPTY is counted as
   failed (loud error / non-success exit), never reported as
   applied/vendored — a defective all-files-dropped patch can no longer
   be silently counted as `applied:1`.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@mikolalysenko
Mikola Lysenko (mikolalysenko) enabled auto-merge (squash) August 12, 2026 23:39
Wenxin-Jiang
Wenxin Jiang (Wenxin-Jiang) approved these changes Aug 12, 2026
View reviewed changes
@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit 5d7eb6f into main Aug 12, 2026
37 of 43 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the fix/get-tolerate-new-file-patch-entries branch August 12, 2026 23:43
@mikolalysenko Mikola Lysenko (mikolalysenko) mentioned this pull request Aug 13, 2026
fix(test): unbreak main — give four get edge-case fixtures a real file for #158's guardrail #168
Merged
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Aug 13, 2026
@mikolalysenko @claude
fix(test): repair telemetry_e2e fixture broken by #158 empty-files gu…
28af3a0
…ardrail

Same root cause as this PR's get_edge_cases_e2e fixes: #158's guardrail
(empty applicable-files map -> "patch has no applicable files", exit 1)
broke get_emits_patch_fetched_telemetry_on_uuid_lookup_success, whose mock
patch-view used "files": {} while asserting a successful get. Give it a
recordable new-file entry (afterHash = git-blob sha256 of the decoded blob)
so the success + telemetry assertions hold; guardrail untouched. Verified
with a full `cargo test --workspace --no-fail-fast` pass that these two
binaries held the only 5 casualties, now all green.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Aug 13, 2026
@mikolalysenko @claude
fix(test): give four get edge-case fixtures a real file so #158's gua…
b109645
…rd passes (#168)

Merged PR #158 (5d7eb6f, "fix(get,scan): retain patch-added new files;
guard empty patch records") added a correct guardrail: a fetched patch
view whose `files_for_manifest` map is EMPTY is now an exit-1 failure
("patch has no applicable files") instead of a silent `applied:1`. But
#158 only touched get.rs and left four tests in get_edge_cases_e2e.rs
carrying stale `"files": {}` patch-view fixtures that still assert the
`get` succeeds (exit 0). Those four now fail on main:

  - get_with_id_flag_selects_specific_patch
  - get_uuid_returns_paid_patch_with_token_succeeds
  - get_on_vendored_purl_warns_about_uuid_drift
  - get_uuid_replacing_existing_manifest_entry_reports_updated

Their real concern is selection / paid-token / drift-warning /
manifest-replacement logic, not "a patch with zero files" — the empty
map was only a lazy stand-in. Give each view fixture one recordable
net-new file (all-zero `beforeHash`, real git-blob `afterHash` via the
shared `common::git_sha256` oracle, matching base64 `blobContent`),
modeled on the passing `get_invariants::patch_response_json` fixture, so
`files_for_manifest` is non-empty and the guard is satisfied. Every
existing assertion (found==1, selected UUID, drift warning, `updated`
reporting + `oldUuid`, manifest move) is unchanged. The guardrail in
get.rs is untouched.

get_edge_cases_e2e now 22/22; lib (350) and get_invariants /
get_update_summary (15) unaffected.

Co-authored-by: Claude Opus 4.8 <[email protected]>
Mikola Lysenko (mikolalysenko) added a commit that referenced this pull request Aug 13, 2026
@mikolalysenko
Merge origin/main (pick up #168: get edge-case fixtures gain a real f…
c2b09b0
…ile for #158's empty-record guard)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Wenxin-Jiang Wenxin Jiang (Wenxin-Jiang) Wenxin-Jiang approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@mikolalysenko @Wenxin-Jiang
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.