fix(node): harden Arweave anchoring and add verification (#26)#224
fix(node): harden Arweave anchoring and add verification (#26)#224Gravirei wants to merge 9 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change replaces Irys uploads with bundler requests, embeds chained ref certificates and pusher signature metadata, adds anchor lifecycle persistence, and exposes gateway-based Arweave transaction verification. ChangesArweave integrity flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant git_receive_pack
participant issue_ref_certificate
participant Bundler
participant verify_anchor_endpoint
participant verify_anchor
participant ArweaveGateway
participant Db
Client->>git_receive_pack: Authenticated push request
git_receive_pack->>issue_ref_certificate: Pass pusher signature and proof
issue_ref_certificate->>Db: Store chained certificate
git_receive_pack->>Bundler: POST /v1/tx with certificate anchor
Bundler-->>git_receive_pack: Return transaction ID
Client->>verify_anchor_endpoint: GET /api/v1/arweave/verify/{tx_id}
verify_anchor_endpoint->>verify_anchor: Verify transaction ID
verify_anchor->>ArweaveGateway: GET gateway/{tx_id}
ArweaveGateway-->>verify_anchor: Return anchored payload
verify_anchor->>Db: Load predecessor certificate
verify_anchor-->>Client: Return validity, errors, and certificate
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
beardthelion
added
crate:node
0801800 to
bd09c35
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/gitlawb-node/src/arweave.rs`:
- Around line 351-373: Update the prev-linkage validation in verify_anchor to
fetch and hash the local certificate at sequence c.seq - 1, rather than the
newest certificate returned by get_most_recent_cert. Only perform the comparison
when that predecessor exists, while preserving the existing mismatch error
handling and payload hashing behavior.
- Around line 319-343: Update the signature decoding in the certificate
verification flow to use the URL-safe, no-padding base64 engine matching
node_keypair.sign_b64 output, while preserving the existing 64-byte validation
and error-result handling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fee43d49-2e4b-4bbd-9921-8248f57fea48
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
crates/gitlawb-node/src/api/arweave.rscrates/gitlawb-node/src/api/events.rscrates/gitlawb-node/src/api/repos.rscrates/gitlawb-node/src/arweave.rscrates/gitlawb-node/src/auth/mod.rscrates/gitlawb-node/src/cert.rscrates/gitlawb-node/src/config.rscrates/gitlawb-node/src/db/mod.rscrates/gitlawb-node/src/server.rscrates/gitlawb-node/src/test_support.rs
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Use the configured gateway's data URL when verifying an anchor
crates/gitlawb-node/src/arweave.rs:265
arweave_gatewaydefaults tohttps://arweave.net, but this requests/v1/tx/{id}, which is the bundler API used for uploads rather than an Arweave gateway data URL. Consequently every normally uploaded anchor is reported invalid with the documented default configuration. Fetch the item through the gateway's data path (or add a separately named bundler-read configuration), and cover the default configuration rather than a mock of the bundler path. -
[P1] Bind each anchor to the certificate for its own ref update
crates/gitlawb-node/src/api/repos.rs:1306
Certificates are issued once per update above this block, but every iteration subsequently reads the repository-wide latest certificate. In a multi-ref push, all permanent anchors therefore embed the last update's certificate; another completed push can also win the asynchronous race. Sinceverify_anchornever compares the certificate's repo/ref/old/new fields with the outer anchor, it returns valid for an anchor whose advertised transition was never signed. Preserve the returned certificate per update and reject a mismatch during verification. -
[P1] Preserve certificate history and fail closed on a missing predecessor
crates/gitlawb-node/src/db/mod.rs:2013
The existing(repo_id, ref_name)upsert overwrites the predecessor whenever that ref is pushed again, while the newseq/prevdesign requires that predecessor to remain available.verify_anchorthen silently skips the check whenget_cert_by_seqreturnsNoneor errors, so an ordinary repeated push produces a truncated chain reported as valid. Make chain entries append-only and treat an unavailable declared predecessor as invalid (or explicitly unverifiable). -
[P2] Allocate chain sequence numbers atomically
crates/gitlawb-node/src/cert.rs:31
Sequence allocation is a read-then-increment with no transaction, lock, or unique(repo_id, seq)constraint. Concurrent successful pushes can receive the same sequence and predecessor;get_cert_by_seqthen selects an arbitrary row. This makes a signed chain nondeterministic under normal concurrent traffic. Allocate the sequence transactionally and enforce uniqueness, with retry on collision. -
[P1] Do not describe raw signature bytes as a verifiable pusher authorization proof
crates/gitlawb-node/src/auth/mod.rs:252
Only the 64-byte Ed25519 signature is persisted. The RFC 9421Signature-Input, covered component values, method/path, and content digest are discarded, andverify_anchornever verifiespusher_sig. A third party therefore cannot reconstruct the signing string or bind these bytes to this push, yet the endpoint can report the anchor valid. Persist a complete verifiable authorization artifact and validate it, or remove the proof/verification claim. -
[P1] Bound the untrusted response read on the public verification route
crates/gitlawb-node/src/arweave.rs:279
The new unauthenticated, unthrottled route buffers the full gateway response withresp.bytes()before attempting JSON parsing. A caller can repeatedly select large data items and force corresponding memory and bandwidth consumption on the node. Apply a strict response-size limit (and a route-appropriate rate limit) before buffering or parsing the body. -
[P3] Implement the promised anchor failure lifecycle instead of dropping failed uploads
crates/gitlawb-node/src/api/repos.rs:1324
Upload failures only log a warning; no anchor row is created, retried, confirmed, or marked failed. The new pending/confirmed/failed methods are unused, and success-only rows are always inserted aspending. This does not meet the linked issue's stated retry/visible-gap acceptance criterion, so transient bundler failures silently leave history unanchored. Persist pending work before upload and drive it through bounded retry and terminal status handling. -
[P2] Keep the documented anchoring configuration working during the rename
crates/gitlawb-node/src/config.rs:91
This removesGITLAWB_IRYS_URLwithout a fallback, while both.env.exampleandREADME.mdstill instruct operators to set it. Upgrading an existing documented deployment leavesbundler_urlempty and silently disables both anchoring paths. Support the legacy variable for a deprecation period or make the migration explicit and update all operator documentation in the same change. -
[P3] Expose the new signed fields through the certificate API
crates/gitlawb-node/src/api/certs.rs:45
The certificate signing payload now includesseq,prev, andpusher_sig, but both list and get responses omit all three fields. Consumers of the established certificate API (includinggl cert) therefore cannot reconstruct the signed payload or inspect chain continuity after this change. Serialize the new fields and update the client display/verification path accordingly.
|
@Gravirei please rebase to main and fix conflicts |
c94e8ed to
ae4f5fc
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
crates/gitlawb-node/src/test_support.rs (1)
4983-4988: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake
seed_certproduce chain-valid fixtures.This helper creates the 10- and 55-certificate datasets, but every certificate has
seq: 1and a zero predecessor. The tests therefore cannot catch regressions that ignore monotonic ordering orprevlinks. Pass sequence/predecessor values through the helper or add a dedicated chained fixture.🤖 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/gitlawb-node/src/test_support.rs` around lines 4983 - 4988, Update the seed_cert fixture helper so generated certificates form a valid chain: assign increasing sequence values and set each certificate’s prev field to the preceding certificate’s identifier or digest, with the first certificate using the chain’s root predecessor. Ensure both the 10- and 55-certificate datasets exercise monotonic ordering and linked predecessors.
🤖 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/gitlawb-node/src/arweave.rs`:
- Around line 292-293: Update the payload parsing in verify_anchor_endpoint so
serde_json::from_slice failure is handled as an invalid verification result
rather than propagated as an internal error. Return VerifyResult with valid set
to false and an appropriate error string, while preserving the existing JSON
parsing path.
- Around line 281-293: Update the response handling around resp.bytes() in the
verification flow to enforce the 1 MiB limit before unbounded buffering: reject
any Content-Length above 1_048_576, and stream or otherwise cap reads so
responses without a trustworthy length header cannot exceed the limit. Preserve
the existing invalid VerifyResult fields and JSON parsing for accepted payloads.
In `@crates/gitlawb-node/src/db/mod.rs`:
- Around line 923-938: Migration version 13 must backfill distinct
per-repository sequence values before enforcing uniqueness. In the migration’s
`stmts` array, add an update that assigns deterministic, non-colliding `seq`
values to existing `ref_certificates` rows grouped by `repo_id`, then create
`idx_ref_certs_repo_seq`; preserve the existing column additions and append-only
index changes.
- Around line 5157-5162: Ensure each certificate created through make_cert
receives a unique seq value before insertion, either by incrementing it within
make_cert or overriding it at every test call site. Update the affected
certificate setup so list_ref_certificates_respects_limit and
insert_ref_certificate_append_only use distinct sequence numbers and avoid the
(repo_id, seq) uniqueness conflict.
---
Nitpick comments:
In `@crates/gitlawb-node/src/test_support.rs`:
- Around line 4983-4988: Update the seed_cert fixture helper so generated
certificates form a valid chain: assign increasing sequence values and set each
certificate’s prev field to the preceding certificate’s identifier or digest,
with the first certificate using the chain’s root predecessor. Ensure both the
10- and 55-certificate datasets exercise monotonic ordering and linked
predecessors.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e15d3f3-7966-4b86-a8fa-640f7c92e10a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
crates/gitlawb-node/src/api/arweave.rscrates/gitlawb-node/src/api/certs.rscrates/gitlawb-node/src/api/events.rscrates/gitlawb-node/src/api/repos.rscrates/gitlawb-node/src/arweave.rscrates/gitlawb-node/src/auth/mod.rscrates/gitlawb-node/src/cert.rscrates/gitlawb-node/src/config.rscrates/gitlawb-node/src/db/mod.rscrates/gitlawb-node/src/main.rscrates/gitlawb-node/src/server.rscrates/gitlawb-node/src/test_support.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/gitlawb-node/src/api/events.rs
- crates/gitlawb-node/src/server.rs
- crates/gitlawb-node/src/api/arweave.rs
- crates/gitlawb-node/src/config.rs
- crates/gitlawb-node/src/cert.rs
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P1] Backfill legacy certificate sequence numbers before adding the unique index
crates/gitlawb-node/src/db/mod.rs:910
Migration 12 gives every existing certificateseq = 1, but migration 13 then creates a unique(repo_id, seq)index. Version 10 only deduplicated(repo_id, ref_name), so any existing repository with certificates for two refs has duplicate(repo_id, 1)rows and cannot complete this migration or start. This is also immediately exposed by the changedseed_certfixture, which inserts tenseq = 1rows and makeslist_certs_respects_limit_paramfail. Assign deterministic per-repository sequence/chain values before creating the index and update the fixture. -
[P1] Enforce the verification response limit before buffering the gateway body
crates/gitlawb-node/src/arweave.rs:282
The new unauthenticated verification route callsresp.bytes()and only checks the 1 MiB limit after the whole Arweave object has been downloaded and allocated. A caller can select a huge or chunked gateway item and exhaust node memory/bandwidth before the rejection occurs. Stream into a capped reader (and reject a known excessive content length up front) instead of buffering first. -
[P1] Restore cryptographic verification to
gl cert show
crates/gl/src/cert.rs:151
This branch has drifted from its base and removes the base's--verify,--expect-node, Ed25519 verification routine, and tests, although the command is still documented as verifying a certificate. It now only prints a proposed payload and returns success for a modified or self-signed certificate. Please rebase without reverting that fail-closed CLI contract, then update its canonical payload forseq,prev, andpusher_sig. -
[P2] Hold the certificate-chain lock through allocation and insertion
crates/gitlawb-node/src/db/mod.rs:2160
pg_advisory_xact_lockis transaction-scoped, but this standalone pooled query commits beforeissue_ref_certificatereads the previous certificate or inserts the new one. Concurrent pushes can therefore select the same sequence; with three or more contenders the single retry can collide again, leaving an accepted push without its certificate/anchor evidence. Use one acquired connection/transaction for the lock, predecessor read, and insert (or a robust serialization/retry strategy). -
[P2] Bind the embedded certificate to the enclosing Arweave anchor
crates/gitlawb-node/src/arweave.rs:303
Verification checks the copied certificate signature but never compares the untrusted outerrepo,owner_did, ref, SHAs, or node DID to that certificate. An attacker can publish a payload with a valid public certificate while claiming a different ref update and receivevalid: true. Reject field mismatches (and match a locally recorded transaction too if this endpoint is meant to validate local anchors). -
[P2] Keep
gl statuscompatible with the remote created bygl init
crates/gl/src/status.rs:146
This branch regresses the base's multi-remote lookup: the status command now accepts only agitlawb://fetch URL onorigin, whilegl initadds the same URL under thegitlawbremote. Immediately after the supported init flow,gl statusreports that the repository is not a Gitlawb repo and skips the PR/issue queries. Rebase without reverting the base's lookup for thegitlawbremote and other Gitlawb fetch/push URLs. -
[P2] Do not hard-code
mainafter a plaingit init
crates/gl/src/init.rs:41
This branch reverts the base's branch/commit-state handling. Plaingit inithonors the user'sinit.defaultBranch, but the command unconditionally instructsgit push gitlawb main. Onmaster, feature, detached, or unborn HEADs that instruction either targets a nonexistent/wrong ref or fails before the first commit. Rebase without dropping the previous branch/commit-state handling, or initializemainwith the compatibility fallback. -
[P2] Preserve the legacy command-line spelling during the bundler rename
crates/gitlawb-node/src/config.rs:75
The environment fallback runs only after Clap parses arguments. Existing operators invokinggitlawb-node --irys-url …now receive an unknown-argument startup error even though the PR claims compatibility forGITLAWB_IRYS_URL. Add a deprecated long alias or normalize a retained legacy option as well as the environment variable. -
[P2] Do not remove unrelated CI safety gates from this anchoring change
.github/workflows/pr-checks.yml:189
This is stale-base drift rather than part of the Arweave feature: the branch deletes the shipped Windows CLI test lane and the onlygitlawb-coredependency-purity gate, together with its allowlist and checker script. That removes platform regression visibility and a supply-chain control for the shared cryptographic core; rebase without deleting these protections, or justify and replace them in a separately scoped change.
…or prev hash check, add get_cert_by_seq
Gitlawb#26) - Use configured gateway's data URL (/tx_id) instead of bundler API for verify - Bound untrusted response to 1 MiB on public verify route - Bind each anchor to its own ref update certificate (not repo-wide latest) - Make certificate storage append-only with unique (repo_id, seq) constraint - Add migration v13 to drop old unique index and add sequence uniqueness - Allocate chain sequence numbers atomically using per-repo advisory lock - Fail closed on missing predecessor cert during verification - Persist full RFC 9421 HTTP Signature context (signature-input, content-digest, path) - Verify pusher authorization proof in verify_anchor - Add legacy GITLAWB_IRYS_URL fallback for GITLAWB_BUNDLER_URL - Expose seq, prev, pusher_sig through certificate API list/get responses
ae4f5fc to
e9a1a5e
Compare
…ias, cross-check cert, transaction lock - v13 migration: backfill seq values per repo before creating unique (repo_id, seq) index to prevent failures on existing data - verify_anchor: check Content-Length before buffering response body - config: add deprecated --irys-url alias for bundler_url - verify_anchor: cross-check outer anchor fields (repo, ref_name, old_sha, new_sha, node_did) against embedded certificate - cert issuance: wrap lock/lookup/insert in a single Postgres transaction so pg_advisory_xact_lock is held for the full sequence - Add _tx variants of db methods accepting &mut PgConnection - Fix test helpers to use atomic counters for unique seq values - Fix v10 dedup tests to drop v13 index during pre-migration setup
|
@coderabbitai please make an intensive review of this changes |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/gitlawb-node/src/api/events.rs (1)
429-435: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftUse valid per-repository certificate-chain fixtures.
crates/gitlawb-node/src/api/events.rs#L429-L435: replace the process-global sequence generator with per-chain fixture state.crates/gitlawb-node/src/api/events.rs#L447-L452: deriveprevfrom the preceding certificate instead of always using the genesis hash.crates/gitlawb-node/src/test_support.rs#L1481-L1486: avoid assigning a global sequence to an otherwise standalone certificate.crates/gitlawb-node/src/test_support.rs#L4967-L4973: make sequence generation scoped to a repository/chain.crates/gitlawb-node/src/test_support.rs#L4990-L4995: generate matching predecessor hashes for multi-certificate fixtures.🤖 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/gitlawb-node/src/api/events.rs` around lines 429 - 435, Replace the process-global NEXT_FCERT_SEQ/ref_cert_seq state in crates/gitlawb-node/src/api/events.rs:429-435 with sequence state scoped to each certificate chain; update the certificate construction at crates/gitlawb-node/src/api/events.rs:447-452 to derive prev from the preceding certificate. In crates/gitlawb-node/src/test_support.rs:1481-1486, leave standalone certificates without a global sequence; in lines 4967-4973, scope sequence generation to the repository or chain; and in lines 4990-4995, generate predecessor hashes that match the preceding certificates in multi-certificate fixtures.
🤖 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/gitlawb-node/src/api/certs.rs`:
- Around line 55-57: The certificate JSON responses in
crates/gitlawb-node/src/api/certs.rs must include complete pusher-signature
metadata. Update both the listed-certificates response at lines 55-57 and the
single-certificate response at lines 98-100 to include signature_input,
content_digest, and request_path alongside the existing pusher_sig fields.
---
Nitpick comments:
In `@crates/gitlawb-node/src/api/events.rs`:
- Around line 429-435: Replace the process-global NEXT_FCERT_SEQ/ref_cert_seq
state in crates/gitlawb-node/src/api/events.rs:429-435 with sequence state
scoped to each certificate chain; update the certificate construction at
crates/gitlawb-node/src/api/events.rs:447-452 to derive prev from the preceding
certificate. In crates/gitlawb-node/src/test_support.rs:1481-1486, leave
standalone certificates without a global sequence; in lines 4967-4973, scope
sequence generation to the repository or chain; and in lines 4990-4995, generate
predecessor hashes that match the preceding certificates in multi-certificate
fixtures.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5645b021-f503-4733-92c1-424a3ae2349e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
crates/gitlawb-node/src/api/arweave.rscrates/gitlawb-node/src/api/certs.rscrates/gitlawb-node/src/api/events.rscrates/gitlawb-node/src/api/repos.rscrates/gitlawb-node/src/arweave.rscrates/gitlawb-node/src/auth/mod.rscrates/gitlawb-node/src/cert.rscrates/gitlawb-node/src/config.rscrates/gitlawb-node/src/db/mod.rscrates/gitlawb-node/src/main.rscrates/gitlawb-node/src/server.rscrates/gitlawb-node/src/test_support.rs
|
@coderabbitai make an intensive review on this please |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/gitlawb-node/src/arweave.rs (1)
303-304: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNon-JSON gateway payload still returns 500 instead of
valid: false.serde_json::from_slice(&body_bytes)?propagates throughverify_anchor_endpoint(mapped toAppError::Internal), inconsistent with the other graceful branches and the "could be JSON or raw bytes" comment. Convert a parse failure intoVerifyResult { valid: false, .. }with an error string.🤖 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/gitlawb-node/src/arweave.rs` around lines 303 - 304, Update the anchor parsing in verify_anchor_endpoint so serde_json::from_slice failures are handled as an invalid verification result rather than propagated as AppError::Internal. Return VerifyResult with valid set to false and an error string for non-JSON payloads, while preserving the existing successful JSON path and other graceful branches.
🧹 Nitpick comments (2)
crates/gitlawb-node/src/server.rs (1)
222-228: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider throttling the unauthenticated verify route. Each
GET /api/v1/arweave/verify/{tx_id}triggers an outbound gateway fetch plus a DB lookup with no auth or per-IP brake, so it's an amplification/DoS surface (node → gateway) reachable by anonymous callers. Given the other cost-bearing routes here carry a per-IPIpRateLimiter, consider wrappingarweave_routessimilarly. (tx_idis a fixed-host path segment, so this is a load concern, not SSRF.)🤖 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/gitlawb-node/src/server.rs` around lines 222 - 228, Wrap the arweave_routes router, including GET /api/v1/arweave/verify/{tx_id}, with the existing per-IP IpRateLimiter used by other cost-bearing routes. Preserve the current list_anchors and verify_anchor_endpoint handlers while ensuring anonymous requests are throttled before triggering gateway or database work.crates/gitlawb-node/src/db/mod.rs (1)
2834-2870: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
cert_idis never persisted on anchor rows.RecordAnchorInputV2has nocert_idfield andrecord_arweave_anchor's INSERT omits it, so thecert_idcolumn added in migration v12 stays NULL for every anchor even thoughlist_arweave_anchors/list_pending_anchorsproject it. The push path inapi/repos.rsalready has the issued certificate in scope (ref_certs_clone), so the anchor→certificate DB linkage this column was added for is currently unreachable. Consider threading the cert id through so audits can join anchors to their certs. (gateway_urlon the input is likewise accepted but ignored by this function.)🤖 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/gitlawb-node/src/db/mod.rs` around lines 2834 - 2870, The anchor record flow does not persist the issued certificate ID. Add a cert_id field to RecordAnchorInputV2, pass the corresponding ID from the push path using ref_certs_clone, and include it in record_arweave_anchor’s INSERT and bindings so cert_id is stored on each anchor row; also remove or persist gateway_url consistently instead of silently ignoring it.
🤖 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.
Duplicate comments:
In `@crates/gitlawb-node/src/arweave.rs`:
- Around line 303-304: Update the anchor parsing in verify_anchor_endpoint so
serde_json::from_slice failures are handled as an invalid verification result
rather than propagated as AppError::Internal. Return VerifyResult with valid set
to false and an error string for non-JSON payloads, while preserving the
existing successful JSON path and other graceful branches.
---
Nitpick comments:
In `@crates/gitlawb-node/src/db/mod.rs`:
- Around line 2834-2870: The anchor record flow does not persist the issued
certificate ID. Add a cert_id field to RecordAnchorInputV2, pass the
corresponding ID from the push path using ref_certs_clone, and include it in
record_arweave_anchor’s INSERT and bindings so cert_id is stored on each anchor
row; also remove or persist gateway_url consistently instead of silently
ignoring it.
In `@crates/gitlawb-node/src/server.rs`:
- Around line 222-228: Wrap the arweave_routes router, including GET
/api/v1/arweave/verify/{tx_id}, with the existing per-IP IpRateLimiter used by
other cost-bearing routes. Preserve the current list_anchors and
verify_anchor_endpoint handlers while ensuring anonymous requests are throttled
before triggering gateway or database work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d81eba6b-bb9a-4ee3-af1e-5e62923f6b5f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
crates/gitlawb-node/src/api/arweave.rscrates/gitlawb-node/src/api/certs.rscrates/gitlawb-node/src/api/events.rscrates/gitlawb-node/src/api/repos.rscrates/gitlawb-node/src/arweave.rscrates/gitlawb-node/src/auth/mod.rscrates/gitlawb-node/src/cert.rscrates/gitlawb-node/src/config.rscrates/gitlawb-node/src/db/mod.rscrates/gitlawb-node/src/main.rscrates/gitlawb-node/src/server.rscrates/gitlawb-node/src/test_support.rs
…ate limit - serde_json parse failure in arweave verify returns VerifyResult instead of propagating as AppError::Internal - RecordAnchorInputV2 replaces unused gateway_url with cert_id, persisted in arweave_anchors INSERT, sourced from push-time certificate - arweave routes wrapped with per-IP IpRateLimiter
Summary
Replace the Irys upload path with a generic Bundler endpoint, capture the pusher RFC 9421 HTTP Signature in ref certificates, add monotonic sequence numbers with SHA-256 chaining to certificate chains, and expose a
/api/v1/arweave/verify/{tx_id}endpoint that fetches the anchor from Arweave and validates the embedded certificate chain.Motivation & context
Closes #26
Kind of change
What changed
gitlawb-node
GITLAWB_IRYS_URLrenamed toGITLAWB_BUNDLER_URL;GITLAWB_ARWEAVE_GATEWAYadded.{url}/uploadto{url}/v1/tx; content-typeapplication/octet-stream; headerx-bundler-tags; Bundler response parsing.RefAnchornow carries an optional embeddedRefCertificate. Newverify_anchor()function fetches from gateway, extracts the cert, verifies the node Ed25519 signature and checksprevhash linkage against the local DB.RefCertificategainsseq(i64),prev(String),pusher_sig(Option).ArweaveAnchorgainsstatus,deadline_height,receipt_sig,cert_id;irys_tx_id→arweave_tx_id. Migration v11 upgrades existing databases.RecordAnchorInputV2replaces the old input struct. Addedget_most_recent_cert(),confirm_arweave_anchor(),fail_arweave_anchor(),list_pending_anchors().issue_ref_certificateacceptspusher_sig, chains from the previous cert via SHA-256 prev hash, builds a v2 signing payload.require_signaturemiddleware injectsPusherSignatureextension (base64-encoded Ed25519 sig bytes).git_receive_packextractsPusherSignature, passes it to cert issuance.RefAnchorconstruction fetches latest cert from DB. UsesRecordAnchorInputV2.GET /api/v1/arweave/verify/{tx_id}endpoint.RefCertificateinitializers for new fields.How a reviewer can verify
DATABASE_URL=postgresql://gitlawb:changeme@172.19.0.2:5432/gitlawb cargo test --package gitlawb-nodeAll 495 tests pass.
Before you request review
cargo test --workspacepasses locally (495 passed)cargo fmt --allandcargo clippy --workspace --all-targets -- -D warningsare cleanfix(...)).env.exampleupdated if behavior or config changed (or N/A)Protocol & signing impact
did:key, Ed25519 / RFC 9421 signatures, UCAN, ref certs, or P2P wire formatsNotes for reviewers
The
gateway_urlfield inRecordAnchorInputV2is passed through by callers but not persisted in the DB — it is used by the caller to construct the Arweave URL at query time. Thecert_idcolumn onarweave_anchorsis reserved for a follow-up linking pass.Summary by CodeRabbit
New Features
GET /api/v1/arweave/verify/:tx_id, returning validity, error details, and optional embedded certificate info.Improvements
seq,prev, and related optional HTTP signature fields.Configuration