Skip to content

Report key-signed artifacts as such at install - #6471

Closed
lorenzozanee wants to merge 1 commit into
stacklok:mainfrom
lorenzozanee:fix/key-signed-verification
Closed

lorenzozanee wants to merge 1 commit into
stacklok:mainfrom
lorenzozanee:fix/key-signed-verification

Conversation

@lorenzozanee

Copy link
Copy Markdown
Contributor

Summary

A skill or plugin pushed with thv skill push --key produces a validly key-signed artifact, but install-time verification is keyless-only. The install previously failed with a generic signature error that --allow-unsigned could not override. This change classifies the key-signed layout distinctly and reports an actionable refusal.

Fixes #6442

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)

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.

A skill or plugin pushed with --key produces a validly key-signed
artifact, but install verification is keyless-only. The artifact
was therefore refusing with a generic signature failure and
--allow-unsigned could not override it because the error was
ErrSignatureInvalid rather than ErrUnsigned.

Classify the layout via Bundle.HasCertificate and surface
ErrKeySigned with an actionable message stating that key-pair
signatures cannot be verified at install time, that
--allow-unsigned does not apply, and that the artifact should
be re-published with keyless signing. Add the same handling
for catalog-constrained and sync/upgrade paths and document
the limitation on thv skill push --key.

Closes #6442

Signed-off-by: lorenzozanee <wyz0707@proton.me>
@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.03%. Comparing base (0fb54d4) to head (07f2750).

Files with missing lines Patch % Lines
pkg/skills/verifier/oci.go 75.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6471   +/-   ##
=======================================
  Coverage   78.02%   78.03%           
=======================================
  Files         767      767           
  Lines       74273    74298   +25     
=======================================
+ Hits        57955    57975   +20     
- Misses      16313    16318    +5     
  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.

@samuv

samuv commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking this up. The same change landed on main in #6443 (merged 2026-08-31), which added verifier.ErrKeySigned and the actionable 403 for key-signed artifacts on both surfaces, so this PR is superseded and no longer applies cleanly.

Since then the scope of #6442 has moved on: the skills half is complete (#6444, #6447, #6478) and what remains is bringing the plugins surface to parity — install-time key verification, sync/upgrade re-verify against a pinned key, and then restoring --key on thv ai-plugin push. The sequenced plan is on the issue if you'd like to take one of those pieces.

Closing this as superseded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--key push signing has no install-time verification path (plugins)

2 participants