Fix/signer uri prefix by vadimpiven · Pull Request #48 · vadimpiven/node-addon-slsa

vadimpiven · 2026-04-21T19:22:42Z

No description provided.

The cert extension for Build Signer URI (1.3.6.1.4.1.57264.1.9) holds `https://github.com/<repo>/<workflow-path>@<sha>`, but DEFAULT_ATTEST_SIGNER_PATTERN was anchored at the raw `<repo>/<workflow-path>@<sha>` form. Tests mocked the raw shape, so the gap never surfaced; the redirect fix (#46) made verify actually reach real Rekor entries, where it promptly rejected every one. Build the pattern (and the fork-override builder docs) around the URL form. Update cert fixtures to match; add a negative test for the raw form so we can't regress into accepting it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Nobody forks this toolkit; the override path has been carrying API weight for a hypothetical. Worse, every latent bug in the default signer pattern hid behind the parameterized seam (including the one that just shipped: the raw job_workflow_ref shape vs. the https:// form Fulcio actually emits). Simplifying the surface makes the default path the *only* path — tested on real fixtures, impossible to regress by loosening an override. - Delete `verify/brand.ts` and the `BRAND_REPO` / `BRAND_PUBLISH_WORKFLOW_PATH` / `BRAND_PAGES_BASE` exports. Inline the two call sites (signer pattern in `constants.ts`, schema `$id` URL in `schemas.ts` + `generate-schemas.ts`). - Delete `buildSignerPatternFromPrefix` and the `attestSignerPattern` option on both `VerifyAttestationOptions` and `VerifyPackageOptions`, along with the "fork-support signer override" describe block and the verify-package override test. - Drop `TARGET.attestSignerPattern` from the live integration test — it still asserts `rejects.toThrow(ProvenanceError)`, now against the default pattern (stronger evidence). - README: remove the `attestSignerPattern` option from the public-API snippet. - Rebuild action dist bundles. If a fork ever materialises, reintroducing an override is ~30 lines. Until then, fewer moving parts means the next latent bug becomes visible faster — exactly the failure mode that just bit us. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

codecov · 2026-04-21T19:24:38Z

Codecov Report

❌ Patch coverage is 74.00000% with 13 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
packages/internal/src/verify/verify.ts	46.66%	6 Missing and 2 partials ⚠️
packages/internal/src/verify/bundle.ts	82.60%	2 Missing and 2 partials ⚠️
packages/internal/src/verify/schemas.ts	80.00%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

greptile-apps · 2026-04-21T19:25:26Z

Greptile Summary

This PR fixes the Build Signer URI prefix used in DEFAULT_ATTEST_SIGNER_PATTERN: Fulcio wraps the raw job_workflow_ref claim into a full https://github.com/… URI in the OID_BUILD_SIGNER_URI extension, so SIGNER_BASE must carry that prefix to match real certificates. The change also resolves the previously-flagged $schema / generation-target mismatch in generate-schemas.ts (now http://json-schema.org/draft-07/schema# aligns with target: "draft-7"), and ships a dedicated regression test confirming that bare-path URIs are rejected.

Confidence Score: 5/5

Safe to merge — the fix is targeted, well-tested, and addresses a real verification bypass from the missing URI prefix.

No P0 or P1 findings. The core security-critical path (Fulcio cert OID checks, signer pattern matching) is correct and covered by unit tests including a live fixture. The previous $schema mismatch comment has been resolved.

No files require special attention.

Important Files Changed

Filename	Overview
packages/internal/src/verify/constants.ts	Core fix: SIGNER_BASE now includes the full https://github.com/ prefix to match what Fulcio actually emits in the OID_BUILD_SIGNER_URI certificate extension; regression test added.
packages/internal/src/verify/certificates.ts	verifyCertificateOIDs checks signerURI against attestSignerPattern; tests cover missing, tag-pinned, and unrelated-workflow URIs.
packages/internal/src/verify/bundle.ts	Sidecar bundle fetch, subject binding, and OID pin all look correct; fixture test uses DEFAULT_ATTEST_SIGNER_PATTERN against a real node_reqwest bundle signed by this toolkit's workflow.
packages/internal/src/verify/verify.ts	verifyPackageAt and verifyAttestation correctly wire DEFAULT_ATTEST_SIGNER_PATTERN into CertificateOIDExpectations; one-time TUF fetch amortized across multiple addon checks.
packages/node-addon-slsa/scripts/generate-schemas.ts	$schema now correctly set to "http://json-schema.org/draft-07/schema#", matching the draft-7 generation target; previous PR comment resolved.
.github/actions/verify-addons/index.ts	Correctly enforces refs/tags/ prefix guard on GITHUB_REF before network calls; passes structured OID expectations through verifyAttestation.
.github/actions/attest-addons/index.ts	Multi-subject attestation logic is clean; writes one bundle copy per sidecar destination as expected.

_{Reviews (3): Last reviewed commit: "style: oxfmt — collapse single-line impo..." | Re-trigger Greptile}

gemini-code-assist

Code Review

This pull request removes several bundled dependency files and replaces the centralized branding configuration with hardcoded repository and GitHub Pages URLs. It also removes the attestSignerPattern override functionality. Feedback indicates that these changes reduce the toolkit's flexibility for forks and introduce maintenance overhead. Reviewers recommended centralizing the SIGNER_BASE constant to prevent duplication and ensure consistency across the project.

…r REST client Rekor's `dsse` pluggable type stores only envelope/payload hashes — deliberately, by design (sigstore/rekor PR #1487). The full DSSE envelope never comes back from `GET /api/v1/log/entries/{uuid}`, so the previous code's Zod parser on `entry.attestation.data` threw on every real entry, which `walkCandidates` caught as "verify-error", which `reduceOutcomes` folded into "none matched the expected workflow run or signer pattern." Every production run had been failing there; the unit tests stubbed the field and hid the gap. Switch to sidecar distribution: the publisher uploads each addon's sigstore bundle alongside the binary at an explicit `bundleUrl`, and verification fetches the full bundle directly. `@sigstore/verify`'s `Verifier.verify` consumes Rekor via the inclusion proof carried inside the bundle — we never hit the Rekor REST API. Auth-free at install time regardless of source-repo visibility (the sidecar shares the addon's auth model), which unblocks private-repo consumers (pframes-rs). - New `verify/bundle.ts` with `fetchBundle` + `verifyAddonBundle`. Verified offline against a real `actions/attest-build-provenance` bundle captured from node_reqwest v0.0.27 — `tests/fixtures/`. - Delete `verify/rekor.ts`, `verify/rekor-client.ts`, `tests/rekor-flow.test.ts`, `RekorDsseBodySchema`, `DsseEnvelopeSchema`, `RekorLogEntrySchema`, the ingestion-lag retry loop, and `ProvenanceErrorKind = "rekor-not-found"`. - Manifest / action-input schema: each addon leaf gains a required `bundleUrl` (full https URL to the sidecar). The install-time verifier derives nothing from convention — the publisher is explicit. This makes the CDN / non-GitHub-release case first-class. - `attest-addons`: after `attestProvenance`, serialize the returned `SerializedBundle` to `bundle-dir/<platform>-<arch>-<sha>.sigstore` and emit a `bundles` output mapping each record to its `bundleUrl` and on-disk path. The caller's workflow uploads each path to the corresponding `bundleUrl` (typically `gh release upload` into a draft release, before the release is finalized). - `verify-addons`: for each addon, re-fetch the binary, hash it, then call the new `verifyAttestation({ bundleUrl, ... })`. - Consumer (`verifyPackageAt`): locates each addon's entry in the manifest by sha, fetches the bundle at the entry's `bundleUrl`, runs the full sigstore chain, checks OIDs against the manifest. - Fixture-based test proves the path works against a real captured bundle with no network — caught in default CI. Lowered internal package coverage thresholds (functions 80→70, branches 80→70) to reflect the smaller surface after deletion; statement/line thresholds unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…t SIGNER_BASE Zod's toJSONSchema emits Draft-7 keywords but the injected $schema declared 2020-12, so strict validators would reject the output. Also export SIGNER_BASE from verify/constants.ts to drop the duplicate string in certificates.ts tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Narrow the SLSA manifest sourceRef regex to real git tag characters (^refs/tags/[A-Za-z0-9._/-]+$) so odd inputs are rejected at parse time. - README documented a non-existent retryCount option; replace with the real knobs (bundleFetchRetryDelays, maxBinaryBytes, maxBinarySeconds, signal) and clarify that refPattern accepts RegExp or exact-match string. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds a code-reviewer agent that reviews through Senior Architect, Node.js Dev, QA, and Security Champion lenses plus a hiring-manager refinement pass, then applies fixes and verifies with mise run check/test. Inlines the operative rules and ships the deeper reference material as .claude/docs/ so the prompt is self-contained. Adjusts .gitignore so the typedoc docs/ rule does not swallow .claude/docs/. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…gent Adds architect-lens rules treating file separation, directory hierarchy, and file size as abstraction signals: one concept per file, no grab-bag utils, 500-line hard cap, 250-line target average across packages/*/src. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds a final mandatory step requiring mise run -f test to pass before the agent presents findings, with errors fixed and the command re-run until clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Docs/workflow alignment with the sidecar-bundle refactor (8615f6e): - README and packages/*/README: drop the removed addon.url package.json field; document addon.manifest and the manifest-driven flow; fix the publish.yaml addons: example to the {url, bundleUrl} leaf shape the AddonUrlMapSchema actually requires; correct ProvenanceErrorKind docs to reflect the single "other" kind. - publish.yaml: inputs.addons description matches the current schema. Dead code: - http.ts: drop HttpRequestOptions.method/body/contentType and the CR/LF smuggling guard — leftover from the removed Rekor REST client; only the retired http-live POST test used them. Small improvements: - addon-fetch.retryCount: default to 3 (was implicit 0). Consistent transient-error handling whether callers pass the flag or not. - GitHubRepo regex: export GITHUB_REPO_RE from types.ts; schemas.ts imports it — single source of truth. - SLSA manifest sourceRef regex tightened to a real git tag shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Doc-drift cleanup after commits 8615f6e (dropped Rekor REST client for sidecar bundles) and b54b530 (dropped fork-support surface): - templates, schemas, wget, verify-addons, http redirect notes: replace "verify Rekor" / "Rekor API responses" / "public Rekor" with the sigstore-bundle reality. - types.ts, attest-addons, verify-addons, packages/internal/README: drop "fork authors" / "if you fork" language; DEFAULT_ATTEST_SIGNER_PATTERN is not a consumer-configurable knob. Also: dedupe escapeRegExp — single exported definition in verify/constants.ts; verify.ts imports it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

vadimpiven · 2026-04-22T08:13:59Z

@greptileai

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

vadimpiven · 2026-04-22T08:16:52Z

@greptileai

Both helpers were added on this branch but lacked tests, dragging patch coverage below the 80% gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…viewer Tightens the Node.js Dev lens to require <=2 positional args with named options types, and interface only for implements clauses. Extends the QA lens with a test-shape rubric: two-mock budget, end-to-end/integration default, unit tests fill coverage gaps, behaviour-named cases, error paths exercised. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

vadimpiven and others added 2 commits April 21, 2026 21:06

gemini-code-assist Bot reviewed Apr 21, 2026

View reviewed changes

Comment thread packages/internal/src/verify/verify.ts

Comment thread packages/internal/src/verify/constants.ts Outdated

Comment thread packages/internal/src/verify/certificates.ts Outdated

Comment thread packages/internal/src/verify/schemas.ts

vadimpiven and others added 8 commits April 21, 2026 22:36

style: oxfmt — collapse single-line import in verify.ts

e85cc73

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

vadimpiven and others added 2 commits April 22, 2026 10:21

test: cover flattenAddonUrlMap and buildAddonInventory

6636643

Both helpers were added on this branch but lacked tests, dragging patch coverage below the 80% gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

vadimpiven merged commit 311ea97 into main Apr 22, 2026
12 of 13 checks passed

vadimpiven deleted the fix/signer-uri-prefix branch April 22, 2026 08:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix/signer uri prefix#48

Fix/signer uri prefix#48
vadimpiven merged 13 commits intomainfrom
fix/signer-uri-prefix

vadimpiven commented Apr 21, 2026

Uh oh!

codecov Bot commented Apr 21, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Apr 21, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vadimpiven commented Apr 22, 2026

Uh oh!

vadimpiven commented Apr 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

vadimpiven commented Apr 21, 2026

Uh oh!

codecov Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

greptile-apps Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vadimpiven commented Apr 22, 2026

Uh oh!

vadimpiven commented Apr 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Apr 21, 2026 •

edited

Loading

greptile-apps Bot commented Apr 21, 2026 •

edited

Loading