fix: fail fast for unsupported Sigstore publish auth#194
Conversation
jcfischer
left a comment
There was a problem hiding this comment.
Review: fix: fail fast for unsupported Sigstore publish auth
Lenses applied: CodeQuality, Security, Architecture, Performance
Summary
Clean, well-scoped fix for #193. The core design — resolveSigningAuth() as a pure discriminated-union classifier, fail-fast in both publish() (pre-upload) and signSigstoreBundle() (pre-cosign-spawn), temp-file for inline tokens with finally cleanup — is sound. The fail-closed posture (reject unknown auth rather than falling through to interactive device flow) is the right security call.
Findings
Nits (0 blockers, 0 majors, 2 nits):
-
Test env save/restore duplication — The 6-var save/delete/restore pattern is copy-pasted identically across
publish.test.tsandcosign.test.ts(~25 lines each). A small helper likewithCleanSigningEnv(fn)intest/helpers/would collapse both to one-liners and prevent drift if new env vars are added later. Not blocking — the tests are correct as-is. -
Missing
resolveSigningAuthunit tests — The function is exported and has four return branches, but only theunsupportedpath is tested (indirectly viasignSigstoreBundle). Direct unit tests forgithub-actions,identity-token, andidentity-token-filebranches would catch regressions cheaply — same pattern as the existingresolveSignerIdentitytests above. Follow-up material, not blocking.
What's good
- Fail-closed by default — local cosign device/browser auth is explicitly rejected, not silently attempted. Matches the trust model where verification trusts only GitHub Actions OIDC.
- Token never in process args — inline
ARC_SIGSTORE_IDENTITY_TOKENis written to a 0600 temp file and cleaned up infinally. Keeps tokens out of/proc/*/cmdline. - Pre-upload gate in
publish()— catches unsupported auth before the tarball upload, so no wasted registry state on inevitable signing failure. - Clean discriminated union —
SigningAuthtype makes exhaustive matching natural; theelsebranch insignSigstoreBundlecorrectly narrows toidentity-token. - Error message is actionable — tells the operator exactly what's needed (GH Actions OIDC permissions or explicit token env vars).
Verdict: approve. The two nits are follow-up quality — this unblocks @metafactory/soma 0.8.1 publishing.
Closes #193.\n\nChanges:\n- Reject official Sigstore publishes before upload when the environment would fall back to local cosign device/browser auth.\n- Allow only GitHub Actions OIDC or explicit ARC_SIGSTORE_IDENTITY_TOKEN_FILE / ARC_SIGSTORE_IDENTITY_TOKEN for non-interactive signing.\n- Restrict cosign to the GitHub Actions OIDC provider in CI and keep token values out of process args by writing inline tokens to a temp file.\n\nValidation:\n- rtk bun test test/commands/publish.test.ts test/unit/cosign.test.ts\n- rtk bunx tsc --noEmit\n- rtk bun run lint\n- rtk bun test