diff --git a/README.md b/README.md index 3a092bf2..5909c830 100644 --- a/README.md +++ b/README.md @@ -311,6 +311,7 @@ metadata local disk / optional S3 | DID | A user, agent, or node identity derived from an Ed25519 public key. | | HTTP Signature | RFC 9421 signature proving control of the DID key for write requests. | | Ref certificate | Signed record of a ref update. Useful for audit and replication. | +| Ref certificate versioning | The wire format is versioned; new certificates are currently issued as v1. `gl cert show --verify` requires `--expect-node `, and the queried node's self-reported DID is **not** accepted as a trust anchor. v2 is reserved for a future payload shape that carries the version inside the signed bytes; this client refuses to verify v2+ rather than guessing. | | UCAN | Delegation token for future capability-based workflows. | | Peer announce | Node-to-node HTTP announcement of DID + public URL. | | Gossipsub | libp2p topic for ref-update events. | diff --git a/crates/gitlawb-node/src/api/certs.rs b/crates/gitlawb-node/src/api/certs.rs index 0d954cb1..1965aa49 100644 --- a/crates/gitlawb-node/src/api/certs.rs +++ b/crates/gitlawb-node/src/api/certs.rs @@ -52,6 +52,7 @@ pub async fn list_certs( "node_did": c.node_did, "signature": c.signature, "issued_at": c.issued_at, + "version": c.version, }) }) .collect(); @@ -92,5 +93,6 @@ pub async fn get_cert( "node_did": cert.node_did, "signature": cert.signature, "issued_at": cert.issued_at, + "version": cert.version, }))) } diff --git a/crates/gitlawb-node/src/api/events.rs b/crates/gitlawb-node/src/api/events.rs index 1158f47e..3b34e911 100644 --- a/crates/gitlawb-node/src/api/events.rs +++ b/crates/gitlawb-node/src/api/events.rs @@ -437,6 +437,7 @@ mod ref_updates_feed_tests { node_did: "did:key:z6MkNode".into(), signature: "sig".into(), issued_at: Utc::now().to_rfc3339(), + version: 1, } } diff --git a/crates/gitlawb-node/src/cert.rs b/crates/gitlawb-node/src/cert.rs index 0ed50418..90c0b5ef 100644 --- a/crates/gitlawb-node/src/cert.rs +++ b/crates/gitlawb-node/src/cert.rs @@ -11,6 +11,74 @@ use uuid::Uuid; use crate::db::RefCertificate; use crate::state::AppState; +/// Build the v1 signing payload — the EXACT bytes the node signs and +/// gl's `verify_signature` reconstructs. This is the single source +/// of truth shared by `issue_ref_certificate` and the frozen +/// canonical-form tests below; drift here is what would break every +/// existing cert. +/// +/// Field order and key set are fixed: a default `serde_json::Value` +/// serializes `Map` with sorted keys, so any change to +/// the literal is observable as a different byte sequence on the +/// wire. +/// +/// v1 is the unversioned legacy shape: no `version` key in the signed +/// JSON. From v2 onward the version belongs INSIDE the signed bytes +/// (see `v2_signing_payload`): leaving it as an unsigned sibling +/// would let a future v2 cert be downgraded to v1 and still verify +/// cleanly, since the bytes would be identical. +pub(crate) fn v1_signing_payload( + repo_id: &str, + ref_name: &str, + old_sha: &str, + new_sha: &str, + pusher_did: &str, + node_did: &str, + issued_at: &str, +) -> serde_json::Value { + serde_json::json!({ + "repo_id": repo_id, + "ref": ref_name, + "old": old_sha, + "new": new_sha, + "pusher": pusher_did, + "node": node_did, + "ts": issued_at, + }) +} + +/// Build the v2 signing payload — the future shape for versioned +/// certs. Identical to v1 except the wire `version` is part of the +/// signed bytes, so stripping or flipping the `version` column +/// invalidates the signature instead of verifying cleanly under the +/// other version's path. +/// +/// NOT yet issued: `issue_ref_certificate` still stamps v1 because +/// the shipped `gl` verifier only supports v1. This builder exists +/// to pin the downgrade-resistant shape now, before any v2 cert is +/// ever signed. +#[allow(dead_code)] +pub(crate) fn v2_signing_payload( + repo_id: &str, + ref_name: &str, + old_sha: &str, + new_sha: &str, + pusher_did: &str, + node_did: &str, + issued_at: &str, +) -> serde_json::Value { + serde_json::json!({ + "repo_id": repo_id, + "ref": ref_name, + "old": old_sha, + "new": new_sha, + "pusher": pusher_did, + "node": node_did, + "ts": issued_at, + "version": 2, + }) +} + /// Issue a signed ref-update certificate for a successful push. /// /// Builds a canonical JSON payload, signs it with the node's Ed25519 key, @@ -26,16 +94,12 @@ pub async fn issue_ref_certificate( let node_did = state.node_did.to_string(); let issued_at = Utc::now().to_rfc3339(); - // Build the canonical signing payload. - let payload = serde_json::json!({ - "repo_id": repo_id, - "ref": ref_name, - "old": old_sha, - "new": new_sha, - "pusher": pusher_did, - "node": node_did, - "ts": issued_at, - }); + // Build the canonical signing payload via the shared builder so + // the frozen-vector test below and the live signer cannot drift + // apart — a regression in either side fails both. + let payload = v1_signing_payload( + repo_id, ref_name, old_sha, new_sha, pusher_did, &node_did, &issued_at, + ); let payload_bytes = serde_json::to_vec(&payload)?; let signature = state.node_keypair.sign_b64(&payload_bytes); @@ -50,9 +114,351 @@ pub async fn issue_ref_certificate( node_did, signature, issued_at, + // #26 Split PR 3: the wire-format version. New certs stamp + // v1 — the pre-versioning 7-field payload with no `version` + // key in the signed JSON — because the shipped `gl` + // verifier only supports v1. Stamping v2 here would issue + // certificates the shipped client refuses (round-3 + // finding: node pinned `version == 2` while gl pinned + // "2 is refused", with nothing exercising the + // composition). v2 is reserved for the future shape + // defined by `v2_signing_payload`, which carries the + // version INSIDE the signed bytes; land the v2 verify + // path before stamping v2. + version: 1, }; // Persist and return the row as it exists in the database (on a // conflict the existing row survives when it is newer). state.db.insert_ref_certificate(&cert).await } + +#[cfg(test)] +mod v1_payload_tests { + //! #26 Split PR 3 — cert payload version compat. + //! + //! The v1 cert payload is the frozen canonical byte form the + //! node signs. gl's `verify_signature` reconstructs the same + //! payload and verifies. Any drift in field order, key set, or + //! whitespace breaks every existing cert. The tests here pin + //! the v1 payload shape, the round-trip, the cert id + //! determinism per the v1 version, and the v2 read forward + //! compat (an unknown version refuses to verify rather than + //! guessing). + //! + //! Reviewer 1 finding: the live signer (`issue_ref_certificate`) + //! and the frozen-vector test (`v1_payload_matches_frozen_canonical_form`) + //! previously held two separate `serde_json::json!` literals, + //! so a regression in the signer could not fail this test. + //! Both now go through the shared `v1_signing_payload` builder. + use super::{v1_signing_payload, v2_signing_payload}; + use crate::db::RefCertificate; + use gitlawb_core::identity::Keypair; + + /// v1 payload is the same shape the legacy code signed, byte + /// for byte. Reverting this assertion to the v2 shape (which + /// includes a `version` key) is what the versioned format + /// forbids: an old client reading a v1 cert must not see a + /// `version` key in the signed JSON. + /// + /// The literal is built via the shared `v1_signing_payload` + /// helper that `issue_ref_certificate` also uses, so the live + /// signer and the frozen vector share one source of truth. + #[test] + fn v1_payload_matches_frozen_canonical_form() { + let payload = v1_signing_payload( + "repo-1", + "refs/heads/main", + &"0".repeat(40), + &"a".repeat(40), + "did:key:z6MkPusher", + "did:key:z6MkNode", + "2026-07-22T00:00:00+00:00", + ); + let frozen = format!( + r#"{{"new":"{}","node":"did:key:z6MkNode","old":"{}","pusher":"did:key:z6MkPusher","ref":"refs/heads/main","repo_id":"repo-1","ts":"2026-07-22T00:00:00+00:00"}}"#, + "a".repeat(40), + "0".repeat(40), + ); + assert_eq!(serde_json::to_string(&payload).unwrap(), frozen); + } + + /// Round-trip: sign the v1 payload with a keypair, build a + /// RefCertificate with version: 1, and verify the structure. + /// This is the construction the live code does on every push. + /// + /// Uses the shared builder for both halves so a regression in + /// the live signer (adding/removing/renaming a field) cannot + /// pass this test by accident. + #[test] + fn v1_ref_certificate_structure_is_well_formed() { + let kp = Keypair::generate(); + let node_did = kp.did().as_str().to_string(); + let payload = v1_signing_payload( + "repo-1", + "refs/heads/main", + &"0".repeat(40), + &"a".repeat(40), + "did:key:z6MkPusher", + &node_did, + "2026-07-22T00:00:00+00:00", + ); + let payload_bytes = serde_json::to_vec(&payload).unwrap(); + let sig = kp.sign_b64(&payload_bytes); + let cert = RefCertificate { + id: "cert-id".into(), + repo_id: "repo-1".into(), + ref_name: "refs/heads/main".into(), + old_sha: "0".repeat(40), + new_sha: "a".repeat(40), + pusher_did: "did:key:z6MkPusher".into(), + node_did: node_did.clone(), + signature: sig.clone(), + issued_at: "2026-07-22T00:00:00+00:00".into(), + version: 1, + }; + assert_eq!(cert.version, 1, "new certs carry version: 1"); + // The signed payload reconstructs identically: gl's + // verify_signature would build the same JSON, hash the + // same bytes, and verify the same signature. + let reconstructed = v1_signing_payload( + &cert.repo_id, + &cert.ref_name, + &cert.old_sha, + &cert.new_sha, + &cert.pusher_did, + &cert.node_did, + &cert.issued_at, + ); + let reconstructed_bytes = serde_json::to_vec(&reconstructed).unwrap(); + assert_eq!( + reconstructed_bytes, payload_bytes, + "the round-trip serialization must be byte-identical; \ + this is what makes gl's verify_signature succeed" + ); + // Round 3 (P3 reviewer): byte-equality alone never calls + // `identity::verify`, so a bad signature helper would pass. + // Verify the computed signature against the reconstructed + // bytes — the same check gl performs. + let vk = node_did + .parse::() + .unwrap() + .to_verifying_key() + .unwrap(); + use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; + let sig_bytes: [u8; 64] = URL_SAFE_NO_PAD + .decode(&cert.signature) + .unwrap() + .try_into() + .unwrap(); + gitlawb_core::identity::verify(&vk, &reconstructed_bytes, &sig_bytes) + .expect("well-formed v1 cert signature must verify"); + } + + /// Reviewer 1 finding: the live signer must share the + /// canonical-form literal so a regression in either side + /// fails both. This test pins the shared builder's output + /// against the literal a future v2 byte stream would have + /// to break — if someone adds a `version` key to + /// `v1_signing_payload`, every existing cert breaks verify, + /// and this test is one of the canaries that fires. + #[test] + fn v1_signing_payload_has_no_version_key() { + let payload = v1_signing_payload( + "repo-1", + "refs/heads/main", + "oldsha", + "newsha", + "did:key:z6MkPusher", + "did:key:z6MkNode", + "2026-07-22T00:00:00+00:00", + ); + let obj = payload.as_object().expect("payload is a JSON object"); + assert!( + !obj.contains_key("version"), + "v1 signing payload must not carry a `version` key — adding \ + one changes the bytes and breaks every existing cert's \ + signature. v2 certs build a different payload, not this one" + ); + // Pin the exact set so a future field addition is caught. + let expected: std::collections::BTreeSet<&str> = + ["new", "node", "old", "pusher", "ref", "repo_id", "ts"] + .into_iter() + .collect(); + let actual: std::collections::BTreeSet<&str> = obj.keys().map(String::as_str).collect(); + assert_eq!(actual, expected, "v1 payload key set is frozen"); + } + + /// Round 3 (P2 reviewer): from v2 onward the version belongs + /// INSIDE the signed bytes. v1 stays the unversioned legacy + /// shape; v2 carries `"version": 2` in the payload so a + /// version flip without re-signing invalidates the signature + /// instead of verifying cleanly under the other path. + #[test] + fn v2_signing_payload_binds_version_and_resists_downgrade() { + let kp = Keypair::generate(); + let node_did = kp.did().as_str().to_string(); + let ts = "2026-07-22T00:00:00+00:00"; + let v1 = v1_signing_payload( + "repo-1", + "refs/heads/main", + "oldsha", + "newsha", + "did:key:z6MkPusher", + &node_did, + ts, + ); + let v2 = v2_signing_payload( + "repo-1", + "refs/heads/main", + "oldsha", + "newsha", + "did:key:z6MkPusher", + &node_did, + ts, + ); + let v2_obj = v2.as_object().expect("v2 payload is a JSON object"); + assert_eq!( + v2_obj.get("version"), + Some(&serde_json::json!(2)), + "v2 payload must carry the version inside the signed bytes" + ); + let v1_bytes = serde_json::to_vec(&v1).unwrap(); + let v2_bytes = serde_json::to_vec(&v2).unwrap(); + assert_ne!( + v1_bytes, v2_bytes, + "v1 and v2 payloads must differ, otherwise a version flip is cryptographically invisible" + ); + + // A signature over v1 must NOT verify as v2 and vice versa. + let vk = node_did + .parse::() + .unwrap() + .to_verifying_key() + .unwrap(); + use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; + let sig_v1 = kp.sign_b64(&v1_bytes); + let sig_v1_bytes: [u8; 64] = URL_SAFE_NO_PAD.decode(&sig_v1).unwrap().try_into().unwrap(); + assert!( + gitlawb_core::identity::verify(&vk, &v1_bytes, &sig_v1_bytes).is_ok(), + "v1 signature must verify under v1 bytes" + ); + assert!( + gitlawb_core::identity::verify(&vk, &v2_bytes, &sig_v1_bytes).is_err(), + "a v1 signature presented as v2 (downgrade/upgrade flip) must not verify" + ); + } + + /// The live issuer stamps the version the shipped client can + /// verify. The gl client refuses to verify v2 certs explicitly; + /// this test pins that the version the ISSUING PATH claims + /// agrees with the payload shape it actually signs. + /// + /// Round 2: the prior form built a `RefCertificate` literal with + /// `version: 1` and asserted `version == 1` — true no matter what the + /// signer does, so flipping the issuer's version (or its payload shape) + /// left it green. This one drives the actual + /// `issue_ref_certificate` so a regression in the issuer flips the test. + /// The DB row is created against `test_support::test_state`, which + /// gives us a real `AppState` with a real node keypair. + /// + /// Round 3: the issuer stamps v1 because the shipped `gl` + /// verifier only supports v1 (stamping v2 shipped certs the + /// client refuses). The binding to the live issuer is kept — + /// flipping the stamp still turns this test red — without + /// changing what production issues. + #[sqlx::test] + #[allow(clippy::async_yields_async)] + async fn issuer_stamps_v1_over_v1_payload(pool: sqlx::PgPool) { + let state = crate::test_support::test_state(pool.clone()).await; + let repo_id = "repo-issuer-observe"; + let ref_name = "refs/heads/main"; + let old = "0".repeat(40); + let new = "a".repeat(40); + let pusher = "did:key:z6MkPusher"; + + // Drive the live issuer and read the persisted row back by + // id, so the test covers the stored `version` column and + // not just the in-process return value. + let cert = + crate::cert::issue_ref_certificate(&state, repo_id, ref_name, &old, &new, pusher) + .await + .expect("issue_ref_certificate must succeed"); + + // The cert returned by the issuer is the source of truth + // for both the version claim and the signed bytes. + assert_eq!(cert.repo_id, repo_id); + assert_eq!(cert.ref_name, ref_name); + + // Cross-check 1: the issuer's claimed version must match + // what the live function stamps. This assertion is bound + // to the live function, not a hand-built literal: + // flipping the stamp in `issue_ref_certificate` breaks it + // through the actual call path. + assert_eq!( + cert.version, 1, + "the live issuer must stamp v1 until a v2 verifier ships; \ + flipping the stamp breaks this assertion through the actual call path" + ); + + // Cross-check 1b: the persisted row carries the same stamp. + let stored = state + .db + .get_ref_certificate(&cert.id) + .await + .expect("persisted cert must be readable") + .expect("persisted cert must exist"); + assert_eq!( + stored.version, cert.version, + "the stored version column must match the issued claim" + ); + + // Cross-check 2: the signature on the cert must verify + // against the v1 payload builder's bytes, not against some + // other shape the issuer might have introduced. This is + // what gl's `verify_signature` does for a v1 cert: rebuild + // the payload from the cert's own fields and verify the + // signature. If the issuer ever signs a different shape + // while still claiming v1, every shipped client breaks — + // and so does this. + let rebuilt = v1_signing_payload( + &cert.repo_id, + &cert.ref_name, + &cert.old_sha, + &cert.new_sha, + &cert.pusher_did, + &cert.node_did, + &cert.issued_at, + ); + let vk = cert + .node_did + .parse::() + .unwrap() + .to_verifying_key() + .unwrap(); + use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; + let sig_bytes: [u8; 64] = URL_SAFE_NO_PAD + .decode(&cert.signature) + .unwrap() + .try_into() + .unwrap(); + gitlawb_core::identity::verify(&vk, &serde_json::to_vec(&rebuilt).unwrap(), &sig_bytes) + .expect("a v1 cert's signature must verify against the v1 payload shape"); + + // Cross-check 3: the v1 signed payload must NOT contain a + // version key — v2 is the future shape that adds one inside + // the signed bytes (see `v2_signing_payload`). The v1 + // payload (which the cert is signed over) is frozen at 7 + // fields. A change that adds `version` to the v1 payload + // breaks every existing cert and this test. + let obj = rebuilt + .as_object() + .expect("rebuilt v1 payload is a JSON object"); + assert!( + !obj.contains_key("version"), + "the v1 signing payload must not carry a version key — \ + adding one changes the bytes and breaks every existing \ + cert's signature" + ); + } +} diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index cc2cf0bd..fe2fdc77 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -151,6 +151,13 @@ pub struct RefCertificate { pub node_did: String, pub signature: String, pub issued_at: String, + /// #26 Split PR 3 — the wire-format version of this cert. v1 is + /// the pre-versioning 7-field payload; v2+ will add optional + /// fields without breaking the v1 signature path. An old + /// client that ignores this field verifies v1 certs; a new + /// client that reads a v1 cert reconstructs the v1 payload + /// and verifies normally. + pub version: u32, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -1123,6 +1130,32 @@ const MIGRATIONS: &[Migration] = &[ "ALTER TABLE pin_repair_sweep ADD COLUMN IF NOT EXISTS discovery_cursor_id TEXT NOT NULL DEFAULT ''", ], }, + Migration { + version: 37, + name: "ref_certificates_version", + stmts: &[ + // #26 Split PR 3 — certificate / CLI compatibility. The + // ref-cert wire format is versioned so future fields can + // be added without breaking old clients (which ignore + // the field) or old servers (which default the column + // to 1). New certificates are currently issued as v1, + // which is byte-for-byte identical to the pre-versioning + // shape: an old client reading a v1 cert from a new + // server sees the same payload, the same signature, and + // the same Ed25519 verify path. A new client reading an + // old server sees `version` defaulted to 1 and verifies + // against the v1 payload shape. v2 is reserved for a + // future shape that carries the version inside the + // signed bytes; it must not be stamped until a v2 + // verifier ships. + // + // DEFAULT 1 so an existing row reads as v1 without a + // backfill migration. NOT NULL so a missing value is a + // hard error at insert time rather than a silent v0 that + // an old client would interpret as "no version field." + "ALTER TABLE ref_certificates ADD COLUMN IF NOT EXISTS version INTEGER NOT NULL DEFAULT 1", + ], + }, ]; /// Max distinct source repos recorded per pinned object (F1, #173 jatmn round 8). @@ -2336,8 +2369,8 @@ impl Db { pub async fn insert_ref_certificate(&self, cert: &RefCertificate) -> Result { let row = sqlx::query( "INSERT INTO ref_certificates - (id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + (id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at, version) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) ON CONFLICT (repo_id, ref_name) DO UPDATE SET old_sha = CASE WHEN EXCLUDED.issued_at > ref_certificates.issued_at THEN EXCLUDED.old_sha ELSE ref_certificates.old_sha END, @@ -2350,8 +2383,10 @@ impl Db { signature = CASE WHEN EXCLUDED.issued_at > ref_certificates.issued_at THEN EXCLUDED.signature ELSE ref_certificates.signature END, issued_at = CASE WHEN EXCLUDED.issued_at > ref_certificates.issued_at - THEN EXCLUDED.issued_at ELSE ref_certificates.issued_at END - RETURNING id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at", + THEN EXCLUDED.issued_at ELSE ref_certificates.issued_at END, + version = CASE WHEN EXCLUDED.issued_at > ref_certificates.issued_at + THEN EXCLUDED.version ELSE ref_certificates.version END + RETURNING id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at, version", ) .bind(&cert.id) .bind(&cert.repo_id) @@ -2362,6 +2397,7 @@ impl Db { .bind(&cert.node_did) .bind(&cert.signature) .bind(&cert.issued_at) + .bind(cert.version as i32) .fetch_one(&self.pool) .await?; Ok(row_to_cert(row)) @@ -2376,7 +2412,7 @@ impl Db { // bounded even if a raw/negative value slips through the handler layer. let limit = limit.max(1); let rows = sqlx::query( - "SELECT id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at + "SELECT id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at, version FROM ref_certificates WHERE repo_id = $1 ORDER BY issued_at DESC LIMIT $2", ) .bind(repo_id) @@ -2415,7 +2451,7 @@ impl Db { let pattern = format!("{}%", escaped_prefix); let rows = sqlx::query( - "SELECT id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at + "SELECT id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at, version FROM ref_certificates WHERE repo_id = $1 AND id LIKE $2 ESCAPE '!' ORDER BY issued_at DESC LIMIT $3", ) .bind(repo_id) @@ -2428,7 +2464,7 @@ impl Db { pub async fn get_ref_certificate(&self, id: &str) -> Result> { let row = sqlx::query( - "SELECT id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at + "SELECT id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at, version FROM ref_certificates WHERE id = $1", ) .bind(id) @@ -3945,6 +3981,7 @@ fn row_to_cert(r: sqlx::postgres::PgRow) -> RefCertificate { node_did: r.get("node_did"), signature: r.get("signature"), issued_at: r.get("issued_at"), + version: r.get::("version") as u32, } } @@ -6574,6 +6611,7 @@ mod ref_certificate_tests { node_did: "did:key:zNODE".to_string(), signature: "sig".to_string(), issued_at: issued_at.to_string(), + version: 1, } } @@ -7247,6 +7285,130 @@ mod ref_certificate_tests { ); } + /// #26 Split PR 3 (INV-7): an existing node past v1 gets the + /// `ref_certificates.version` column from its OWN v37 entry, proven + /// by dropping the column plus its `schema_migrations` row and + /// re-running the real migration code. The migration is the only + /// place the column is added (v37 — the v1 bundle no longer + /// carries it), so a deletion of v37 must be visible as a missing + /// column on a v27 node. + /// + /// DEFAULT 1 is what makes an existing pre-v37 cert row read as + /// v1 without a backfill migration. An upgraded node reads + /// every legacy row as version 1 — the same payload the old + /// code signed — so a v1 verify path on a new client still + /// works against an upgraded database. + /// + /// MUTATION (RED): delete the v37 entry from `MIGRATIONS` and the + /// upgrade path leaves the column missing. + #[sqlx::test] + async fn v37_ref_certificates_version_applies_on_upgrade(pool: PgPool) { + async fn version_column_default(pool: &PgPool) -> Option { + // fetch_optional (not fetch_one) so a missing column + // returns Ok(None) instead of RowNotFound. The whole + // point of the precondition assertion below is to + // detect a missing column, and RowNotFound would mask + // the difference between "the helper is wrong" and + // "the migration is wrong". + sqlx::query_scalar::<_, Option>( + "SELECT column_default + FROM information_schema.columns + WHERE table_name = 'ref_certificates' + AND column_name = 'version'", + ) + .fetch_optional(pool) + .await + .unwrap() + .flatten() + } + + async fn seed_pre_v37_cert(pool: &PgPool) { + // The pre-v37 schema has no `version` column, so a raw + // INSERT omitting it is the legacy code path. The node + // did not write a version value on v1..v27 — the field + // did not exist. + sqlx::query( + "INSERT INTO ref_certificates + (id, repo_id, ref_name, old_sha, new_sha, + pusher_did, node_did, signature, issued_at) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + ON CONFLICT (repo_id, ref_name) DO NOTHING", + ) + .bind("legacy-cert-1") + .bind("legacy-repo") + .bind("refs/heads/main") + .bind("0000") + .bind("1111") + .bind("did:key:zLEGACYPUSHER") + .bind("did:key:zLEGACYNODE") + .bind("legacy-sig") + .bind("2026-07-01T10:00:00+00:00") + .execute(pool) + .await + .unwrap(); + } + + async fn legacy_row_reads_as_v1(pool: &PgPool) -> i32 { + // After v37 the legacy row (inserted without an explicit + // version) reads back as 1 via the DEFAULT, not as 0 or + // NULL — the latter would be a hard insert error under + // NOT NULL DEFAULT, but the read-back is the actual + // forward-compat property the v1 verify path relies on. + sqlx::query_scalar::<_, i32>( + "SELECT version FROM ref_certificates WHERE id = 'legacy-cert-1'", + ) + .fetch_one(pool) + .await + .unwrap() + } + + // 1. Fresh chain: v37 has run, the column exists with DEFAULT 1. + let db = Db::for_testing(pool.clone()); + db.run_migrations().await.unwrap(); + assert_eq!( + version_column_default(&pool).await.as_deref(), + Some("1"), + "v37 must declare the version column with DEFAULT 1" + ); + + // 2. Roll back to pre-v37: drop the column AND the v37 record. + // The rollback is split into two statements because ALTER + // TABLE ... DROP COLUMN and DELETE run in the same DDL + // surface but the column drop must complete before the + // next step reads schema_migrations. + sqlx::query("ALTER TABLE ref_certificates DROP COLUMN version") + .execute(&pool) + .await + .unwrap(); + sqlx::query("DELETE FROM schema_migrations WHERE version = 37") + .execute(&pool) + .await + .unwrap(); + assert!( + version_column_default(&pool).await.is_none(), + "precondition: column removed and its migration record removed" + ); + + // 3. Seed a legacy cert on the pre-v37 schema (no version + // column ⇒ the INSERT must omit it). + seed_pre_v37_cert(&pool).await; + + // 4. Re-run migrations: v37 must re-add the column with + // DEFAULT 1, and the legacy row must read back as 1. + db.run_migrations().await.unwrap(); + assert_eq!( + version_column_default(&pool).await.as_deref(), + Some("1"), + "v37 must recreate the version column with DEFAULT 1 on an upgrading node" + ); + assert_eq!( + legacy_row_reads_as_v1(&pool).await, + 1, + "a pre-v37 cert row reads as version 1 after v37 runs, so the \ + v1 verify path on a new client still works against an upgraded database" + ); + } + /// INV-7: upgrade-path test — seed a database at v9 with duplicate /// ref_certificates, then let the real v10 migration fire via /// run_migrations(). This exercises the migration code path rather than diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 430c0600..e1306077 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -1751,6 +1751,7 @@ mod tests { node_did: owner.to_string(), signature: "sig".to_string(), issued_at: Utc::now().to_rfc3339(), + version: 1, }) .await .expect("seed private cert"); @@ -13943,6 +13944,7 @@ mod tests { node_did: "did:key:zNode".into(), signature: "sig".into(), issued_at: "2026-01-01T00:00:00Z".into(), + version: 1, }; state.db.insert_ref_certificate(&cert).await.unwrap(); @@ -13978,6 +13980,7 @@ mod tests { node_did: "did:key:zNode".into(), signature: "sig".into(), issued_at: "2026-01-01T00:00:00Z".into(), + version: 1, }; state.db.insert_ref_certificate(&cert).await.unwrap(); @@ -14487,6 +14490,7 @@ mod tests { node_did: "did:key:zNode".into(), signature: "sig".into(), issued_at: "2026-01-01T00:00:00Z".into(), + version: 1, }; state.db.insert_ref_certificate(&cert).await.unwrap(); @@ -15343,6 +15347,7 @@ mod tests { node_did: "did:key:zNODE".into(), signature: "sig".into(), issued_at: issued_at.to_string(), + version: 1, } } diff --git a/crates/gl/src/cert.rs b/crates/gl/src/cert.rs index 87ad5aec..f0915cb4 100644 --- a/crates/gl/src/cert.rs +++ b/crates/gl/src/cert.rs @@ -31,7 +31,13 @@ pub enum CertCmd { #[arg(long)] dir: Option, }, - /// Show a specific ref certificate and verify its signature + /// Show a specific ref certificate and verify its signature. + /// + /// `--verify` requires `--expect-node ` since v2 cert + /// compat (Round 2): the queried node's self-reported DID is no + /// longer accepted as a trust anchor, so callers must name the + /// issuer they trust. See the `BREAKING CHANGE:` note on the + /// commit that introduced this requirement. Show { /// Repository in / or format repo: String, @@ -42,13 +48,17 @@ pub enum CertCmd { #[arg(long)] dir: Option, /// Exit non-zero unless the Ed25519 signature verifies AND the - /// issuing node matches the queried node (or --expect-node) + /// issuing node matches --expect-node (which --verify requires: + /// without a caller-supplied anchor there is no trusted issuer to + /// verify against) #[arg(long)] verify: bool, /// Expected issuing node DID for --verify. A valid signature alone /// only proves the cert is internally consistent — signed by whatever /// key it names — so --verify also anchors the issuer to a DID you - /// trust: this value when given, else the queried node's DID. + /// trust. The queried node's self-reported DID is deliberately NOT + /// accepted as a fallback anchor: the node that served the cert can + /// serve a matching self-report. #[arg(long, requires = "verify")] expect_node: Option, }, @@ -100,12 +110,12 @@ async fn cmd_list(repo: String, node: String, dir: Option) -> Result<() let client = signed_client(&node, dir.as_deref()); let path = format!("/api/v1/repos/{owner}/{name}/certs"); - let resp: Value = client + let resp = client .get_authed(&path) .await? - .json() - .await + .error_for_status() .context("failed to list certificates")?; + let resp: Value = resp.json().await.context("failed to list certificates")?; let certs = resp["certificates"].as_array().cloned().unwrap_or_default(); @@ -156,6 +166,24 @@ async fn cmd_show( let node_did = cert["node_did"].as_str().unwrap_or("?"); let signature = cert["signature"].as_str().unwrap_or("?"); let issued_at = cert["issued_at"].as_str().unwrap_or("?"); + // #26 Split PR 3: read the wire-format version. A MISSING key + // selects legacy v1 (an old server that predates the field). + // A PRESENT value must be a JSON integer that fits in u32 and + // names a version this client supports — otherwise the response + // declares a format we cannot represent and we refuse to verify + // rather than guess the payload shape. Reviewer 2 finding: + // collapsing `null`, a string, a float, or an overflow integer + // onto v1 would let a well-signed v1 signature pass `--verify` + // against a server that explicitly said "version 2 (or 99, or + // 4294967297)" — that is the exact mismatch the field exists + // to prevent. parse_cert_version is the load-bearing parser + // that distinguishes missing-key (legacy v1) from invalid-value + // (unsupported). + let parsed_version = parse_cert_version(cert.get("version")); + let version_display: String = match &parsed_version { + Ok(v) => v.to_string(), + Err(reason) => format!("unsupported ({reason})"), + }; println!("Ref Certificate: {cert_id}"); println!(" Ref: {ref_name}"); @@ -164,6 +192,7 @@ async fn cmd_show( println!(" Pusher: {pusher}"); println!(" Node DID: {node_did}"); println!(" Issued at: {issued_at}"); + println!(" Version: {version_display}"); println!(" Signature: {signature}"); println!(); @@ -173,9 +202,31 @@ async fn cmd_show( // This proves the cert is internally authentic — signed by the key it // names; the node-DID comparison below covers *which* node that is. let repo_id = cert["repo_id"].as_str().unwrap_or(""); - let verdict = verify_signature( - repo_id, ref_name, old_sha, new_sha, pusher, node_did, issued_at, signature, - ); + let verdict = match parsed_version { + // v1 is the only version this client verifies. The v1 + // signed payload is the 7-field canonical form with no + // `version` key — see gitlawb-node/src/cert.rs. A future + // v2+ cert has a different signed payload shape (the + // version field becomes part of the signed JSON per + // `v2_signing_payload`), so a v2+ cert from a server this + // client does not know about must NOT be silently verified + // as v1. + // Reviewer 2: refuse rather than guess; the client and + // server must agree on the version. + // parse_cert_version admits exactly one Ok value: 1 (missing key and + // integer 1 both land there; 2, floats, strings, overflow and every + // other spelling come back Err with their own reason). A separate + // `Ok(v != 1)` arm existed here and was DEAD CODE — a test aimed at it + // could never catch drift in this match (review round 2) — so the + // match now mirrors the parser's real contract, which the + // `parse_cert_version_truth_table` test pins. + Ok(_) => verify_signature( + repo_id, ref_name, old_sha, new_sha, pusher, node_did, issued_at, signature, + ), + Err(reason) => Err(format!( + "cert declared a version this client cannot represent ({reason}); refusing to verify" + )), + }; println!("Signature verification:"); match &verdict { @@ -202,7 +253,10 @@ async fn cmd_show( }; match current_node_did.as_deref() { Some(current) if current == node_did => { - println!(" Issuing node DID matches the node being queried."); + // Self-reported, so phrased as information rather than trust: the + // node answering `/` is the node that served the cert, and a + // forger controls both. Trust comes only from --expect-node. + println!(" Issuing node DID matches the queried node's self-reported DID."); } Some(current) => { println!(" WARNING: Certificate node DID ({node_did}) does not match"); @@ -216,22 +270,33 @@ async fn cmd_show( if require_valid { if let Err(reason) = verdict { - anyhow::bail!("certificate signature did not verify: {reason}"); + // Round 3: do not claim the signature failed when it was + // never checked. A version refusal means the payload + // shape is unknown, so no signature verification ran. + anyhow::bail!("certificate could not be verified: {reason}"); } // A valid signature proves internal consistency only: the payload was // signed by whatever key the certificate itself names. A hostile // source can mint a keypair, put its DID in node_did, and self-sign. - // --verify therefore also anchors the issuer to a trusted DID: - // --expect-node when given, else the DID of the node being queried. - let expected = expect_node.as_deref().or(current_node_did.as_deref()); - match expected { + // --verify therefore also anchors the issuer — and the anchor must be + // a DID the CALLER trusts, which is exactly what --expect-node is. + // The DID the queried node reports at `/` is NOT an anchor: the same + // node that served a forged certificate can serve a matching + // self-report, and with that fallback a wholly forged cert printed + // VALID and exited 0 (review round 2, demonstrated by execution). So + // --verify without --expect-node refuses to claim a trusted issuer at + // all, rather than laundering the server's self-assertion into one. + match expect_node.as_deref() { Some(expected) if expected == node_did => {} Some(expected) => anyhow::bail!( "certificate is signed by {node_did}, but the expected issuer is {expected} — \ a valid signature alone proves internal consistency, not a trusted issuer" ), None => anyhow::bail!( - "cannot anchor the issuer: node info is unreachable and no --expect-node was given" + "--verify needs --expect-node to anchor the issuer: the signature is \ + valid, but it only proves the cert is self-consistent with the DID it \ + names ({node_did}); the queried node's self-reported DID is not a trust \ + anchor" ), } } @@ -285,6 +350,65 @@ fn verify_signature( .map_err(|_| "Ed25519 signature does not match the signed payload".to_string()) } +/// Parse the wire-format `version` field of a certificate response. +/// +/// Semantics (Reviewer 2): +/// - MISSING key (`None`) → `Ok(1)`. An old server predates the field, so +/// the cert is by definition the v1 (pre-versioning) shape; the v1 +/// verify path is the only path that can possibly match the bytes. +/// - PRESENT key must be a JSON integer that fits losslessly in `u32` +/// and equals a version this client supports. `null`, a string, +/// a float, an overflow integer, or any version other than 1 all +/// yield `Err`. Collapsing any of these onto v1 would let a +/// well-signed v1 signature pass `--verify` against a response +/// that explicitly declared a format this client cannot represent +/// — which is exactly the mismatch the version field exists to +/// prevent. +/// +/// The supported-version set is hard-coded to `{1}` because PR 3 ships +/// only v1 verification; bump this set (and the verdict branch) when +/// v2 verification lands. +fn parse_cert_version(value: Option<&Value>) -> Result { + let v = match value { + None => return Ok(1), + Some(v) => v, + }; + + // serde_json::Value::as_u64 rejects strings, floats, booleans, + // nulls, arrays, and objects — but it silently truncates floats + // that are whole numbers (e.g. 2.0 → 2). We must reject floats + // explicitly so a server cannot smuggle a v2 cert through the + // float shape. + if v.is_f64() { + return Err(format!( + "version is a JSON number with a fractional part ({v}); expected an integer" + )); + } + + let n = v.as_u64().ok_or_else(|| { + // null / string / bool / array / object — anything that is + // not a JSON integer. + format!("version is {v}; expected a JSON integer") + })?; + + // Lossy narrowing: refuse anything that does not fit in u32. + // u32::MAX is 4_294_967_295; serde_json only goes up to u64. + if n > u32::MAX as u64 { + return Err(format!( + "version {n} does not fit in u32 (max {})", + u32::MAX + )); + } + + let n = n as u32; + if n != 1 { + return Err(format!( + "this client supports cert version 1 only; server returned {n}" + )); + } + Ok(1) +} + async fn resolve_cert_id(client: &NodeClient, owner: &str, name: &str, id: &str) -> Result { if id.len() >= 36 { return Ok(id.to_string()); @@ -397,4 +521,397 @@ mod tests { ); assert!(garbage.is_err(), "malformed signature must not verify"); } + + /// #26 Split PR 3: a missing `version` field on a cert defaults + /// to 1, so an old server's response is forward-compatible with + /// a new client. The v1 verify path is taken, and a + /// well-signed v1 cert round-trips through `verify_signature`. + /// + /// Reviewer 1 finding: the previous shape only parsed the field + /// and never called `verify_signature`, so the test could not + /// fail if the verdict branch was wired to the wrong path. This + /// version signs and verifies the full round-trip. + #[test] + fn missing_version_defaults_to_1_and_verifies() { + let kp = gitlawb_core::identity::Keypair::generate(); + let node_did = kp.did().as_str().to_string(); + let payload = serde_json::json!({ + "repo_id": "repo-1", + "ref": "refs/heads/main", + "old": "0".repeat(40), + "new": "a".repeat(40), + "pusher": "did:key:z6MkPusher", + "node": node_did, + "ts": "2026-07-22T00:00:00+00:00", + }); + let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap()); + + // Simulate the JSON a v1 server returns: no `version` key. + let cert_json = serde_json::json!({ + "id": "test-id", + "repo_id": "repo-1", + "ref_name": "refs/heads/main", + "old_sha": "0".repeat(40), + "new_sha": "a".repeat(40), + "pusher_did": "did:key:z6MkPusher", + "node_did": node_did, + "signature": sig.clone(), + "issued_at": "2026-07-22T00:00:00+00:00", + // no `version` field + }); + + // Forward-compat: the parse yields v1 (the only version this + // client verifies), and the verdict arm matching Ok(1) feeds + // the v1 payload through verify_signature end to end. + assert_eq!(parse_cert_version(cert_json.get("version")).unwrap(), 1); + let verdict = verify_signature( + "repo-1", + "refs/heads/main", + &"0".repeat(40), + &"a".repeat(40), + "did:key:z6MkPusher", + &node_did, + "2026-07-22T00:00:00+00:00", + &sig, + ); + assert!( + verdict.is_ok(), + "missing version must take the v1 verify path: {verdict:?}" + ); + } + + /// #26 Split PR 3: an explicit `version: 1` on a cert is the v1 + /// verify path. A v1 cert with an explicit version verifies the + /// same as a v1 cert without one — round-trip through + /// `verify_signature`. + #[test] + fn explicit_version_1_takes_v1_path() { + let kp = gitlawb_core::identity::Keypair::generate(); + let node_did = kp.did().as_str().to_string(); + let payload = serde_json::json!({ + "repo_id": "repo-1", + "ref": "refs/heads/main", + "old": "0".repeat(40), + "new": "a".repeat(40), + "pusher": "did:key:z6MkPusher", + "node": node_did, + "ts": "2026-07-22T00:00:00+00:00", + }); + let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap()); + + // Simulate a v1 cert with an explicit `version: 1` field. + let cert_json = serde_json::json!({ + "id": "test-id", + "repo_id": "repo-1", + "ref_name": "refs/heads/main", + "old_sha": "0".repeat(40), + "new_sha": "a".repeat(40), + "pusher_did": "did:key:z6MkPusher", + "node_did": node_did, + "signature": sig.clone(), + "issued_at": "2026-07-22T00:00:00+00:00", + "version": 1, + }); + + // The parse arms the Ok(1) verdict branch, which calls + // verify_signature on the v1 payload. A signature mismatch + // here would mean the verdict branch was wired to the + // wrong path or the payload drifted. + assert_eq!(parse_cert_version(cert_json.get("version")).unwrap(), 1); + let verdict = verify_signature( + "repo-1", + "refs/heads/main", + &"0".repeat(40), + &"a".repeat(40), + "did:key:z6MkPusher", + &node_did, + "2026-07-22T00:00:00+00:00", + &sig, + ); + assert!( + verdict.is_ok(), + "explicit version: 1 must take the v1 verify path: {verdict:?}" + ); + } + + /// #26 Split PR 3 + Reviewer 2: parse_cert_version is the + /// load-bearing gate that distinguishes a missing-key (legacy + /// server) from a present-but-malformed value. The four cases + /// below pin every cell of that truth table; collapsing any + /// pair would let a server advertise "version 2" (or 99, or + /// 4294967297, or "two") while the client runs the v1 + /// signature path on a different signed payload — a hostile + /// misconfiguration that `--verify` must not silently accept. + #[test] + fn parse_cert_version_truth_table() { + // MISSING key → legacy v1 (the v1 path is the only one that + // could match pre-versioning bytes). + assert_eq!( + parse_cert_version(None).unwrap(), + 1, + "a missing version field is the legacy v1 path" + ); + + // Explicit 1 → v1. + assert_eq!( + parse_cert_version(Some(&serde_json::json!(1))).unwrap(), + 1, + "explicit version 1 is the v1 path" + ); + + // Explicit 2 → Err, NOT Ok(1). Reviewer 2: a v1 signature + // must not verify against a server that said "version 2". + assert!( + parse_cert_version(Some(&serde_json::json!(2))).is_err(), + "explicit version 2 must not collapse to v1" + ); + + // u32 overflow (2^32 + 1) → Err, NOT Ok(1). The previous + // `.map(|v| v as u32)` silently truncated this to 1. + let overflow = serde_json::json!(u64::from(u32::MAX) + 1); + assert!( + parse_cert_version(Some(&overflow)).is_err(), + "a version that overflows u32 must not truncate to 1" + ); + + // Non-numeric: a string, a float, null, bool — every shape + // that is not a JSON integer is rejected. None of these + // may collapse to v1. + for bad in [ + serde_json::json!("two"), + serde_json::json!(2.0), // serde_json::Value::is_f64 is true for this + serde_json::json!(2.5), // obviously fractional + serde_json::json!(true), + serde_json::json!(null), + serde_json::json!([2]), + serde_json::json!({"v": 2}), + ] { + assert!( + parse_cert_version(Some(&bad)).is_err(), + "non-integer version {bad} must not collapse to v1" + ); + } + } + + /// #26 Split PR 3, round 2: a `version: 2` certificate must fail + /// `--verify` THROUGH `cmd_show` itself, not through a copy of its + /// match. The prior form of this test duplicated the verdict match in + /// the test body and aimed at an `Ok(v != 1)` arm the real parser can + /// never produce, so gutting `cmd_show`'s rejection kept every test + /// green (demonstrated by the reviewer by execution). Driving the + /// command end to end is what makes this a guard: the mock serves a + /// cert whose SIGNATURE IS VALID for v1 bytes but which declares + /// version 2, and the command must exit Err with the version named — + /// the v1 verify path must not launder it through. + #[tokio::test] + async fn cmd_show_verify_rejects_v2_even_with_valid_v1_signature() { + let kp = gitlawb_core::identity::Keypair::generate(); + let node_did = kp.did().as_str().to_string(); + + // Sign a v1-shaped payload so the ONLY disqualifier is the version. + let payload = serde_json::json!({ + "repo_id": "repo-1", + "ref": "refs/heads/main", + "old": "0".repeat(40), + "new": "a".repeat(40), + "pusher": "did:key:z6MkPusher", + "node": node_did, + "ts": "2026-07-22T00:00:00+00:00", + }); + let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap()); + + let cert_json = serde_json::json!({ + "id": "0123456789abcdef0123456789abcdef0123", + "repo_id": "repo-1", + "ref_name": "refs/heads/main", + "old_sha": "0".repeat(40), + "new_sha": "a".repeat(40), + "pusher_did": "did:key:z6MkPusher", + "node_did": node_did, + "signature": sig, + "issued_at": "2026-07-22T00:00:00+00:00", + "version": 2, + }); + + let mut server = mockito::Server::new_async().await; + // 36-char id short-circuits resolve_cert_id; "owner/repo" input + // short-circuits resolve_repo — the cert GET is the only required + // route. `/` may be probed for the contextual DID line; serving it + // keeps the test about the verdict, not about network noise. + let _cert = server + .mock( + "GET", + "/api/v1/repos/o/r/certs/0123456789abcdef0123456789abcdef0123", + ) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(cert_json.to_string()) + .create_async() + .await; + let _root = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(serde_json::json!({ "did": node_did }).to_string()) + .create_async() + .await; + + let err = cmd_show( + "o/r".to_string(), + "0123456789abcdef0123456789abcdef0123".to_string(), + server.url(), + None, + true, + Some(node_did.clone()), + ) + .await + .expect_err("a version-2 cert must fail --verify even with a valid v1 signature"); + let msg = format!("{err:#}"); + assert!( + msg.contains("version"), + "the failure must name the version mismatch, not a generic invalid signature: {msg}" + ); + } + + /// Round 2's second demonstrated escape: with no --expect-node, the old + /// fallback anchored the issuer to the DID the QUERIED NODE reports at + /// `/` — which a forger controls along with the cert, so a wholly forged + /// self-signed certificate passed --verify with exit 0. `cmd_show` with + /// --verify and no --expect-node must now refuse to claim a trusted + /// issuer, even when the served cert's signature is valid and the node's + /// self-report matches it. + #[tokio::test] + async fn cmd_show_verify_without_expect_node_refuses_self_asserted_anchor() { + let kp = gitlawb_core::identity::Keypair::generate(); + let node_did = kp.did().as_str().to_string(); + let payload = serde_json::json!({ + "repo_id": "repo-1", + "ref": "refs/heads/main", + "old": "0".repeat(40), + "new": "a".repeat(40), + "pusher": "did:key:z6MkPusher", + "node": node_did, + "ts": "2026-07-22T00:00:00+00:00", + }); + let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap()); + let cert_json = serde_json::json!({ + "id": "0123456789abcdef0123456789abcdef0123", + "repo_id": "repo-1", + "ref_name": "refs/heads/main", + "old_sha": "0".repeat(40), + "new_sha": "a".repeat(40), + "pusher_did": "did:key:z6MkPusher", + "node_did": node_did, + "signature": sig, + "issued_at": "2026-07-22T00:00:00+00:00", + }); + + let mut server = mockito::Server::new_async().await; + // P3 (reviewer round 2): both mocks are required to be + // hit for the test to mean anything. `mockito 1.7`'s + // `assert_on_drop` is off by default, so the previous + // `_cert` and `_root` bindings would silently let a + // regression that skipped the root fetch pass. + // `.expect(2)` pins each mock's call count (the cert mock + // is hit twice because the test runs `cmd_show` twice) and + // `.assert_async()` at the end fires if any are unmet. + let cert_mock = server + .mock( + "GET", + "/api/v1/repos/o/r/certs/0123456789abcdef0123456789abcdef0123", + ) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(cert_json.to_string()) + .expect(2) + .create_async() + .await; + // The forger's matching self-report — the exact shape + // that used to convert into a trust anchor. The guard's + // whole point is to make the test FAIL if a future + // regression stops calling `/` here: `.expect(2)` + + // `.assert_async()` makes that dependency visible. + let root_mock = server + .mock("GET", "/") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(serde_json::json!({ "did": node_did }).to_string()) + .expect(2) + .create_async() + .await; + + let err = cmd_show( + "o/r".to_string(), + "0123456789abcdef0123456789abcdef0123".to_string(), + server.url(), + None, + true, + None, + ) + .await + .expect_err("--verify without --expect-node must not mint a trust anchor from the node's self-report"); + let msg = format!("{err:#}"); + assert!( + msg.contains("--expect-node"), + "the refusal must tell the caller how to anchor trust: {msg}" + ); + + // Positive control: the same cert with the DID explicitly trusted + // passes, so the guard is about the anchor, not the signature. + cmd_show( + "o/r".to_string(), + "0123456789abcdef0123456789abcdef0123".to_string(), + server.url(), + None, + true, + Some(node_did), + ) + .await + .expect("explicitly anchored --verify of a valid v1 cert must pass"); + + // P3 (reviewer round 2): confirm the mocks were actually + // hit. Without this, a regression that skipped the cert or + // root fetches would still produce a passing test (the + // `expect_err` arm succeeds, the positive control succeeds) + // and the test would mean nothing. + cert_mock.assert_async().await; + root_mock.assert_async().await; + } + + /// P2 (reviewer round 3): a gated `GET /api/v1/repos/{o}/{r}/certs` + /// must surface as an Err, not as "No ref certificates …" with + /// exit 0. The pre-fix shape called `.json()` straight through and + /// let a 404 body that happened to lack `certificates` fall into + /// the empty-list branch, silently claiming an empty repository. + /// Driving `cmd_list` end to end against a 404 mock is what makes + /// this guard load-bearing: removing `error_for_status()` and the + /// test fails. + #[tokio::test] + async fn cmd_list_surfaces_gated_404_as_err() { + let mut server = mockito::Server::new_async().await; + let list_mock = server + .mock("GET", "/api/v1/repos/o/r/certs") + .with_status(404) + .with_header("content-type", "application/json") + // Body that lacks the `certificates` key — this is the + // exact shape the pre-fix code swallowed into the + // empty-list branch. A passing test here therefore proves + // the new error path is taken, not just that 404 produces + // a non-200. + .with_body(r#"{"error":"not found"}"#) + .expect(1) + .create_async() + .await; + + let err = cmd_list("o/r".to_string(), server.url(), None) + .await + .expect_err("a gated 404 on the cert list must surface as an Err, not an empty list"); + let msg = format!("{err:#}"); + assert!( + msg.contains("failed to list certificates"), + "the failure must point at the list endpoint, not at an empty result: {msg}" + ); + + list_mock.assert_async().await; + } }