fix(cli): wsc --version reports wsc, not wsc-cli - #273
Open
avrabe wants to merge 4 commits into
Open
Conversation
…260) varve (#260) already produces cosign bundles (keyless, GitHub-OIDC) but wsc could only verify artifacts it signed itself — SigstoreBundle can emit wsc signatures but nothing converts an existing cosign bundle back into a KeylessSignature. Adds KeylessSignature::from_sigstore_bundle(json) parsing BOTH wire shapes: - Legacy `rekorBundle` JSON (cosign v2.4.x — what varve's v0.28.0 ships): {base64Signature, cert, rekorBundle:{SignedEntryTimestamp, Payload}}. - Protobuf `bundle.sigstore.dev/v0.3+json` envelope (verificationMaterial + messageSignature + tlogEntries). Faithful extraction (verified against two REAL fixtures committed here): - module_hash is read from the hashedrekord body's spec.data.hash.value, never recomputed; the negative-control test flips one hex char and asserts the extracted hash changes. - integratedTime (unix int) -> RFC3339, the form RekorEntry documents and verify_cert_chain parses (confirmed: cert-chain + body-binding both accept the ingested varve bundle). - v0.3 requires a Fulcio certificate; a raw-public-key (non-keyless) bundle is rejected with a specific error, proven on a real cosign v0.3 bundle. Round-trip fidelity test: from_sigstore_bundle -> from_keyless_signature -> to_json -> from_json preserves signature, module_hash, cert chain and rekor fields (uuid/inclusion_proof intentionally empty for legacy — documented). KNOWN LIMITATION → REQ-28 (#231, the verify half): cosign emits ECDSA signatures in ASN.1 DER (varve's is 71 bytes, 3045…), but the offline verifier's verify_crypto uses P256Signature::from_slice (fixed 64-byte P1363), so it currently rejects an ingested DER signature. Making the verifier accept DER (from_der fallback) and handling the empty Rekor uuid on the offline path is REQ-28's scope — that is where "verify an ingested cosign bundle offline" completes. from_sigstore_bundle here is the faithful ingestion half. Fixtures: legacy = varve v0.28.0 public release; v0.3 = real cosign --new-bundle-format output (see fixtures README). Tests: wsc lib 610 pass/3 ignored; sigstore_bundle 7 pass. Refs: #260, #231 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012aR3Md1h46K9wAUWMQiESH
…ap (#260) Two parts: a real interop DEFECT FIX found by a round-trip test, and the coverage gap that was hiding it. ## The coverage gap (codecov/patch was failing: 164 of 397 new lines uncovered) from_v03_bundle was almost entirely unexercised: the only v0.3 fixture is a LOCAL-KEY bundle, so parsing bailed at the "requires a Fulcio certificate" check and the whole v0.3 happy path never ran. We claimed "supports both wire shapes" while only one shape's happy path was tested — the vacuous-oracle shape REQ-30/#258 exists to kill, and exactly what varve warned about in #260 ("supporting only one will surprise someone"). Fixed by building a genuine cert-bearing v0.3 bundle from REAL material: legacy fixture -> from_sigstore_bundle -> SigstoreBundle::from_keyless_signature -> to_json -> from_sigstore_bundle again, asserting field-by-field fidelity. Plus a spec-shaped keyless positive (the singular certificate.rawBytes branch real cosign keyless bundles use), a v0.3 negative control, and 43 error-path unit tests each asserting a specific message. Added lines uncovered: 164 -> 4, and those 4 are provably unreachable (a map_err closure guarded by an is_ascii_hexdigit + length check, and two test-helper panic arms). ## The defects that round-trip test found (emitter was non-conformant) Ground truth: a real `cosign sign-blob --new-bundle-format` bundle emits `logId.keyId` as BASE64 and places the SET under `inclusionPromise`, with no top-level field. 1. logId.keyId encoding. The emitter wrote RekorEntry::log_id (hex, the Rekor REST form) straight into LogId.key_id, which the Sigstore protobuf spec types as `bytes` — base64 in JSON. A 64-char hex string is ALSO valid base64, so it did not error: it decoded to 48 junk bytes, corrupting the Rekor log identity with no diagnostic (c0d23d6a…801d -> 734776dd…7dce). Fixed: transcode hex -> base64; a non-hex value passes through unchanged rather than emitting mangled base64. 2. SET placement. The emitter wrote the SET at the top-level tlogEntries[].signedEntryTimestamp; every other implementation (and wsc's own ingest) reads inclusionPromise.signedEntryTimestamp. The SET — the only offline transparency proof a legacy bundle carries — was silently dropped on wsc's own round trip. Fixed: emit the spec location via a new InclusionPromise type. Both mean bundle.rs's documented claim that emitted bundles verify with `cosign verify-blob --bundle` was false. Backward compatible on read: the legacy top-level SET is still accepted (deserialize-only field + a signed_entry_timestamp() accessor), with a test proving pre-0.11.0 bundles still round-trip. The two KNOWN-DEFECT assertions that pinned the buggy behaviour are replaced with true losslessness assertions, so the round-trip test now proves fidelity rather than documenting corruption. Tests: wsc lib 655 pass/3 ignored (+45); sigstore_bundle 10 pass (+3). Vacuous-oracle gate clean. Clippy clean. Refs: #260, #231 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012aR3Md1h46K9wAUWMQiESH
Closes #255. The package is `wsc-cli`; the binary it installs is `wsc`. `crate_name!()` expands to CARGO_PKG_NAME, so `wsc --version` printed `wsc-cli 0.10.0` — a name that appears on no PATH. `env!("CARGO_BIN_NAME")` instead, so the package keeps its name and the binary reports its own. The now-unused `crate_name` import is dropped rather than left to warn. Measured on this branch by building and running it: before wsc-cli 0.10.0 after wsc --version wsc 0.11.0 exit 0 wsc --help exit 0 wsc --not-a-flag exit 2 pulseengine-cli-conventions rule 1. Reported in pulseengine.eu#183, which measured 5 of 9 tools in the signed layer breaking it. This one matters more than tidiness: wsc is the signer. The tool whose whole job is attestation could not state its own identity in the conventional form, which is the same class of problem it exists to solve for everything else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G5qpB7zvxLvdrvn5YRyj4R
CARGO_BIN_NAME is set by Cargo and NOT by rules_rust. This crate is built both
ways (//src/cli:wasmsign_cli), so `env!` compiled fine under cargo and failed
CI with:
error: environment variable `CARGO_BIN_NAME` not defined at compile time
I verified the first version with `cargo build --bin wsc` alone, which cannot
see the Bazel path. option_env!(...).unwrap_or("wsc") builds under both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G5qpB7zvxLvdrvn5YRyj4R
Contributor
Author
|
The two failing
In this job the steps that matter to this PR all passed: What I did verify for this changeThe first version of this PR used The fixup uses Both build systems now agree on the reported name. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #255. Raised from pulseengine.eu#183, which measured 5 of 9 tools in the signed varve layer breaking CLI conventions rule 1.
The package is
wsc-cli; the binary it installs iswsc.crate_name!()expands toCARGO_PKG_NAME, sowsc --versionprinted a name that appears on no PATH.env!("CARGO_BIN_NAME")instead — the package keeps its name, the binary reports its own. The now-unusedcrate_nameimport is dropped rather than left to warn.Measured by building and running it
Rule 2 already passed and still does.
Why this one is worth more than tidiness
wsc is the signer. The tool whose entire job is attestation could not state its own identity in the conventional form — the same class of problem it exists to solve for everything else. Any provenance record answering "which binary produced this signature" had to special-case it.
🤖 Generated with Claude Code
https://claude.ai/code/session_01G5qpB7zvxLvdrvn5YRyj4R