Skip to content

fix: provenance-file takes precedence over OIDC auto-generated provenance - #9882

Open
yunseo-kim wants to merge 3 commits into
npm:latestfrom
yunseo-kim:fix/provenance-file-precedence
Open

fix: provenance-file takes precedence over OIDC auto-generated provenance#9882
yunseo-kim wants to merge 3 commits into
npm:latestfrom
yunseo-kim:fix/provenance-file-precedence

Conversation

@yunseo-kim

@yunseo-kim yunseo-kim commented Aug 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Why Needed

When publishing with an externally generated provenance bundle under OIDC trusted publishing
(npm publish <tarball> --provenance-file=<bundle>), npm silently discarded the supplied bundle and published
its own auto-generated provenance instead. Three layers interacted:

  1. lib/utils/oidc.js auto-enabled provenance (opts.provenance = true) whenever the provenance config was at
    its default, without checking whether a provenance-file was supplied.
  2. In libnpmpublish's buildMetadata(), the inner provenance === true branch then ran generateProvenance(),
    so the verifyProvenance(subject, provenanceFile) branch, the only code path that reads the supplied file,
    never executed.
  3. Every documented way to disable automatic provenance was unusable in combination with --provenance-file
    (config-layer mutual exclusivity error, env carve-out, or publishConfig flatten timing).

What Changes

  • lib/utils/oidc.js: skip auto-enabling provenance when a provenance file is configured (opts.provenanceFile).
    opts already carries provenanceFile from every config source (CLI/env/npmrc/publishConfig) by the time the
    OIDC flow runs, so this covers all entry paths. The supplied bundle is now verified via verifyProvenance() and
    published, as documented.
  • libnpmpublish: buildMetadata() now throws EPROVENANCECONFLICT when both provenance: true and
    provenanceFile are provided.
  • Docs: config descriptions for provenance / provenance-file and the libnpmpublish README now state the
    precedence rule.

⚠️ New error code (feedback requested)

This PR introduces EPROVENANCECONFLICT in libnpmpublish, thrown when both provenance: true and
provenanceFile are provided programmatically.
Rationale: the README already documents the two as mutually exclusive, and silently preferring either direction
discards a cryptographically meaningful artifact. Note the CLI's config layer reports the same conflict as a
TypeError without an error code (pre-existing). Happy to align on EUSAGE or another convention per
review.

Testing

  • New CLI regression test: OIDC trusted publishing + provenance-file config asserts the published packument's
    sigstore attachment deep-equals the supplied bundle (and that sigstore generation is never invoked).
  • New libnpmpublish test: both options set → rejects with EPROVENANCECONFLICT, no registry PUT,
    generation not invoked.
  • Full root suite green with 100% coverage; libnpmpublish workspace suite green; lint clean.

References

Fixes #9879

Out of scope (noted for follow-up)

  • publishConfig.provenance: false does not block the OIDC auto-enable (publishConfig flattens into opts only,
    so config.isDefault('provenance') stays true). A separate behavioral question about isDefault semantics.
  • config.set('provenance', true, 'user') bypassing load-time exclusivity is unreachable in this flow after this fix;
    hardening config.set itself is an @npmcli/config semver-major conversation.

@yunseo-kim
fix: provenance-file takes precedence over OIDC auto-generated proven…
7b2ab6d 
…ance

When publishing with an externally generated provenance bundle under OIDC
trusted publishing, the auto-enable logic in the OIDC flow unconditionally
set `provenance: true`, which caused libnpmpublish to silently discard the
supplied `--provenance-file` bundle and publish its own generated
provenance instead.

Skip auto-enabling provenance when a provenance file is configured, so the
explicitly supplied bundle is verified and published as documented.

Fixes: npm#9879
Signed-off-by: Yunseo Kim <git@yunseo.kim>
@yunseo-kim
fix(libnpmpublish): throw when provenance and provenanceFile are both…
66710dc 
… set

The two options are documented as mutually exclusive, but buildMetadata
previously preferred generation when both were set, silently discarding
the supplied bundle. Throw an EPROVENANCECONFLICT usage error instead of
silently discarding a caller-supplied cryptographically meaningful
artifact.

Signed-off-by: Yunseo Kim <git@yunseo.kim>
@yunseo-kim
docs: document provenance-file precedence over automatic provenance
df4e300 
Signed-off-by: Yunseo Kim <git@yunseo.kim>
@yunseo-kim
yunseo-kim requested review from a team as code owners August 16, 2026 10:32
This was referenced Aug 16, 2026
Open
Merged
yunseo-kim added a commit to windlasstech/slsa-builder that referenced this pull request Aug 16, 2026
@yunseo-kim
docs(decisions): Add ADR 0082 pinning the publish-stage npm CLI versi…
b98dfaf 
…on (#99)

## Summary

Adds ADR 0082, deciding that the **publish-stage** npm CLI is
provisioned as an explicitly pinned, integrity-verified distribution
selected from a reviewed allowlist, replacing the current floating
node-24-bundled npm **on the publish job only**. Build-stage npm
selection is unchanged and remains governed by ADR 0016/0017/0027.

- What changed?
- New ADR 0082: pin the publish-stage npm CLI version with
integrity-verified provisioning (version + distribution SHA-256 in the
publish job), a reviewed allowlist recorded in the specification, and a
bump procedure requiring an npm-internals diff-review checklist
(`oidc.js` flow, `buildMetadata()` branch structure, `@npmcli/config`
exclusive enforcement, `publishConfig` flatten timing, sigstore-js
major) plus per-version compatibility fixtures.
- Publish-side contract change (implementation lands with the #97 fix):
the publish job's npm validates against the allowlist pin, replacing the
current exact-equality check against the build-time
`runtime.npm_version` record, since build npm (toolchain, floating) and
publish npm (pinned) diverge by design. Build continues recording its
actual npm in `runtime.npm_version`.
- Relations updates (frontmatter only; accepted ADR bodies untouched):
`partially-supersedes ADR-0027` scoped to the publish-stage npm
provisioning clause (reverse edge added), `see-also` ADR
0016/0017/0029/0081 (reverse edges added).
- Index rows for 0082 in `docs/decisions/README.md` and `README.ko.md`.
- Why is this needed?
- The #97 remediation mechanism depends on npm internals that already
vary across the currently allowed range (11.5.1 / 11.17.0 / 12.0.2:
`oidc.js` structure, exclusive env carve-out, sigstore-js v3–v5). The
fourth dogfood showed fail-closed read-back protects acceptance but not
registry state (`@windlass/vers-js@0.1.2` is permanently burned). A
floating toolchain under a source-dependent publish mechanism leaves the
verification surface open, and a node-24 image update is a
correlated-failure vector for all callers. The decision completes the
builder-owned npm principle (ADR 0016/0017) and ADR 0027's recorded
pinned-setup intent on the publish path, and makes the specification's
per-version fixture obligation satisfiable. Upstream npm/cli#9882
adoption becomes a deliberate pinned bump.
- Scope note (from review): an earlier draft pinned npm across all jobs;
per reviewer decision the pin is publish-only, and a manifest-selected
build npm option was considered and excluded from this ADR (build-stage
selection may be revisited by a separate future decision).
- How to test: documentation-only; `adr-relations-check` passes (83
ADRs, 184 edges, NO PROBLEMS); prettier and markdownlint clean.

## Related Issues

- Related #97 (motivating defect), #30 (dogfood evidence)

## Change Type

- [x] Documentation

## Changelog

- Category: None
- User-facing note: Architecture decision record; no runtime behavior
change in this PR (spec amendments and provisioning changes follow in
the #97 fix implementation).

Changelog update:

- [x] Not needed because this change is not user-facing

## Checklist

### General

- [x] PR title follows [Conventional
Commits](https://www.conventionalcommits.org/) format: `type(scope):
Summary`
- [x] This PR does not expose backend/internal implementation details in
a public repo.
- [x] No secrets, tokens, keys, or private endpoints are included.
- [x] Changes stay within this repository's intended scope.

### Protocol / Compatibility Impact

- [x] No protocol/spec impact in this PR; ADR 0082's consequences (spec
allowlist, publish-side pin validation, provisioning) land in the
follow-up implementation PR.

## Testing

- [x] Lint and format pass
- [x] Manual verification performed

Describe test evidence: `adr-relations-check` reports 83 ADRs / 184
edges / NO PROBLEMS; prettier and markdownlint clean on
`docs/decisions/`; verified via git diff that existing ADRs changed only
in frontmatter `relations` (bodies immutable per convention).

## Documentation

- [x] Spec/docs updated (this PR is the ADR; index READMEs updated in
both languages)
- [x] Changelog decision completed above

## Rollout / Risk

- Risk level: Low (documentation only)
- Rollback plan: revert this branch.

## Reviewer Checklist

- [x] Scope is clear and minimal
- [x] Security and boundary checks passed
- [x] Tests and docs are sufficient
- [x] Compatibility impact is correctly handled

---------

Signed-off-by: Yunseo Kim <git@yunseo.kim>
@yunseo-kim yunseo-kim mentioned this pull request Aug 17, 2026
Merged
30 tasks
yunseo-kim added a commit to windlasstech/slsa-builder that referenced this pull request Aug 17, 2026
@yunseo-kim
docs(decisions): Add ADR 0083-0085 pinning the npm CLI toolchain acro…
9a140e6 
…ss build and publish stages (#100)

## Summary

- What changed: three architecture decision records completing the npm
CLI version-pinning decision chain for the JS/TS npm package profile:
- **ADR 0083** — defer the npm M1 publish remediation (#97) to the
upstream npm/cli#9882 fix; the first reviewed npm release containing the
fix becomes the initial publish npm pin, with an explicit revisit
trigger (unmerged close, materially different precedence semantics, or
no merge by 12026-10-01).
- **ADR 0084** — select the publish-stage npm provisioning mechanism:
download the exact recorded registry tarball URL, verify the committed
SHA-512 digest (SRI form, byte-identical to the registry-native
`dist.integrity`) over the compressed bytes before extraction, and
execute via a basename-`npm` launcher. Partially supersedes ADR 0082's
SHA-256 designation; amends its open mechanism choice.
- **ADR 0085** — pin the build-stage toolchain pair: the reusable
workflow resolves Node.js to an exact 24 patch recorded together with
the npm version that patch bundles, and each job that uses the bundled
npm asserts `node --version`/`npm --version` equality with the recorded
pair before the first npm invocation. Partially supersedes ADR 0027's
floating toolchain resolution clause only.
- Why is this needed: the fourth M1 dogfood (#97) showed a floating npm
on the publish path can publish-then-reject and permanently burn version
numbers. ADR 0082 pinned the publish npm and deliberately left two
questions open — what the #97 fix is (settled by 0083), which mechanism
provisions the pin (settled by 0084) — plus the build-stage version
selection review (settled by 0085 with an escalation trigger toward an
independently provisioned build npm).
- How to test: documentation-only change. Validation evidence below.

## Related Issues

- Related #97 — this PR records the remediation *decisions*;
implementation lands when the fixed npm release ships (ADR 0083 deferral
scope), so #97 stays open.
- Related #30 — dogfood attempt evidence referenced by the ADRs.

## Change Type

- [ ] Bug fix
- [ ] Feature
- [ ] Refactor
- [x] Documentation
- [ ] Test/CI
- [ ] Breaking change
- [ ] Other: <!-- describe -->

## Changelog

- Category: None
- User-facing note: Architecture decision records only — no user-facing
behavior changes. The implementing specification, workflow, and Go
changes land later under ADR 0083's deferral scope and ADR 0085's own
schedule.

Changelog update:

- [ ] `CHANGELOG.md` `[Unreleased]` updated
- [x] Not needed because this change is not user-facing

## Checklist

### General

- [x] PR title follows [Conventional
Commits](https://www.conventionalcommits.org/) format: `type(scope):
Summary`
- [x] This PR does not expose backend/internal implementation details in
a public repo.
- [x] No secrets, tokens, keys, or private endpoints are included.
- [x] Changes stay within this repository's intended scope.

### Protocol / Compatibility Impact

- [x] No protocol/spec impact
- [ ] Protocol/spec updated
- [ ] Conformance tests updated
- [ ] Breaking change is versioned and migration notes are included

ADR texts only; no specification files are modified. Each ADR records
which future specification amendments it governs.

## Testing

- [ ] Unit tests added/updated
- [ ] Integration or conformance tests added/updated
- [ ] Tests pass
- [x] Lint and format pass
- [ ] Type check passes
- [x] Manual verification performed

Describe test evidence:

- `prettier --write` on all touched files: clean.
- `markdownlint-cli2 docs/decisions/**/*.md`: 89 files, 0 errors.
- `python3
.agents/skills/adr-relations-check/scripts/check_relations.py`: **86
ADRs, 216 edges — NO PROBLEMS** (forward + reverse relation edges
symmetric, status grammar valid).
- Lefthook pre-commit (prettier, markdownlint) and commit-msg
(dco-signoff) passed on all three commits.

## Documentation

- [x] README updated — `docs/decisions/README.md` and `README.ko.md`
sequence, inventory, and traceability tables (bilingual, same change).
- [x] Spec/docs updated — three new ADRs; reverse relation edges added
to ADR 0016/0017/0027/0067/0082/0083/0084; `docs/decisions/AGENTS.md`
index updated.
- [x] Changelog decision completed above

## Rollout / Risk

- Risk level: Low — documentation-only; no runtime, workflow, or
specification behavior changes.
- Rollback plan: revert the merge commit; ADR 0065's immutability rule
applies after acceptance, so any post-merge decision change would be a
new ADR instead.

## Reviewer Checklist

- [x] Scope is clear and minimal
- [x] Security and boundary checks passed
- [x] Tests and docs are sufficient
- [x] Compatibility impact is correctly handled

---------

Signed-off-by: Yunseo Kim <git@yunseo.kim>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Trusted publishing auto-provenance silently discards the --provenance-file bundle

1 participant

@yunseo-kim