mirror of
https://github.com/Twigpine/node.git
synced 2026-11-09 05:20:05 -10:00
38bab4191b
* 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.
* 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.
* 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()`.
* 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.
* 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.
* fix(node): bind cert v2 test to live issuer via sqlx::test
* 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
* 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.
* style(gl): fix fmt in cert list response parsing
* 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.
* docs(node): align RefCertificate.version rustdoc with the signed-bytes v2 contract
---------
Co-authored-by: Gravirei <[email protected]>
Co-authored-by: Kevin Codex <[email protected]>