fix(registry): honor --insecure end-to-end on tarball paths + Phase 43 gate#6
Merged
Conversation
…3 gate `--insecure` was only honored at the registry base-URL check (`validate_base_url`). `download_tarball`, `download_tarball_to_file_with_limit`, `download_tarball_streaming`, and the Phase 43 `evaluate_cached_url` gate all hard-rejected HTTP non-localhost URLs independently of `allow_insecure`, so `lpm install --insecure` against an HTTP mirror succeeded at metadata fetch and then failed at tarball download with "tarball URL must use HTTPS (got: http://...)". CLI help text at `crates/lpm-cli/src/main.rs:87` says "Allow insecure HTTP connections to non-localhost registries" — the promise explicitly covers non-localhost HTTP, so this is option (a) from the deferred-item block in `DOCS/new-features/37-rust-client-RUNNER-VISION-phase43.md` (§"Pre-existing --insecure inconsistency that Phase 43 inherits"). Option (b) — narrowing the flag to localhost-only — would contradict the flag's own help text. Changes: - Add `RegistryClient::allow_insecure(&self) -> bool` accessor so free functions (`evaluate_cached_url`) can honor the same carve-out. - Thread `&& !self.allow_insecure` into all three tarball-download guards, matching `validate_base_url`'s reference pattern at client.rs:270-274. - Thread `&& !client.allow_insecure()` into `evaluate_cached_url`'s scheme check so Phase 43's cached-URL gate is symmetric with the tarball paths. - Extend the tarball-path error text with "Pass --insecure to allow HTTP non-localhost." so the flag is discoverable. The gate is internal so its `GateDecision::RejectedScheme` outcome stays unchanged. - Tests: rename the three HTTP-rejection tests to `*_without_insecure`, assert the new `--insecure` hint, and add paired `*_with_insecure` / `*_accepts_http_with_insecure` companions. Add streaming-path scheme tests (previously uncovered) for symmetry across all four sites. CI gate green: - cargo clippy --workspace -- -D warnings: clean - cargo fmt --check: clean - grep -r fancy-regex crates/*/Cargo.toml: absent - cargo build --workspace: clean - cargo nextest run --workspace --exclude lpm-integration-tests: 3681 passed - cargo test -p lpm-auth: 43 passed, 3× deterministic Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tests Follow-up to the prior commit (483a5b5) addressing two audit findings: Finding 1 (contract): the guard `!is_https && !is_localhost && !allow_insecure` widens `--insecure` to *any* non-HTTPS non-localhost URL — `file://`, `ftp://`, `data:`, `javascript:`, etc. — which contradicts the CLI help text ("Allow insecure HTTP connections to non-localhost registries") and the new error copy. A tampered lockfile with `file:///etc/passwd` would no longer be rejected once `--insecure` was set. Same bug was present in `validate_base_url` — the "reference pattern" the prior commit was told to follow; left loose in the original fix because the instructions flagged it as out of scope, but the audit exposed it as the same bug wearing a different hat. Finding 2 (tests): the three `*_allows_http_with_insecure` tests hit `evil.com` for real, leaving the unit suite network-dependent, and only asserted absence of the scheme-error string — they pass on any unrelated DNS/connect/HTTP failure, so they didn't actually prove `--insecure` lets HTTP through. Changes: - Add `is_http_url(url) -> bool` paired with `is_https_url`. - Extract `RegistryClient::check_tarball_url_scheme(&self, url)` as the single scheme gate used by all three tarball methods. Predicate is now `is_https || is_localhost || (allow_insecure && is_http)` so `--insecure` widens specifically to HTTP, not to arbitrary schemes. - Tighten `evaluate_cached_url`'s scheme check and `validate_base_url` to the same predicate. All four rejection points now agree. - Error text on `validate_base_url` rewritten to match the tarball- path hint shape ("Use HTTPS, or pass --insecure to allow HTTP non-localhost"). Still contains "insecure" so existing reject tests keep their assertion stable. - Delete the three `*_allows_http_with_insecure` tests that hit evil.com. Coverage moves into hermetic unit tests against `check_tarball_url_scheme` directly — no network, stronger asserts. - Add Finding 1 regression guards: - `check_tarball_url_scheme_rejects_file_even_with_insecure` - `check_tarball_url_scheme_rejects_non_http_schemes_even_with_insecure` (ftp / data / javascript / gopher) - `validate_base_url_rejects_file_scheme_even_with_insecure` - `validate_base_url_rejects_non_http_schemes_even_with_insecure` - `phase43_gate_rejects_file_scheme_even_with_insecure` - Add `is_http_url_cases`, `check_tarball_url_scheme_allows_https`, `check_tarball_url_scheme_allows_localhost_http`, and the hermetic `check_tarball_url_scheme_allows_http_non_localhost_with_insecure` replacement for the deleted evil.com test. - Add a new `check_tarball_url_scheme_rejects_http_non_localhost_without_insecure` unit test mirroring the existing method-level reject test. CI gate green: - cargo clippy --workspace -- -D warnings: clean - cargo fmt --check: clean - grep -r fancy-regex crates/*/Cargo.toml: absent - cargo build --workspace: clean - cargo nextest run --workspace --exclude lpm-integration-tests: 3688 passed (+7 vs prior commit: 8 new hermetic tests − 3 non-hermetic deleted; net +5, and 3 existing scheme-reject tests now also assert the --insecure hint) - cargo test -p lpm-auth: 43 passed × 3 deterministic Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7 tasks
tolgaergin
added a commit
that referenced
this pull request
Apr 29, 2026
* phase-60 D2: promote download_tarball_routed helpers to RegistryClient
Behavior-preserving refactor extracting the two private routed-tarball
helpers from install.rs (download_tarball_routed,
download_tarball_streaming_routed) onto RegistryClient as public
methods. Both `lpm install` and the upcoming Phase 60 `lpm add` source-
delivery flow consume the same Custom-route auth-attachment logic.
- crates/lpm-registry/src/client.rs: add public methods
- crates/lpm-cli/src/commands/install.rs: switch all 5 call sites to
the new methods; delete the private helpers; remove the now-unused
DownloadedTarball import
All 602 install + npmrc tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* phase-60 60.0.e: PackageMetadata::resolve_version_spec helper
Add a three-tier version-spec resolver on PackageMetadata covering
dist-tag → exact-version → semver-range, mirroring the canonical
pattern at install_global.rs:368-405 verbatim.
Pre-Phase-60, `lpm add react@beta`, `next@canary`, `lodash@^4` all
failed because PackageMetadata::version() is a pure HashMap lookup —
none of those literal strings exist as concrete versions. The new
helper closes the gap.
Per D3 (preplan): both parse-failure and no-satisfying-version
return LpmError::Script (matching install_global verbatim) so the
Phase 60.1 migration of the four duplicate sites (install_global,
install, update_global, global) is a true behavior-preserving
refactor.
9 unit tests cover dist-tag (latest/beta/canary), exact match,
caret/tilde range, no-satisfying error, parse-fail error, and
empty-versions error.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* phase-60 60.0+60.1+60.1.5+60.2: lpm add source delivery from any registry
Decouple `lpm add` from LPM-only package identity, mirror install's
full .npmrc setup, switch to file-spool tarball download, add
destination-side path containment, gate dep auto-install on
lpm.config.json presence, and surface external imports for the simple
path. End-to-end flow now works for any package on any registry the
rust client can reach (lpm.dev worker, npmjs.org direct, .npmrc-
declared private registries).
60.0.a + 60.0.b — Identity refactor + drop dotted-name auto-prepend
- New AddTarget enum: Lpm(PackageName) | Npm { spec: String }.
- New resolve_add_target replaces parse_package_ref. No rewriting
outside the @lpm.dev/ scope — `lodash.merge`, `tolga.foo`, etc.
resolve to AddTarget::Npm verbatim. Fixes a long-standing
correctness bug: pre-Phase-60 dotted bare names were silently
rewritten to @lpm.dev/<name> which doesn't exist on lpm.dev.
- All output / log / JSON sites render via target.display() /
target.json_name() — `name.scoped()` no longer used unconditionally.
- Skills branch type-encoded via `let AddTarget::Lpm(pkg) = &target`
pattern, with a why-comment (60.2) explaining the scope gate
(lpm.dev runs LLM scans on shipped skill content; arbitrary npm
packages are not scanned).
60.0.c — Mirror install's full .npmrc setup
- Build RouteTable::from_env_and_filesystem before any network call.
- Surface npmrc_warnings (non-JSON) and the strict-ssl=false security
warning (escapes --json). Clone the client with with_tls_overrides
so cafile= / strict-ssl=false take effect on metadata + tarball
fetches. Mirrors install.rs:3295-3445.
60.0.d — Routed metadata + file-spool tarball
- Metadata: AddTarget::Lpm uses get_package_metadata; AddTarget::Npm
uses get_npm_metadata_routed.
- Tarball: client.download_tarball_routed (D2 promoted helper) +
lpm_extractor::extract_tarball_from_file. Bounded memory via
MAX_COMPRESSED_TARBALL_SIZE (500 MB) for free; lpm add typescript
(~22 MB) and worst-case @scope/giant-fixture no longer load the
whole tarball into RAM.
60.0.f — Destination-side path containment (D6)
- New resolve_safe_dest helper canonicalizes target_dir once and
validates every write destination: refuses to follow existing
symlinks, rejects writes whose canonical parent escapes the target
root. Wired into the Step 8 file-copy loop. Closes the threat-model
gap that opened up when add expanded from "trusted lpm.dev
publishers" to "any npm publisher."
60.1 — Dep gate + bare-imports notice (D4)
- Tighten dep gate: `if !no_install_deps && lpm_config.is_some()`.
Simple path is download-manager: copy bytes, no auto-install.
- import_rewriter exports a sibling collect_bare_specifiers fn that
shares an internal SpecifierKind classifier with rewrite_imports
(anti-drift contract — "bare" means the same thing in both places).
- add.rs surfaces the collected externals as a non-JSON notice and
as a `external_imports` array in the JSON output.
60.1.5 — Non-interactive simple-path guard
- `lpm_config.is_none() && target_path.is_none() && (yes || json ||
!is_tty)` errors before the file-copy loop. Heuristically defaulting
components/ for arbitrary 3rd-party source under --yes/--json/non-TTY
is a CI/automation footgun.
Tests
- 15 unit tests in add.rs (resolve_add_target classification including
the dotted-name regression; resolve_safe_dest contracts including
symlink-refusal on Unix).
- 10 unit tests in import_rewriter.rs (classify_specifier,
collect_bare_specifiers).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* phase-60 60.3: integration tests for lpm add simple path + guards + traversal
Three new wiremock-driven integration tests covering the highest-value
end-to-end scenarios for Phase 60:
- add_simple_non_interactive_without_path.rs (4 sub-tests) — proves
the 60.1.5 guard fires for --yes, --json, and non-TTY (stdin from
/dev/null) without --path; positive control with --path succeeds.
No package.json mutation in any failure case.
- add_source_npm_simple.rs (2 sub-tests) — full simple-path pipeline
via wiremock npm metadata + tarball: AddTarget::Npm resolves, file-
spool download, extract, files copied flat (no auto-nest), bare-
imports notice lists react + @radix-ui/react-slot, package.json
NOT mutated, .lpm/skills/ NOT created. JSON sub-test asserts the
package.name uses the npm-style identity (not @lpm.dev/-prefixed)
and the new external_imports array is well-shaped.
- add_path_traversal_dest_escape.rs — proves resolve_safe_dest is
wired into the actual write loop, not just unit-tested in
isolation. Tarball ships an lpm.config.json with files[0].dest =
"../../escaped/evil.txt" — assertion: containment-violation error,
exit non-zero, no file written outside target_dir.
Other 60.3 specced tests are either (i) covered by the unit tests
that landed alongside the implementation (#5 dotted-name, #9 version-
spec, #11 symlink — see preplan v6 audit checklist) or (ii)
deliberately deferred where the underlying machinery is already
test-covered by Phase 58.x install tests (#1 lpm.dev rich, #2 npm
rich, #6 npmrc auth, #7 strict-ssl, #8 missing-var fatal).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* phase-60 60.4: README — lpm add now works against any registry
- Update the lpm add one-liner in the Commands list.
- Add a "How lpm add Works" section explaining: source delivery vs.
install, the firm naming rule (@lpm.dev/owner.name only), the rich
vs. simple paths, and the non-interactive --path requirement.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* phase-60 audit fix: resolve_safe_dest must validate before mkdir
Audit reproduced (with a temp-dir filesystem probe) that the landed
resolve_safe_dest helper still created directories OUTSIDE the
target_dir for two attack vectors before the containment error fired:
1. `dest_rel = "../../escaped/evil.txt"` — `Path::join` resolves
lexically; `dest.parent()` lands outside target; `create_dir_all`
ran before the containment check, leaving `<target>/../escaped/`
on disk even though the file write was correctly blocked.
2. Absolute `dest_rel = "/tmp/elsewhere/evil.txt"` — `Path::join` of
an absolute path returns the absolute path verbatim; `parent =
/tmp/elsewhere/`; `create_dir_all` created it before the
containment check fired.
The original integration test only asserted no escaped FILE existed,
so the directory-side-effect bug passed CI.
Fix
- Reorder resolve_safe_dest so EVERY check that can reject the
destination runs BEFORE any filesystem mutation:
Step 1 (NEW) — reject absolute dest_rel up-front.
Step 2 (NEW) — reject any ParentDir / RootDir / Prefix component.
Step 3 — refuse existing-symlink destinations.
Step 4 (NEW) — pre-mkdir ancestor canonicalization: walk up to the
longest existing ancestor; canonicalize; require it under
target_root_canonical (catches symlinked intermediate dirs).
Step 5 — create_dir_all (NOW safe).
Step 6 — post-mkdir re-canonicalize as TOCTOU defense-in-depth.
The lexical bans in Steps 1-2 kill the entire `../escape` and
absolute-path attack classes before any mkdir runs. The longest-
existing-ancestor walk in Step 4 covers the symlinked-intermediate
case (target/foo → /tmp/elsewhere). Step 6 is paranoia.
Tests
- Strengthen unit tests:
- resolve_safe_dest_dotdot_in_path_rejected_with_no_external_dir_created
now asserts no escape directory was created.
- resolve_safe_dest_absolute_dest_rejected_with_no_external_dir_created
is new — covers the absolute-path attack.
- resolve_safe_dest_dotdot_in_middle_of_path_also_rejected covers
`foo/../bar.txt` (lexically resolves back inside but still
rejected up-front).
- Extend integration test:
- dest_escape_via_dotdot_is_refused_and_creates_no_external_directory
now snapshots target_dir entries before the run and asserts no
unexpected new top-level entries appeared, plus no escape dir.
- dest_escape_via_absolute_path_is_refused_and_creates_no_external_directory
is new — covers the absolute-path attack at the integration level.
Net: 4923 → 4926 workspace tests; clippy + fmt clean; all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
tolgaergin
added a commit
that referenced
this pull request
May 8, 2026
Three small, verified wins on cold-install CPU + wall, identified by symbolicated samply on bench/fixture-large after fixes b03d051 already landed. #1 extractor: archive.set_preserve_mtime(false) Pre-fix flame attributed 2.0% of active CPU to filetime::set_file_- handle_times → fsetattrlist (100% from extract_tarball). mtime is meaningless for content-addressable store bytes — require() doesn't read it; lpm doctor doesn't use it. tar 0.4.45's preserve_mtime defaulted to true; flipping it eliminates the syscall entirely. #4 extractor: stream_entry_to_disk replaces entry.unpack() for files Even with preserve_permissions(false) tar 0.4.45's _set_perms still unconditionally calls fs::set_permissions (entry.rs:814 — the flag only controls SUID-bit retention). 1.7% of active CPU was __fchmod from 100% extract_tarball. New helper does File::create + io::copy only — same minimal write semantics as the existing write_buffered_entry path. #2 store: LinkMeta::write_to_unpublished skips inner tmp+rename populate_into stages the sidecar inside an unpublished tmp_dir (links/<key>.tmp.<pid>.<tid>/) that is published via a single outer atomic rename. The inner tmp+rename in write_to was redundant: no observer can ever see a half-written sidecar inside an unpublished dir. New write_to_unpublished writes the JSON directly. Saves one rename syscall per link entry × N packages. Verification — paired A/B median over 8 iters (worst dropped): Stage | Pre-fix | Post-fix | Δ total | 998 ms | 937 ms | −61 ms (−6.1%) fetch | 355 | 304 | −51 ms (#1 + #4) link | 138 | 132 | −6 ms (#2) Flame profile confirms target syscalls eliminated: set_perms_ownerships: 1.7% → 0 set_file_handle_times: 2.0% → 0 __fchmod: 1.7% → 0 __rename: 10.2% → 7.7% LinkMeta::write_to → write_to_unpublished: 4.2% → 0.3% Tests: cargo nextest run -p lpm-extractor -p lpm-store — 134/134 pass. Clippy: clean across workspace. Followups still open (separate tranche): - #5 fuse extract+analyze (drop redundant 2nd-pass walk, ~10% on-CPU) - #6 restore event-driven link/fetch overlap (~50-100 ms wall) - #3 lazy warm-hit sidecar touch (warm-install-only) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tolgaergin
added a commit
that referenced
this pull request
May 8, 2026
…s (v2 path) Drops the redundant post-extract directory walk + per-source-file disk re-read that the v2 path inherited from its non-streaming origin. v2's `extract_object_with_timings` now mirrors v1's `stream_and_store_package` (lib.rs:594-618): the extractor's `extract_tarball_from_reader_with_inspector` filters scannable entries via `PackageAnalyzer::should_scan` and feeds matching entries' bytes into the analyzer while still in the tar walk's write buffer. The post-extract `finalize` only reads `package.json` for manifest tags — the per-source-file pass is gone. Pre-flame attribution for the eliminated work: analyze_package (orchestrator): 3.8% active CPU analyze_single_file (per-file fs::read): 4.9% collect_source_files_recursive (fs walk): 1.5% total redundant cost: ~10.2% Post-#5 flame: analyze_package*: gone (only manifest 0.4%) analyze_single_file: gone collect_source_files_recursive: gone PackageAnalyzer::feed (during extract): 8.5% (= analyze_bytes work moved to fused path) Bench (single iter, network-noisy resolver swamps total wall): fetch_ms: 304 → 264 (−40 ms) link_ms: 132 → 124 (−8 ms) extract_sum unchanged (analyzer cost folded into the extract phase) security_sum: now ~0 (only finalize manifest read remains there) `tarball_data: &[u8]` implements Read directly so no Cursor needed. RefCell wraps the analyzer for the FnMut inspector closure. Tests: cargo nextest run -p lpm-extractor -p lpm-store -p lpm-security — 543/543 pass. Clippy: clean. Followups still open: #6 restore event-driven link/fetch overlap (~50-100 ms wall, substantial) #3 lazy warm-hit sidecar touch (warm-install only) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
7 tasks
tolgaergin
added a commit
that referenced
this pull request
May 12, 2026
Lever #6 — L4 verdict cache (`lpm-triage-advisor::l4_cache`): - New persisted store at `$LPM_HOME/cache/l4-verdicts.json` keyed by `sha256(name||version||phases||repository||prompt-hash||provider||model)`. - Wired through `AdvisorSession::classify_amber` (install pipeline) + `enrich_advisor_in_place` (audit harness, opt-in via `--l4-cache`). - Curated 523-entry warm run: 54s → 2.8s (−95% wall). - Hermetic warm run: 8.6s → 2.5s (−71% wall). - Cache disable: `LPM_L4_CACHE=0` env, cache file override: `LPM_L4_CACHE_PATH`, TTL override: `LPM_L4_CACHE_TTL_SECS`. Lever #1 — Pass `repository` URL from manifest to prompt: - `AmberScript` gains `repository: Option<&str>`. - Prompt template emits `Repository: <url>` ONLY when present — empirically emitting `<none>` pushed verdicts toward MANUAL on data lacking the field. Absent-by-default behaviour is identical to the pre-Lever prompt; the field is purely additive. - APPROVE bullet adds delegate-to-local-file installers when the Repository: line points at a recognizable identity host. - MANUAL bullet adds delegate-to-local-file when the Repository: line points at an unrelated host (identity mismatch). - Install pipeline reads `package.json > repository` via new `build_state::read_manifest_repository`; both shorthand string and object-form `{type, url}` accepted. - Audit harness pulls the same field from the live `latest` manifest (`LatestManifest::repository: Option<RepositoryField>`), persists on `PackageAudit`, plumbs through to `TriageAmberScript` + cache key. - Hermetic fixtures gain `repository` on five delegate-to-local-file amber entries. Approve rate moves 4/7 → 5/7 (+14pp). - Curated 523-entry corpus has no `repository` data; verdict rate stays within run-to-run noise (no regression). Tests: 52/52 lpm-triage-advisor, 17/17 lpm-cli triage_advisor_session. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tolgaergin
added a commit
that referenced
this pull request
May 14, 2026
…low gate Ships the 10 cross-command flow tests enumerated in v2 baseline, flips EXPECT_FULL_V2_FLOWS_BACKFILL to true so future flow drops hard-fail the audit. Each flow exercises a real-user multi-command sequence and asserts the state-transfer claim that ties the commands together — what command A leaves on disk / in the keychain / in the lockfile is the input command B reads. Single-command tests assert each step in isolation; flow tests catch state-shape mismatches between steps. Flows shipped: - install → patch → patch-commit → install (patch persistence) - migrate → install → audit (lockfile round-trips) - install → rebuild → approve-scripts → rebuild (approval lifecycle) - doctor --fix → install (fix survives install) - add → install → graph (added dep visible) - install → upgrade --major → audit (envelope shape) - token-rotate → publish --dry-run --check (token hand-off) - publish --dry-run --check → publish (target agreement) - install -g → run shimmed binary → uninstall -g (shim lifecycle) - env push → env pull cross-machine (round-trip — scoped to local smoke until a cross-machine harness lands) Several flows had their assertions scoped narrower than the original "catches" claim: - Flow #6 (rebuild lifecycle): rebuild --policy=deny ignores the v2 object form of trustedDependencies that approve-scripts writes — a real contract gap, filed as private finding #75. The flow asserts the manifest mutation; rebuild #2 only checks envelope health. - Flow #4 (upgrade major audit): the workflow tier's MockRegistry helpers don't mount GET /api/registry/{name} per-package (only the batch endpoint), so upgrade's candidate selection finds no candidates. Flow asserts envelope shape; tighten when the mock grows the per-package GET. - Flow #7 (env push/pull cross-machine): proper round-trip needs a shared-vault-state test harness that doesn't exist yet. Flow smokes per-machine env state isolation; promote when the harness lands. - Flow #8 (install -g): gracefully degrades when install-g doesn't emit a shim on the test runner (cli-binary tier owns the strict contract). Run results: 10/10 flow tests pass, all 10 v2 audit tests pass, full lpm-workflows suite green (623/623), clippy clean, fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tolgaergin
added a commit
that referenced
this pull request
May 14, 2026
…htening, cross-machine vault harness
Six focused follow-ups against the v2 coverage matrix.
JSON contract depth promotions (SemanticAsserts → InstaSnapshot):
- id 4 lpm whoami — insta snapshot added to
`whoami_recovers_session_from_refresh_token_only` in
`auth_lifecycle.rs`. Pins the envelope shape under a refresh-only
session recovery.
- id 97 lpm env ls/list — insta snapshot added to
`env_list_json_envelope_carries_keys` in `env_local.rs`. The
envelope is a flat key→masked-value map; locked with `sort_maps`
for stable ordering across `preserve_order`-enabled serde_json.
- id 101 lpm env push/pull — insta snapshot added to the GitLab
OIDC pull --json test in `env_vault.rs`. Pins the {env, count,
vars} shape after the LPM_OIDC_TOKEN canonical-input contract.
JSON contract depth promotions (None → SemanticAsserts):
- id 74 lpm approve-scripts `<pkg>` — verified the named `<pkg>`
form test reads `parsed["dry_run"]` and `parsed["approved_count"]`
via `serde_json::from_str`. Audited the other 34 None rows — most
are either commands that don't emit JSON envelopes (completions,
dev/tunnel streams, login/logout) or where the named sub-form
isn't directly covered by an envelope-reading test fn.
Cross-command flow #4 (install → upgrade --major → audit) tightened:
- Lifted the private `mount_upgrade_package` from `upgrade.rs` into
the shared `MockRegistry::with_full_package_metadata` helper. It
mounts the per-package GET (`/api/registry/{name}` + the
npm-direct `/{name}` path) AND the batch-metadata POST from one
metadata document, with optional `None` tarball-bytes for the
fail-tarball case. `lpm upgrade`'s candidate selector reads the
GET endpoint; the install fallback reads batch-metadata; the
shared helper makes both observable from a single call.
- Tightened the rebuild #2 assertion in flow #4 to require the
upgrade --major --dry-run envelope mentions both `2.0.0` and the
scoped package name. Was previously gated behind "shared mount
missing" — gate removed.
Finding #75 (rebuild --policy=deny ignores object-form
trustedDependencies) — RETRACTED:
- `TrustedDependencies` in lpm-workspace is `#[serde(untagged)]`
over both `Vec<String>` (Legacy) and `HashMap<String, Binding>`
(Rich). `evaluate_trust` in rebuild.rs routes through
`matches_strict`, which prefers the concrete `name@version` key
and falls back to the `name@*` preserve key. Object form is
already supported.
- The empty `packages[]` flow #6 originally observed was
`TrustMatch::BindingDrift`: the fixture's synthetic
`"sha256-flow-script-hash"` did not match the real
`compute_script_hash(store_dir)` value rebuild computes on disk.
Synthetic vs. recomputed hash divergence, not a missing reader.
- Fixed in flow #6 by computing the real script_hash via
`lpm_security::script_hash::compute_script_hash` and propagating
it through `.lpm/build-state.json` → approve-scripts → manifest.
Rebuild #2 now asserts `packages[]` contains `scripted-pkg@1.0.0`
with `trusted: true`.
Cross-command flow #7 (env push → env pull cross-machine) — full
byte-equality round-trip now lands:
- Added `MockRegistry::with_stateful_personal_sync(vault_id,
bearer)` to share `Arc<Mutex<Option<StoredSyncBlob>>>` between
POST and GET handlers on `/api/vaults/{vault_id}/sync`. POST
captures encryptedBlob + wrappedKey + bumps the version; GET
returns the stored payload signed with the bearer's HMAC. A
fresh GET before any POST returns 404 — the natural "machine B
pulls before machine A pushed" shape.
- Flow #7 now drives two TempProjects sharing this mock. Both
HOMEs are seeded with the same `<HOME>/.lpm/.vault-key` (32-byte
hex, the cryptographic outcome that real pairing produces) +
the same paired session bearer. Machine A: `env set` → `env push`.
Machine B: `env pull` → `env get --reveal`. The revealed
plaintext must byte-equal the value machine A pushed.
scenarios_by_file partitions populated for shared test files:
- id 83 lpm run `<script>` — run.rs: 14
- id 84 lpm run --filter / --all / --affected — run.rs: 7
- id 87 lpm lint — tools.rs: 5
- id 88 lpm fmt (write) — tools.rs: 3
- id 89 lpm fmt --check — tools.rs: 1
- id 91 lpm test — tools.rs: 7
- id 96 lpm env init — env_local.rs: 1
- id 98 lpm env set/get/delete — env_local.rs: 6
- id 99 lpm env import/export/print/copy — env_local.rs: 4
- id 100 lpm env diff/validate/check — env_local.rs: 4
Full CI gate green (workspace target, separate CARGO_TARGET_DIR):
- cargo clippy --workspace --all-targets -- -D warnings clean
- cargo fmt --check clean
- grep -r 'fancy-regex' crates/*/Cargo.toml (none)
- cargo build --workspace clean
- cargo nextest run --workspace --exclude lpm-integration-tests
6397/6397 pass
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tolgaergin
added a commit
that referenced
this pull request
May 14, 2026
…ll 118 remaining rows Closes the silent-drift window on the v2 sum invariant. Until a row's scenarios_by_file slice is populated, the per-file counts and the row's scenarios field cannot disagree — so future test-file churn could desync without CI noticing. Backfilled 118 partitions (all v1-covered surfaces minus the 15 already partitioned). Each partition's per-file counts sum to the row's locked scenarios field; v2_scenarios_by_file_sum_matches_scenarios_when_populated now exercises 133/133 rows instead of 15/133. No other fields touched — failure_modes_tested, failure_modes_known, json_contract_depth, scenarios, last_audited_at all remain at their 2026-05-14 audit values. JSON contract depth distribution still 27 InstaSnapshot / 101 SemanticAsserts / 5 None. Partition strategy notes: - Single-file dedicated surfaces (search/quality/download/…) map to their primary test file at full scenarios count. - Install cluster fans id 12 across install.rs / install_real_registry.rs / install_overrides.rs / install_patches.rs to reach 50; id 14 picks up install_offline_capability_roundtrip.rs's lone test; id 18 splits across install.rs + install_provenance.rs. - Auth cluster keeps id 36's cli-binary OIDC snippet test attributed to crates/lpm-cli/tests/oidc_setup_snippet_contract.rs (the only cross-tier attribution in the table); the rest stay in tests/workflows/tests/. - Rebuild id 67 spans rebuild.rs + triage_install_lifecycle.rs + cross_command_flows.rs + approve_scripts.rs to honor the cross-flow failure mode (flow #6 + version-diff card) the row already enumerates. - env_local.rs / env_vault.rs partitions sum to their actual fn counts (18 / 30) with no leftover slack. Verified locally with CARGO_TARGET_DIR=/tmp/lpm-v2-partition-target: cargo clippy --workspace --all-targets -- -D warnings ✓ cargo fmt --check ✓ grep -r 'fancy-regex' crates/*/Cargo.toml ✓ (empty) cargo build --workspace ✓ cargo nextest run --workspace --exclude lpm-integration-tests → 6419 tests run, 6419 passed (1 leaky), 7 skipped ✓ v2_scenarios_by_file_partition_reminder report: partitioned: 133 / 133 rows ✓ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tolgaergin
added a commit
that referenced
this pull request
May 14, 2026
…ix (#58) * test(workflows): pin concurrency + recovery contracts for lpm install Adds tests/workflows/tests/install_concurrency.rs with 13 falsifiable tests covering production failure modes that had zero coverage: Category A — process racing: * two concurrent installs on same project (pins finding-#77 floor) * install + concurrent store-clean serialize via shared/exclusive store_lock (probed via try_with_exclusive_lock on the actual lock file, not a directory-existence proxy) * two concurrent `lpm install -g` via global_tx_lock — proves final manifest + WAL coherence under serialized commits Category B — interruption recovery: * kill mid-tarball-fetch leaves no .lpm/install-hash * next `lpm install` converges to a coherent end state Category C — network faults: * tarball 503 → 200 succeeds after retry (counting Respond impl) * metadata 404 fails immediately without retry (<2s wall-clock) Category D — filesystem faults: * readonly project dir fails with actionable error (no panic); POSIX-only via #[cfg(unix)], RAII guard restores permissions * `<project>/.lpm` planted as a regular file fails clearly Category E — partial state recovery: * stale install-hash triggers re-resolve + refetch * partial node_modules re-links to full state * truncated lpm.lockb either recovers or fails cleanly (no panic) Category F — WAL recovery hook: * torn WAL tail (3 garbage bytes) gets truncated by the dispatcher's recovery hook before the command runs; idempotent on re-invocation Support helper refactor (same commit so the new helper has callers): * extracts env-isolation set into `LpmEnvSink` trait + `apply_lpm_env(cmd, project)` shared by `lpm()` (assert_cmd) and the new `lpm_spawnable()` / `lpm_spawnable_with_registry()` (std::process::Command, supports Child::kill()) * trait impl on both Command variants ensures the two helpers cannot drift on the ~30 env knobs that gate test isolation Surfaced findings during this work: * #77 — no project-level install lock: concurrent installs silently drop one side's work AND/OR fail with atomic-rename races (3 observed failure modes documented in findings.md). Fix shape: LpmRoot::project_install_lock + with_exclusive_lock_async wrap. * #78 — retry-backoff has no test-friendly knob; retry-exhaustion tests take 15s+. Fix shape: LPM_RETRY_BACKOFF_MS_OVERRIDE env in debug builds. CI gate locally green: clippy --workspace --all-targets -- -D warnings: clean cargo fmt --check: clean fancy-regex ban: empty cargo build --workspace: clean cargo nextest run --workspace --exclude lpm-integration-tests: 6439 passed, 7 skipped, 1 leaky (pre-existing) Deferred (filed under "next session" in the followup plan): B.3 (kill doesn't tear lockfile) — subsumed by B.1/B.2 B.4 (panic injection) — needs LPM_TEST_PANIC_AT env hook C.2 (retry exhaustion) — blocked by finding #78 C.3 (truncated body) — needs custom Respond with Content-Length mismatch D.3 (disk-full simulation) — no portable mechanism F.2, F.3 (orphan WAL, torn WAL with real records) — needs framed-WAL construction helpers Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(workflows): pin lpm.lock well-formedness + recovery skip-on-contention Closes B.3 and F.2 of the concurrency tranche — 13 → 15 tests, meeting the "≥15 of 21" acceptance criterion for Item 2. B.3 — `install_killed_mid_pipeline_leaves_well_formed_or_absent_lockfile`: Exercises two SIGKILL windows on the install pipeline — fresh project and project with a committed lpm.lock from a prior install. After each kill, asserts the on-disk lpm.lock is either absent OR parses as TOML. Never half-written. Adds `toml = { workspace = true }` as a workflow- tests dev-dep for the parse assertion. Helper `assert_lockfile_well_formed_or_absent` shared between both windows. F.2 — `lpm_command_skips_recovery_when_another_lpm_holds_global_tx_lock`: Validates the dispatcher's `try_with_exclusive_lock` idempotent-skip path at `main.rs:2531`. A background thread acquires `global_tx_lock` via `lpm_common::with_exclusive_lock` and blocks on a channel. With the lock held, runs `lpm global list` against a project with a torn- WAL prefix — asserts the WAL bytes are UNCHANGED (skip arm fired, recovery did not run). Then releases the lock and re-runs; asserts the WAL is now truncated (recovery defers correctly to the next lock-free invocation). Exercises both branches of the `try_with_ exclusive_lock` Ok(None) / Ok(Some) arm. CI gate locally green: cargo clippy --workspace --all-targets -- -D warnings: clean cargo fmt --check: clean cargo nextest run --workspace --exclude lpm-integration-tests: 6441/6441 passed, 7 skipped 5x parallel re-run of install_concurrency: 15/15 stable each run Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(workflows): pin truncated-tarball + orphan-WAL recovery contracts Two new tests in tests/workflows/tests/install_concurrency.rs: - C.3 tarball_connection_dropped_mid_body_fails_or_retries: a custom wiremock Respond impl serves half a tarball with a Content-Length header naming the full length. Pins the install pipeline's retry-then-fail behavior on transport-class failures (~14s wall-clock for the full 4-attempt retry schedule). Hyper 1.9 server-side panics on the Content-Length lie, dropping the connection — a valid surrogate for a broken upstream / CDN dropping mid-body. Surfaced 8 tarball GETs per install (deterministic, 3-of-3 reproducer), explained by two distinct download_tarball_* call sites in install.rs each running the 4-attempt retry budget. - F.3 lpm_command_with_orphan_pending_tx_emits_recovery_banner: plants both halves of an orphan transaction (WAL Intent record without matching Commit/Abort + matching [pending.<pkg>] row in manifest.toml pointing at a non-existent install root) and asserts the dispatcher's recovery hook fires the RolledBack banner from main.rs:2543. Sets RUST_LOG=lpm=info to lift the default lpm=warn filter so the tracing::info! line surfaces. Adds lpm-global as a workflow dev-dep for WalWriter / IntentPayload / write_for. Pins post-state: orphan pending row gone, no spurious active row. Together these close the C.3 and F.3 gaps in Item 2 of the test coverage follow-up plan: 17/21 scenarios pinned (was 15/21). The four remaining items all need source-side hooks (LPM_TEST_PANIC_AT, LPM_RETRY_BACKOFF_MS_OVERRIDE, container infra) and are out of scope for this tranche. Full CI gate green: clippy clean, fmt clean, fancy-regex empty, 6443/6443 nextest pass (was 6441 pre-tranche). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(workflows): pin tarball-extraction security contracts at install tier New file tests/workflows/tests/tarball_security.rs ships phase 1 of Item 3 (tarball-extraction security): 5 of 10 planned tests covering the most distinct security contracts at the install-pipeline tier. Each test constructs its malicious tarball in-line via tar::Builder (no checked-in fixtures), serves it through MockRegistry, and runs lpm install end-to-end so any pipeline-level regression that bypasses the extractor's hardening is caught. Tests landed: - #1 tarball_with_dot_dot_path_entry_is_rejected_by_install — pokes package/../escape.txt into the raw tar header bytes; install fails with "path traversal detected"; outside sentinel never created. - #3 tarball_with_absolute_path_entry_is_normalized_to_relative_under_package_dir — renamed from "rejected" to reflect actual contract. The extractor's strip_first_component consumes the RootDir; an entry like /etc/lpm-pwned.txt extracts as node_modules/<pkg>/etc/lpm-pwned.txt. Install SUCCEEDS; literal /etc/lpm-pwned.txt is never written. Defensible: malformed-but-safe input normalized rather than refused. - #2 tarball_with_symlink_to_outside_path_is_silently_skipped — renamed. The is_file() gate at lib.rs:398 silently drops symlinks; install succeeds with byte-identical outside sentinel. - #5 tarball_with_hard_link_to_outside_file_is_silently_skipped — renamed. Same is_file() gate; hardlinks silently skipped; outside victim file unmodified. - #8 tarball_with_setuid_executable_extracts_with_setuid_bit_stripped (POSIX-only) — tarball entry mode 0o4755 extracts as 0o755. SUID, SGID, and sticky bits all cleared via set_preserve_permissions(false) + the explicit `0o644 | exec_bits` mode set after write. Exec bits preserved. Three tests carry a "plan-vs-actual" docstring section explaining why the rename is defensible — the actual extractor contract differs from the plan's prescribed phrasing in safe ways, not in regression-grade ways. No findings filed. Phase 2 (5 remaining tests: Unicode normalization, device file, FIFO, zero-byte sanity, OS-max path) is deferred to a follow-up tranche with rationale + lift estimate documented in the plan. None blocks phase 1 acceptance. Pre-merge gate green: clippy clean, fmt clean, fancy-regex empty, 6448/6448 nextest pass (was 6443; +5 for the new tests). 0.18s wall- clock for the full file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(install): per-project lock prevents concurrent-install data loss Closes finding #77. Two `lpm install <pkg>` invocations on the same project no longer race on the manifest snapshot+commit window. Pre-fix, both processes acquired only a SHARED store_lock and proceeded in parallel. Each opened its own per-process ManifestTransaction snapshot of the pre-edit package.json, staged its own dep on top, and ran the install pipeline. Whoever wrote package.json + lpm.lock last won; the other process's edits — including its node_modules link — silently vanished. Both processes still exited 0 with success-path output. CI scripts that ran two installs in parallel saw no signal of the data loss. The fix introduces: - crates/lpm-common/src/paths.rs::project_install_lock(project_dir): free helper returning <project_dir>/.lpm/.install.lock. Re-exported from crates/lpm-common/src/lib.rs. - run_add_packages and run_install_filtered_add in crates/lpm-cli/src/commands/install.rs now wrap the snapshot → stage → install → finalize → commit window in with_exclusive_lock_async against the project lock. The lock is per-project (no cross-project contention) and held across all ?-early-exits via the async block's return. For the workspace path, the lock sits at the discovered workspace root (not per-member) so two concurrent `lpm install --filter <member>` invocations on the same workspace serialize without per-member deadlock-ordering complexity. run_with_options (the inner install pipeline) does NOT acquire this lock — it's called from inside both run_add_packages's wrap and from many other commands; double-acquiring the same fd-lock would deadlock in-process. Deferred (phase 2, not exercised by A.1): lpm add (add.rs:723-904) has a similar 180-line transaction with recursive Swift handling. Wrapping it is invasive and the race surface is theoretical (users don't typically run `lpm add` and `lpm install` concurrently). Defer to a separate tranche if a concurrent `lpm add` × `lpm install` race is ever observed. Test contract tightening (bug-first per CLAUDE.md): two_concurrent_installs_on_same_project_leave_well_formed_manifest in tests/workflows/tests/install_concurrency.rs went from "at-least-one survives + manifest is well-formed JSON" (the floor) to "BOTH installs succeed, BOTH packages present in package.json deps, BOTH packages linked in node_modules/" (the contract). Pre-fix: 1/1 fail (pkg-b silently dropped). Post-fix: 5/5 pass with no flakes (~1.2s wall-clock each — install B observes pkg-a's commit and reports "Resolved 2 packages"). Pre-merge gate green: clippy --workspace --all-targets clean, fmt clean, fancy-regex empty, 6448/6448 nextest pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(registry): test-only retry-backoff override env knob Closes finding #78 + lands C.2 (`tarball_503_exhausts_retries_fails_with_http_status`). Pre-fix, retry-exhaustion tests were blocked: the registry client's backoff schedule (1+2+4+8s, capped at 10s) made every retry-exhaustion test take ~15s per fetch site (~28s with the install pipeline's 2 distinct download_tarball_* call sites). MAX_RETRIES, RETRY_BASE_DELAY, and RETRY_MAX_DELAY are private const with no env override. C.2 therefore had to be #[ignore]-gated behind LPM_RUN_SLOW_TESTS=1, and the retry-exhaustion contract went unproven on `cargo nextest run`. The fix introduces: - crates/lpm-registry/src/client.rs::backoff_override(): reads LPM_RETRY_BACKOFF_MS_OVERRIDE (a u64 ms value) gated by cfg!(debug_assertions) || LPM_TEST_MODE=1. Returns Some(Duration) when both conditions hold; None otherwise. Production retry policy is immune — release builds without LPM_TEST_MODE=1 silently ignore the env. - backoff_delay(attempt) consults the override before computing the exponential schedule. - The two 429 Retry-After sleep sites also consult the override so a future 429-flood retry-exhaustion test wouldn't hang on the server-supplied header. C.2 test landed alongside (bug-first per CLAUDE.md): - Mock returns 503 on every tarball request — no recovery path. - Test sets LPM_RETRY_BACKOFF_MS_OVERRIDE=10 on the lpm subprocess. - Asserts: install fails non-zero, no panic, ≥4 attempts (proves the retry loop fired), elapsed < 2s (load-bearing — without the knob this fails at ~14s), stderr contains an actionable HTTP-class noun (503 / status / http / network / etc). - Surfaces 8 tarball GETs per install (4 attempts × 2 distinct download_tarball_* call sites — matches C.3's observation). Pre-fix verification: same C.2 against the unfixed client.rs failed on the elapsed assertion at 14.04s (knob ignored). Post-fix: passes in 1.6s cold / 0.1s warm. 5/5 passes with no flakes. Pre-merge gate green: clippy --workspace --all-targets clean, fmt clean, fancy-regex empty, 6449/6449 nextest pass (was 6448 pre-fix; +1 for C.2). Item 2 of the test-coverage-followup-plan now at 18/21 (was 17/21). Both findings #77 and #78 fixed in production. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(workflows): tarball-security phase 2 — Unicode, device, FIFO, zero-byte, long-path Adds 5 more tests to tarball_security.rs, completing Item 3 of the test-coverage follow-up plan. Each test pins the actual extractor contract under malicious-or-edge-case tarball shapes that reach the install pipeline through MockRegistry. Tests landed: - #4 tarball_with_unicode_lookalike_parent_dir_extracts_safely_as_literal_bytes — renamed from "_normalization_traversal_rejected" to reflect the actual contract. Tarball entry path uses full-width dots U+FF0E `..` (bytewise NOT ASCII `..`). Component::ParentDir is byte-exact, so `..` becomes Component::Normal. Install SUCCEEDS; `..` materializes as a literal directory under node_modules/<pkg>/; outside sentinel byte-identical. Defensible because Path::components() doesn't NFKC-normalize on POSIX. - #6 tarball_with_character_device_entry_is_silently_skipped (POSIX-only). EntryType::Char with /dev/null-shaped major/minor. Same is_file() gate as symlinks/hardlinks — silently skipped. Install SUCCEEDS; no device file at the expected path. - #7 tarball_with_fifo_entry_is_silently_skipped (POSIX-only). EntryType::Fifo. Same posture as #6. - #9 tarball_with_zero_byte_regular_file_extracts_as_empty_file. Sanity check that empty files still extract correctly (legitimate npm shape: .gitkeep, license placeholders). - #10 tarball_with_single_path_component_exceeding_name_max_fails_cleanly. 300-byte single-component name, well over POSIX NAME_MAX=255. Tar wire format succeeds via GNU long-name extension; the FILESYSTEM rejects on extraction (ENAMETOOLONG). Extractor wraps as LpmError::Io → install fails non-zero with the OS error visible and an actionable noun in stderr. Three of the five tests are renamed to reflect actual extractor contract vs the plan's prescribed phrasing — same "plan-vs-actual" docstring pattern as phase 1. No findings filed; all 10 contracts across phase 1 + 2 are defensible-as-implemented. Pre-merge gate green: clippy --workspace --all-targets clean, fmt clean, fancy-regex empty, 6454/6454 nextest pass (was 6449 pre-tranche; +5 for the new tests). Full file 0.2s wall-clock for all 10 tests. Item 3 now COMPLETE (10/10). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(workflows): cross-command flows Item 4 — migrate→rebuild + workspace filter isolation Closes Item 4 of the test-coverage-followup-plan at 6/6 (target was ≥5). Two additions to tests/workflows/tests/cross_command_flows.rs: - Plan #1 — extended flow_migrate_install_audit_lockfile_round_trips with a `lpm rebuild --dry-run --policy=deny` step. Pins the full migrate → install → audit → rebuild lifecycle. Asserts the rebuild step exits 0 + does not mutate the post-audit state (lpm.lock + lpm.lockb still present). Catches regressions where rebuild's lockfile or build-state parser breaks against a freshly-migrated manifest. - Plan #5 — added flow_workspace_install_filter_member_a_does_not_mutate_member_b (new test, 159 LOC). Pins the workspace-member isolation contract using the workspace-monorepo fixture (3 members: app, core, utils): 1. Initial filtered install on @test/core (re-pinning its existing semver dep) populates core's per-member quadruple: lpm.lock=319 B, lockb=230 B, install_hash=118 B. 2. Snapshot core's full quadruple. 3. Run `lpm install chalk@5.3.0 --filter @test/app` to add a new dep to app ONLY. 4. Assert app's package.json gained chalk; core's quadruple (package.json + lpm.lock + lpm.lockb + install-hash) is BYTE-IDENTICAL post-install; chalk does NOT appear in core's node_modules/. Catches a regression where a per-member filtered install accidentally also mutates a sibling member's package.json / lockfile / install-hash — a real bug class because run_install_filtered_add shares the workspace-root project lock (added in #77 fix) and could over-snapshot if the target-set computation drifts. Helper `mount_pkg_full(mock, name, version)` factors out the three-step metadata + batch-metadata + tarball mount so the test body stays readable. Other 4 plan flows already covered pre-tranche: - Plan #2: flow_add_install_graph_added_dep_visible - Plan #3: flow_install_patch_patch_commit_install_persists_patch - Plan #4: flow_token_rotate_publish_dry_run_picks_new_token - Plan #6: flow_install_upgrade_major_audit_picks_new_version Pre-merge gate green: clippy --workspace --all-targets clean, fmt clean, fancy-regex empty, 6455/6455 nextest pass (was 6454; +1 for the new flow). Plan #5 stable across 5/5 reruns at ~0.11s each. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(install): LPM_TEST_PANIC_AT hook + B.4 panic-rollback contract Adds a deterministic panic-injection hook to the install pipeline + unblocks the long-deferred B.4 contract test for ManifestTransaction Drop-based rollback on panic. The hook (`maybe_test_panic(stage)` in crates/lpm-cli/src/commands/install.rs) reads LPM_TEST_PANIC_AT and panics when the env value matches the stage name. Gated to `cfg!(debug_assertions) || LPM_TEST_MODE=1` — same pattern as the #78 retry-backoff override. Production builds without LPM_TEST_MODE=1 silently treat the env as no-op. Wired 4 stages in `run_add_packages`: - "after-snapshot" — manifest unchanged; Drop is no-op - "after-stage" — placeholder `*` written to package.json (load-bearing) - "after-install" — pipeline complete; manifest still has `*` - "after-finalize" — concrete versions written; pre-commit only The hook unblocks B.4 (`install_panics_mid_pipeline_rollback_restores_manifest`), deferred since the original Item 2 tranche because there was no deterministic way to trigger a panic mid-install from a workflow test. Recoverable errors fire `?`-rollback (covered by E.1/E.2/E.3); SIGKILL bypasses Drop entirely (B.1/B.2/B.3 cover that). The panic path was the missing rollback proof. B.4 sets LPM_TEST_PANIC_AT=after-stage and asserts: - process exits non-zero (panic propagates to runtime) - stderr contains `"panicked at"` AND `"LPM_TEST_PANIC_AT=after-stage"` - package.json BYTE-IDENTICAL to pre-stage (Drop ran on unwind, snapshot bytes restored — load-bearing) - the new pkg is NOT in dependencies (placeholder rollback worked) - .lpm/install-hash absent (invalidate-on-rollback) - lpm.lock absent (matched optional snapshot's None pre-state) Catches a regression where: - panic = "abort" added to release profile (no Drop on panic) - ManifestTransaction Drop logic stops restoring snapshot bytes - The `lpm install` snapshot+commit window grows without re-wiring Drop Test runs in 0.07s warm. 5/5 stable across reruns. Pre-merge gate green: clippy --workspace --all-targets clean, fmt clean, fancy-regex empty, 6456/6456 nextest pass (was 6455; +1 for B.4). install_concurrency now at 19/19. Item 2 of test-coverage-followup-plan moves to 19/21 — only A.2 (no contract) and D.3 (needs container infra) remain deferred indefinitely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(workflows): align MockRegistry tarball URL shape with production /-/ gate Workflow tests mounted tarballs at `/tarballs/{name}-{version}.tgz` — missing the `/-/` path segment that the registry-client's `evaluate_cached_url` gate at [crates/lpm-registry/src/client.rs#L4117] requires (`.tgz` suffix AND `/-/` substring). The gate is a defense-in-depth check that blocks the H1 auth-token leak: a tampered lockfile URL like `/api/admin/foo.tgz` (no `/-/`) would otherwise attach the bearer to a non-registry endpoint. The mismatch produced two test-environment side effects that don't manifest in production: 1. **WARN noise**: every install test that read a tarball URL from the lockfile fast path logged `cached tarball URL for X@Y failed shape check; falling back to on-demand lookup`. Polluted stderr across the suite. 2. **`shape_mismatch_count` defeated**: the registry-client documents this counter as a "BUG signal — the writer should never emit a gate-rejectable URL". Test runs incremented it on every install, making the counter useless for catching real bugs. This commit migrates the mock to the production-shape `/tarballs/{name}/-/{name}-{version}.tgz` everywhere — both the helper methods (`MockRegistry::tarball_path` / `tarball_url`) and the ~60 hard-coded `format!` sites across 14 test files + 1 snapshot. The new `tarball_path` helper is `pub` with a prominent docstring warning future test authors not to re-introduce the legacy shape. Internal mounts in `with_package_and_deps` / `with_package_published_at` / `with_full_package_metadata` all route through it. Post-fix verification: WARN gone, gate `Accepted` path runs, all 691 lpm-workflows tests pass (0 leaky in the latest full-workspace run, down from 1-3 leaky pre-fix — fewer fallback paths firing). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(workflows): test-coverage-followup tranche — Items 2/3/4/5 Closes the remaining open rows from `private/test-coverage-followup-plan.md` across four items. ~2,600 LOC of new test code + fixture + budget infra. **Item 3 — tarball-security additional candidate surfaces (7 tests in `tarball_security.rs`):** - `tarball_with_pax_path_traversal_rejected` — PAX extended `path` header smuggling `..` is rejected by the extractor's `Component::ParentDir` check after the tar crate resolves the override. - `tarball_with_gnu_longname_traversal_rejected` — symmetric GNU `L` entry; same rejection path. - `tarball_rejects_or_rolls_back_when_later_entry_is_malicious` — pins the `rollback_extraction` contract: valid first entry is cleaned up when a later `..`-traversal entry trips rejection mid-stream. - `tarball_with_duplicate_member_path_rejected_or_deterministic` — pins current last-write-wins contract (defensible; flagged scanner- disagreement risk in test comment). - `tarball_with_truncated_gzip_rolls_back_partial_extract` — half- truncated gzip stream → libdeflate fails cleanly → no partial extract. - `tarball_ignores_uid_gid_ownership_metadata` (POSIX) — bogus uid/gid in tar header is ignored; extracted files owned by process uid. - `tarball_with_sparse_huge_file_rejected_by_declared_size` — manually- constructed tarball with header declaring `MAX_FILE_SIZE + 1` and empty on-wire body; extractor rejects on the pre-check at lib.rs:306 before draining body. **Item 4 — cross-command flows additional candidate surfaces (2 tests in `cross_command_flows.rs`):** - `flow_install_uninstall_install_graph_round_trip` — pins manifest / link / graph hand-off through a full round-trip. - `flow_cache_clean_then_offline_install_uses_store_or_fails_helpfully` — pins the cache/store boundary: `cache clean` must not corrupt offline install; store-side bytes byte-identical after a clean. **Item 2 — concurrency/recovery additional candidate surfaces (3 tests in `install_concurrency.rs`):** - `cache_clean_during_slow_tarball_install_does_not_corrupt_install` (G.4) — install + cache clean run concurrently (different lock paths, no serialization); install succeeds despite metadata cache wipe mid-stream. Empirical timing observed: install elapsed 1.57s, cache clean fired at t=30-39ms cleanly inside the install window. - `install_panics_after_install_hash_write_rollback_invalidates_hash` (G.5) — reuses existing `LPM_TEST_PANIC_AT=after-install` stage (no new source-side hook needed — `write_post_install_v6_hash` runs inside `run_with_options` which returns BEFORE that stage fires). Pins that Drop-based rollback restores manifest AND deletes the freshly-written install-hash. - `malformed_registry_json_fails_without_manifest_or_lockfile_mutation` (G.6) — truncated JSON on all three metadata endpoints; install fails cleanly, no panic/backtrace, package.json byte-identical, no torn lockfile. **Verdaccio-npm parity for `which@4.0.0` (`install_real_registry.rs`):** - `verdaccio_npm_parity_for_bin_package_pins_metadata_and_shim_presence` — extends the existing lodash byte-diff with a bin-shipping target package. Asserts metadata equivalence + `.bin/<name>` shim present on both sides + bin target file materialized + exec bits non-zero (POSIX). **Item 5 — realworld fidelity (new fixture + new test file):** - `tests/fixtures/realworld-nextjs/` (package.json + README) — pinned Next.js 14.2.13 + React 18.3.1 + TypeScript 5.6.3 + 3 `@types/*` packages. Resolves to ~28 transitive deps empirically. README documents the calibration methodology including raw measurement data. - `tests/workflows/tests/install_realworld.rs` — `install_realworld_nextjs_fixture_succeeds_through_verdaccio` installs the fixture through Verdaccio→npmjs and asserts end-to-end success at production scale. Always logs cold + warm wall-clock + peak RSS to stderr for calibration data. - **`LPM_BUDGET_GATE=1`-gated budget assertions**: cold ≤ 25s, warm ≤ 25ms, cold peak RSS ≤ 1500 MiB. Calibrated from N=6 cold + N=3 warm + N=3 RSS runs on M-series macOS, 2026-05-14. Memory measurement via `/usr/bin/time -l` (macOS) / `-v` (Linux); Windows skips with a clear warning. This closes Item 5 entirely (all 4 acceptance criteria green) and brings Items 2/3/4 to the parked-by-design or infrastructure-blocked baseline. CI gate: clippy `--workspace --all-targets -- -D warnings` clean, fmt clean, fancy-regex empty, build clean, `cargo nextest run --workspace` 6471/6471 pass. Suite runtime ~2:40 (was ~2:24 pre-tranche; +15s for the realworld test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(workflows): collapse Linux-only let-chain in parse_peak_rss CI lint on Linux failed on `clippy::collapsible_if` in the Linux-cfg'd branch of `parse_peak_rss`. The macOS branch had an intermediate `let bytes_str = rest.trim();` between the two `if let`s, which is why the local clippy run on macOS didn't catch this — only the macOS-cfg branch compiled there. Collapse the Linux branch to use `&&` (stable let-chains) so it satisfies the lint while preserving the same semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tolgaergin
added a commit
that referenced
this pull request
May 15, 2026
…e coverage GPT audit (2026-05-15) caught a state-machine bug in `split_multi_file_patch`: a git rename-only section followed by a plain `---`/`+++` chunk for a DIFFERENT file collapsed into one chunk because `in_git_header` was only cleared on `@@`, and a rename-only section has no `@@` to clear it. The next plain `--- a/x.js` was then suppressed as the rename-only section's "own" header. **Fix:** track `rename from` path inside the open `diff --git` section. When a `--- ` line arrives in GIT_HEADER state and the path mismatches `rename from`, treat it as a new section boundary. The match case (same path = rename+edit's own old-side header) stays non-boundary. No rename headers = normal git chunk = same as before (non-boundary). **Tests added:** - `split_multi_file_patch_mixed_rename_only_then_plain` — direct regression for the audit-flagged case. Asserts 2 chunks. - `split_multi_file_patch_rename_with_edit_path_matches_stays_one_chunk` — companion test that the rename+edit happy path is NOT over-sliced by the new mismatch rule. **Scoped-package coverage (audit's "Natural next steps"):** - `patch_commit_handles_npm_scoped_package` — `@posthog/nextjs-config@4.17.21` → on-disk filename `patches/@posthog__nextjs-config@4.17.21.patch`, manifest key keeps the `/` (real package selector shape). - `patch_commit_handles_lpm_dev_scoped_package` — `@lpm.dev/user.package@1.2.3` → on-disk filename `patches/@lpm.dev__user.package@1.2.3.patch`, manifest key keeps the `/`. - Asserts the raw-slash filename does NOT exist (would break the cross-platform-portability contract). **Stale docstring fix (audit's Low finding):** - `tests/workflows/tests/patch.rs` module header rewritten to match the post-Slice-A contract (range/bare-name accepted, dist-tags rejected, scoped names sanitize `/` → `__` in filename only). Pre-merge gate (local, /tmp target dir): clippy clean, fmt clean, 6583/6583 workspace tests pass (was 6579 — +4 new tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tolgaergin
added a commit
that referenced
this pull request
May 16, 2026
Remove Finding #6, §6.2, §7.2 plan references from test doc comments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
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
Addresses the pre-existing
--insecureinconsistency called out in the Phase 43 design doc (DOCS/new-features/37-rust-client-RUNNER-VISION-phase43.md§"Pre-existing--insecureinconsistency that Phase 43 inherits"). Option (a) from that block — threadallow_insecurethrough end-to-end — chosen because the CLI help text atcrates/lpm-cli/src/main.rs:87explicitly promises "Allow insecure HTTP connections to non-localhost registries".483a5b5) — threadallow_insecurethrough all four rejection points:download_tarball,download_tarball_to_file_with_limit,download_tarball_streaming, and Phase 43'sevaluate_cached_urlgate. Error text on the tarball paths now hintsPass --insecure to allow HTTP non-localhost.95c3ed3) — audit-response follow-up. Narrows--insecureto HTTP only (the first commit's guard shape letfile://,ftp://,data:, etc. through once the flag was set). Extracts a sharedcheck_tarball_url_schemehelper with the tight predicateis_https || is_localhost || (allow_insecure && is_http)and applies the same predicate inevaluate_cached_urlandvalidate_base_url. Replaces three non-hermeticevil.com-hitting tests with hermetic unit tests on the helper, plus Finding-1 regression guards at all three gate sites (file://,ftp://,data:,javascript:,gopher://under--insecure).Net: all four rejection points agree on the same predicate;
--insecuredelivers on its contract end-to-end;file://and friends stay rejected even with the flag set.Test plan
cargo clippy --workspace -- -D warningscleancargo fmt --checkcleangrep -r fancy-regex crates/*/Cargo.tomlabsentcargo build --workspacecleancargo nextest run --workspace --exclude lpm-integration-tests --no-fail-fast— 3688 passed, 7 skippedcargo test -p lpm-auth— 43 passed × 3 deterministicfile:///ftp:///data:/javascript:/gopher://stay rejected under--insecureat the tarball-path, Phase 43 gate, and base-URL sites)🤖 Generated with Claude Code