From 0721d64188a10f1074eec05f32ea7ed7ea885b62 Mon Sep 17 00:00:00 2001 From: mehmet turac Date: Mon, 25 May 2026 13:52:35 +0300 Subject: [PATCH] fix: require provenance for trusted publisher evidence (#11911) * fix: require provenance for trusted publisher evidence * test: align provenance fixtures with registry types * chore: include pnpm CLI in changeset The repo guideline requires every changeset that touches a published package to list the pnpm CLI explicitly so the fix appears in the CLI's release notes. * fix(resolving-npm-resolver): require provenance for trusted publisher evidence Ports pnpm's fea5fd41da: `get_trust_evidence` now only returns `TrustedPublisher` when the version carries both `_npmUser.trustedPublisher` *and* `dist.attestations.provenance`. Without the attestation, the publisher flag is metadata a staged publish could mint, so it can't be ranked above plain provenance. Refs #11887. --------- Co-authored-by: Zoltan Kochan --- .changeset/tidy-trust-publishers.md | 6 ++ .../crates/registry/src/package_version.rs | 12 ++-- .../src/create_npm_resolution_verifier.rs | 6 +- .../create_npm_resolution_verifier/tests.rs | 12 ++-- .../src/trust_checks.rs | 30 +++++---- .../src/trust_checks/tests.rs | 64 ++++++++++++++++++- .../src/createNpmResolutionVerifier.ts | 4 +- resolving/npm-resolver/src/trustChecks.ts | 2 +- .../test/createNpmResolutionVerifier.test.ts | 14 ++-- .../npm-resolver/test/trustChecks.test.ts | 26 +++++++- 10 files changed, 140 insertions(+), 36 deletions(-) create mode 100644 .changeset/tidy-trust-publishers.md diff --git a/.changeset/tidy-trust-publishers.md b/.changeset/tidy-trust-publishers.md new file mode 100644 index 0000000000..ef2d514d94 --- /dev/null +++ b/.changeset/tidy-trust-publishers.md @@ -0,0 +1,6 @@ +--- +"@pnpm/resolving.npm-resolver": patch +"pnpm": patch +--- + +Require provenance before treating trusted publisher metadata as the strongest trust evidence. diff --git a/pacquet/crates/registry/src/package_version.rs b/pacquet/crates/registry/src/package_version.rs index 08c2a2c213..3fd740085b 100644 --- a/pacquet/crates/registry/src/package_version.rs +++ b/pacquet/crates/registry/src/package_version.rs @@ -32,12 +32,14 @@ pub struct PackageVersion { pub peer_dependencies: Option>, /// npm registry's per-version publisher metadata. When - /// `trusted_publisher` is present the version was published - /// through an OIDC-backed trusted-publisher integration, which - /// counts as the higher (`trustedPublisher`) trust rank that - /// upstream's [`getTrustEvidence`](https://github.com/pnpm/pnpm/blob/2a9bd897bf/resolving/npm-resolver/src/trustChecks.ts#L119-L127) + /// `trusted_publisher` is present alongside + /// `dist.attestations.provenance`, the version was published + /// through an OIDC-backed trusted-publisher integration *and* + /// shipped a provenance attestation, which together count as the + /// higher (`trustedPublisher`) trust rank that upstream's + /// [`getTrustEvidence`](https://github.com/pnpm/pnpm/blob/fea5fd41da/resolving/npm-resolver/src/trustChecks.ts#L119-L127) /// checks before falling back to the `provenance` attestation - /// rank. + /// rank. The publisher flag without provenance is ignored. /// /// Mirrors pnpm's /// [`PackageInRegistry._npmUser`](https://github.com/pnpm/pnpm/blob/2a9bd897bf/resolving/registry/types/src/index.ts#L29-L36) diff --git a/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs b/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs index d049e43fb8..7c946af619 100644 --- a/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs +++ b/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs @@ -412,9 +412,9 @@ impl NpmResolutionVerifier { /// /// No attestation fast-path: presence of provenance on the current /// version is not sufficient to clear a downgrade. The package may - /// have shipped earlier versions under a `trustedPublisher` (the - /// higher-rank evidence) and then dropped to plain provenance — - /// `fail_if_trust_downgraded` correctly flags that. + /// have shipped earlier versions under a `trustedPublisher` with + /// provenance (the higher-rank evidence) and then dropped to plain + /// provenance — `fail_if_trust_downgraded` correctly flags that. async fn run_trust_check( &self, registry: &str, diff --git a/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs b/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs index c5ed638084..e886f1566e 100644 --- a/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs +++ b/pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier/tests.rs @@ -74,9 +74,12 @@ fn min_age_packument_json(name: &str, version: &str, published_at: &str) -> serd }) } -/// Packument with two versions: earlier (`prior_version`) has -/// `_npmUser.trustedPublisher`, current has only `dist.attestations.provenance`. -/// This is the canonical "trusted-publisher → provenance" downgrade. +/// Packument with two versions: earlier (`prior_version`) has both +/// `_npmUser.trustedPublisher` *and* `dist.attestations.provenance` +/// — `get_trust_evidence` only ranks the publisher flag as the +/// strongest evidence when the version also ships an attestation — +/// while current has only `dist.attestations.provenance`. This is the +/// canonical "trusted-publisher → provenance" downgrade. fn trust_downgrade_packument(name: &str) -> serde_json::Value { serde_json::json!({ "name": name, @@ -93,7 +96,8 @@ fn trust_downgrade_packument(name: &str) -> serde_json::Value { "dist": { "integrity": FAKE_INTEGRITY, "shasum": "0000000000000000000000000000000000000000", - "tarball": format!("https://registry/{name}-1.0.0.tgz") + "tarball": format!("https://registry/{name}-1.0.0.tgz"), + "attestations": { "provenance": { "predicateType": "https://slsa.dev/provenance/v1" } } } }, "1.1.0": { diff --git a/pacquet/crates/resolving-npm-resolver/src/trust_checks.rs b/pacquet/crates/resolving-npm-resolver/src/trust_checks.rs index bac0a8ecc1..6b7b49e4ad 100644 --- a/pacquet/crates/resolving-npm-resolver/src/trust_checks.rs +++ b/pacquet/crates/resolving-npm-resolver/src/trust_checks.rs @@ -8,10 +8,10 @@ //! For each it asks [`get_trust_evidence`] which "rank" of evidence //! the version exposes: //! -//! - `trustedPublisher` (rank 2) — `_npmUser.trustedPublisher` is -//! present. +//! - `trustedPublisher` (rank 2) — `_npmUser.trustedPublisher` and +//! `dist.attestations.provenance` are both present. //! - `provenance` (rank 1) — `dist.attestations.provenance` is -//! present (and no trusted-publisher record). +//! present without a trusted-publisher record. //! - `None` (rank 0 / no evidence). //! //! The strongest rank seen across the prior history is the @@ -36,9 +36,11 @@ use pacquet_registry::{Package, PackageVersion}; pub enum TrustEvidence { /// `dist.attestations.provenance` is set. Provenance, - /// `_npmUser.trustedPublisher` is set (overrides provenance — - /// it's a stronger signal that a known upstream pipeline - /// published the version). + /// `_npmUser.trustedPublisher` and `dist.attestations.provenance` + /// are both set. Without the attestation the publisher flag is + /// just metadata a future staged-publish flow could mint, so it + /// only counts as the stronger signal when the version also + /// shipped a provenance attestation. TrustedPublisher, } @@ -243,14 +245,20 @@ fn detect_strongest_trust_evidence_before( best } -/// `_npmUser.trustedPublisher` outranks `dist.attestations.provenance`; -/// absence of both yields `None`. Mirrors pnpm's -/// [`getTrustEvidence`](https://github.com/pnpm/pnpm/blob/2a9bd897bf/resolving/npm-resolver/src/trustChecks.ts#L119-L127). +/// `_npmUser.trustedPublisher` outranks `dist.attestations.provenance` +/// only when the version also carries a provenance attestation; +/// otherwise the publisher flag is ignored and the version falls back +/// to the provenance rank or `None`. Mirrors pnpm's +/// [`getTrustEvidence`](https://github.com/pnpm/pnpm/blob/fea5fd41da/resolving/npm-resolver/src/trustChecks.ts#L119-L127). pub fn get_trust_evidence(version: &PackageVersion) -> Option { - if version.npm_user.as_ref().and_then(|user| user.trusted_publisher.as_ref()).is_some() { + let has_provenance = + version.dist.attestations.as_ref().and_then(|att| att.provenance.as_ref()).is_some(); + let has_trusted_publisher = + version.npm_user.as_ref().and_then(|user| user.trusted_publisher.as_ref()).is_some(); + if has_trusted_publisher && has_provenance { return Some(TrustEvidence::TrustedPublisher); } - if version.dist.attestations.as_ref().and_then(|att| att.provenance.as_ref()).is_some() { + if has_provenance { return Some(TrustEvidence::Provenance); } None diff --git a/pacquet/crates/resolving-npm-resolver/src/trust_checks/tests.rs b/pacquet/crates/resolving-npm-resolver/src/trust_checks/tests.rs index ed5036a8a8..eb2391b78b 100644 --- a/pacquet/crates/resolving-npm-resolver/src/trust_checks/tests.rs +++ b/pacquet/crates/resolving-npm-resolver/src/trust_checks/tests.rs @@ -13,14 +13,17 @@ enum Evidence { /// Build a JSON object for a single version with the trust-evidence /// shape the verifier reads (`_npmUser.trustedPublisher` or -/// `dist.attestations.provenance`). +/// `dist.attestations.provenance`). A `TrustedPublisher` fixture +/// includes both fields: per `get_trust_evidence`, the publisher +/// flag only outranks plain provenance when the version also ships a +/// provenance attestation. fn version_json(name: &str, version: &str, evidence: Evidence) -> serde_json::Value { let mut dist = serde_json::json!({ "integrity": "sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==", "shasum": "0000000000000000000000000000000000000000", "tarball": format!("https://registry/{name}-{version}.tgz") }); - if matches!(evidence, Evidence::Provenance) { + if matches!(evidence, Evidence::Provenance | Evidence::TrustedPublisher) { dist["attestations"] = serde_json::json!({ "provenance": { "predicateType": "https://slsa.dev/provenance/v1" } }); @@ -318,3 +321,60 @@ fn prior_version_missing_time_does_not_mask_trust_history() { .expect_err("missing-time on a prior version must not mask the 1.0.0 baseline"); assert!(matches!(err, TrustViolation::TrustDowngrade { .. }), "got {err:?}"); } + +mod get_trust_evidence { + use pacquet_registry::PackageVersion; + + use super::{Evidence, version_json}; + use crate::trust_checks::{TrustEvidence, get_trust_evidence}; + + fn parse(version: serde_json::Value) -> PackageVersion { + serde_json::from_value(version).expect("deserialize fixture PackageVersion") + } + + /// `_npmUser.trustedPublisher` without `dist.attestations.provenance` + /// is ignored — the publisher flag alone is metadata a staged + /// publish could mint, so it cannot stand in for the attestation. + #[test] + fn trusted_publisher_without_provenance_is_none() { + let mut version = version_json("acme", "1.0.0", Evidence::None); + version["_npmUser"] = serde_json::json!({ + "trustedPublisher": { "id": "github", "oidcConfigId": "release" } + }); + assert!(get_trust_evidence(&parse(version)).is_none()); + } + + /// `_npmUser.trustedPublisher` *with* provenance ranks as + /// `TrustedPublisher` (the strongest evidence). + #[test] + fn trusted_publisher_with_provenance_ranks_strongest() { + let version = version_json("acme", "1.0.0", Evidence::TrustedPublisher); + assert!(matches!( + get_trust_evidence(&parse(version)), + Some(TrustEvidence::TrustedPublisher) + )); + } + + /// `dist.attestations.provenance` alone ranks as `Provenance`. + #[test] + fn provenance_alone_ranks_as_provenance() { + let version = version_json("acme", "1.0.0", Evidence::Provenance); + assert!(matches!(get_trust_evidence(&parse(version)), Some(TrustEvidence::Provenance))); + } + + /// Neither field present → `None`. + #[test] + fn no_evidence_returns_none() { + let version = version_json("acme", "1.0.0", Evidence::None); + assert!(get_trust_evidence(&parse(version)).is_none()); + } + + /// An `_npmUser` record without a `trustedPublisher` field is + /// ignored. + #[test] + fn npm_user_without_trusted_publisher_is_none() { + let mut version = version_json("acme", "1.0.0", Evidence::None); + version["_npmUser"] = serde_json::json!({ "name": "alice", "email": "alice@example.com" }); + assert!(get_trust_evidence(&parse(version)).is_none()); + } +} diff --git a/resolving/npm-resolver/src/createNpmResolutionVerifier.ts b/resolving/npm-resolver/src/createNpmResolutionVerifier.ts index fff0422dd8..ab949c3aa1 100644 --- a/resolving/npm-resolver/src/createNpmResolutionVerifier.ts +++ b/resolving/npm-resolver/src/createNpmResolutionVerifier.ts @@ -329,8 +329,8 @@ async function runAgeCheck ( * attestation endpoint is cheaper than the packument: presence of * provenance on the current version is not sufficient to clear a * downgrade. A package could have shipped earlier versions under a - * `trustedPublisher` (the higher-rank evidence) and then dropped - * back to plain provenance for the version we're verifying — + * `trustedPublisher` with provenance (the higher-rank evidence) and + * then dropped back to plain provenance for the version we're verifying — * `failIfTrustDowngraded` correctly flags that, and a "has any * attestation → pass" shortcut would silently miss it. */ diff --git a/resolving/npm-resolver/src/trustChecks.ts b/resolving/npm-resolver/src/trustChecks.ts index 258245eca2..4a7d1006af 100644 --- a/resolving/npm-resolver/src/trustChecks.ts +++ b/resolving/npm-resolver/src/trustChecks.ts @@ -117,7 +117,7 @@ function detectStrongestTrustEvidenceBeforeDate ( } export function getTrustEvidence (manifest: PackageInRegistry): TrustEvidence | undefined { - if (manifest._npmUser?.trustedPublisher) { + if (manifest._npmUser?.trustedPublisher && manifest.dist?.attestations?.provenance) { return 'trustedPublisher' } if (manifest.dist?.attestations?.provenance) { diff --git a/resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts b/resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts index a778a186b3..c5c95ae236 100644 --- a/resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts +++ b/resolving/npm-resolver/test/createNpmResolutionVerifier.test.ts @@ -50,7 +50,7 @@ test('createNpmResolutionVerifier() returns undefined when no policy is active', }) test('createNpmResolutionVerifier() flags a trustedPublisher → provenance downgrade', async () => { - // 0.0.1 was published by a trustedPublisher → rank 2. + // 0.0.1 was published by a trustedPublisher with provenance → rank 2. // 0.0.2 is provenance-only (rank 1, weaker) → downgrade vs 0.0.1. // This is exactly the case the resolver-time trustChecks unit tests // cover, but routed through the lockfile verifier. The verifier must @@ -62,7 +62,11 @@ test('createNpmResolutionVerifier() flags a trustedPublisher → provenance down '0.0.1': { name: 'demo', version: '0.0.1', - dist: { tarball: 'https://registry.npmjs.org/demo/-/demo-0.0.1.tgz', shasum: 'aa' }, + dist: { + tarball: 'https://registry.npmjs.org/demo/-/demo-0.0.1.tgz', + shasum: 'aa', + attestations: { provenance: { predicateType: 'https://example.org/p' } }, + }, _npmUser: { trustedPublisher: { id: 'gha', oidcConfigId: 'cfg' } }, }, '0.0.2': { @@ -71,7 +75,7 @@ test('createNpmResolutionVerifier() flags a trustedPublisher → provenance down dist: { tarball: 'https://registry.npmjs.org/demo/-/demo-0.0.2.tgz', shasum: 'bb', - attestations: { provenance: { url: 'https://example.org/p' } }, + attestations: { provenance: { predicateType: 'https://example.org/p' } }, }, }, }, @@ -109,7 +113,7 @@ test('createNpmResolutionVerifier() passes a same-evidence-level version', async dist: { tarball: 'https://registry.npmjs.org/demo/-/demo-0.0.1.tgz', shasum: 'aa', - attestations: { provenance: { url: 'https://example.org/p1' } }, + attestations: { provenance: { predicateType: 'https://example.org/p1' } }, }, }, '0.0.2': { @@ -118,7 +122,7 @@ test('createNpmResolutionVerifier() passes a same-evidence-level version', async dist: { tarball: 'https://registry.npmjs.org/demo/-/demo-0.0.2.tgz', shasum: 'bb', - attestations: { provenance: { url: 'https://example.org/p2' } }, + attestations: { provenance: { predicateType: 'https://example.org/p2' } }, }, }, }, diff --git a/resolving/npm-resolver/test/trustChecks.test.ts b/resolving/npm-resolver/test/trustChecks.test.ts index 7e8b629d9e..001137f31d 100644 --- a/resolving/npm-resolver/test/trustChecks.test.ts +++ b/resolving/npm-resolver/test/trustChecks.test.ts @@ -5,7 +5,7 @@ import type { PackageInRegistry, PackageMetaWithTime } from '@pnpm/resolving.reg import { failIfTrustDowngraded, getTrustEvidence } from '../src/trustChecks.js' describe('getTrustEvidence', () => { - test('returns "trustedPublisher" when _npmUser.trustedPublisher exists', () => { + test('returns undefined when _npmUser.trustedPublisher exists without provenance', () => { const manifest: PackageInRegistry = { name: 'foo', version: '1.0.0', @@ -22,10 +22,10 @@ describe('getTrustEvidence', () => { tarball: 'https://registry.example.com/foo/-/foo-1.0.0.tgz', }, } - expect(getTrustEvidence(manifest)).toBe('trustedPublisher') + expect(getTrustEvidence(manifest)).toBeUndefined() }) - test('returns "trustedPublisher" even when attestations.provenance exists', () => { + test('returns "trustedPublisher" when attestations.provenance also exists', () => { const manifest: PackageInRegistry = { name: 'foo', version: '1.0.0', @@ -285,6 +285,11 @@ describe('failIfTrustDowngraded', () => { dist: { shasum: 'def456', tarball: 'https://registry.example.com/foo/-/foo-2.0.0.tgz', + attestations: { + provenance: { + predicateType: 'https://slsa.dev/provenance/v1', + }, + }, }, }, '3.0.0': { @@ -339,6 +344,11 @@ describe('failIfTrustDowngraded', () => { dist: { shasum: 'def456', tarball: 'https://registry.example.com/foo/-/foo-2.0.0.tgz', + attestations: { + provenance: { + predicateType: 'https://slsa.dev/provenance/v1', + }, + }, }, }, '3.0.0': { @@ -388,6 +398,11 @@ describe('failIfTrustDowngraded', () => { dist: { shasum: 'def456', tarball: 'https://registry.example.com/foo/-/foo-2.0.0.tgz', + attestations: { + provenance: { + predicateType: 'https://slsa.dev/provenance/v1', + }, + }, }, }, '3.0.0': { @@ -404,6 +419,11 @@ describe('failIfTrustDowngraded', () => { dist: { shasum: 'ghi789', tarball: 'https://registry.example.com/foo/-/foo-3.0.0.tgz', + attestations: { + provenance: { + predicateType: 'https://slsa.dev/provenance/v1', + }, + }, }, }, },