fix(gem): hosted grant rotation refreshes the source block in place — no more nested Gemfile sources - #211
Conversation
…ocks with the CLI's empty token Regression tests for the P0: the CLI's only production DepOverride construction site (scan/hosted.rs) leaves token empty, so the gem rewriter's grant-rotation idempotency guard (which wildcards only non-empty rotating segments) never recognizes the previous grant's source block. A re-scan under a rotated grant wraps the old block's indented gem line in a NEW nested source block, keeps the stale token URL live, corrupts the ledger revert chain, and reports success. - core unit: rotated re-run with CLI-shaped (empty-token) overrides must refresh the URL in place — currently nests (2 blocks). - core unit: gems.rb/Gemfile identical twins + rotated grant + empty token must not be trapped behind spellings-diverge — currently is. - e2e (real host bundler): scan A -> re-scan A (byte-idempotent, holds today) -> re-scan B rotated — currently nests exactly like the campaign repro (hosted-b1/b2/b4 idempotent-rerun), then fresh install of the rotated pair. All three captured red at this commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pulating DepOverride.token
Defense in depth for the P0 rotated-grant nesting corruption:
- core: gem_index_url_pattern now derives the grant-token path level
from the index URL itself (the segment immediately preceding the
patch uuid — production's /patch-registry/gem/{token}/{uuid}/ shape)
and wildcards it unconditionally, so the block recognizer, the
refresh-in-place branch, and gem_spelling_residue's footprint
erasure all survive a caller that leaves token empty.
- cli: scan/hosted.rs (the only production DepOverride construction
site) now recovers the token via the new pub
grant_token_path_segment helper from the registryOverride indexUrl
(artifact URL fallback) instead of hard-coding String::new() —
every path-token'd ecosystem gets a truthful token.
- polish: the redirect_gem_source_option refusal prescribes
'socket-patch vendor --revert' when the blocking path: option is
socket-patch's own .socket/vendor wiring.
Red tests from the previous commit now pass; the rotated-grant e2e leg
(token A -> A -> B, same uuid, real host bundler) proves exactly one
source block, a redirect_gemfile_source_url ledger edit, and a green
fresh-checkout install of the patched bytes.
NOTE: the depscan TS twin (registry-rewrite gem.ts) must be ported to
match — cross-repo follow-up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reviewer P2: the new code introduced 5 rustfmt diffs on its own lines (main's versions of both files are fmt-clean). Scoped rustfmt run on the two touched files only; no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9a47426. Configure here.
| would override the Socket source block; run `socket-patch \ | ||
| vendor --revert` first, then re-run the hosted scan", | ||
| dep.name | ||
| ) |
There was a problem hiding this comment.
Remediation understates revert blast radius
Medium Severity
The new redirect_gem_source_option branch recommends socket-patch vendor --revert without stating that command’s full side effects. vendor --revert restores lockfile fragments and deletes .socket/vendor/ artifacts for every vendored package in the project, not only the blocking gem—so following the advice can eject unrelated vendored deps.
Triggered by learned rule: Remediation text must accurately state the full blast radius of recommended commands
Reviewed by Cursor Bugbot for commit 9a47426. Configure here.
| ); | ||
| let env: serde_json::Value = serde_json::from_str(&stdout).expect("rotation envelope JSON"); | ||
| assert_eq!(env["redirect"]["redirected"], 1, "envelope: {env}"); | ||
| let gemfile = std::fs::read_to_string(fx.proj.join("Gemfile")).unwrap(); |
There was a problem hiding this comment.
New tests use bare unwrap
Low Severity
The new grant-rotation e2e capstone reads the Gemfile and redirect ledger with bare .unwrap() in several places. On failure those panics omit which setup or assertion step broke, which makes CI triage harder than .expect("…") with a step-specific message.
Triggered by learned rule: Prefer .expect("context") over bare .unwrap() in test code
Reviewed by Cursor Bugbot for commit 9a47426. Configure here.
1 participant
Defect
A hosted gem re-scan under a rotated grant (production rotates the grant-token path segment of the patch-registry URL per request) did not recognize the Socket source block a previous run wrote — it wrapped it in a new one, leaving nested
source "..." doblocks in the Gemfile. On gems.rb/Gemfile twin spellings the same blindness tripped theredirect_gem_gemfile_spellings_divergetrap instead of refreshing. Silent Gemfile corruption on the exact re-scan path production exercises.Root cause
gem_index_url_pattern(crates/socket-patch-core/src/patch/redirect/mod.rs:3001): the rotation-idempotency wildcard was keyed onDepOverride.token— and the CLI never populated that field, so the pattern degraded to an exact-URL match. A block written under grant token A was invisible to a run under token B, and the rewriter appended a fresh block around it.crates/socket-patch-cli/src/commands/scan/hosted.rs:286: the grant token is not a top-level reference-API field, so the CLI shippedtoken: "".Fix
Two-sided, so the guard is caller-independent:
gem_index_url_pattern), and a rotated grant refreshes the URL in place (ledger editredirect_gemfile_source_url, original = old URL) — never nests. New helpergrant_token_path_segment(mod.rs:2979) derives the token from the URL shape.hosted.rsnow populates a truthful token for all ecosystems via the same generic path-segment-before-uuid derivation, so any futuredep.tokenconsumer is safe. (Audit:gem_index_url_patternwas the only production consumer repo-wide; bun/npm/cargo/maven/nuget rotation idempotency does not use it.)Campaign repro
Hosted+vendored real-data sweep,
hosted-b2/idempotent-rerunleg: token A → A → B on the same patch uuid. Run 2 was byte-idempotent; run 3 (rotated grant) nested two source blocks — the exact Gemfile shape from that lane'srun.logis pinned in the red commit. The lane'srun.shscripts pass as-written on the fixed binary (their green path is the fix).Tests (red → green)
8bd8960, test-only): unitgemfile_rerun_with_rotated_grant_and_cli_empty_token_never_nestsfailed with the exact nested-2-blocks Gemfile;gems_rb_twins_rotated_grant_with_cli_empty_token_refreshesfailed on the divergence trap; new e2e capstonegem_hosted_rotated_grant_rescan_refreshes_source_block_and_installsfailed through the real binary + real bundler 4.0.15 with the campaign's production failure shape.1def4d0): both units pass; full e2e suite 5/5 — the rotation leg proves exactly one source block, theredirect_gemfile_source_urlledger edit (original=token-A URL, new=token-B URL), and a green fresh install of byte-verified patched gems. Plusgrant_token_path_segment_shapes(7 URL-shape edges incl. uuid-as-host and uuid-in-first-segment) and Gemfilesource:-option refusal units.9a47426): scoped rustfmt on the two touched files (reviewer P2 — main's versions were fmt-clean, the drift was introduced here).Final gate at head: core lib redirect units green,
e2e_redirect_gem_build -- --ignored5/5 against real host bundler 4.0.15,cargo clippy -p socket-patch-core -p socket-patch-cli --all-targets— zero warnings in touched files (5 pre-existing warnings live in untouched test files, present on main).Reviewer sign-off
Adversarial review approved, red→green independently reproduced at unit and e2e level. Disclosed gaps riding this PR: in-container docker-matrix legs (bundler 1.17/2.7/4.0.18) were not re-run (disk headroom fell below the 15 GB stop threshold; no images were built); maven/golang rotation idempotency was not deep-audited (recommend a cross-lane check).
Cross-repo follow-up
The depscan TS twin (
registry-rewritegem.ts) must be ported to match: token-independent grant-rotation recognition — wildcard the path level before the patch uuid unconditionally. Shared golden suite should pin the nested-block red case.🤖 Generated with Claude Code
Note
Medium Risk
Changes hosted gem Gemfile rewrite and idempotency logic on a production re-scan path; scope is gem redirect + CLI override construction, with strong unit and e2e coverage but no broad cross-ecosystem rotation audit in this PR.
Overview
Hosted gem re-scans under a rotated grant (production rotates the grant-token segment in patch-registry URLs per request) no longer nest new
source "…" doblocks around existing Socket blocks. The rewriter refreshes the index URL in place, records aredirect_gemfile_source_urlledger edit, and stays byte-idempotent when the grant is unchanged.Core: New
grant_token_path_segmentrecovers the token from URL shape (path segment before the patch uuid).gem_index_url_patternnow wildcards that segment fromDepOverride.tokenor from the URL itself, so idempotency does not break when callers leavetokenempty.CLI:
scan/hostedpopulatesDepOverride.tokenfrom registry override or artifact URLs via the same helper.Also: When a
gemline blocks redirect because ofpath:into.socket/vendor/, the warning prescribessocket-patch vendor --revertinstead of a generic skip message.Tests: Unit cases for empty-token rotation,
gems.rbtwins,grant_token_path_segmentedges, and vendored-path warnings; e2e capstonegem_hosted_rotated_grant_rescan_refreshes_source_block_and_installs(token A → A → B through real bundler).Reviewed by Cursor Bugbot for commit 9a47426. Configure here.