publish: Sigstore-sign official package versions

[jcfischer]

Summary

• sign official-source publish tarballs with cosign keyless signing
• upload Sigstore bundles to /api/v1/storage/bundle/:sha256 and register signing metadata
• fail closed unless --allow-unsigned-official is used for legacy rollout

Closes #191.

Verification

• bunx tsc --noEmit
• bunx eslint --quiet src/lib/cosign.ts src/lib/publish.ts src/commands/publish.ts src/cli.ts test/unit/publish.test.ts test/commands/publish.test.ts test/unit/cosign.test.ts
• bun test: 962 pass, 3 skip, 0 fail

[jcfischer]

Code Review — CodeQuality + Security lenses

PR: publish: Sigstore-sign official package versions
Lenses: CodeQuality, Security, Architecture

Summary

Solid feature — fail-closed signing for official tiers, injectable signer for testing, proper cleanup in finally, idempotent 409 handling on bundle upload. Two findings worth addressing before merge.

Findings

#	Severity	File	Issue
1	major	src/lib/publish.ts:396	signed_at truthiness check drops 0
2	nit	src/lib/publish.ts:340	Content-Type/body mismatch on bundle upload

See inline comments for details.

verdict: blockers=0 majors=1 nits=1 — recommend: request-changes. One falsiness bug in signed_at conditional spread; one Content-Type nit.

[jcfischer]

[major] Truthiness check on signed_at will silently drop the field if its value is 0. While a Unix epoch of 0 (1970-01-01) is unlikely in practice, using truthiness for numeric optional fields is a latent bug — the type system says number | undefined, and the conditional should match.

-    ...(payload.signed_at ? { signed_at: payload.signed_at } : {}),
+    ...(payload.signed_at !== undefined ? { signed_at: payload.signed_at } : {}),

Same pattern applies to signature_bundle_key and signer_identity on the two lines above — those are strings so empty-string is the falsy edge case. Less likely to bite, but !== undefined is more precise for all three.

[jcfischer]

[nit] Content-Type: application/json with an ArrayBuffer body. The Sigstore bundle is JSON, so this happens to work — but reading into an ArrayBuffer and sending raw bytes with a JSON content type is semantically off. Either:

• Use await file.text() and send the string (cleaner, explicit JSON), or
• Use application/octet-stream if the server treats this as an opaque blob

Low-risk since the bytes are valid JSON either way, but worth aligning intent with implementation.

[jcfischer]

Code Review — CodeQuality + Security + Architecture + EcosystemCompliance lenses

Lenses: CodeQuality, Security, Architecture, EcosystemCompliance

Summary

Clean, well-structured feature. Previous review's two findings (truthiness check on signed_at, Content-Type/body mismatch) both addressed correctly.

Security posture — strong:

• Fail-closed design — official sources require signing unless --allow-unsigned-official
• Injectable signer option — clean DI pattern for testing
• Bun.spawnSync with array args — no command injection vector
• Temp bundle cleanup in finally — no file leaks on error paths
• Bundle key mismatch validation — good defensive check

Code quality — solid:

• 8 new tests: happy path, fail-closed, unsigned override, 409 idempotency, metadata propagation, zero-valued signed_at
• Types clean (pre-existing jetstream errors only, unrelated)
• Error paths return structured results, no throws leaking

Architecture — appropriate layering: signing in cosign.ts, upload in publish.ts, orchestration in commands/publish.ts, CLI in cli.ts.

Findings

#	Severity	Location	Issue
1	nit	commit msg	Commit says fix publish sigstore signing — this is a feature (feat:), not a bug fix. Minor conventional-commit mismatch with PR title.

verdict: blockers=0 majors=0 nits=1 — recommend: approve. Previous findings addressed. Solid fail-closed signing implementation with comprehensive tests.

[jcfischer]

Code Review — publish: Sigstore-sign official package versions

Lenses applied: CodeQuality, Security, Architecture, Performance

Summary

Clean, well-structured PR that adds Sigstore keyless signing to official-source publishes. Fail-closed design is the right call — official packages unsigned by default would be a supply chain risk. The injectable signer option is good DI for testing.

Findings

blockers=0, majors=0, nits=2

Nits

1. signSigstoreBundle captures signedAt before spawnSync (src/lib/cosign.ts): The timestamp is taken before cosign runs, not after it signs. In practice the delta is small (sub-second for local signing), but if cosign blocks on OIDC (keyless flow can prompt a browser), the gap widens. Consider capturing after the successful exitCode === 0 check for accuracy — or document the "sign-initiated" semantic.

2. registerVersion conditional spreads (src/lib/publish.ts): The three ...(payload.X !== undefined ? { X: payload.X } : {}) lines work but are verbose. A helper like pickDefined({ signature_bundle_key, signer_identity, signed_at }) would reduce noise — but this is style, not correctness, and the existing pattern matches the rest of the file.

What's good

• Fail-closed default — official sources require signing unless --allow-unsigned-official
• Injectable signer — clean DI, tests never shell out to real cosign
• Sigstore bundle cleanup in finally — temp file cleaned on both success and failure paths
• 409 idempotency on bundle upload — matches the existing tarball upload pattern
• Bundle key verification — server response validated against expected key, catches server-side routing bugs
• Comprehensive test coverage — happy path, fail-closed, unsigned override, unit tests for identity resolution, upload, and registration payload

Verdict

recommend: approve — blockers=0 majors=0 nits=2. Ship it.

🤖 Reviewed by Ivy (CodeReview skill)