Complete witness receipts and plurality law hardening#552
Merged
flyingrobots merged 14 commits intoJun 17, 2026
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 32 minutes and 25 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughImplements Goalposts 4 and 5 of the braids/strands hardening roadmap. Adds three new ChangesGP4 + GP5: Witness Receipts, Sealed Membership, and Named Plurality Laws
Sequence Diagram(s)sequenceDiagram
participant Caller
participant WitnessBackendSimulator
participant WitnessReceipt
participant PluralityLawReading
participant BraidShellAudit
Caller->>WitnessBackendSimulator: verify(WitnessRequest::self_witness(subject, evidence))
WitnessBackendSimulator->>WitnessReceipt: self_witness(subject_digest, evidence_digest)
WitnessReceipt-->>WitnessBackendSimulator: receipt(IntegrityOnly, E1Scaffold)
WitnessBackendSimulator-->>Caller: Ok(WitnessReceipt)
Caller->>PluralityLawReading: new(law_ref, shell_digest, witness_receipt, disclosure_budget)
Note over PluralityLawReading: infers EvidencePosture from WitnessKind::SelfWitness + IntegrityOnly
PluralityLawReading-->>Caller: reading with disclosure-budget-tagged digest
Caller->>BraidShellAudit: audit_braid_shell(shell, witness_digest)
Note over BraidShellAudit: computes member DisclosureBudget (Public vs AuthorityScoped)<br/>constructs WitnessReceipt::self_witness<br/>builds PluralityLawReading
BraidShellAudit-->>Caller: BraidShellAudit { law_reading, witness_receipt, member_facts[disclosure_budget] }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/warp-core/src/braid_shell.rs`:
- Around line 1716-1740: The test
replay_audit_labels_sealed_member_disclosure_budget currently only asserts the
member-level disclosure budget via audit.member_facts[0].disclosure_budget but
missing validation of the shell-level disclosure budget. Add an assertion to
check audit.law_reading.disclosure_budget() to ensure the shell-level disclosure
budget is validated and prevent regressions in shell_disclosure_budget function.
The assertion should verify that the shell-level disclosure budget matches the
expected DisclosureBudget value (likely DisclosureBudget::AuthorityScoped).
In `@crates/warp-core/src/plurality_law.rs`:
- Around line 375-399: The digest method of PluralityLawCard calls hash_tag_vec
three times with self.requires, self.emits, and self.conceals, and each call
allocates a temporary Vec<u8> in the hash_tag_vec helper function. To eliminate
these hot-path allocations, refactor the hash_tag_vec helper to accept a Hasher
reference and update it directly without creating intermediate Vec allocations,
then modify all three calls in the digest method to pass the hasher and iterator
directly to the refactored helper instead of having the helper create and return
a Vec.
- Around line 148-154: The settlement_policy function directly constructs a
PluralityLawRef struct instead of using the proper constructor validation
method, which bypasses the non-zero-name guard that exists in the new()
constructor at line 133. This creates an inconsistency where settlement_policy
can create invalid instances (such as with an all-zero policy_id) that would be
rejected by new(). Refactor settlement_policy to use the proper constructor
validation logic instead of directly constructing the struct, ensuring all
invariants are consistently enforced regardless of which function creates the
PluralityLawRef.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a38d0ed6-cb79-4852-93ca-8e49a860a31c
📒 Files selected for processing (11)
CHANGELOG.mdcrates/warp-core/src/braid_shell.rscrates/warp-core/src/lib.rscrates/warp-core/src/plurality_law.rscrates/warp-core/src/sealed_membership.rscrates/warp-core/src/witness.rscrates/warp-core/tests/plurality_law_public_api_tests.rscrates/warp-core/tests/witness_public_api_tests.rsdocs/design/braids-and-strands-hardening/goalpost-04-witness-receipts-and-sealed-capabilities.mddocs/design/braids-and-strands-hardening/goalpost-05-named-plurality-laws.mddocs/design/braids-and-strands-roadmap.md
![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: 0ca9eb2c1c
ℹ️ 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".
Owner
Author
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ca9eb2c1c
ℹ️ 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".
Owner
Author
Activity Summary
Resolved review threads: all 12 thread IDs currently returned by GraphQL are Local validation after the final commit:
Pre-push hooks also reran the configured exact Rust slices and markdown formatting successfully. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
42-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncomplete: GP4 CHANGELOG entry omits the known sealed-membership field-visibility gap.
The entry describes the validation that sealed presentations perform at construction time, which is accurate. However, it does not mention that the P2 severity issue remains unresolved:
SealedMembershipPresentationfields are public and can be mutated after construction, circumventing the validation. The design doc (lines 92–94 of goalpost-04) incorrectly claims fields are read-only post-construction.Consider adding a note like "Remediation of field-visibility constraints is planned as a follow-up" to signal that the implementation is incomplete on the security boundary, or leave this to the design doc to state more clearly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` around lines 42 - 52, The CHANGELOG entry for the warp-core fourth goalpost describes the sealed membership presentation validation but omits mention of the known P2 severity security gap where SealedMembershipPresentation fields are public and can be mutated after construction, circumventing the validation. Add a note to the CHANGELOG entry acknowledging this known issue and indicating that remediation of field-visibility constraints is planned as follow-up work to signal that the security boundary implementation is incomplete.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/warp-core/src/braid_shell.rs`:
- Around line 758-763: The check_policy_id function only validates the main
policy_id parameter but ignores the collapse_policy field in Derived outcomes,
allowing shells with invalid (all-zeros) collapse_policy values to pass
assembly-time validation and fail only during replay. Extend the
check_outcome_law function to validate the collapse_policy field when it is
present in a Derived outcome, ensuring it is not all zeros similar to how
check_policy_id validates the main policy_id, so that invalid shells fail fast
at assembly time rather than during replay/audit operations.
In `@crates/warp-core/tests/witness_public_api_tests.rs`:
- Around line 160-190: The current test
public_sealed_membership_presentation_rejects_unbound_receipts only exercises
the WitnessSubjectMismatch error path of SealedMembershipPresentation::new. Add
a new dedicated test function that creates a WitnessReceipt with a correct
subject digest but an incorrect evidence digest (pass a different value than
evidence_digest() to the self_witness call), then verify that
SealedMembershipPresentation::new returns the
SealedMembershipPresentationError::WitnessEvidenceMismatch variant to ensure
complete error-path coverage.
In
`@docs/design/braids-and-strands-hardening/goalpost-04-witness-receipts-and-sealed-capabilities.md`:
- Around line 92-94: The design document claims that fields in
SealedMembershipPresentation are read-only after construction, but this is
factually incorrect since the current implementation exposes all fields as pub
in sealed_membership.rs, allowing direct mutation after construction. Either
update the document to accurately describe the current implementation state
(public mutable fields with a validation closure gap) or explicitly reference
the planned remediation work that will address this security issue through
private fields and read-only accessors. Do not assert read-only semantics that
do not currently exist in the code.
---
Outside diff comments:
In `@CHANGELOG.md`:
- Around line 42-52: The CHANGELOG entry for the warp-core fourth goalpost
describes the sealed membership presentation validation but omits mention of the
known P2 severity security gap where SealedMembershipPresentation fields are
public and can be mutated after construction, circumventing the validation. Add
a note to the CHANGELOG entry acknowledging this known issue and indicating that
remediation of field-visibility constraints is planned as follow-up work to
signal that the security boundary implementation is incomplete.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f75943ab-8448-4d5e-93a1-cc29fe36f0ac
📒 Files selected for processing (10)
CHANGELOG.mdcrates/warp-core/src/braid_shell.rscrates/warp-core/src/lib.rscrates/warp-core/src/plurality_law.rscrates/warp-core/src/sealed_membership.rscrates/warp-core/src/witness.rscrates/warp-core/tests/plurality_law_public_api_tests.rscrates/warp-core/tests/witness_public_api_tests.rsdocs/design/braids-and-strands-hardening/goalpost-04-witness-receipts-and-sealed-capabilities.mddocs/design/braids-and-strands-hardening/goalpost-05-named-plurality-laws.md
Owner
Author
Self-Code Review Findings@codex please confirm these findings against
Checks run during review:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd18b984cb
ℹ️ 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".
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/warp-core/src/plurality_law.rs (1)
521-530: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
PluralityLawAuthorizationpublic fields allow bypassing registry validation.External code can construct this directly without calling
registry.authorize(), creating forged authorization evidence. Same pattern as the P2 issue flagged forSealedMembershipPresentationin this PR.If the remediation plan (private fields + read-only accessors) applies here too, consider bundling it.
Proposed encapsulation
/// Successful law authorization evidence. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct PluralityLawAuthorization { - /// Authorized law reference. - pub law_ref: PluralityLawRef, - /// Digest of the registered Law Card. - pub card_digest: Hash, - /// Authority that authorized execution, if any. - pub authorized_by: Option<AuthorityDomainRef>, + law_ref: PluralityLawRef, + card_digest: Hash, + authorized_by: Option<AuthorityDomainRef>, +} + +impl PluralityLawAuthorization { + pub const fn law_ref(&self) -> PluralityLawRef { self.law_ref } + pub const fn card_digest(&self) -> Hash { self.card_digest } + pub const fn authorized_by(&self) -> Option<AuthorityDomainRef> { self.authorized_by } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/warp-core/src/plurality_law.rs` around lines 521 - 530, The public fields on the PluralityLawAuthorization struct (law_ref, card_digest, and authorized_by) allow external code to construct valid-looking authorization evidence without going through the registry authorization process. Make these three fields private instead of public, and add public getter methods for each field that return the same types they currently expose. This ensures the struct can only be properly instantiated through the intended registry.authorize() code path, preventing forged authorization evidence from being created by external callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/warp-core/src/plurality_law.rs`:
- Around line 521-530: The public fields on the PluralityLawAuthorization struct
(law_ref, card_digest, and authorized_by) allow external code to construct
valid-looking authorization evidence without going through the registry
authorization process. Make these three fields private instead of public, and
add public getter methods for each field that return the same types they
currently expose. This ensures the struct can only be properly instantiated
through the intended registry.authorize() code path, preventing forged
authorization evidence from being created by external callers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2250b624-a5bc-4a38-86ef-6219d88df28e
📒 Files selected for processing (5)
CHANGELOG.mdcrates/warp-core/src/braid_shell.rscrates/warp-core/src/lib.rscrates/warp-core/src/plurality_law.rscrates/warp-core/tests/plurality_law_public_api_tests.rs
Owner
Author
Activity Summary@codex review-thread remediation complete against
Review-thread state: all four remaining threads resolved via GraphQL; a fresh thread inventory returns Verification:
|
Summary
Testing / Checks
|
Owner
Author
Activity SummaryCode Lawyer re-audit complete against
Review inventory:
Verification:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the remaining braids/strands hardening roadmap slices for Goalposts 4 and 5.
Self-Review Fixes Included
WitnessError::UnsupportedCompatibilityinstead of minting stable public identity for scaffolding evidence.PluralityLawRef::new(...)rejects all-zero law names and zero versions before registration.Validation
cargo test -p warp-core --libcargo test -p warp-core --test witness_public_api_testscargo test -p warp-core --test plurality_law_public_api_testscargo fmt --all -- --checkcargo clippy -p warp-core --lib -- -D warningscargo clippy -p warp-core --test witness_public_api_tests -- -D warningscargo clippy -p warp-core --test plurality_law_public_api_tests -- -D warningspnpm exec markdownlint-cli2 CHANGELOG.md docs/design/braids-and-strands-roadmap.md docs/design/braids-and-strands-hardening/goalpost-04-witness-receipts-and-sealed-capabilities.md docs/design/braids-and-strands-hardening/goalpost-05-named-plurality-laws.mdscripts/check_spdx.shscripts/check-no-app-nouns-in-core.shgit diff --check origin/main...HEADSummary by CodeRabbit