rfc(pnpr): integrity-addressed patch revisions - #19
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds two RFCs for immutable, registry-scoped digest tarballs and revision metadata. They define provider ingestion, projected revision history, pnpm lockfile and fetch behavior, revision selection, compatibility checks, refresh operations, and audit rules. ChangesIntegrity-addressed revisions
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Provider
participant pnprRegistry
participant RegistryPackument
participant pnpmResolver
participant Lockfile
Provider->>pnprRegistry: submit signed artifact revision
pnprRegistry->>RegistryPackument: publish verified revision metadata
pnpmResolver->>RegistryPackument: resolve name@version and revision
pnpmResolver->>Lockfile: record integrity and revision
pnpmResolver->>pnprRegistry: fetch registry-scoped digest URL
pnprRegistry-->>pnpmResolver: return immutable tarball bytes
pnpmResolver->>Lockfile: verify SRI and install selected revision
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
PR Summary by QodoRFC: Integrity-addressed patch revisions and pnpm fetching
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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.
Inline comments:
In `@pnpr/text/0000-integrity-addressed-patch-revisions.md`:
- Around line 181-204: The “Provider input remains integrity-pinned”
specification must define anti-rollback enforcement for signed patch manifests.
Add a persisted, signed monotonic checkpoint for each provider identity and
require pnpr to reject manifests selecting revisions older than the checkpoint;
document an explicit, authorized rollback mechanism and its verification
requirements.
- Around line 491-505: Update the provider refresh sequence to make `(registry,
name@version, digest)` revision allocation unique and idempotent: claim or reuse
an existing digest mapping within a transaction/unique constraint before
assigning an ordinal, and append revision metadata plus select the mapping
atomically. Ensure concurrent refreshes and retries reuse the same stable
ordinal rather than allocating duplicates.
- Around line 282-292: Update the public CDN caching and policy sections
describing successful immutable responses, including the related sections on
revocation and cache behavior, to define authorization-aware edge enforcement
and purge/invalidation when a revision is blocked. If immediate revocation on
warm immutable hits cannot be guaranteed, weaken the policy wording to state the
actual propagation behavior instead of promising an immediate 403.
- Around line 157-171: Clarify the immutability and access-policy rules in the
RFC sections covering tarball URLs, historical artifact screening, and legacy
lockfiles: define immutability as successful responses returning unchanged
bytes, specify whether vulnerability screening applies to canonical URLs as well
as integrity URLs, and document the response legacy lockfiles receive when
access is denied.
In `@text/0000-integrity-addressed-registry-tarballs.md`:
- Around line 207-209: Restrict the registry-base portability guarantee and its
tests to hostname aliases or deployment moves of the same logical registry.
Update the digest URL resolution and hydration behavior described in the
registry portability section, including the identity gap noted around the
lockfile discussion, so changing to an unrelated configured registry is not
treated as equivalent; alternatively persist and validate registry identity
before hydration.
- Around line 312-316: Define the lockfile version gate for revision-aware
integrity and the `?vN` representation, including when pnpm emits it and how
hydrated lockfile parsing rejects it in older clients. Update the affected
proposal sections consistently and add a compatibility test verifying older
clients fail fast on the gated lockfile rather than reconstructing the canonical
`dist.tarball` URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 39f74957-4168-4ccf-809c-ebc7cea26467
📒 Files selected for processing (2)
pnpr/text/0000-integrity-addressed-patch-revisions.mdtext/0000-integrity-addressed-registry-tarballs.md
📜 Review details
🧰 Additional context used
🪛 LanguageTool
pnpr/text/0000-integrity-addressed-patch-revisions.md
[grammar] ~465-~465: Ensure spelling is correct
Context: ...is longer by one sha512 digest. Patched packuments contain a small revision list. Both cos...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~481-~481: This phrase is redundant. Consider writing “same”.
Context: ...ked registry package, pnpm resolves the same exact name@version. If dist.integrity cha...
(SAME_EXACT)
[grammar] ~506-~506: Ensure spelling is correct
Context: ...validate projected full and abbreviated packuments. No tarball cache object is invalidated o...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~518-~518: To elevate your writing, try using an alternative expression here.
Context: ... returns a policy-specific 403. This matters for a vulnerable original. A reproducib...
(MATTERS_RELEVANT)
text/0000-integrity-addressed-registry-tarballs.md
[grammar] ~145-~145: Use a hyphen to join words.
Context: ...egrity`; 4. the digest is complete, well formed, and unambiguous; 5. the selected...
(QB_NEW_EN_HYPHEN)
[style] ~349-~349: This phrase is redundant. Consider writing “same”.
Context: ... pnpm resolves current metadata for the same exact name@version. If dist.integrity cha...
(SAME_EXACT)
🔇 Additional comments (2)
pnpr/text/0000-integrity-addressed-patch-revisions.md (1)
1-156: LGTM!Also applies to: 172-180, 205-281, 293-408, 428-449, 470-490, 523-690, 691-725
text/0000-integrity-addressed-registry-tarballs.md (1)
1-206: LGTM!Also applies to: 210-266, 278-311, 318-475, 477-499
Replace the empty-option/`?vN` markers with explicit `?r0`/`?rN` ordinals: `r` abbreviates *registry revision*, matches `dist.revisions` numbering, and follows Alpine/Gentoo `-rN` precedent, while an always-non-empty option removes the raw-SRI-parsing fragility. Specify workspace revision selection through ordinary overrides whose targets carry the ordinal as semver build metadata (`<version>+rN`): `+r0` pins the original, positive ordinals adopt or freeze replacements, unknown ordinals fail hard, and registry policy 403s still win. Require each `dist.revisions` entry to carry a `manifest` object with the abbreviated-packument field subset, derived from the verified tarball and never provider-declared, so a pinned non-selected revision resolves with its own dependency metadata. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pnpr/text/0000-integrity-addressed-patch-revisions.md (1)
328-332: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDefine revision-reference equality separately from artifact equality.
Line 328 says the
?rNoption is excluded from equality, while selection still depends on the digest plus ordinal anddist.revisions[]advertises each ordinal as a distinct artifact. Document that canonical digest equality ignores options, but resolving or checking?rNmust match the canonicaldist.revisions[].revisionfor that integrity, and reject or canonicalize forged mismatched ordinals.🤖 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 `@pnpr/text/0000-integrity-addressed-patch-revisions.md` around lines 328 - 332, Clarify the revision semantics in the document: canonical artifact digest equality must ignore the ?rN option, while resolving or validating a revision reference must match the canonical dist.revisions[].revision for the corresponding integrity digest. Specify that forged or inconsistent ordinals are rejected or canonicalized, and keep option-only changes from creating new stored artifacts.
🤖 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.
Inline comments:
In `@pnpr/text/0000-integrity-addressed-patch-revisions.md`:
- Line 718: Resolve the ordinal contract before retaining the conformance
requirement: explicitly require stable monotonic rN ordinals across provider
changes, or define the migration behavior that applies when providers change.
Update the surrounding unresolved questions and the conformance statement for
the first replacement so they consistently reflect the chosen rule.
- Around line 509-514: Update the revision-override handling for +rN selections
to persist the addressed registry together with the package identity and pinned
ordinal. When resolving or applying an override, require the stored registry and
name@version to match the current package before using the revision; ensure pnpm
update --patches recognizes these workspace overrides as pinned and does not
refresh them.
In `@text/0000-integrity-addressed-registry-tarballs.md`:
- Around line 149-155: Update the integrity-validation wording to compare the
decoded URL digest with the decoded payload from dist.integrity after removing
its canonical rN option, while retaining the sha512- algorithm prefix handling.
Clarify that literal comparison against the full integrity string is invalid
because the URL contains only the digest.
---
Outside diff comments:
In `@pnpr/text/0000-integrity-addressed-patch-revisions.md`:
- Around line 328-332: Clarify the revision semantics in the document: canonical
artifact digest equality must ignore the ?rN option, while resolving or
validating a revision reference must match the canonical
dist.revisions[].revision for the corresponding integrity digest. Specify that
forged or inconsistent ordinals are rejected or canonicalized, and keep
option-only changes from creating new stored artifacts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d0504695-6831-4e86-b826-b5927fe1d04b
📒 Files selected for processing (2)
pnpr/text/0000-integrity-addressed-patch-revisions.mdtext/0000-integrity-addressed-registry-tarballs.md
📜 Review details
🧰 Additional context used
🪛 LanguageTool
pnpr/text/0000-integrity-addressed-patch-revisions.md
[style] ~648-~648: Consider an alternative for the overused word “exactly”.
Context: ...ements. Rejected: - an empty option is exactly the state a generic SRI library is most...
(EXACTLY_PRECISELY)
text/0000-integrity-addressed-registry-tarballs.md
[style] ~596-~596: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n policy for pnpm update --patches? - What should the global posture setting that ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (2)
pnpr/text/0000-integrity-addressed-patch-revisions.md (1)
19-21: LGTM!Also applies to: 24-27, 31-33, 94-96, 118-123, 146-147, 164-166, 266-267, 308-327, 334-337, 357-388, 409-424, 458-462, 643-659, 705-706, 716-717, 719-724, 766-767
text/0000-integrity-addressed-registry-tarballs.md (1)
367-372: 🗄️ Data Integrity & IntegrationSpecify verification of revision manifests before using them for resolution.
dist.revisions[N].manifestbecomes authoritative for dependencies, peers, engines, and install scripts, but the fetch steps only verify tarball bytes against the digest. Explicitly require the companion manifest-verification contract—including its binding to the selected name, version, revision, and digest—and fail before subtree resolution when that metadata is stale or mismatched.Also applies to: 546-548
Exact-version override selectors already match intersecting declared ranges in current pnpm (pnpm/pnpm#13470, retained for npm parity), so no resolver semantics change is needed. <version>+rN becomes a plain revision-addressed spec — usable as an override target or a directly declared dependency — resolved by picking the version normally, then selecting the revision from dist.revisions. State the version-pinning trade-off and the npm degradation (pin without revision selection) honestly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Provider manifests carry a signed monotonic sequence; pnpr rejects replayed lower-sequence documents, so selection rollback requires a fresh manifest or an explicitly authorized operator action. - Revision allocation is transactional: (registry, name, version, digest) maps to exactly one ordinal, idempotent under retries. - Immutability means stable bytes, not availability: policy refusal requires edge purge or non-shared caching, applies identically to canonical and digest routes, and the digest route authorizes against the principal's package-level access. - Ordinals are documented as registry-scoped; portability guarantees integrity, not identical ordinal meaning across registries. - Override rewriting applies once (no chains or cycles), and conflicting revision demands for one name@version fail explicitly. - deprecated is registry-managed mutable metadata, excluded from the artifact-derived revision manifest subset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Allocation serializes on (registry, name, version) with separate uniqueness constraints on the ordinal and the digest mapping, so concurrent refreshes with different digests cannot share an ordinal. - The registry owns the ordinal history: provider renames, replacements, or removals never renumber existing revisions (previously an open question, now normative since rN pins are a public API). - The provider sequence checkpoint advances atomically with durable manifest acceptance, keeping a crashed sequence retryable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The ordinal moves out of the integrity value: dist.revision on the registry side and resolution.revision in the lockfile, present only for replacements (N >= 1). Originals are never marked — the canonical URL is pinned to revision zero by invariant, so plain integrity-only entries stay correct on every registry, a lockfile with no adopted patches is byte-identical to today's format, and only revision entries need the lockfile-version gate. dist.integrity returns to a standard SRI value that strict consumers parse. Registry feature detection (the pnpr handshake) is specified as advisory only — diagnostics and an optional originals-by-digest optimization — never load-bearing for the fetch convention, which the lockfile field alone determines. Digest-route tarballs without dist.revision are normalized to integrity-only entries so hostnames never enter the lockfile. Document that npm/yarn patch delivery rides dist.tarball (unchanged by canonical immutability), that convention-reconstructing clients fail loudly rather than silently, and add a client-matrix verification question. The SRI-option and mark-everything designs move to rejected alternatives; +rN spec syntax is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pnpr/text/0000-integrity-addressed-patch-revisions.md (1)
203-218: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftConstrain provider artifact fetches before download.
entries.*.tarballis an arbitrary server-side URL, but the RFC does not restrict its origin, redirects, DNS rebinding, or private-network targets. A signed or compromised provider manifest could therefore turn refresh into SSRF. Require HTTPS provider-origin allowlists, private-IP/egress checks, redirect validation, and ensure provider credentials are never forwarded to artifact hosts.🤖 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 `@pnpr/text/0000-integrity-addressed-patch-revisions.md` around lines 203 - 218, The artifact download flow described for manifest entries.*.tarball must enforce SSRF protections before fetching: require HTTPS URLs from an approved provider-origin allowlist, reject private or otherwise disallowed resolved addresses and egress targets, validate every redirect against the same rules, and never forward provider credentials to artifact hosts. Apply these checks before download and preserve complete integrity verification afterward.
🤖 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.
Inline comments:
In `@pnpr/text/0000-integrity-addressed-patch-revisions.md`:
- Around line 438-443: Update the abbreviated historical metadata specification
to retain each history entry’s integer revision ordinal alongside its integrity,
digest URL, and manifest fields. Ensure +rN resolution can map N to the correct
historical record, and add a focused test covering revision ordinals in
abbreviated packuments.
- Around line 438-443: Define a transactional carry-forward/merge rule for
revision swaps so projected version entries preserve registry-managed mutable
fields, including deprecated, when refreshed. Update the revision
refresh/projection process described in the metadata rules, keeping immutable
historical provenance separate while merging existing mutable metadata into the
replacement entry.
- Around line 220-230: Update the per-provider sequence checkpoint logic to
persist both the highest accepted sequence and its canonical manifest digest.
Treat an exact replay of the same sequence and digest as an idempotent no-op,
but reject any manifest using the same sequence with a different digest as
equivocation; retain advancement only for higher sequences and keep checkpoint
updates atomic with durable manifest acceptance.
- Around line 334-346: Define and document a single validation policy for
dist.revision across registry and client validation, covering absent,
non-integer, out-of-range, and mismatched values. Ensure full and abbreviated
packuments apply the same policy, and explicitly reconcile malformed-value
rejection with structured revision metadata being authoritative; update the
relevant test matrix and rules accordingly.
In `@text/0000-integrity-addressed-registry-tarballs.md`:
- Around line 376-381: Make revision refresh graph-transactional: when updating
the resolved entry’s revision and manifest, re-resolve the entire affected
dependency graph using the refreshed revision metadata, including dependency and
peer edges, engines, binaries, and install-script state. Commit all graph,
snapshot, and peer-resolution changes atomically, and roll back the refresh if
re-resolution produces conflicts. Add coverage for a revision whose dependency
metadata changes, including the corresponding lines around the revision refresh
logic.
- Around line 207-225: Update the revisioned digest tarball fetch requirements
at this section and the corresponding sections around lines 612 and 642 to
reject every 3xx response as a hard failure, not only redirects crossing
origins. Preserve the single authenticated GET behavior and prohibit following
same-origin or cross-origin redirects.
---
Outside diff comments:
In `@pnpr/text/0000-integrity-addressed-patch-revisions.md`:
- Around line 203-218: The artifact download flow described for manifest
entries.*.tarball must enforce SSRF protections before fetching: require HTTPS
URLs from an approved provider-origin allowlist, reject private or otherwise
disallowed resolved addresses and egress targets, validate every redirect
against the same rules, and never forward provider credentials to artifact
hosts. Apply these checks before download and preserve complete integrity
verification afterward.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 92882297-ba60-41e2-9dfc-8fc8fe0a3e60
📒 Files selected for processing (2)
pnpr/text/0000-integrity-addressed-patch-revisions.mdtext/0000-integrity-addressed-registry-tarballs.md
📜 Review details
🧰 Additional context used
🪛 LanguageTool
text/0000-integrity-addressed-registry-tarballs.md
[grammar] ~137-~137: Use a hyphen to join words.
Context: ...egrity; 4. the digest is complete, well formed, and unambiguous; 5. dist.revisi...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (6)
pnpr/text/0000-integrity-addressed-patch-revisions.md (2)
1-14: LGTM!Also applies to: 26-58, 62-138, 141-192, 232-263, 265-283, 290-333, 347-410, 424-437, 445-505, 507-552, 554-604, 606-735, 737-758, 775-796, 798-835
15-22: 🗄️ Data Integrity & IntegrationManual verification needed for revision selection rules.
The excerpt raises a plausible concern about inconsistent keying between
dist.tarballencoding, multiple SRI hash expressions, missingdist.revision, and absent revision handling, but the draft context is needed before ruling this as actionable.text/0000-integrity-addressed-registry-tarballs.md (4)
130-138: Compare the URL digest with the SRI payload, not the full SRI token.
dist.integritycontainssha512-<base64>, while the URL contains only digest bytes. Define validation as decoding the SRI payload after removing the algorithm prefix and comparing those bytes with the decoded base64url path digest.
315-319: Specify the concrete lockfile gate.The RFC requires revision fields to be gated but still leaves the lockfile version and emission/rejection behavior unresolved. Define the exact version, writer behavior, hydrated-lockfile rejection, and mixed-version compatibility matrix.
Also applies to: 606-608, 676-678
151-161: 🗄️ Data Integrity & Integration | ⚡ Quick winDefine the ordinal’s numeric bounds and canonical encoding.
“Positive integer” is insufficient for a JSON/JavaScript protocol: without a safe upper bound or canonical decimal-string representation, large ordinals can round or compare inconsistently and break revision selection. Add overflow, leading-zero, and maximum-value tests.
Also applies to: 636-637
357-368: 🎯 Functional CorrectnessState that
+rNoverrides need new override behavior.The linked pnpm issue is not publicly available as a stable contract here, so this should not present pnpm’s existing override behavior as sufficient if the feature allows a version-pinning override to force a selected version regardless of the declared range.
| The signed document carries a monotonic `sequence`. pnpr persists the highest | ||
| sequence it has accepted per provider and rejects any manifest whose sequence | ||
| is not greater: a valid signature over an older document must not be able to | ||
| replay earlier selections and reselect a vulnerable revision. Moving selection | ||
| backward — withdrawing a bad patch, restoring the original — therefore always | ||
| requires either a new, higher-sequence manifest from the provider or an | ||
| explicit, separately authorized operator operation, never a replayed | ||
| document. The checkpoint advances atomically with — never before — the | ||
| durable acceptance of the manifest's work: a crash mid-ingestion leaves the | ||
| previous checkpoint in place, so the same sequence can be retried rather | ||
| than becoming permanently unacceptable. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make sequence checkpoints idempotent and detect equivocation.
Persist the highest sequence together with the canonical manifest digest. An exact replay of an accepted (sequence, digest) should be a no-op, while the same sequence with different contents must be rejected. The current sequence > highest rule makes successful refreshes appear invalid after timeout/retry and cannot distinguish duplicate delivery from provider equivocation.
🤖 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 `@pnpr/text/0000-integrity-addressed-patch-revisions.md` around lines 220 -
230, Update the per-provider sequence checkpoint logic to persist both the
highest accepted sequence and its canonical manifest digest. Treat an exact
replay of the same sequence and digest as an idempotent no-op, but reject any
manifest using the same sequence with a different digest as equivocation; retain
advancement only for higher sequences and keep checkpoint updates atomic with
durable manifest acceptance.
| pnpr applies these rules: | ||
|
|
||
| - it emits `dist.revision` exactly when the selected artifact is a | ||
| replacement (`N ≥ 1`); originals serve unmarked metadata; | ||
| - it assigns `1` to the first distinct accepted replacement and monotonically | ||
| increasing values to later replacements; | ||
| - it never uses the provider name or provider revision in the field; | ||
| - it preserves ordinals in `dist.revisions`, audit output, and logs; | ||
| - it excludes the field from object lookup, equality, authorization, VEX, and | ||
| integrity verification; | ||
| - it treats structured revision metadata as authoritative if the field is | ||
| forged or inconsistent; | ||
| - a field-only change cannot create a new stored artifact. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Define one response to invalid dist.revision values.
This section says structured history is authoritative when dist.revision is forged or inconsistent, while the test matrix requires malformed values to be rejected. Define consistent behavior for absent, non-integer, out-of-range, and mismatched values across registry and client validation; otherwise full and abbreviated packuments may resolve differently.
Also applies to: 762-774
🤖 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 `@pnpr/text/0000-integrity-addressed-patch-revisions.md` around lines 334 -
346, Define and document a single validation policy for dist.revision across
registry and client validation, covering absent, non-integer, out-of-range, and
mismatched values. Ensure full and abbreviated packuments apply the same policy,
and explicitly reconcile malformed-value rejection with structured revision
metadata being authoritative; update the relevant test matrix and rules
accordingly.
| additionally carry all provenance fields. Abbreviated metadata should keep | ||
| current `dist` plus the integrity, digest URL, and `manifest` of historical | ||
| revisions, so revision pinning works from the metadata pnpm actually | ||
| fetches. An unpatched version does not need a `dist.revisions` array or a | ||
| `dist.revision` field; it serves ordinary metadata, and its `dist.tarball` | ||
| may use the canonical or the digest URL. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Retain the ordinal in abbreviated historical metadata.
+rN resolution needs the mapping from N to a history record, but this requirement lists only historical integrity, URL, and manifest fields and omits revision. Retain the integer ordinal with every historical entry, and test this specifically in abbreviated packuments.
🤖 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 `@pnpr/text/0000-integrity-addressed-patch-revisions.md` around lines 438 -
443, Update the abbreviated historical metadata specification to retain each
history entry’s integer revision ordinal alongside its integrity, digest URL,
and manifest fields. Ensure +rN resolution can map N to the correct historical
record, and add a focused test covering revision ordinals in abbreviated
packuments.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve mutable projected metadata during revision swaps.
The RFC excludes deprecated from immutable history and stores it on the projected version, but revision refreshes replace the entire projected version entry. Define a transactional carry-forward/merge rule for deprecated and all other registry-managed mutable fields; otherwise patch refreshes can erase existing policy metadata.
🤖 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 `@pnpr/text/0000-integrity-addressed-patch-revisions.md` around lines 438 -
443, Define a transactional carry-forward/merge rule for revision swaps so
projected version entries preserve registry-managed mutable fields, including
deprecated, when refreshed. Update the revision refresh/projection process
described in the metadata rules, keeping immutable historical provenance
separate while merging existing mutable metadata into the replacement entry.
| For an entry carrying `revision: N`, pnpm: | ||
|
|
||
| 1. converts the complete base64 digest to base64url; | ||
| 2. resolves `-/tarballs/<algorithm>/<digest>` against the effective registry | ||
| base; | ||
| 3. sends one ordinary authenticated GET; | ||
| 4. verifies the response against the complete SRI digest before admitting it | ||
| to the content-addressed store. | ||
|
|
||
| If URL construction fails or the response does not match, installation fails. | ||
| pnpm must not fetch current package metadata, change the checksum, retry the | ||
| canonical URL, or fall forward to another revision. Falling back to the | ||
| canonical URL can never succeed anyway — it serves revision zero, whose bytes | ||
| cannot match a replacement's integrity — so the failure surfaces immediately | ||
| rather than after a wasted download. | ||
|
|
||
| The endpoint is same-origin with the configured registry, so ordinary registry | ||
| authentication and credential-scoping behavior applies. Redirects to another | ||
| origin are not part of this protocol. Knowledge of a digest is not |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'text/0000-integrity-addressed-registry-tarballs.md' . || true
echo "== file exists? =="
test -f text/0000-integrity-addressed-registry-tarballs.md && echo yes || echo no
echo "== relevant ranges =="
sed -n '195,230p' text/0000-integrity-addressed-registry-tarballs.md
echo "== nearby line counts =="
wc -l text/0000-integrity-addressed-registry-tarballs.md
echo "== references around 600-650 =="
sed -n '590,655p' text/0000-integrity-addressed-registry-tarballs.md
echo "== search redirect/tarball terms =="
rg -n "redirect|3xx|30[12]|tarballs|one GET|ordinary GET|credential|auth|same-origin|fallback|fall" text/0000-integrity-addressed-registry-tarballs.mdRepository: pnpm/rfcs
Length of output: 7610
Reject all redirects for revisioned digest tarballs.
The protocol says “one ordinary authenticated GET” and “no redirect”, but the text only explicitly requires cross-origin redirect rejection. Require that every 3xx response during this fetch is a hard failure, including same-origin redirects, since following redirects can expose different registry behavior/keys/routes while still claiming “one GET.”
Also applies to lines 612 and 642.
🤖 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 `@text/0000-integrity-addressed-registry-tarballs.md` around lines 207 - 225,
Update the revisioned digest tarball fetch requirements at this section and the
corresponding sections around lines 612 and 642 to reject every 3xx response as
a hard failure, not only redirects crossing origins. Preserve the single
authenticated GET behavior and prohibit following same-origin or cross-origin
redirects.
| 2. selects registry revision `N` from the resolved entry's `dist.revisions` | ||
| instead of the selected `dist`: its integrity, its digest URL, and its | ||
| `manifest` record. Revisions may legally differ in dependencies, optional | ||
| and peer dependencies, `bin`, engines, and install-script posture, so the | ||
| pinned entry's own `manifest` drives subtree resolution — never the current | ||
| projected version's top-level fields. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make revision refresh graph-transactional.
Because revision manifests may change dependencies, peers, engines, binaries, or scripts, updating only integrity, revision, and one snapshot can leave stale graph edges or peer resolutions. Re-resolve the affected graph atomically and roll back on conflicts; add a test where a revision changes dependency metadata.
Also applies to: 450-455
🤖 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 `@text/0000-integrity-addressed-registry-tarballs.md` around lines 376 - 381,
Make revision refresh graph-transactional: when updating the resolved entry’s
revision and manifest, re-resolve the entire affected dependency graph using the
refreshed revision metadata, including dependency and peer edges, engines,
binaries, and install-script state. Commit all graph, snapshot, and
peer-resolution changes atomically, and roll back the refresh if re-resolution
produces conflicts. Add coverage for a revision whose dependency metadata
changes, including the corresponding lines around the revision refresh logic.
|
pnpm might start supporting vendors like echo, that upload multiple releases that resolve to the same npm version (but differ in tarball and integrity). a feature to consider is the ability to refresh a lockfile to get the newest release while keeping the same version. Since a query of the version would only return the release the registry exposes there is no decision logic to be made by pnpr/pnpm, it should probably only query each version in the lockfile directly and replace the entry if it's different. pnpm has a feature that does something similar (npm install --fix-lockfile --lockfile-only), but what could be added is restriction to only update the hash if it belongs to a pre configured vendor etc |
| Versions such as `2.7.4-echo.2` are prereleases under semver. Ordinary ranges | ||
| containing `2.7.4` do not necessarily select them, peer ranges may reject them, | ||
| and the package no longer reports the version the application was tested | ||
| against. Build metadata is unsuitable because semver ignores it for precedence |
There was a problem hiding this comment.
There's a possibility of using build metadata. npm clients will ignore it, thereby seeing it as just the regular version, but the registry itself can be organized to serve the newest revision whenever its published. This (mostly) will not break any old lockfile as long as the urls of different versions are different (you can encode them with pkg-version+vendor.N.tgz for example). If the old urls stay on the registry users can always get the latest version when they ask for it, and they will not break if they're lockfile is out of date.
There was a problem hiding this comment.
Right, we do leverage the build metadata in this rfc for explicitly specifying which revision to install via overrides. For the tarball URL I have suggested to use the integrity checksum of the tarball. Alternatively we could consider adding the revision number to the url as you suggest.
sure, such feature would make sense. The current architecture would make it slow though. We'd have to read the whole package document for every package found in the lockfile. I wonder if there is a better way. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The SRI-option rejection asserted that strict consumers may reject options. The real failure is worse: pnpm's Rust ssri has no option grammar, so it absorbs the option into the digest, round-trips it through Display, and fails far from the parse. Integrity-string equality is also byte equality across pnpm, so an ordinal inside the value would make a revision-only difference read as a content change. Record encoding the ordinal in the lockfile package key as an alternative, since the registry-qualified keys of pnpm/pnpm#13528 invite it. It is rejected on churn: a dep path repeats once per dependent, so one revision bump would rewrite roughly fifty lines for a package as common as semver, and patchedDependencies already demonstrates this with patch_hash. The key may carry extra data; it may not carry data designed to change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
Summary
This design supersedes #14 (closed in its favor). It contains two companion
RFCs:
pnpr/text/0000-integrity-addressed-patch-revisions.mdprojects providerbuilds over the original
name@version. Every artifact is served from animmutable, registry-scoped URL keyed by its complete sha512 digest; the
canonical
name/-/name-version.tgzURL stays pinned to the original bytesforever.
dist.revisionsretains an append-only history of every acceptedartifact — integrity, provider, ordinal, VEX/provenance, and each
artifact's own resolution-relevant
manifestfields.text/0000-integrity-addressed-registry-tarballs.mdteaches pnpm thelockfile representation and the
<version>+rNrevision-addressed spec.The revision field
The ordinal is a plain integer field, present only for replacements:
Originals are never marked. The canonical URL is pinned to revision zero
by invariant, so integrity-only entries stay correct on every registry —
revision-aware or not — and a lockfile with no adopted patches needs no new
format. Only
revision: Nentries are gated by lockfile version and fetchedfrom the digest route (one direct CDN request; falling back to the canonical
URL is impossible by construction, since original bytes cannot match a
replacement's integrity). Registry feature detection (the pnpr handshake) is
advisory only — better diagnostics, optional originals-by-digest optimization
— never load-bearing: the lockfile field alone determines the fetch
convention.
Explicit revision selection:
<version>+rNThe ordinal carried as semver build metadata makes a revision-addressed spec,
usable as an override target or a directly declared dependency:
{ "pnpm": { "overrides": { "ejs@2.7.4": "2.7.4+r0" } } } // keep the originalNo new override semantics: exact-version selectors already match intersecting
declared ranges (pnpm/pnpm#13470, retained for npm parity). pnpm resolves the
version half normally, then selects revision
Nfromdist.revisions—using that revision's own
manifestfields, since revisions may legallydiffer in dependencies. Unknown ordinals fail hard; rewriting applies once;
conflicting revision demands for one
name@versionare an explicit error.Registry policy wins: refused bytes fail the install, they are never
substituted.
Important properties
dist.tarballonfresh resolutions; pnpm additionally gets revision management (host-free
lockfile entries,
+rNpinning,pnpm update --patches). Clients thatreconstruct conventional URLs fail loudly, never silently wrong.
canonical immutability makes that honest instead of breaking them.
sequence; replayed olderdocuments are rejected, and the checkpoint advances atomically with durable
acceptance, so rollback requires a fresh manifest or an explicitly
authorized operator action.
(registry, name, version)with separateuniqueness constraints on ordinal and digest; the registry owns the ordinal
history — provider changes never renumber.
against the principal's package-level access; knowledge of a digest is not
authorization. Full sha512 values are required.
identically to canonical and digest routes and requires edge purge or
non-shared caching.
guarantees integrity, never identical ordinal meaning.
deprecatedand other registry-managed mutable metadata stay outside theimmutable revision records, which carry only artifact-derived fields.
Compatibility
revisionentries are byte-identical to the currentformat — no gate, readable by older pnpm.
whole unpatched graph installable; only replacement entries fail, as
genuine unavailability.
dist.integrityis a standard SRI value; npm-compatible clients ignore theunknown
dist.revisionfield and followdist.tarballnormally.lockfile-identity problem.
Summary by CodeRabbit
pnpm update --patches.