Substrate catch-up to the Server 0.6 triple, and CC 1.0-rc5 licensure conformance - #140
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4679dd3f28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let row: Option<(uuid::Uuid,)> = sqlx::query_as( | ||
| "SELECT org_id FROM organizations WHERE partner_id = $1 \ | ||
| ORDER BY created_at ASC, org_id ASC LIMIT 1", | ||
| ) | ||
| .bind(uuid) |
There was a problem hiding this comment.
When a UUID-shaped partner has a linked organization, this lookup cannot return it successfully: migration 001_initial_schema.sql defines both organizations.partner_id and organizations.org_id as PostgreSQL TEXT, but the query binds a uuid::Uuid and attempts to decode the selected TEXT as uuid::Uuid. PostgreSQL/SQLx will report a parameter or column type mismatch; partner_composition catches that error and silently emits an empty licensure array for every affected partner. Bind partner_id as &str and decode the row as String.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and already fixed on the current head. This review ran against 4679dd3; 10d14f2 binds partner_id as text and decodes org_id as String, and the emit site now checks the value parses as a UUID before publishing it as an authority_id (a non-UUID org id is withheld with a warning).
The same defect was found independently by booting the build against Postgres 16 and seeding partners: every licensure array came back empty. After the fix, seeded ACTIVE / SUSPENDED / REVOKED partners serve a UUID authority and three distinct statuses.
…d into revoked (CC 1.0-rc5)
CC 1.0-rc5 ratified the authority triad (CC 2.4.1.2.1, from CIRISConstitution#100)
and, in doing so, named this Registry twice. Both citations were defects here.
## authority_id was the tax id
part_3 §3.3.9: when a licensing authority is an organisation, the `authority_id`
segment of `licensure:{authority_id}` is its `org_id` — the UUID
`org_membership.org_id` joins on, through which the organisation's keys resolve —
and explicitly NOT "the legal registration number the reference Registry's
`PartnerRecord.organization_id` carries (jurisdiction-scoped, mutable,
PII-adjacent)."
`/v1/partner/{key_id}` emitted exactly that: `partners.organization_id`, which
the schema comments as "Tax ID / Registration number". So a public, unauthenticated
surface was publishing a tax id AND naming the wrong thing as the licence's
provenance.
The correct id was one join away and unused: `organizations.org_id`, reachable
via `organizations.partner_id REFERENCES partners(partner_id)` (covered by
`idx_organizations_partner`). New `db::org_id_for_partner` does that reverse
join. The column is indexed but not unique, so it takes the oldest by
`created_at` with an `org_id` tiebreak — an authority_id that flipped between
rows would be worse than a wrong one: unstable licence provenance.
No org row now yields NO licence entry rather than a guess. Emitting a licence
whose authority we cannot name asserts provenance we do not have; absent is both
honest and fail-secure.
## suspended and revoked were the same string
rc5: "`suspended` is reversible; `revoked` is terminal — the one operational
distinction a consumer MUST honour, and a status a consumer MUST NOT infer from
the other."
The mapping was `if status == 1 { "active" } else { "inactive" }`, collapsing
PARTNER_SUSPENDED(2) and PARTNER_REVOKED(3) into one token. A consumer reading
`"inactive"` could not tell whether a partner may return — the exact inference
rc5 forbids, destroyed at the emitter. `"active"` was also outside the canonical
vocabulary.
Now: 1 → `issued`, 2 → `suspended`, 3 → `revoked`. UNSPECIFIED(0) and any
unrecognised code map to a deliberately NON-canonical `"unspecified"`, not to
`issued`: the row exists so the licence is not absent, but nothing licensed it,
and a consumer that does not recognise the token withholds — the correct
outcome. Mapping unknown to `issued` would escalate "we do not know" to "valid".
Three tests pin both rules, each of which regressed once: suspended and revoked
are distinguishable; emitted statuses are canonical or deliberately not, and
unknown never borrows a canonical one; authority_id parses as a UUID while the
tax-id shapes it used to carry do not.
Green: 111 lib + 19 crypto_properties + 26 capability_properties.
Refs CIRISConstitution#100, CC 2.4.1.2.1, CC part_3 §3.3.9
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Arv1jQhQPe5Ab92mJfyLLG
…kup binds TEXT (#139) Found by booting against a fresh Postgres 16 and seeding partners. ebe637a added responsible_party and public_contact_email to every partner query but put the ALTER only in the legacy database/migrations/ tree, never in the sqlx tree the binary runs. On a database this binary migrated itself, lookup_partner fails with 'column responsible_party does not exist': gRPC LookupPartner errors and /v1/partner/{key_id} serves an empty composition, because the handler discards the error. Migration 031 adds both columns with ADD COLUMN IF NOT EXISTS, so a database that already has them is untouched. The same boot showed org_id_for_partner was wrong as first written: it bound and decoded UUIDs, but organizations.org_id and .partner_id are TEXT in the schema this binary migrates (the UUID types are the legacy tree's). It now binds text, and the emit site checks the org id parses as a UUID before publishing it as an authority_id; a non-UUID value is withheld with a warning. Exercised end to end over seeded ACTIVE / SUSPENDED / REVOKED partners: UUID authority, three distinct statuses, no tax id in any response, and 031 applied over existing rows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VhhR4ntuczmABtdh6RifqF
…218 substrate)
Substrate to CIRISServer 0.5.218's exact triple: persist v52.0.1, edge v38.1.0,
verify v18.0.0. The set is coherent (edge pins persist 52.0.1, both pin verify
18.0.0), and the lock carries one of each. No source change was needed for the
bump; the one new upstream field (Revocation) is on a type this crate never
constructs.
CEG-Version header 1.0-rc4 -> 1.0-rc5. rc6 is open upstream but not cut, so
this claims the last released text, whose authority triad the licensure
surface implements.
MAJOR because /v1/partner/{key_id} changes on the wire (authority_id value,
status vocabulary) and the persist migrations through V166 set a rollback
floor: a v3.0.0 image will not boot against a database this release migrated.
Verified: 117 lib + 19 + 26 + 3 fold_router; the no-default-features fold build
passes with sqlx / tonic / tokio-postgres absent; booted against Postgres 16
with persist on Postgres (161 migrations to V166, sqlx to 031, 6/6 identity
keys).
Closes #139
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VhhR4ntuczmABtdh6RifqF
4679dd3 to
f7d80a6
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Two commits, independently reviewable. The first is routine; the second fixes a defect the constitution named this Registry for.
4fc9c96— substrate catch-uppersist
v40.0.0 → v42.1.0, edgev20.1.1 → v21.1.0, verifyv14.1.0 → v15.0.0.This is CIRISServer 0.5.204's exact triple, which is the point: registry-core folds into that workspace, so a divergence here becomes a duplicate substrate rev in the composed build. The set is internally coherent — edge 21.1.0 pins persist 42.1.0, and edge and persist both pin the verify family at 15.0.0 — so there is one verify in the graph, not two. Confirmed in the lockfile: crypto, keyring and verify-core all resolve to exactly 15.0.0.
Zero source changes.
KeyRecord/Attestation/Revocation/ScrubSigare field-identical across the gap, and every symbol this crate imports survives (put_attestation,list_attestations_by,list_keys_by_identity_type,lookup_public_key,federation_directory,ThresholdMemberwith fields unchanged,ThresholdSignature,verify_threshold_signatures,jcs::canonicalize).verify v15.0.0 is a major — FedCode becomes
non_exhaustive, admission unconstructible unless verified (CIRISVerify#274). Registry consumes neither, so the break does not reach us. MSRV stays 1.86.Re-verified the two pins that bite in production rather than CI: a single
libsqlite3-sys 0.28(thebundledunion holds), and no persist federation type is serialized into an HTTP response.4679dd3— CC 1.0-rc5 licensure conformancerc5 ratified the authority triad (CC 2.4.1.2.1, from CIRISConstitution#100) and named this Registry twice while doing it. Both citations were live defects on
GET /v1/partner/{key_id}— a public, unauthenticated surface.authority_idwas the tax idpart_3 §3.3.9 rules the
authority_idis the organisation'sorg_idUUID, and explicitly not "the legal registration number the reference Registry'sPartnerRecord.organization_idcarries (jurisdiction-scoped, mutable, PII-adjacent)."The emit site was
authority_id: row.organization_id.clone()— the column the schema comments asTax ID / Registration number. So the surface published a tax id and named the wrong thing as the licence's provenance.The correct id was one join away and unused:
organizations.org_id, reachable throughorganizations.partner_id REFERENCES partners(partner_id), covered byidx_organizations_partner. Newdb::org_id_for_partnerdoes that reverse join, taking the oldest row bycreated_atwith anorg_idtiebreak — the column is indexed but not unique, and anauthority_idthat flipped between rows would be worse than a wrong one: unstable licence provenance.No org row now yields no licence entry rather than a guess. Emitting a licence whose authority we cannot name asserts provenance we do not have.
suspendedandrevokedwere the same stringrc5: "
suspendedis reversible;revokedis terminal — the one operational distinction a consumer MUST honour, and a status a consumer MUST NOT infer from the other."The mapping was
if status == 1 { "active" } else { "inactive" }, collapsingPARTNER_SUSPENDED(2)andPARTNER_REVOKED(3)into one token. A consumer reading"inactive"could not tell whether a partner may return — the exact inference rc5 forbids, destroyed at the emitter."active"was also outside the canonical vocabulary.Now
1 → issued,2 → suspended,3 → revoked.UNSPECIFIED(0)and unrecognised codes map to a deliberately non-canonical"unspecified"rather thanissued: the row exists so the licence is not absent, but nothing licensed it, and a consumer that does not recognise the token withholds. Mapping unknown toissuedwould escalate "we do not know" to "valid".⚠ Wire change
/v1/partner/{key_id}→licensure[].authority_idchanges value (tax id → org UUID) andlicensure[].statuschanges vocabulary (active/inactive→issued/suspended/revoked/unspecified). Both are the fix; anything keyed on the old strings sees new ones. A partner with no organisation row now returns an emptylicensure[].Three tests pin the rules, each of which regressed once: suspended and revoked are distinguishable; emitted statuses are canonical or deliberately not, and unknown never borrows a canonical one;
authority_idparses as a UUID while the tax-id shapes it used to carry do not.Green: 111 lib + 19 crypto_properties + 26 capability_properties.
Refs CIRISConstitution#100, #76, #62
🤖 Generated with Claude Code
https://claude.ai/code/session_01Arv1jQhQPe5Ab92mJfyLLG