Skip to content

Plumb an identity token through skill push for keyless signing - #6385

Merged
samuv merged 1 commit into
mainfrom
skills-keyless/02-push-token-plumbing
Aug 20, 2026
Merged

samuv merged 1 commit into
mainfrom
skills-keyless/02-push-token-plumbing

Conversation

@samuv

@samuv samuv commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Stacked on #6383. Per the #6307 (keyless push signing) design, the CLI
acquires the OIDC identity token and forwards it in the push request; the
server exchanges it with Fulcio for a certificate, signs, and records a
Rekor entry (toolhive-core container/signer, already wired in #6383). This
PR is the server-side/API half of that plumbing — the token forward and
validation, not yet the CLI flag or acquisition ladder (that's the next PR
in the stack).

  • Add PushOptions.IdentityToken, threaded through the API DTO
    (pushSkillRequest.identity_token) and the Go HTTP client's pushRequest
    DTO, so a value set on PushOptions actually reaches the server instead of
    silently stopping at either DTO boundary.
  • skillsvc.Push now requires exactly one of Key, IdentityToken, or
    NoSign — previously it only checked "key or no_sign"; combining two
    credentials, or a credential with no_sign, is now a 400 instead of
    silently picking one.
  • TOOLHIVE_SIGSTORE_FULCIO_URL / TOOLHIVE_SIGSTORE_REKOR_URL env var
    overrides, forwarded into core's signer.Options, for E2E/staging use
    only (documented as such in the comment — not a production knob).

No CLI changes yet: PushOptions.IdentityToken has no way to be set from
thv skill push until the next PR in the stack adds --identity-token and
the token-acquisition ladder.

Type of change

  • New feature

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)

task test passes for pkg/skills/... and pkg/api/.... One unrelated
failure appeared in pkg/transport/proxy/streamable from a stray leftover
process holding a hardcoded test port in this local environment (same
pre-existing flake noted in #6383, untouched package).

Added round-trip tests for identity_token across both DTO boundaries (API
request → PushOptions, and PushOptions → HTTP client body) — the DTO
trap this repo has hit before, called out explicitly per team convention.

API Compatibility

  • This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Changes

File Change
pkg/skills/options.go Add PushOptions.IdentityToken
pkg/api/v1/skills_types.go, skills.go Add identity_token to the push request DTO and forward it
pkg/skills/client/dto.go, client.go Mirror identity_token in the Go client's request DTO
pkg/skills/skillsvc/build.go New validateSigningInputs (exactly one of key/token/no_sign); pass IdentityToken and Fulcio/Rekor URL overrides into signer.Options
pkg/skills/skillsvc/build_verify_test.go Table-driven validation matrix; new keyless-signing test with env overrides
pkg/api/v1/skills_test.go, pkg/skills/client/client_test.go Round-trip tests for identity_token
docs/server/* Regenerated via task docs

Does this introduce a user-facing change?

No — IdentityToken has no way to be set yet (no CLI flag), so this changes
no observable behavior for existing callers except tightening the push
validation error (combining --key with no_sign is now rejected instead
of silently ignoring --key).

Special notes for reviewers

Base branch is skills-keyless/01-core-signer (#6383), not main — this PR
only makes sense stacked on top of it.

@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Aug 19, 2026
@samuv samuv self-assigned this Aug 19, 2026
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.05%. Comparing base (0b8acf2) to head (bb7f62b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6385      +/-   ##
==========================================
+ Coverage   73.04%   73.05%   +0.01%     
==========================================
  Files         745      745              
  Lines       79208    79237      +29     
==========================================
+ Hits        57857    57890      +33     
+ Misses      17300    17297       -3     
+ Partials     4051     4050       -1     

☔ 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/01-core-signer to main August 19, 2026 15:20
@samuv
samuv force-pushed the skills-keyless/02-push-token-plumbing branch from 5a3e9db to d36d2b1 Compare August 19, 2026 15:21
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Aug 19, 2026
Add PushOptions.IdentityToken end to end (API DTO, HTTP client, and
service validation) so the CLI can forward a caller-acquired OIDC
token for keyless signing (#6307), while keeping cosign key-pair
signing and --no-sign as the other two mutually exclusive choices.

Pushing now requires exactly one of key, identity_token, or no_sign;
Fulcio/Rekor can be overridden via TOOLHIVE_SIGSTORE_FULCIO_URL /
TOOLHIVE_SIGSTORE_REKOR_URL for E2E and staging use.

Signed-off-by: Samuele Verzi <samu@stacklok.com>
@samuv
samuv force-pushed the skills-keyless/02-push-token-plumbing branch from d36d2b1 to bb7f62b Compare August 19, 2026 15:49
@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Aug 19, 2026
@samuv
samuv merged commit d278b33 into main Aug 20, 2026
48 checks passed
@samuv
samuv deleted the skills-keyless/02-push-token-plumbing branch August 20, 2026 13:36
@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/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants