fix(composer): redirect confirmation, discovery, setup fidelity, revert safety - #195
Merged
Merged
Conversation
The composer.lock rewriter wrote `\/`-escaped URLs while the post-rewrite confirmation probe searched only raw and percent-encoded spellings, so every successful composer redirect reported `redirected: 0`, persisted no patch records into the ledger, and left `vex` with nothing to attest. The rewriter now emits composer-native raw slashes (PHP `JSON_UNESCAPED_SLASHES`) and shares one `artifact_url_present` predicate with the probe — both spellings accepted — so writer and prober can never drift again. Three more rewriter defects fixed: - Entry lookup is bounded by a string-literal-aware brace matcher, so a source-only (dist-less) target fails closed with `redirect_composer_no_dist` instead of silently rewriting the NEXT package's dist url+shasum — a checksum-clean wrong-code install. - Names match case-insensitively and the locked version must match the patched one through composer's leading-`v` normalization; a drifted lock fails closed with `redirect_composer_version_mismatch`. - A dist block without a `shasum` key now gets the sha1 pin inserted (composer's key order) instead of redirecting unverified. New codes: `redirect_composer_no_dist_url`, `redirect_composer_lock_malformed`. The basic golden is converted to the raw-slash format real composer writes, with five new cases (escaped-slash lock, no-shasum-key, source-only bystander, version-mismatch, mixed-case + v-prefix). A new in-process e2e proves the full loop: rewritten lock, `redirected: 1`, patch record and revert edit in the ledger. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Discovery hardcoded `cwd/vendor`, so a project using composer.json `config.vendor-dir` or `COMPOSER_VENDOR_DIR` scanned as "not yet installed (lockfile-only)" and apply resolved its packages as not-found — installed vulnerable code could never be patched, with no error. The vendor dir now resolves the way composer does (`COMPOSER_VENDOR_DIR` > `config.vendor-dir` relative to the manifest > `vendor`), shared by crawl, find, and apply. Unsafe config values (traversal, absolute) fail closed rather than falling back to a tree composer never installed into. installed.json's `install-path` was ignored, so packages installed through composer/installers (WordPress plugins, Drupal modules — the classic PHP CVE carriers) were invisible and unpatchable even though their exact path sat unread in the record. It is now the authoritative package dir, gated lexically to the project root; a rejected path drops the entry instead of falling back to the conventional dir a poisoned record could squat. lock_inventory now shares the crawler's `normalize_version`, so a composer-legal `V1.2.3` tag no longer double-counts as an installed row plus a phantom lockfile-only row (both previously POSTed to the API). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
`setup` re-serialized composer-authored manifests from their native 4-space indent (PHP `JSON_PRETTY_PRINT`) to 2-space, so it produced a whole-file diff and `--remove` could never restore the original bytes. It now detects and keeps the file's own indentation and trailing-newline shape via the shared detect_indent/serialize_json helpers; a composer-written manifest round-trips byte-identically. The write also went through the non-mode-preserving atomic writer — the only manifest editor that did — resetting e.g. 0744 to 0644. Switched to `atomic_write_bytes_preserving_mode` like every sibling editor; the in-tree RED test documenting the bug is un-ignored and passing. The setup-matrix composer case never staged a composer.json before running `setup`, so it reported `no_files` and the case was deterministically red under `--features setup-e2e` (and a silent 0-test green without it), mislabeling working functionality as a baseline gap. The driver now stages a minimal manifest like the npm arms, and `baseline_supported` flips to true so a future failure reads REGRESSION instead of BASELINE GAP. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Reverting a repair-reconstructed composer entry (wiring lost, ledger rebuilt from the wired lock) reported success while deleting the vendored artifacts and leaving composer.lock pointing at the deleted paths — the next install failed "Source path ... is not found" with no guidance. The pristine dist survives only in `wiring[].original`, which such an entry does not have, so synthesizing a restore would mean guessing; revert now refuses the entry instead, keeps the artifacts and the ledger row, and names the escape hatch (`composer update --no-install <pkg>`, then revert again). Dry and wet runs agree. The GC orphan sweep deleted un-ledgered uuid dirs under `.socket/vendor` on the false premise that no ledger means no live wiring — with state.json lost, dirs still referenced by a lockfile were swept and fresh installs broke. Candidates are now cross-checked against `scan_vendor_references` (the same all-ecosystem scan repair reconstructs from); referenced dirs are kept and surfaced as `vendor_orphan_still_wired`, and reclaimed counts no longer include them. The docker vendor capstone's networked `composer update` gains the sibling suite's 3-attempt retry — that fetch is the documented dominant flake. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The e2e_composer matrix job invoked its suite with `-- --ignored`, but e2e_composer.rs has no #[ignore] tests, so the job permanently passed while running zero tests. The shared run step now takes a per-entry `test_filter` and composer sets `--include-ignored` — the two real tests run, and a future #[ignore]-gated composer capstone still runs on this leg. Every other matrix entry renders the same command as before. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 734a8e0. Configure here.
Composer-legal `config.vendor-dir` values with `.` segments or backslash separators (`./vendor`, `./lib/deps`, `lib\deps`) were refused by the safety gate with no fallback, regressing those projects to zero discovery — worse than the old hardcoded `vendor/` (Bugbot review finding). The value is now reduced to plain `a/b` segments first (`.` drops, `..` resolves lexically and fails closed above or at the project root, absolute stays refused), then gated. The crawler e2e tests injected absolute paths into fixture JSON via `display()`, which on Windows embeds raw backslashes and makes the document unparseable — installed.json discovery collapsed to zero and install_path_escaping_project_root_is_rejected failed for the wrong reason (and the absolute vendor-dir refusal test passed vacuously). Both injections now JSON-escape backslashes. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Wenxin Jiang (Wenxin-Jiang)
approved these changes
Aug 14, 2026
Mikola Lysenko (mikolalysenko)
enabled auto-merge (squash)
August 14, 2026 20:24
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Union of both sides' redirect unit tests (composer helpers/tests from this branch, pnpm v5/v6 + CRLF tests from main); validated with the redirect, crawler, and composer e2e suites. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Union splice of the composer and cargo (#193) in-process redirect test blocks appended at the same anchor; suite passes 33/33. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Aug 14, 2026
Composer redirect work (#195) landed on main. Same conflict shape as the previous merge, in `tests/in_process_redirect.rs`: both sides append to the end of the file — this branch's hosted `--prune` warning contract test, main's composer redirect fixtures and tests. Independent, so both were kept. Assisted-by: Claude Code:claude-opus-5
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Aug 14, 2026
…er-cleanup Picks up #195 (composer redirect confirmation/pinning), which landed on main while this branch was being reconciled. No overlap with the cargo takeover work; merged clean. Assisted-by: Claude Code:claude-opus-5
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.
LLM Description written by Claude Code:claude-fable-5
What
Fixes the 13 verified bugs from a full audit of composer (PHP) support — code review across every composer code path plus behavioral testing against real composer 2.10.2 / PHP 8.5.8, with each finding adversarially verified before being fixed. Five logical commits, one per area.
Hosted / registry-redirect (
94e51ec)redirected: 0— the rewriter wrote\/-escaped URLs, the confirmation probe searched raw/percent-encoded only. No patch records reached the ledger; VEX had nothing to attest. The rewriter now emits composer-native raw slashes and shares oneartifact_url_presentpredicate with the probe.shasumkey was redirected with no integrity pin; the sha1 is now inserted.redirected: 1with ledger records end to end — the coverage gap that hid all of this.Scan discovery (
e55d6cf)config.vendor-dir/COMPOSER_VENDOR_DIRwere ignored: installed packages were misreported as lockfile-only and apply silently couldn't locate them. Vendor-dir resolution now mirrors composer's precedence.install-pathwas dropped, making composer/installers packages (WordPress plugins, Drupal modules) invisible and unpatchable. It is now the authoritative package dir, lexically contained to the project root, with no conventional-path fallback for rejected values.V1.2.3tags double-counted (crawler strippedv|V, lock inventory onlyv); both now sharenormalize_version.Setup (
f13889c)--removenever restored bytes; serialization now keeps the input's indent and newline shape (byte-identical round-trip, proven against PHP-generated fixtures).--features setup-e2e; it now stages a minimal manifest and passes 15/15 against real composer, andbaseline_supportedflips true so future failures read REGRESSION.Vendored (
7c57ac4)scan_vendor_referencesand kept withvendor_orphan_still_wired.composer updategains the documented 3-attempt retry.CI (
734a8e0)e2e_composermatrix job ran-- --ignoredagainst a suite with zero ignored tests — a permanent 0-test green. It now runs the real tests via a per-entrytest_filter(--include-ignored); all other entries render unchanged. (FYI:e2e_cargoande2e_golanghave the same vacuous shape; left out of scope.)Testing
Combined-tree battery, all green: full
socket-patch-coresuite;e2e_composer2/2;in_process_redirect24/24 (incl. the new composer confirmation e2e, whose failure mode was falsified against the pre-fix code first);e2e_vendor_composer_build7/7 plus the real-composer host capstone (vendor → fresh-checkout install → revert);ecosystem_dispatch_e2e22/22;in_process_vendor27/27;setup_matrix_composer15/15 under--features setup-e2ein host mode;cargo fmt --checkclean. Crawler fixes additionally validated live against real composer projects (custom vendor-dir, composer/installers WP-plugin layout,COMPOSER_VENDOR_DIR).Not run here:
docker_e2e_composer/docker_e2e_vendor_composer— the local Docker daemon is wedged (the feature compiles; suites passed pre-fix on 2026-08-13 and deserve one real run once a daemon is available).Follow-up (depscan)
The TS twin
workspaces/app/src/patches/registry-rewrite/composer-lock.tshas the same rewriter bugs and is already ported to mirror this behavior byte-for-byte (verified against these goldens). It lands in depscan together with the socket-patch submodule bump after this merges.🤖 Generated with Claude Code
Note
Medium Risk
Changes touch lockfile rewriting, patch target resolution, and vendor revert/delete paths where mistakes could break installs or delete the wrong artifacts; scope is Composer-focused with substantial new tests and fail-closed guards.
Overview
Fixes a cluster of Composer ecosystem bugs across redirect, scan/apply discovery,
setup, vendor revert, and CI.Registry redirect now emits composer-native raw slashes in
composer.lock, sharesartifact_url_presentwith the hosted scan confirmation probe (so successful redirects are counted and ledger/VEX records land), bounds dist edits to the correct package entry (version + case-insensitive name), inserts missingshasumpins, and fails closed on source-only targets and version mismatches instead of rewriting a neighbor package’s dist.Discovery respects
config.vendor-dir,COMPOSER_VENDOR_DIR, andinstall-path(with project-root containment), and aligns lock inventory version normalization with the crawler so packages are not double-counted.setuppreserves manifest indent/newline shape and file mode oncomposer.jsonedits; the setup-matrix composer case now stages a manifest and is marked baseline-supported.Vendor revert refuses repair-reconstructed entries with no restorable wiring while the lock still points at vendored paths; orphan sweeps keep dirs still referenced in lockfiles and warn with
vendor_orphan_still_wired.CI runs
e2e_composerwith--include-ignoredso non-ignored tests actually execute; docker vendor composer gets a retry loop oncomposer update.Reviewed by Cursor Bugbot for commit 734a8e0. Configure here.