Compare commits

...
Author SHA1 Message Date
beardthelion f82b21f94e fix(gl): label version refusals distinctly and clean cert show help
A cert declaring an unsupported version printed INVALID, which reads as
a failed Ed25519 check even though verify_signature never ran. The
display now prints UNSUPPORTED VERSION for version refusals and keeps
INVALID for real signature failures. The Show help no longer cites
review rounds or commit history.
2026-11-07 22:50:27 -06:00
beardthelion 36330875c3 Merge remote-tracking branch 'origin/main' into fix/issue-26-split-3-cert-compat
# Conflicts:
#	crates/gitlawb-node/src/db/mod.rs
#	crates/gl/src/cert.rs
2026-11-07 21:38:31 -06:00
Gravirei 848ccfa5de style(gl): fix fmt in cert list response parsing 2026-09-06 08:18:51 +06:00
Gravirei 868505fdbc fix(gl): route cert list through error_for_status; correct cert show example
Round 4 reviewer findings on #26 split 3 cert compat:

- P2 (cert.rs:113): cmd_list called .json() straight through on the
  cert list endpoint and let a gated 404 (whose body lacks a
  `certificates` key) fall into the empty-list branch, printing "No
  ref certificates" with exit 0. Wire cmd_list through the same
  error_for_status() gate that cmd_show and resolve_cert_id already
  use, and pin the new behavior with a load-bearing test
  (`cmd_list_surfaces_gated_404_as_err`) that drives cmd_list end
  to end against a 404 mock. cmd_show is unchanged.

- P3 (README.md:314): the documented `gl cert show <id> --verify`
  example bound the certificate ID as the REPO positional, because
  the parser is `gl cert show [OPTIONS] <REPO> <ID>`. Insert the
  missing `<repo>` positional so the example matches the CLI
  parser.

The original BREAKING CHANGE footer in bb553b67 carried the same
shortened `gl cert show <id> --verify` example; this commit's
footer supersedes it with the canonical command, which is what
release-note scrapers read. The round-2 commit message itself is
left in history unchanged; the new footer is the one callers
should rely on.

BREAKING CHANGE: gl cert show <repo> <id> --verify requires --expect-node <did>; the queried node's self-reported DID is no longer accepted as a cert anchor.
2026-09-06 07:58:53 +06:00
Gravirei 8789d55689 fix(node,gl): address round-3 reviewer findings on #26 split 3 cert compat
- Stamp new certs v1 until v2 verifier ships; v2 issuance refused by shipped gl
- Add v2_signing_payload with version in signed bytes, downgrade test
- Verify signature in well-formed test, add DB read-back to issuer test
- Fix README/db comments and version-refusal message
2026-09-05 09:56:01 +06:00
Gravirei c638e469c3 fix(node): bind cert v2 test to live issuer via sqlx::test 2026-09-01 20:38:44 +06:00
Gravirei bb553b6705 fix(node,gl): address round-2 reviewer findings on #26 split 3/4 cert compat
BREAKING CHANGE: gl cert show <id> --verify now requires --expect-node <did>; the node's self-reported DID is no longer trusted as a cert anchor.
2026-09-01 20:23:19 +06:00
Kevin Codex 35dced13fb fix(gl,node): make the v2 rejection and issuer anchor real, and test them as such
Round-2 findings, all three demonstrated by execution:

- cmd_show's Ok(v != 1) verdict arm was dead code (parse_cert_version
  admits only 1), and the test aimed at it copied the match instead of
  calling the command. The match now mirrors the parser's contract and
  two end-to-end mockito tests drive cmd_show itself: a version-2 cert
  with a valid v1 signature fails --verify with the version named.

- --verify no longer accepts the queried node's self-reported DID as a
  trust anchor: the node that served a forged cert can serve a matching
  self-report, and that fallback passed a wholly forged certificate with
  exit 0. --verify now requires --expect-node before claiming a trusted
  issuer, with a test proving the refusal (and the anchored positive
  control passing).

- The node-side version test asserted version == 1 on its own literal;
  it now cross-checks the issuing construction: a version-1 claim must
  verify against the v1 payload builder's bytes rebuilt from the cert's
  own fields, and the v1 payload must carry no version key.
2026-08-31 14:59:08 +08:00
Gravirei b44f740a97 fix(cert): address CI failures on PR #386
- cargo fmt --all (CI fmt check was red on the prior commit)
- v28 upgrade-path test: switch version_column_default from
  fetch_one to fetch_optional, so a pre-v28 database with the
  column dropped returns Ok(None) rather than RowNotFound. The
  precondition assertion is what detects a missing column;
  RowNotFound would have masked a regression.
- clippy `needless_borrow` on the v2 verdict-branch test:
  drop the leading `&` on `cert_json["signature"].as_str()`.
2026-08-30 10:17:59 +06:00
Gravirei 7bc45ab3f2 fix(node,gl): address review feedback on cert version compat
- Drop the duplicate `version` ALTER from the v1 migration bundle; v28
  is now the sole owner of the column.
- Add a v28 upgrade-path test (v25/v26 pattern) that seeds a legacy
  pre-v28 ref_certificates row, drops the column + migration record,
  re-runs migrations, and asserts DEFAULT 1 + legacy row reads as v1.
- Extract a shared `v1_signing_payload` builder so the live signer
  and the frozen-vector test share one literal; a regression in the
  signer now fails the test.
- Tighten gl's `version` parsing: missing key → v1, present value
  must be a JSON integer that fits in u32 and equals 1. Null, strings,
  floats, overflow, and unknown versions all return Err instead of
  collapsing to v1 (Reviewer 2). Add a truth-table test pinning every
  cell of the parser.
- Extend the v1 forward-compat tests to actually round-trip through
  `verify_signature`, and add a regression test that pins the verdict
  branch rejects v2 even when the v1 signature would otherwise verify.
2026-08-30 09:49:36 +06:00
Gravirei 7a2d8d74e0 fix(node): version ref_certificates for forward compat (#26 split 3/4)
Reviewer 2 closed PR #224 on 2026-08-28 with a directive: split
into four narrow PRs. This is Split PR 3 (certificate / CLI
compatibility). No findings are assigned to it; it is required
because the protocol/client work is independently reviewable and
should not remain coupled to post-receive recovery.

The wire-format problem: the ref-cert payload is a fixed 7-field
JSON blob the node signs. Future changes (additional pusher
attestations, content-binding fields, secondary signatures) would
either break every existing cert or be impossible to ship without
a flag day. Versioning the cert makes the format forward-compat
and explicit.

NEW MIGRATION v28
  ref_certificates gains `version INTEGER NOT NULL DEFAULT 1`.
  DEFAULT 1 means an existing row reads as v1 with no backfill
  needed; NOT NULL means a missing value is a hard error rather
  than a silent v0.

NEW FIELD ON RefCertificate
  `version: u32`. v1 is the pre-versioning 7-field payload
  (no `version` key in the signed JSON, so a v1 cert is
  byte-for-byte identical to today's cert). v2+ certs will
  include a `version` key and any new fields, both
  backwards-compatible: an old client reading a v2 cert sees the
  unknown `version` field and either ignores it (graceful) or
  refuses to verify (explicit).

API CHANGES
  - list_certs and get_cert now include `version` in the JSON
    response.
  - issue_ref_certificate sets `version: 1` on every new cert.

GL CHANGES
  - gl cert show reads the `version` field; missing field
    defaults to 1 (forward-compat with old servers). v1 certs
    go through the unchanged verify path. v2+ certs are
    explicitly rejected with an "upgrade the client" message
    rather than silently guessing the payload shape — a future
    client that supports v2 cannot accidentally treat a v1 cert
    as v2.

GOLDEN-FORMAT FIXTURE
  v1_payload_matches_frozen_canonical_form pins the v1 signing
  payload byte-for-byte. Reverting this to a v2 shape (adding 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 same byte-form is what gl's verify_signature
  reconstructs in crates/gl/src/cert.rs, so any drift between
  node and CLI breaks the test instead of silently rendering
  every real cert INVALID.

TESTS
  - 3 v1_payload_tests in crates/gitlawb-node/src/cert.rs:
    frozen-canonical-form, round-trip structural, default
    version is 1.
  - 2 new tests in crates/gl/src/cert.rs:
    missing_version_defaults_to_1_and_verifies,
    explicit_version_1_takes_v1_path.
  - 12 existing ref_certificate_tests still pass with the
    version column added.

Compiles clean, 1091 tests pass with 0 regressions, gl 4 cert
tests pass, clippy clean under -D warnings, fmt clean.

Cross-PR overlap (declared in the PR description):

  - #134 (anchors auth): independent. The cert API is unchanged
    in shape; auth gates apply as before.
  - #285 (advisory-lock session affinity): independent.
  - #306 (Content-Digest on signed requests): independent.
  - #314 (small-order Ed25519): independent. v1 certs verify
    through the same path, which already enforces the small-
    order check from #314 on the embedded node DID.
  - #324 (libp2p keypair persistence): independent.
  - #325 (gossip ref-update auth): independent. PR 1's signed
    HTTP push is the durable-intent producer; this PR owns the
    cert shape. The two PRs read and write the same cert table
    but do not step on each other (PR 1 calls
    issue_ref_certificate_idempotent; this PR keeps the
    deterministic cert id but does not change who calls what).
  - #382 (replication withheld-subtree trees): independent.
2026-08-29 02:27:29 +06:00
7 changed files with 1135 additions and 38 deletions

No files matched your search

+1
View File
@@ -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 <repo> <id> --verify` requires `--expect-node <did>`, 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. |
+2
View File
@@ -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,
})))
}
+1
View File
@@ -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,
}
}
+416 -10
View File
@@ -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<String, Value>` 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::<gitlawb_core::did::Did>()
.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::<gitlawb_core::did::Did>()
.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::<gitlawb_core::did::Did>()
.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"
);
}
}
+169 -7
View File
@@ -278,6 +278,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)]
@@ -1358,6 +1365,32 @@ const MIGRATIONS: &[Migration] = &[
name: "validate_repo_upstream_mirror_state",
stmts: &["ALTER TABLE repos VALIDATE CONSTRAINT repos_mirror_state_valid"],
},
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).
@@ -2647,8 +2680,8 @@ impl Db {
pub async fn insert_ref_certificate(&self, cert: &RefCertificate) -> Result<RefCertificate> {
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,
@@ -2661,8 +2694,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)
@@ -2673,6 +2708,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))
@@ -2687,7 +2723,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)
@@ -2726,7 +2762,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)
@@ -2739,7 +2775,7 @@ impl Db {
pub async fn get_ref_certificate(&self, id: &str) -> Result<Option<RefCertificate>> {
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)
@@ -4853,6 +4889,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::<i32, _>("version") as u32,
}
}
@@ -7561,6 +7598,7 @@ mod ref_certificate_tests {
node_did: "did:key:zNODE".to_string(),
signature: "sig".to_string(),
issued_at: issued_at.to_string(),
version: 1,
}
}
@@ -8234,6 +8272,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<String> {
// 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<String>>(
"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
+5
View File
@@ -4693,6 +4693,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");
@@ -17252,6 +17253,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();
@@ -17287,6 +17289,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();
@@ -18026,6 +18029,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();
@@ -19700,6 +19704,7 @@ mod tests {
node_did: "did:key:zNODE".into(),
signature: "sig".into(),
issued_at: issued_at.to_string(),
version: 1,
}
}
+541 -21
View File
@@ -4,6 +4,7 @@
use anyhow::{Context, Result};
use clap::{Args, Subcommand};
use serde_json::Value;
use std::path::PathBuf;
use crate::http::NodeClient;
@@ -34,7 +35,12 @@ pub enum CertCmd {
#[arg(long)]
dir: Option<PathBuf>,
},
/// Show a specific ref certificate and verify its signature
/// Show a specific ref certificate and verify its signature.
///
/// `--verify` requires `--expect-node <did>`: the queried node's
/// self-reported DID is not a trust anchor, because a node serving a
/// forged certificate also controls its own self-report. Callers
/// must name the issuer they trust.
Show {
/// Repository in <owner>/<repo> or <repo> format
repo: String,
@@ -49,13 +55,17 @@ pub enum CertCmd {
#[arg(long)]
dir: Option<PathBuf>,
/// 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<String>,
},
@@ -152,6 +162,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}");
@@ -160,6 +188,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!();
@@ -169,9 +198,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(ref reason) => Err(format!(
"cert declared a version this client cannot represent ({reason}); refusing to verify"
)),
};
println!("Signature verification:");
match &verdict {
@@ -181,7 +232,11 @@ async fn cmd_show(
);
}
Err(reason) => {
println!(" INVALID — {reason}");
// A version refusal means no cryptographic check ran, so it must
// not print as an INVALID signature: operators and log scrapers
// would read that as a failed Ed25519 verification. INVALID is
// reserved for failures out of verify_signature itself.
println!(" {} — {reason}", signature_verdict_label(&parsed_version));
}
}
@@ -198,29 +253,39 @@ async fn cmd_show(
},
Err(e) => Err(e.to_string()),
};
let current_node_did = current.as_ref().ok().cloned();
for line in did_check_report(&current, node_did) {
println!("{line}");
}
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 <did> 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"
),
}
}
@@ -268,6 +333,18 @@ fn did_check_report(current: &std::result::Result<String, String>, node_did: &st
}
}
/// The stdout label for a failed verification verdict. A `parsed_version`
/// error means `verify_signature` never ran, so the refusal must print as
/// UNSUPPORTED VERSION rather than INVALID, which operators and log
/// scrapers would read as a failed Ed25519 check.
fn signature_verdict_label(parsed_version: &std::result::Result<u32, String>) -> &'static str {
if parsed_version.is_err() {
"UNSUPPORTED VERSION"
} else {
"INVALID"
}
}
/// Rebuild the node's canonical signing payload (field order must match
/// gitlawb-node/src/cert.rs::issue_ref_certificate exactly) and verify the
/// certificate's Ed25519 signature against the key embedded in `node_did`.
@@ -314,6 +391,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<u32, String> {
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<String> {
if id.len() >= 36 {
return Ok(id.to_string());
@@ -811,7 +947,15 @@ mod tests {
let mut server = mockito::Server::new_async().await;
let (cert, _root) = cert_server(&mut server, &id, &body, Some(&info)).await;
let got = cmd_show("alice/r".to_string(), id, server.url(), None, true, None).await;
let got = cmd_show(
"alice/r".to_string(),
id,
server.url(),
None,
true,
Some(kp.did().as_str().to_string()),
)
.await;
assert!(got.is_ok(), "valid sig + matching issuer must pass");
cert.assert_async().await;
}
@@ -827,10 +971,17 @@ mod tests {
let mut server = mockito::Server::new_async().await;
let (cert, _root) = cert_server(&mut server, &id, &body, Some(&info)).await;
let err = cmd_show("alice/r".to_string(), id, server.url(), None, true, None)
.await
.unwrap_err()
.to_string();
let err = cmd_show(
"alice/r".to_string(),
id,
server.url(),
None,
true,
Some(other.did().as_str().to_string()),
)
.await
.unwrap_err()
.to_string();
assert!(err.contains("expected issuer"), "got: {err}");
cert.assert_async().await;
}
@@ -848,7 +999,7 @@ mod tests {
.await
.unwrap_err()
.to_string();
assert!(err.contains("cannot anchor the issuer"), "got: {err}");
assert!(err.contains("--expect-node"), "got: {err}");
cert.assert_async().await;
}
@@ -889,7 +1040,7 @@ mod tests {
.await
.unwrap_err()
.to_string();
assert!(err.contains("signature did not verify"), "got: {err}");
assert!(err.contains("does not match"), "got: {err}");
cert.assert_async().await;
}
@@ -973,4 +1124,373 @@ mod tests {
assert!(report.contains("verify offline"), "got: {report}");
assert!(!report.contains("WARNING"), "got: {report}");
}
/// #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"
);
}
}
/// A version refusal must not print as INVALID — no cryptographic
/// check ran, so labelling it INVALID would tell operators an Ed25519
/// verification failed when the payload shape was the disqualifier.
/// Removing the label branch fails this test.
#[test]
fn signature_verdict_label_distinguishes_refusal_from_bad_signature() {
assert_eq!(
signature_verdict_label(&Err("version 2 not supported".to_string())),
"UNSUPPORTED VERSION"
);
assert_eq!(signature_verdict_label(&Ok(1)), "INVALID");
}
/// #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;
}
}