Skip to content

Enable keyless signing for CI skill pushes - #6402

Merged
samuv merged 5 commits into
mainfrom
skills-keyless/04-ci-and-staging-e2e
Aug 21, 2026
Merged

samuv merged 5 commits into
mainfrom
skills-keyless/04-ci-and-staging-e2e

Conversation

@samuv

@samuv samuv commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Final PR of the keyless skill-signing stack (#6383#6385#6390 → this).

  • Release pushes have been publishing unsigned artifacts. skills-build-and-publish.yml carried a --no-sign stopgap with a TODO(#6307), added while the signing path was still being built. The signer, the server-side token plumbing, and the CLI credential ladder all exist now, so the stopgap is removed: a bare thv skill push picks up the job's ambient OIDC token and signs against Fulcio and Rekor. This requires id-token: write on both the reusable workflow's job and on releaser.yml's calling job — GitHub caps a reusable workflow's permissions at what its caller declares, so granting it in one place only would silently yield no token.
  • Nothing proved the signature is one the wider ecosystem accepts — only that ToolHive's own verifier reads back what ToolHive wrote. New skills-keyless-signing-e2e.yml signs a throwaway skill against Sigstore's staging Fulcio and Rekor, pushes it to a local registry:2 service container, and verifies it with stock cosign. A signature only ToolHive can read fails the job.
  • Non-blocking by design. Staging carries no SLO guarantee and re-signs its TUF metadata every few days. continue-on-error sits on the called workflow's own job, because GitHub rejects that key on a job that uses uses:.
  • Docs: records the push-then-sign ordering caveat (a signing failure after a successful upload leaves the artifact live and unsigned — pre-existing, not changed here), how CI signs release pushes, and what the staging job covers.

Closes #6307

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

No Go code changes in this PR, so there is nothing new for the Go suites to cover. What was actually run:

  • actionlint on all five workflows — clean. It caught one real error during development: continue-on-error is not permitted on a job that calls a reusable workflow, which is why it now lives on the inner job.
  • zizmor 1.29.0 --persona regular --min-severity medium (the exact gate from security-scan.yml) over .github/ — no findings.
  • codespell with the repo's .codespellrc over every changed file — clean.
  • task test: one failure, TestMCPGoClientInitializeAndPing in pkg/transport/proxy/streamable, from a stray local process holding port 8096. Untouched package, unrelated to this PR.
  • task lint-fix: fails locally with the nilness/sentry panic that 39a85c8 pinned CI away from — my local golangci-lint is v2.13.0, CI pins v2.12.2. No Go files change here.

Changes

File Change
.github/workflows/skills-build-and-publish.yml Drop --no-sign from both pushes; add id-token: write
.github/workflows/releaser.yml Add id-token: write to the only job that pushes with push: true
.github/workflows/skills-keyless-signing-e2e.yml New: sign against staging, verify with stock cosign
.github/workflows/run-on-pr.yml Wire in the new job; skip on fork PRs, which cannot get id-token: write
.github/workflows/run-on-main.yml Wire in the new job
docs/arch/12-skills-system.md Push-then-sign caveat, CI signing note, staging job; fix a stale trust-model line that predated keyless push

Does this introduce a user-facing change?

Yes. Skills published by the release pipeline are now Sigstore-signed rather than unsigned, so project-scoped installs of them no longer need --allow-unsigned.

Special notes for reviewers

This PR is stacked on #6390 and must not merge before it. The dependency is semantic, not textual: on main today there is no --identity-token flag and no acquisition ladder, so a bare thv skill push is rejected with a 400 signing credential required. Dropping --no-sign only works once #6390 lands. GitHub will retarget this to main automatically when that happens.

Two implementation details that are non-obvious and were found by reading the dependency's source rather than by guessing:

  • TOOLHIVE_DEV=true on the thv serve process is mandatory for the local registry. newOCIRegistryClient (pkg/api/server.go) only uses plain HTTP for loopback registries in dev mode; without it the oras push attempts TLS against the no-TLS container and dies in the handshake before signing is reached. The signature manifest is attached by go-containerregistry, which independently picks HTTP for a localhost:<port> host — which is why the reference is localhost:5000/... and not a bare localhost host, where the two halves would disagree on scheme.
  • --new-bundle-format=false on cosign verify is required. cosign v3 defaults it to true (OCI 1.1 referrers bundle), but toolhive-core attaches the classic cosign signature manifest — a sha256-<hex>.sig tag whose layer annotations carry signature, certificate and Rekor entry. With the default left on, verification finds no signature at all, which would read as a signing bug rather than a format mismatch.

Two things researched from Sigstore's docs and cosign v3.0.6's source but not confirmable without a live run — expect to iterate once on CI logs:

  1. Whether staging Fulcio accepts GitHub Actions OIDC with the sigstore audience exactly as production does. Staging is documented as operated the same way, so this is likely fine.
  2. Whether cosign verify after cosign initialize --staging resolves the staging Rekor key purely from the pinned trust root, with no explicit --rekor-url. This repo has no prior staging-Sigstore usage to confirm against.

releaser.yml's change is inert until an actual release runs, so it can only be smoke-tested by inspection here.

Unrelated pre-existing issue noticed while linting, not fixed here to keep scope clean: releaser.yml's notify-release-failure references needs.extract-release-actor, a job that does not exist, so that Slack field always falls back to github.actor.

Generated with Claude Code

@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed size/M Medium PR: 300-599 lines changed and removed size/S Small PR: 100-299 lines changed labels Aug 20, 2026
@samuv samuv self-assigned this Aug 20, 2026
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.70%. Comparing base (b48013e) to head (4f2d370).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6402   +/-   ##
=======================================
  Coverage   77.69%   77.70%           
=======================================
  Files         752      756    +4     
  Lines       72717    72788   +71     
=======================================
+ Hits        56500    56561   +61     
- Misses      16212    16222   +10     
  Partials        5        5           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Base automatically changed from skills-keyless/03-cli-token-acquisition to main August 21, 2026 08:29
samuv added 2 commits August 21, 2026 10:29
Release skill pushes have been publishing unsigned artifacts behind a
`--no-sign` stopgap since the keyless signing work started. The signer,
the server-side token plumbing, and the CLI's credential ladder are all
in place now, so the stopgap can go: a bare `thv skill push` picks up the
job's ambient OIDC token and signs against Fulcio and Rekor. That needs
`id-token: write` on both the reusable workflow's job and on releaser's
calling job, since GitHub caps a reusable workflow at what its caller
grants.

Nothing so far proves the resulting signature is one the wider Sigstore
ecosystem accepts, only that ToolHive's own verifier reads it back. Add a
workflow that signs a throwaway skill against Sigstore's staging Fulcio
and Rekor, pushes it to a local registry container, and verifies it with
stock cosign. It runs on every PR and on main, non-blocking: staging has
no SLO guarantee and re-signs its TUF metadata every few days, so its
outages must not gate merges.

Also document the push-then-sign ordering, which leaves an artifact
published but unsigned if signing fails after the upload succeeds.

Signed-off-by: Samuele Verzi <samu@stacklok.com>
GitHub validates a called workflow's declared permissions against the
calling job's statically, before inputs are evaluated, so the build-only
callers must grant id-token: write even though only the release caller
ever reaches the signing code. Without it the whole PR Checks run fails
at startup rather than the job simply running unprivileged.

Signed-off-by: Samuele Verzi <samu@stacklok.com>
@samuv
samuv force-pushed the skills-keyless/04-ci-and-staging-e2e branch from 07de929 to aaa60ef Compare August 21, 2026 08:29
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Aug 21, 2026
sigstore-go tests the certificate SAN with Regexp.MatchString, which is
unanchored, so the previous pattern accepted any SAN merely containing the
expected workflow identity. Anchor it and escape the literal dots, since
proving the *correct* identity round-trips is the whole point of the job.

Also correct the architecture doc: the job skips fork PRs, which cannot be
granted the id-token permission the ambient token needs.

Signed-off-by: Samuele Verzi <samu@stacklok.com>
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Aug 21, 2026
The build runs repository code — thv serve and thv skill build — which on a
pull request is code from the pull request itself. Because GitHub validates
a called workflow's permissions against its caller's statically, a single
workflow behind a push: input forced every caller to grant the union, so PR
CI held the id-token: write that signs releases. That contradicts the rule
run-on-pr.yml already states for the test job: a job executing repository
code has no business holding release signing keys.

Publishing now lives in skills-publish.yml, called only from releaser.yml,
and pull requests and main call a build-only skills-build.yml that holds
neither packages: write nor id-token: write.

Renaming the publishing workflow is free right now and would not stay that
way: the signing certificate identity is the workflow path, and no skill has
ever been published signed, so no consumer has recorded provenance pointing
at the old name.

Signed-off-by: Samuele Verzi <samu@stacklok.com>
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Aug 21, 2026
The job needs id-token: write to mint an ambient OIDC token, and it builds
and runs repository code to use it — on a pull request, code from that pull
request. It could not impersonate the release signer, since a different
workflow path yields a different certificate SAN and lock provenance pins
the path, but the request variables in its environment let that code mint a
token for any other audience, which any external trust policy matching this
repository without pinning the ref would accept.

It was already non-blocking, so running it post-merge from run-on-main.yml
costs no merge gating. Dispatching "Main build" on a branch still exercises
it on demand.

Signed-off-by: Samuele Verzi <samu@stacklok.com>
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Aug 21, 2026
@samuv
samuv merged commit e25de16 into main Aug 21, 2026
45 checks passed
@samuv
samuv deleted the skills-keyless/04-ci-and-staging-e2e branch August 21, 2026 10:26
@github-actions github-actions Bot mentioned this pull request Aug 26, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keyless OIDC push signing for thv skill push

2 participants