WS06 cosign signing

[brokenbot]

WS06 — Cosign keyless + key-based signature verification

Phase: Adapter v2 · Track: Distribution · Owner: Workstream executor · Depends on: WS04, WS05. · Unblocks: WS07, WS08. · Base branch: adapter-v2

Context

README.md D16–D18: signatures verified by default via cosign keyless (sigstore OIDC). Explicit cosign keys supported. criteria adapter pull --allow-unsigned and a workflow-level verification = "off" | "warn" | "strict" setting (default strict in production, permissive in dev). The lockfile (WS07) records the signer identity.

Prerequisites

• WS04 (OCI cache) + WS05 (manifest parser) merged.
• github.com/sigstore/sigstore-go and github.com/sigstore/cosign/v2 Go modules — add to go.mod. Both are pure Go.

In scope

Step 1 — Verification interface

internal/adapter/signing/verify.go:

type VerificationMode string

const (
    ModeOff    VerificationMode = "off"
    ModeWarn   VerificationMode = "warn"
    ModeStrict VerificationMode = "strict"
)

type SignerIdentity struct {
    Keyless *KeylessIdentity `json:"keyless,omitempty"`
    Key     *KeyIdentity     `json:"key,omitempty"`
}

type KeylessIdentity struct {
    Issuer  string `json:"issuer"`   // OIDC issuer URL
    Subject string `json:"subject"`  // e.g., "https://github.com/org/repo/.github/workflows/publish.yml@refs/tags/v1.2.3"
}

type KeyIdentity struct {
    Algorithm string `json:"algorithm"` // "ed25519" | "ecdsa-p256" | ...
    Fingerprint string `json:"fingerprint"` // SHA-256 of public key DER
}

type Policy struct {
    Mode           VerificationMode
    TrustedIssuers []string  // OIDC issuers accepted for keyless (e.g., "https://token.actions.githubusercontent.com")
    SubjectPatterns []string // glob patterns the subject must match
    TrustedKeys    []KeyIdentity
}

// Verify checks the cosign signature attached as an OCI referrer to the
// adapter artifact at `manifestDigest`. Returns the signer identity that
// produced the signature, or an error if no signature satisfies the policy.
//
// In ModeOff:    skips verification, returns nil identity, nil error.
// In ModeWarn:   logs failures but returns nil error and a nil identity.
// In ModeStrict: returns an error on any failure.
func Verify(ctx context.Context, layout *oci.Layout, manifestDigest digest.Digest, policy Policy) (*SignerIdentity, error)

Step 2 — Cosign keyless verification

Implementation reads the cosign signature blob (attached via OCI referrers per the standard .sig tag convention or v1.1 referrers API). Walks the Rekor inclusion proof. Validates the SCT in the certificate. Extracts issuer + subject from the cert SAN. Matches against policy.TrustedIssuers and policy.SubjectPatterns.

Use sigstore-go's Verify() with the trusted-root from the bundled TUF metadata. Cache the TUF root at ~/.criteria/cache/sigstore/.

Step 3 — Explicit-key verification

When policy.TrustedKeys is non-empty, look for a non-keyless signature first (cosign's --key flow). Match the public key against the trusted set by fingerprint. Validate the signature.

Step 4 — Policy resolution from environment / CLI flags

internal/adapter/signing/policy.go:

// PolicyFor resolves the effective Policy for a pull operation, combining:
//   - global config at ~/.criteria/config.hcl (trusted_issuers, etc.)
//   - workflow-level "verification" setting (off|warn|strict)
//   - --allow-unsigned CLI flag (forces ModeOff for this invocation only)
func PolicyFor(ctx PullContext) (Policy, error)

Default policy when no config is provided: ModeStrict, TrustedIssuers=["https://token.actions.githubusercontent.com", "https://accounts.google.com", "https://gitlab.com"], SubjectPatterns=["*"], no trusted keys.

PullContext carries the workflow's verification setting (parsed from HCL by WS09), CLI flag state, and the global config.

Step 5 — Lockfile entry construction helper

internal/adapter/signing/lockfile.go:

// LockfileFields returns the signer-identity fields to record in a
// lockfile entry. Used by WS07's lockfile writer.
func LockfileFields(id *SignerIdentity) map[string]any

Defers actual lockfile writing to WS07, which owns the file format.

Step 6 — Tests

• verify_test.go — fixture artifacts signed with a test keyless identity (using sigstore staging instance for offline reproducibility) + key-based artifacts signed with an ed25519 testkey. Table-driven over policies + identities.
• policy_test.go — covers every combination of global/workflow/CLI input.
• integration_test.go — pulls a real cosigned artifact from ghcr.io/criteria-test/signed-fixture:1.0.0 (published as part of CI setup) and verifies it.

Out of scope

• Lockfile read/write — WS07.
• CLI flags — WS08.
• Workflow HCL parsing of verification setting — WS09.
• Publishing/signing during build — WS28.

Reuse pointers

• sigstore-go for keyless verification.
• cosign/v2/pkg/cosign for signature manipulation helpers.
• TUF root at ~/.criteria/cache/sigstore/ — fetched lazily; vendored as a fallback for air-gapped use (documented limitation: vendored root may be stale; warning emitted).

Behavior change

No for now (no caller wired yet). WS08 turns on enforcement.

Tests required

• All signing/*_test.go pass.
• Integration test against a real signed fixture passes.

Exit criteria

• internal/adapter/signing/ package compiles and tests pass.
• A documented CI fixture artifact exists at a stable ref and is signed at every CI run.
  Deferred: fixture publishing is not yet set up in CI; integration_test.go contains a skipped placeholder (TestIntegration_KeylessFixture) that documents the expected stable ref ghcr.io/criteria-test/signed-fixture:1.0.0. The keyless integration path was validated indirectly via unit tests with certificate.SummarizeCertificate and a self-signed test certificate.

Files this workstream may modify

• internal/adapter/signing/*.go (all new)
• go.mod, go.sum adding sigstore-go and cosign/v2.
• Test fixtures under internal/adapter/signing/testdata/.

Reviewer notes

• Step 1 — verify.go defines VerificationMode, SignerIdentity, KeylessIdentity, KeyIdentity, Policy, and Verify().
• Step 2 — Keyless verification implemented via sigstore-go (verify.NewVerifier + bundle.NewBundle) for the sigstore-bundle path, and verify.VerifyLeafCertificate for the legacy certificate-only path. TUF root cached at ~/.criteria/cache/sigstore/.
• Step 3 — Explicit-key verification matches trusted keys by fingerprint and validates the Ed25519/ECDSA/RSA signature using sigstore/pkg/signature.LoadVerifier.
• Step 4 — policy.go implements PolicyFor with PullContext. Defaults are ModeStrict, TrustedIssuers from DefaultTrustedIssuers, SubjectPatterns=["*"]. Global HCL config parsing is TODO-deferred until WS08/WS09 provide config schema stability.
• Step 5 — lockfile.go provides LockfileFields, deferring file format to WS07.
• Step 6 — Tests:
  • verify_test.go: 12 table-driven tests covering ModeOff/Strict/Warn, findSignatures (OCI referrer + embedded layer), identityFromCert (issuer + subject + glob), verifyKeyBased (correct + wrong key), fingerprintBytes, matchGlob, LockfileFields.
  • policy_test.go: 7 table-driven tests covering defaults, --allow-unsigned, workflow modes, case insensitivity, and invalid mode errors.
  • integration_test.go: TestIntegration_KeyBased performs an end-to-end OCI layout + Ed25519 key-based verification. TestIntegration_KeylessFixture is skipped pending CI fixture publishing.
• Security checks: No secrets committed. trustedMaterial fetches live TUF root over HTTPS; cache directory has 0o750 permissions. RawKey on KeyIdentity is tagged json:"-" to avoid accidental serialization of public key bytes.
• Test-only override: trustedMaterialOverride package variable allows integration tests to inject a mock Sigstore trusted root without changing the public API. This is a clean testing seam and does not affect production behavior.
• Import boundaries: make lint-imports passes. The signing package does not import from internal/cli/ or workflow/.
• No behavior change: No callers are wired yet; WS08 will integrate Verify into the adapter pull flow.

Files this workstream may NOT edit

• internal/adapter/oci/ — owned by WS04.
• internal/adapter/manifest/ — owned by WS05.
• workflow/ — owned by WS09.
• internal/cli/ — owned by WS08.

Reviewer Notes

Review 2026-05-28 — changes-requested

Summary

The WS06 signing package is well-structured and mostly complete, with solid test coverage for key-based verification, policy resolution, and identity extraction. However, one security-critical finding and one spec deviation require executor remediation before approval. CI gates all pass (make build, make test, make lint-go, make lint-imports, make lint-no-todos, make lint-baseline-check, make validate).

Plan Adherence

• Step 1 (Verification interface) — ✅ Implemented. All types and Verify() match the spec signature. KeyIdentity adds RawKey field with json:"-" (correct security practice). IsKeyless() convenience method added (not in spec but reasonable).
• Step 2 (Keyless verification) — ⚠️ Bundle path correctly uses sigstore-go Verify(). Legacy (certificate-only) path has a security gap — see Required Remediations below.
• Step 3 (Explicit-key verification) — ✅ Fingerprint matching and Ed25519/RSA/ECDSA signature verification via sigstore.LoadVerifier are correct.
• Step 4 (Policy resolution) — ⚠️ PolicyFor implemented with correct defaults and case-insensitive mode parsing. Global HCL config parsing correctly deferred. Missing ModeWarn logging — see Required Remediations below.
• Step 5 (Lockfile helper) — ✅ LockfileFields correctly maps keyless/key identity fields.
• Step 6 (Tests) — ✅ 13 test functions across 3 files. Key-based E2E, ModeOff/Strict/Warn, findSignatures, identityFromCert, matchGlob, fingerprintBytes, LockfileFields, PolicyFor. Keyless integration test properly skipped with documentation.

Required Remediations

1. [Security] verifyKeylessLegacy does not verify the signature against the certificate's public key — Severity: blocker

   • File: internal/adapter/signing/verify.go, verifyKeylessLegacy() (line ~351)
   • Problem: After verifying the certificate chain with verify.VerifyLeafCertificate(), the function immediately calls identityFromCert() without checking that the certificate's public key actually produced the signature in rec.signatureB64. An attacker could attach any valid Fulcio-issued certificate to a signature they did not produce and the legacy path would accept it. The bundle path (verifyKeylessBundle) correctly delegates to sigstore-go's Verify() which checks everything, but the legacy path does not.
   • Acceptance criteria: After VerifyLeafCertificate succeeds, extract the public key from cert and verify that the base64-decoded rec.signatureB64 is a valid signature over rec.payload using that public key. Add a unit test that demonstrates a wrong-signature rejection in the legacy path (self-signed cert + wrong key signature → error). Alternatively, if the legacy path is not expected to be used in practice, document this limitation and consider removing it or gating it behind an explicit opt-in.

2. [Spec deviation] ModeWarn does not log failures — Severity: blocker

   • File: internal/adapter/signing/verify.go, handlePolicyMode() (line ~113)
   • Problem: The workstream spec states ModeWarn: logs failures but returns nil error and a nil identity. The implementation silently discards errors in handlePolicyMode(ModeWarn, nil, err) — there is no slog or log call anywhere in the package. Per AGENTS.md convention ("Keep logs structured — slog JSON style in entrypoints"), warnings should be emitted using slog.
   • Acceptance criteria: Add slog.Warn (or slog.Info at log level warn) calls in handlePolicyMode for the ModeWarn case, including the error message. Add a test that verifies the warning is emitted (e.g., capture slog output with slogtest or a test handler).

Test Intent Assessment

• Strong areas: Key-based verification (TestVerifyKeyBased, TestVerifyKeyBased_WrongKey) correctly asserts that valid keys pass and wrong keys fail. Policy resolution tests cover all mode combinations and edge cases. findSignatures tests validate both OCI referrer and embedded layer discovery. identityFromCert tests validate trusted/untrusted issuers and subject pattern matching.
• Weak areas: verifyKeylessLegacy has no unit test (requires a real Fulcio chain). handlePolicyMode is only tested indirectly via TestVerify_ModeWarn_NoSignatures. matchGlob tests don't cover mid-string wildcards like prefix*suffix — though the current implementation doesn't support them, this should be documented or the function renamed. ModeWarn behavior is tested only for the "no signatures" case, not for "signature found but verification failed."
• Missing scenario: No test verifies that ModeWarn actually logs warning messages. The current test only checks that (nil, nil) is returned, not that a warning was emitted.

Architecture Review Required

None.

Remediations Applied (2026-05-28)

1. Security blocker — verifyKeylessLegacy signature verification:

   • Added verifySignatureWithCert helper in verify.go that extracts the public key from the certificate and verifies the base64-decoded signature over the payload using sigsignature.LoadVerifier.
   • verifyKeylessLegacy now calls verifySignatureWithCert after VerifyLeafCertificate succeeds and before identityFromCert.
   • Added TestVerifyKeylessLegacy_WrongSignature in verify_test.go: creates a self-signed Ed25519 cert, mocks trustedMaterialOverride with a FulcioCertificateAuthority using the cert as root, and asserts that a wrong-key signature is rejected while a correct-key signature is accepted.

2. Spec deviation blocker — ModeWarn logging:

   • Added log/slog import to verify.go.
   • handlePolicyMode now emits slog.Warn("signature verification warning", "mode", mode, "error", err) when mode == ModeWarn and err != nil.
   • Updated TestVerify_ModeWarn_NoSignatures to capture slog output via slog.NewTextHandler and asserts the warning message contains "signature verification warning".

Validation Performed

• make build — PASS
• make test (with -race) — PASS (all packages including internal/adapter/signing)
• make lint-go — PASS (golangci-lint clean)
• make lint-imports — PASS
• make lint-no-todos — PASS
• make lint-baseline-check — PASS (23/23)
• make validate — PASS
• Manual code review of all internal/adapter/signing/*.go files
• Verified import boundaries: signing package does not import internal/cli/ or workflow/

Review 2026-05-28-02 — approved

Summary

Both blockers from the previous review have been resolved. verifyKeylessLegacy now verifies the signature against the certificate's public key via the new verifySignatureWithCert helper. handlePolicyMode now emits structured slog.Warn for ModeWarn failures. All CI gates pass.

Plan Adherence

• Step 1 (Verification interface) — ✅ Complete. Types and Verify() match spec.
• Step 2 (Keyless verification) — ✅ Complete. Bundle path uses sigstore-go. Legacy path now verifies signature against cert's public key via verifySignatureWithCert.
• Step 3 (Explicit-key verification) — ✅ Complete. Fingerprint matching and signature verification correct.
• Step 4 (Policy resolution) — ✅ Complete. ModeWarn now logs via slog.Warn. PolicyFor defaults correct.
• Step 5 (Lockfile helper) — ✅ Complete.
• Step 6 (Tests) — ✅ 15 test functions. New TestVerifyKeylessLegacy_WrongSignature covers good/wrong signature in legacy path. TestVerify_ModeWarn_NoSignatures now asserts slog warning output. Keyless integration test properly skipped.

Previous Blocker Remediations Verified

1. verifyKeylessLegacy signature verification — ✅ Fixed. verifySignatureWithCert extracts cert.PublicKey, loads verifier via sigsignature.LoadVerifier, verifies signatureB64 over payload. Test TestVerifyKeylessLegacy_WrongSignature creates self-signed cert with Fulcio issuer extension, mocks trustedMaterialOverride, and asserts good signature passes while wrong-key signature fails.
2. ModeWarn logging — ✅ Fixed. handlePolicyMode emits slog.Warn("signature verification warning", "mode", mode, "error", err) when mode == ModeWarn && err != nil. Test captures slog output via slog.NewTextHandler and asserts "signature verification warning" appears.

Test Intent Assessment

• TestVerifyKeylessLegacy_WrongSignature validates the security-critical property: a signature not produced by the certificate's key is rejected. This test would fail if verifySignatureWithCert were removed or bypassed, confirming regression resistance.
• TestVerify_ModeWarn_NoSignatures now validates both behavioral output (nil identity, nil error) and observable side effects (slog warning emitted), closing the previous test gap.
• All other tests remain valid and passing.

Architecture Review Required

None.

Validation Performed

• make ci — PASS (build, test -race, lint-go, lint-imports, lint-no-todos, lint-baseline-check 23/23, spec-check, validate, validate-self-workflows, example-plugin)
• go test -race -v ./internal/adapter/signing/ — 15 PASS, 1 SKIP (keyless fixture)
• go vet ./internal/adapter/signing/ — clean
• No new lint issues, no TODO/FIXME markers, import boundaries intact.

[jn-dave]

Review — Approved

Prior blockers verified resolved:

1. verifyKeylessLegacy now validates the signature against the certificate's public key via verifySignatureWithCert. Test TestVerifyKeylessLegacy_WrongSignature confirms wrong-key signatures are rejected.
2. handlePolicyMode now emits slog.Warn for ModeWarn failures. Test TestVerify_ModeWarn_NoSignatures captures and asserts the warning output.

Independent verification:

• 15 tests (14 PASS, 1 SKIP deferred per workstream) — all clean with -race.
• make lint-imports passes; import boundaries intact.
• CI gates all green: Unit tests, Lint, Build & E2E, Proto drift check.
• All workstream exit criteria met (keyless fixture deferred with documented skipped test).
• No security concerns: RawKey is json:"-", no secrets in logs, cache dir 0o750, TUF fetched over HTTPS.
• Code is clean, well-structured, and matches the workstream spec.