fix: skip lockfile minimumReleaseAge/trustPOlicy verification for non-registry tarball#12122
Merged
zkochan merged 2 commits intoJun 2, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
🧰 Additional context used📓 Path-based instructions (1)pacquet/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-05-20T19:40:55.051ZApplied to files:
📚 Learning: 2026-05-22T00:08:44.646ZApplied to files:
📚 Learning: 2026-05-20T23:07:58.444ZApplied to files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThis PR prevents minimumReleaseAge and trustPolicy verification for tarball resolutions whose tarball URL uses non-HTTP(S) schemes (e.g., file:). Both TypeScript and Rust resolvers now parse tarball URLs and skip registry verification for non-http/https protocols; tests and a changeset were added. ChangesSkip minimumReleaseAge verification for file: tarball protocols
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoSkip verification for non-registry tarball protocols
WalkthroughsDescription• Skip minimumReleaseAge/trustPolicy verification for non-registry tarball protocols • Add URL scheme validation to detect local and non-HTTP(S) tarballs • Prevent incorrect npm registry metadata checks on local dependencies • Add test coverage for file: protocol tarball resolutions Diagramflowchart LR
A["Tarball Resolution"] --> B{"Check URL Scheme"}
B -->|"file: or other non-HTTP(S)"| C["Skip Verification"]
B -->|"http: or https:"| D["Perform Verification"]
C --> E["Return Ok"]
D --> F["Check Registry Metadata"]
File Changes1. pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an install-time verification bug where lockfile entries for local tarball dependencies (e.g. file:) were incorrectly treated as npm-registry tarballs and subjected to minimumReleaseAge / trustPolicy checks, which can trigger unnecessary registry metadata requests and failures (closes #12111).
Changes:
- Treat tarball resolutions with non-HTTP(S) URL protocols (e.g.
file:) as non-registry resolutions so verification is skipped. - Add regression tests in both the TypeScript resolver and the Pacquet Rust implementation.
- Add a changeset to ship the fix as a patch for
pnpmand@pnpm/resolving.npm-resolver.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| resolving/npm-resolver/src/createNpmResolutionVerifier.ts | Skips verification for tarballs whose parsed URL protocol is not http:/https:. |
| resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts | Adds a regression test ensuring file: tarball resolutions are not verified. |
| pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs | Mirrors the protocol-based skip logic in the Rust verifier (http/https only). |
| pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs | Adds a regression test ensuring file: tarball resolutions short-circuit to Ok. |
| .changeset/short-lamps-relax.md | Publishes the behavior change as a patch release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Micro-Benchmark ResultsLinux |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs (1)
240-241: ⚡ Quick winDrop the prose comment and let the test name carry the scenario.
These lines just restate what
verify_short_circuits_file_tarball_resolution()already says and what the assertion proves. As per coding guidelines, "Tests are documentation — do not duplicate test scenarios, edge cases, failure modes, or worked examples in prose when they are already captured by tests."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs` around lines 240 - 241, Remove the redundant prose comment above the test: delete the two-line comment block that explains the `file:` tarball resolution behavior so the test name `verify_short_circuits_file_tarball_resolution()` stands alone; ensure no other tests or comments rely on that text and leave the test and its assertions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs`:
- Around line 240-241: Remove the redundant prose comment above the test: delete
the two-line comment block that explains the `file:` tarball resolution behavior
so the test name `verify_short_circuits_file_tarball_resolution()` stands alone;
ensure no other tests or comments rely on that text and leave the test and its
assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 77531a7a-4d78-4a4b-94e3-76b48beea62d
📒 Files selected for processing (5)
.changeset/short-lamps-relax.mdpacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rsresolving/npm-resolver/src/createNpmResolutionVerifier.tsresolving/npm-resolver/test/createNpmResolutionVerifier.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Agent
- GitHub Check: Code Coverage
- GitHub Check: Lint and Test (ubuntu-latest)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Dylint
- GitHub Check: Lint and Test (macos-latest)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Analyze (javascript)
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Follow Standard Style with trailing commas, preferring functions over classes, and declaring functions after they are used (relying on hoisting)
Use a single options object instead of multiple parameters when a function needs more than two or three arguments
Follow Import Order: Standard libraries first, then external dependencies (alphabetically), then relative imports
Write self-documenting code where function names, parameters, and types explain what a function does without requiring prose comments
Do not write comments that restate what the code already says; refactor via renaming, splitting helpers, or restructuring instead
Do not repeat documentation at call sites that already exists in JSDoc on the callee; update JSDoc once for all call sites to benefit
Use JSDoc only for a function's contract (preconditions, postconditions, edge cases, why the function exists), not for re-narrating the body
Do not record past implementation shape, refactor history, or 'the previous code did X' framing in code; use git log and git blame instead
Write comments only when: the reason for code is non-obvious (hidden invariant, workaround for known bug, deliberate exception), or the right name doesn't fit (temporary technical constraint)
Files:
resolving/npm-resolver/test/createNpmResolutionVerifier.test.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use util.types.isNativeError() instead of instanceof Error for error type checking in Jest tests
Files:
resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(...), orstreamParser.write(...), mirror the call site, payload, and ordering so@pnpm/cli.default-reporterparses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plainStringor&strin Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too viaTryFrom<String>and/orFromStrand do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallibleFrom<String>constructor
If upstream TypeScript occasionally constructs a branded string without validation, exposefrom_str_uncheckedin Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization
Derive simple conversions for branded strings using#[derive(derive_more::From)]and#[derive(derive_more::Into)]instead of handwritingimplblocks; use manualimplonly when conversion needs custom logic
Model TypeScript string literal unions (like'auto' | 'always' | 'never') as Rustenums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like`${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide inCODE_STYLE_GUIDE.md— imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching,pipe-trait, error handling, test layout, and cloning ofArcandRc
Choose owned vs. borrowed parameters to minimize copies; widen to t...
Files:
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
🧠 Learnings (6)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
resolving/npm-resolver/test/createNpmResolutionVerifier.test.tsresolving/npm-resolver/src/createNpmResolutionVerifier.ts
📚 Learning: 2026-05-20T19:40:55.051Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.
Applied to files:
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
📚 Learning: 2026-05-22T00:08:44.646Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.
Applied to files:
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
📚 Learning: 2026-05-20T23:07:58.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.
Applied to files:
pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rspacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs
📚 Learning: 2026-05-23T17:29:56.247Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11878
File: resolving/npm-resolver/src/createNpmResolutionVerifier.ts:381-418
Timestamp: 2026-05-23T17:29:56.247Z
Learning: When reviewing the npm resolver code, note that `PackageMetaCache` is intentionally keyed only by `name` and `name:full` (no registry component). As a result, code that shares this cache (e.g., `createNpmResolutionVerifier.ts` via the shared `validateSharedMeta` guard) can prevent cross-package contamination by matching names, but it cannot distinguish two different registries that serve packages with the same name within a single install. Don’t flag (or attempt to fix) this as a local issue in the verifier alone—correctly distinguishing registries would require a coordinated change to the resolver cache key shape. (The Pacquet/Rust cache is already registry-qualified, unlike the npm-resolver cache.)
Applied to files:
resolving/npm-resolver/src/createNpmResolutionVerifier.ts
📚 Learning: 2026-05-26T21:01:06.666Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11966
File: .changeset/require-tarball-integrity.md:6-6
Timestamp: 2026-05-26T21:01:06.666Z
Learning: In pnpm lockfile-related release notes/docs (especially changeset markdown), preserve URL hostnames exactly as they appear in pnpm-lock.yaml tarball resolution entries—keep hosts like `codeload.github.com`, `bitbucket.org`, and `gitlab.com` in lowercase. Do not “correct” them to title-case/preserve brand capitalization (e.g., LanguageTool rules like `GITHUB` capitalization) because these are literal URL fragments, not platform brand names.
Applied to files:
.changeset/short-lamps-relax.md
🔇 Additional comments (5)
resolving/npm-resolver/src/createNpmResolutionVerifier.ts (1)
833-837: LGTM!resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts (1)
224-236: LGTM!pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs (1)
760-765: LGTM!pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs (1)
242-256: LGTM!.changeset/short-lamps-relax.md (1)
1-6: LGTM!
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12122 +/- ##
=======================================
Coverage 87.51% 87.51%
=======================================
Files 261 261
Lines 29640 29646 +6
=======================================
+ Hits 25939 25945 +6
Misses 3701 3701 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.0719092614599997,
"stddev": 0.12430612919303112,
"median": 2.03098830676,
"user": 2.6108468799999995,
"system": 3.35705324,
"min": 1.95967425976,
"max": 2.3753229977599997,
"times": [
2.02524019976,
2.03673641376,
1.9836468147600002,
2.0171981997599997,
2.0629220497599996,
1.95967425976,
2.00558876176,
2.19637868476,
2.3753229977599997,
2.0563842327599997
]
},
{
"command": "pacquet@main",
"mean": 2.04944292936,
"stddev": 0.045476903137469556,
"median": 2.0516468017599996,
"user": 2.64078868,
"system": 3.3007296399999992,
"min": 1.9789279907600001,
"max": 2.12572006976,
"times": [
1.9789279907600001,
2.0306881467599998,
2.0299788037599997,
2.0790153507599998,
2.07736989376,
2.12572006976,
2.0335166977599997,
1.98815075976,
2.06977690576,
2.08128467476
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.64285575726,
"stddev": 0.015310751230294978,
"median": 0.63818573926,
"user": 0.35854157999999997,
"system": 1.31046392,
"min": 0.62975821726,
"max": 0.68345455526,
"times": [
0.68345455526,
0.62975821726,
0.63667492526,
0.63969655326,
0.63533578726,
0.64117961526,
0.64923139326,
0.63627184826,
0.63299629726,
0.64395838026
]
},
{
"command": "pacquet@main",
"mean": 0.68046175306,
"stddev": 0.027419378470993672,
"median": 0.68725019626,
"user": 0.3675950799999999,
"system": 1.31849362,
"min": 0.64187849426,
"max": 0.72349703626,
"times": [
0.72349703626,
0.69215108526,
0.66079050726,
0.68533484926,
0.64187849426,
0.65794245926,
0.70741160126,
0.64616393726,
0.7002820172600001,
0.68916554326
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.28617979728,
"stddev": 0.02854882025198684,
"median": 2.2834353838799997,
"user": 3.7381535,
"system": 3.06008626,
"min": 2.23832946238,
"max": 2.3429083293799997,
"times": [
2.3429083293799997,
2.27823242338,
2.27983821638,
2.29054889838,
2.3171947193799998,
2.26120102138,
2.28703255138,
2.23832946238,
2.2761363813799997,
2.29037596938
]
},
{
"command": "pacquet@main",
"mean": 2.31560608098,
"stddev": 0.05921446417776251,
"median": 2.3232639753799997,
"user": 3.7798271999999997,
"system": 3.1104397599999993,
"min": 2.22644385138,
"max": 2.42410525638,
"times": [
2.26438353538,
2.27599062638,
2.33551136438,
2.2688064563799997,
2.32456207738,
2.22644385138,
2.3843844113799997,
2.32196587338,
2.3299073573799998,
2.42410525638
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.4629965745400004,
"stddev": 0.014557227960400616,
"median": 1.46735931834,
"user": 1.6605675399999995,
"system": 1.82190786,
"min": 1.44302641734,
"max": 1.4826459783400001,
"times": [
1.44551564334,
1.4826459783400001,
1.44895603634,
1.44302641734,
1.45098452034,
1.4776837303400001,
1.46861840034,
1.47538767134,
1.47104711134,
1.46610023634
]
},
{
"command": "pacquet@main",
"mean": 1.54031804514,
"stddev": 0.10090761781553367,
"median": 1.51148811984,
"user": 1.73277234,
"system": 1.8412691600000002,
"min": 1.46930162334,
"max": 1.8206395803400002,
"times": [
1.46930162334,
1.52980475234,
1.49463414734,
1.8206395803400002,
1.50798984134,
1.51951027634,
1.49216488934,
1.51498639834,
1.5497769343399999,
1.50437200834
]
}
]
} |
Contributor
|
Bencher Report| Branch | pr/12122 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,286.18 ms(-2.84%)Baseline: 2,352.91 ms | 2,823.50 ms (80.97%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,463.00 ms(-4.19%)Baseline: 1,526.91 ms | 1,832.29 ms (79.85%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,071.91 ms(+0.08%)Baseline: 2,070.26 ms | 2,484.32 ms (83.40%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 642.86 ms(-2.63%)Baseline: 660.20 ms | 792.24 ms (81.14%) |
… non-registry tarball
680615f to
4572963
Compare
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
zkochan
approved these changes
Jun 2, 2026
zkochan
added a commit
that referenced
this pull request
Jun 2, 2026
## What The lockfile resolution verifier now confirms that a registry entry pinning an explicit `tarball` URL points at the artifact the registry's own metadata lists for that `name@version`. A mismatch — or any entry that can't be confirmed against the registry — is rejected with `ERR_PNPM_TARBALL_URL_MISMATCH`. ## Why Follow-up to the design discussion on #12122. The verifier checked the age/trust of `name@version` against the registry packument but never bound the lockfile's `tarball` URL to it. For the non-standard entries pnpm preserves a tarball URL for (npm Enterprise, GitHub Packages — see `toLockfileResolution`), pnpm fetches straight from that URL. So a **tampered lockfile could pair a trusted `name@version` with an attacker-chosen tarball URL** (plus a matching integrity for the attacker's bytes); verification passed against the legitimate version while the install fetched the attacker's bytes. Defending a checked-in lockfile is explicitly in this feature's threat model. ## How - For a registry-keyed entry that pins an explicit `tarball`, fetch the packument and assert the URL equals `versions[v].dist.tarball`. The comparison canonicalizes away benign differences — http/https scheme, default ports (`:443`/`:80`), and `%2f` scope-separator encoding (case-insensitive) — so only real mismatches are flagged. The packument is fetched from the user's configured registry (the lockfile's tarball host can't redirect it), and named-registry routing uses the same canonicalization so a scheme/`%2f`-only difference doesn't route to the wrong packument. - **The binding is unconditional.** It runs regardless of `minimumReleaseAge`/`trustPolicy` and is **not** narrowed by their exclude lists, because it guards *integrity*, not *maturity/trust*. Disabling the age/trust policies must not silently disable anti-tamper. (`createNpmResolutionVerifier` therefore always returns a verifier.) - **It is fail-closed.** An entry passes only when the registry metadata affirmatively lists the version with a matching tarball URL. If the metadata can't be fetched, doesn't list the version, or omits `dist.tarball`, the entry is rejected — otherwise a tampered lockfile could smuggle a malicious URL past the check by pointing it at a `name@version` the registry can't vouch for. - **Behavior change:** as a result, an install that re-verifies a lockfile (its content changed since the last verified run, so the verification cache no longer short-circuits) now requires the configured registry to be reachable. `trustLockfile` is the opt-out for environments that treat the on-disk lockfile as already trusted. - **Verification cache.** The policy snapshot records a `tarballUrlBinding` marker and `canTrustPastCheck` requires it, so a cache record written before this rule existed is re-verified rather than trusted (closing an upgrade-time bypass). - Entries with no explicit `tarball` reconstruct the URL from name+version+registry and are inherently bound (no check). `file:`/git-hosted resolutions stay out of scope (#12122). - Threads `nonSemverVersion` to the verifier so URL-keyed tarball deps (a remote `https:` tarball that carries a semver `version` copied from its manifest) are recognized as deliberate non-registry deps and skipped — also fixing a latent release-age over-match on them. The candidate dedupe key includes `nonSemverVersion` so a registry snapshot and a URL-keyed snapshot sharing a `name@version` and serialized resolution stay distinct. Mirrored in pacquet (`create_npm_resolution_verifier`). The dedupe-key change is TS-only: pacquet's candidate `version` comes from the lockfile key suffix, so the two shapes never share a key there. ## Tests - TS: confirmed mismatch → violation; non-standard URL matching metadata → pass; default-port/scheme difference → pass; URL-keyed dep → skipped; URL binding runs (and fails closed) with no age/trust policy configured; `canTrustPastCheck` rejects a cache record lacking the binding marker. Regression-verified (the mismatch test fails when the check is disabled). - pacquet: mirror tests + the no-policy / `minimumReleaseAge: 0` / `trustPolicy: off` cases, default-port/scheme equivalence, and the missing-`tarballUrlBinding` cache rejection. A few install-dispatch / resolution-reuse tests that pin a deliberately bogus tarball URL (or run against an unreachable registry to prove resolution reuse) now set `trustLockfile`, since the always-on fail-closed tarball-URL check would otherwise flag the fixture before the path under test runs. - `clippy --deny warnings`, `fmt`, and `dylint` clean.
This was referenced Jun 2, 2026
3 tasks
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.
close #12111
Summary by CodeRabbit