573 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
Ayush7614	fcb5cf3a6f	chore(ci): re-trigger PR checks on the flaky test (beta) run	2026-11-10 22:57:41 +05:30
Ayush7614	01543e6764	test(node): make iCaptcha oversized-body test use a valid padded JWKS ... coderabbit flagged that the size-limit test fed fetch_key a body of repeated x bytes, which is invalid JSON: even without a byte cap the parse would return None, so the test passed regardless of the limit. Pad jwks_body() with JSON whitespace past KEY_MAX_BODY_BYTES so an unbounded read would still parse successfully, making the assertion actually exercise the cap.	2026-11-10 22:29:30 +05:30
Ayush7614	298d56e454	fix(node): pin no-redirect and bound the body read on the iCaptcha public-key fetch ... The node fetches the iCaptcha public key that anchors its proof gate, but built its client with reqwest's default redirect policy and trusted `.json()` to buffer the response. A redirect from the configured ICAPTCHA_URL could move the trust anchor, and an oversized body was buffered without bound. - Build the key client with `redirect::Policy::none()`, matching every other outbound client the node runs. - Read the response body through a capped chunk loop (8 KiB) and parse from the received bytes instead of buffering unboundedly. - Add mockito tests proving a valid JWKS still parses, a 302 to a host serving a real key is NOT followed, and an oversized body is refused. Closes #507	2026-11-10 22:04:41 +05:30
beardthelionandAyush7614	66fe646320	fix(node): close quarantine bypass on encrypted blobs, visibility routes, and sibling mutations (#566) ... * fix(node): close quarantine bypass on encrypted blobs and visibility routes Rebuilt per review: this branch now carries ONLY the quarantine fix (dropped the unrelated task-claim and Windows fetch commits, which live in #275/#312, and the author-local .cursor symlink). The ipfs.rs and db-layer halves of the original commit were superseded by main's paged scan, which already carries per-row quarantine flags; what remains is routing encrypted-blob and visibility handlers through authorize_repo_read so a quarantined repo is an opaque 404 there too, plus the three regression tests for exactly that surface. * fix(node): gate sibling mutation endpoints on quarantine, reconcile 404 contract in gl clone (#276) * fix(node): release quarantined mirror fixtures, bound 404 body read in gl clone (#566) * test(node): pin read-before-owner order on every dual-gated handler (#566) * test(node): pin list_webhooks in the order loop, close-owner second assert, tighter release legs (#566) --------- Co-authored-by: Ayush7614 <[email protected]>	2026-11-10 15:29:23 +00:00
JATMNandClaude Opus 5.5	309fc8360d	ci: run PR checks and CodeQL only for the surfaces a PR touches (#519) ... * ci: run PR checks and CodeQL only for the surfaces a PR touches Every PR ran every PR Checks job on the whole workspace, and CodeQL default setup analyzed every language, including a ~13 min macOS Swift autobuild (55 min when it hung on main) for an 8-file macos-app/ that 4 of the last ~280 commits touched. scripts/ci-surfaces.sh maps a PR's changed paths to the jobs that validate them, and scopes cargo test / build --release to the touched crates plus their transitive reverse dependencies (derived from crates/*/Cargo.toml). Root Cargo.toml/Cargo.lock/.cargo, the CI wiring itself, and unknown crate dirs widen to the full suite. Push to main, merge_group, and the release flow's workflow_dispatch always run everything on the whole workspace. codeql.yml replaces default setup with the same scoping per language and a full analysis on push to main and weekly. It needs default setup disabled. Co-Authored-By: Claude Opus 5.5 <[email protected]> * docs: say which changes widen PR CI to the whole workspace ci-surfaces.sh has two widening tiers. A root Cargo.toml/Cargo.lock/.cargo change (or toolchain config, or an unknown crate dir) runs the Rust jobs on --workspace, while the other jobs still follow their own inputs. Only the CI wiring itself (pr-checks.yml, codeql.yml, ci-surfaces.sh) and non-PR events run every surface. AGENTS.md and the script header described both tiers as "everything" / "the full suite" and left "CI wiring" undefined, so an edit to, say, release.yml read as if it ran the full PR Checks suite; it runs none of those jobs and only CodeQL's Actions analysis. Comment and docs only; the mapper's output is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]> * ci: test gitlawb-node when bootstrap-peers.json changes ci-surfaces.sh attributed Rust crates only by crates/<dir>/ paths, but gitlawb-node also compiles in the repo-root bootstrap-peers.json through include_str!. A PR that changed only that file therefore ran the Docker smoke (docker build + --version, which embeds the JSON without parsing it) and skipped cargo test, so embedded_seed_list_parses_successfully ("embedded bootstrap-peers.json must always parse") no longer guarded it. Add a path -> crate table for files outside crates/ that a crate compiles in, routed through the same check as crates/<dir>/ paths (an unknown crate still widens to --workspace). bootstrap-peers.json now yields rust=true with -p gitlawb-node and keeps its Docker trigger; every other mapper output is unchanged. The header also said anything the script cannot attribute precisely widens the run. That holds only inside the Rust surface; a path no surface lists (docs, README, infra/, other workflows) runs no PR Checks job, as AGENTS.md says. Reword it to the two tiers the script implements. Co-Authored-By: Claude Opus 5.5 <[email protected]> * ci: feed ci-surfaces NUL-delimited paths and pin its mapping in a test The mapper read newline-delimited `git diff --name-only` output. In that mode git C-quotes any path with non-ASCII, tab, double-quote or newline characters, so crates/gl/src/é.rs arrived as "crates/gl/src/\303\251.rs", matched no surface, and a PR whose only Rust change was such a file ran no Rust job (Café.swift likewise skipped Swift CodeQL). Newline-delimited input also cannot carry a path that contains a newline. Both workflows now pass `git diff -z` output and the mapper reads it with `read -r -d ''`; input that is not NUL-terminated is rejected (exit 2) instead of being misread. Mapping output for every ASCII path is unchanged (checked against the last 43 commit ranges). scripts/test-ci-surfaces.sh pins one must-not-skip case per surface row, the crate fan-out and widening rules, --all, the wiring rows, quoted names, and rejection of newline-delimited input. PR Checks runs it in the `changed surfaces` job before mapping, on every event, so a broken mapper turns the run red rather than silently narrowing it. Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: Claude Opus 5.5 <[email protected]>	2026-11-10 00:41:03 +00:00
3 people	f68d0dd3ff	fix(node): durable post-receive outbox for receive-pack (#26 split 1/4) (#558) ... * fix(node): durable post-receive outbox at the DB layer (#26 split 1/4) Reviewer 2 closed PR #224 on 2026-08-28 with a directive: split the work into four narrow PRs. This is Split PR 1 (durable post-receive lifecycle) at the DB layer; the handler refactor in crates/gitlawb-node/src/api/repos.rs:2007 (git_receive_pack) lands in the next slice so the test can drive the failure injection end-to-end. The pre-outbox crash window the reviewer flagged: receive_pack can apply a ref to disk and return Ok, and a process exit, a dropped future, or a DB failure before the bookkeeping at crates/gitlawb-node/src/api/repos.rs:2361 (push event + cert + webhook) loses the recovery record. Startup drain enumerates only sources written from that bookkeeping, so it cannot reconstruct the missing work. The partial fallback that re-derives from a row present in the bookkeeping substitutes did:key:recovered and an empty attestation, which is not equivalent to the original authenticated push. This commit adds the durable boundary the handler will lean on. NEW TABLE pending_ref_transitions (migration v27): - Written by the handler BEFORE smart_http::receive_pack, in state 'prepared', carrying the verified pusher DID, the raw RFC 9421 signature header, signature-input, and content-digest that authorized the push, the request id, and the parsed ref update. - The handler transitions the row to 'applied' on receive_pack Ok or 'cancelled' on Err. The drain reads only 'applied'. - A failed or cancelled receive-pack therefore leaves the row in 'prepared' or 'cancelled', which the drain never promotes. This is what closes the reviewer's second proof ("a failed or cancelled receive-pack does not turn a prepared intent into completed accounting or anchoring"). NEW TABLE anchor_jobs (migration v27, owned by PR 1, consumed by PR 2): - One row per (repo_id, ref_name, old_sha, new_sha) transition. PR 1 inserts it on 'applied'; PR 2 reads it and updates claimed_at. - ON CONFLICT (id) DO NOTHING makes the insert idempotent on the deterministic id, so a recovery re-pass cannot create a second upload request. This is the handoff boundary; the bundler call itself is PR 2. NEW DB METHODS on Db: - insert_pending_ref_transitions: writes one 'prepared' row per ref update, returns the persisted rows. - mark_pending_ref_transitions_applied / _cancelled: state flip, gated on the FROM state, idempotent. - list_pending_ref_transitions_applied: drain query, oldest first. - delete_pending_ref_transition: called by recovery after the artifacts land; a third pass is a no-op. - record_push_with_id: ON CONFLICT (id) DO NOTHING on the deterministic id. - insert_ref_certificate_idempotent: ON CONFLICT (repo_id, ref_name) DO NOTHING, returns None if a live-path cert already exists. - insert_anchor_job_idempotent: ON CONFLICT (id) DO NOTHING on the deterministic per-transition id. NEW HELPERS in db/mod.rs: - deterministic_id: SHA-256 hex with an ASCII Unit Separator between fields so two distinct tuples never collide on prefix overlap. - push_event_id_for, ref_cert_id_for, anchor_job_id_for: the derived ids above, one helper per artifact so a caller cannot derive a wrong id by mistake. NEW STRUCTS: - PendingRefTransition: the row shape. - AnchorJob: the handoff row shape. - pending_state: const strings ('prepared' / 'applied' / 'cancelled') shared by tests, the producer, and the drain so a typo on one side cannot silently mismatch the other. NEW TESTS in db::pending_ref_transition_tests (8 tests, all green): - insert_then_mark_applied_flips_state_for_every_ref: producer contract. - mark_applied_is_idempotent_on_repeat: re-fire is a no-op. - cancelled_rows_are_not_returned_by_the_drain: reviewer's second proof at the DB layer. - prepared_rows_are_not_returned_by_the_drain: same proof for the pre-flip state (handler crashed before reaching post-Ok). - mark_cancelled_is_idempotent_on_repeat: counterpart. - drain_then_re_derive_is_idempotent: reviewer's first proof at the DB layer. Inserts a row in 'applied' state directly via insert_pending_ref_transition_for_test, drains it, derives the artifact ids twice, exercises record_push_with_id and insert_anchor_job_idempotent directly, asserts exactly one push event row and exactly one anchor job row regardless of how many times the drain runs. - deterministic_id_avoids_prefix_overlap_collisions: the separator regression test. - push_event_id_for_is_stable: derived ids match across calls and differ on each varied input. OTHER: - Make RefUpdate and its fields pub(crate) so the DB methods can iterate the parsed ref updates. No public API change. NOT IN THIS SLICE (the handler refactor, next commit): - The receive-pack handler does not yet call insert_pending_ref_ transitions before the receive_pack call, nor mark_applied / mark_cancelled after. The DB layer is in place for it; the handler will call these methods and the startup drain will be wired in main.rs. - The startup drain in main.rs is not yet called; it will iterate list_pending_ref_transitions_applied, re-derive the artifacts, and delete the row. - The cert/push event issuance in cert.rs and the bookkeeping in api/repos.rs:2361 are not yet changed to use the deterministic ids. The helper functions exist and are tested; the callers follow. Compiles clean, clippy clean under -D warnings, fmt clean. * fix(node): wire durable outbox into the receive-pack handler (#26 split 1/4) This is the handler-level half of Split PR 1. The previous commit added the migration and the DB methods; this one threads them through crates/gitlawb-node/src/api/repos.rs:2007 (git_receive_pack), the cert issuer, and the startup drain. CHANGES IN THE HANDLER ====================== In git_receive_pack, AT THE LAST POSSIBLE MOMENT before the smart_http::receive_pack call, the handler now: 1. Generates a per-handler request_id (UUID). 2. Captures the raw Signature, Signature-Input, and Content-Digest headers from the request. 3. Calls db.insert_pending_ref_transitions(request_id, ...) which writes one row per ref update in state 'prepared'. The receive_pack call runs as before. After it returns: 4. On Ok: db.mark_pending_ref_transitions_applied(request_id) — the row is the ONLY thing that promotes a 'prepared' row to 'applied', and the drain reads only 'applied' rows. A process crash before this call leaves the row in 'prepared', which the drain never promotes. 5. On Err: db.mark_pending_ref_transitions_cancelled(request_id) — a failed receive_pack leaves the row in 'cancelled', which the drain never promotes. This is what closes the reviewer's two proofs: Proof 1 (crash window): if the process dies after mark_pending_ref_transitions_applied but before the bookkeeping writes, the row is in 'applied' and the next startup drain re-derives the push event, the per-ref certificate (carrying the ORIGINAL pusher DID, not a placeholder), and the anchor handoff. The drain uses the persisted authentic pusher DID and signature header, not a recovered placeholder. Proof 2 (failed receive-pack): the row is only ever flipped to 'applied' in the explicit Ok branch above. A 'prepared' or 'cancelled' row is invisible to the drain, so a failed or dropped receive_pack cannot turn a prepared intent into completed accounting or anchoring. BOOKKEEPING IS NOW DETERMINISTIC-ID =================================== The post-Ok bookkeeping at api/repos.rs:2448 now uses: - record_push_with_id with push_event_id_for(request_id, first_ref) — ON CONFLICT (id) DO NOTHING, so a recovery re-pass is a no-op. - issue_ref_certificate_idempotent with ref_cert_id_for(request_id, ref_name) — ON CONFLICT (repo_id, ref_name) DO NOTHING, returns None if a live-path cert already exists. - insert_anchor_job_idempotent with anchor_job_id_for(repo_id, ref_name, old_sha, new_sha) — the per-transition tuple key, so two pushes to the same ref produce one anchor upload per landed state. The legacy entry points (record_push, issue_ref_certificate, insert_ref_certificate) remain for callers that prefer a fresh UUID per cert; they are #[allow(dead_code)] for the PR 3 cert/CLI compat pass to decide whether to keep or remove. STARTUP DRAIN ============= crates/gitlawb-node/src/main.rs calls durable_outbox::drain_pending_ref_transitions(state, 1000) ONCE before serving, after migrations and after the existing peer / quarantine prunes. Non-fatal: a transient drain failure logs and leaves the rows for the next startup. durable_outbox::drain_pending_ref_transitions reads every 'applied' row, calls derive_one (which re-derives the three artifacts using the persisted authentic pusher DID and signature header), then deletes the row. A second drain pass is a no-op for both the artifacts (idempotent inserts) and the row (gone after the first pass). NEW END-TO-END TESTS ==================== crates/gitlawb-node/src/durable_outbox.rs adds three end-to-end tests in drain_tests, complementing the eight DB-layer tests in db::pending_ref_transition_tests: - drain_re_derives_all_three_artifacts_for_an_applied_row: the reviewer's first proof. Inserts a row in 'applied' state (the crash window), drains, asserts exactly one push event row, exactly one cert row carrying the original pusher DID (not a placeholder), and exactly one anchor job row. Asserts the deterministic cert id matches. Asserts a second drain pass is a no-op. - cancelled_row_produces_no_artifacts: the reviewer's second proof for the cancelled state. A row in 'cancelled' (receive_pack returned Err) is invisible to the drain. - prepared_row_produces_no_artifacts: the reviewer's second proof for the prepared state. A row in 'prepared' (handler crashed between insert_prepared and the post-Ok branch) is invisible to the drain. Each test names the invariant it pins and the production line it covers. Reverting that line turns the named assertion red. Compiles clean, 1099 tests pass with 0 regressions, clippy clean under -D warnings, fmt clean. Cross-PR overlap (declared in the PR description): - #134 (anchors auth): composes. The /arweave/anchors route already requires auth; this PR does not change the route. - #285 (advisory-lock session affinity): composes. The durable intent is written inside the same handler that holds the lock from #285; no changes to the lock layer. - #306 (Content-Digest on signed requests): composes. PR 1 persists the Content-Digest header that #306 makes mandatory. - #314 (small-order Ed25519): independent. PR 1's tests use strong keys. - #324 (libp2p keypair persistence): independent. PR 1 does not touch p2p identity. - #325 (gossip ref-update auth): independent. PR 1's signed envelope is the HTTP-side equivalent, not the gossip-side. - #382 (replication withheld-subtree trees): independent. PR 1 does not touch replication or pin selection. * fix(node): address four reviewer findings on #26 split 1/4 - P1-A: add startup reconcile step that promotes `prepared` rows to `applied` when the on-disk ref matches the row's `new_sha`. The recovery drain (which only reads `applied` rows) can now pick up a ref that landed when the live handler's `mark_pending_ref_transitions_applied` call errored or was interrupted. Strict SHA equality is the load-bearing check — a `prepared` row whose target did NOT actually land stays `prepared`. - P1-B: route the live handler's cert issuance through `cert::issue_ref_certificate` (the upsert) instead of `issue_ref_certificate_idempotent` (DO NOTHING). A re-push to the same ref now updates the cert's `old_sha` / `new_sha` / `pusher_did` / `issued_at` / `signature` to the new transition while preserving the deterministic `cert_id`. The recovery drain keeps the idempotent variant; both paths collapse to one row. - P2-A: refactor the drain into a `drain_pending_ref_transitions_with` testable seam that does per-row log-and-continue, and add `drain_pending_ref_transitions_all` that loops `DRAIN_PER_PASS_LIMIT=1000` rows for `DRAIN_MAX_PASSES=10` passes. A failing row no longer stalls the batch; a backlog above 1000 rows is fully processed across passes. - P2-B: add a `first_ref_name` column to `pending_ref_transitions` via migration v28. The live handler hoists a `first_ref_name` local and persists it on every row of the same `request_id`. The drain's `derive_one` keys the push event id on `row.first_ref_name` instead of `row.ref_name`, so live and recovery produce the same id and `ON CONFLICT (id) DO NOTHING` collapses a multi-ref push to one push event row (and one trust- score bump). Cert and anchor ids stay per-ref / per-transition. * fix(node): address four reviewer findings on #26 split 1/4 (round 2) * fix(node): address reviewer round-3 findings on #26 split 1/4 - P1: Delete outbox rows after live durable effects complete so they don't replay on every restart - P1: Parse git report-status for per-ref ok/ng results; mark only proven rejections as cancelled, uncertain outcomes as recoverable - P1: Introduce 'uncertain' state for receive-pack errors where some refs may have landed; reconcile checks these against disk at startup - P2: Promote deletion transitions during reconcile (new_sha == ZERO_SHA with absent ref = successful deletion) - P2: Loop reconcile across multiple passes so backlogs beyond the first page are processed in the same startup Closes review round 3 findings from reviewer-1 and reviewer-2. * fix(node): fix CI failures from round-3 changes - Add COMMENT ON TABLE to v29 migration so migration_bodies_are_non_empty passes - Return error on non-zero receive-pack exit (preserving backward compat with tests that expect Err(AppError::Git(_))) while still parsing report-status for outbox row handling * fix(node): update inv22 gate tests for receive_pack_raw refactor - F3 gate: scan for smart_http::receive_pack_raw( instead of receive_pack( - U5 gate: scan for all_refs_ok instead of receive_result.is_ok() * fix(node): require reflog proof before the reconcile promotes a row (#26 split 1/4) Closes the second half of the reviewer's round-3 [P1]: the startup reconcile still promoted on `disk_refs.get(ref) == row.new_sha`, which also matches a coincidental current tip (old=B, new=A while the ref is already A). That is not an exotic case — it is the ordinary shape of a REJECTED push, since git refuses an update whose expected old value does not match the ref. Both the SHA check and the MAX_RECONCILE_AGE window pass, the push never happened, and the drain then writes a push event, a signed certificate and an anchor for it. The live path proves landings from git's report-status body; that body is gone by the time the reconcile runs, so the proof is re-derived from the repository. `git::store::ref_reflog_entries` reads `logs/<ref>` and a row is promoted only when an entry carries its exact `old -> new` pair, stamped at or after the row was written (REFLOG_CLOCK_SKEW absorbs git's whole-second truncation against the row's sub-second `created_at`). The timestamp floor is what separates a landing from a later push that re-introduced the same pair: proof must postdate the intent it proves. Deletions stay exempt from the reflog half and keep the absence-plus-age rule already in place — git removes a ref's reflog along with the ref, so no other evidence can exist for one. `init_bare` now sets `core.logAllRefUpdates`, which bare repos default off; a repo with no reflog yields no proof and its rows stay put for human-attended recovery rather than becoming accounting the node cannot substantiate. Tests: reconcile_refuses_a_coincidental_tip_with_no_reflog_proof, reconcile_promotes_a_row_the_reflog_proves, reconcile_leaves_a_row_prepared_when_the_repo_keeps_no_reflog, reconcile_refuses_a_reflog_entry_older_than_the_row, reconcile_still_promotes_a_landed_deletion_which_can_have_no_reflog, init_bare_keeps_reflogs_so_a_landing_can_be_proven, ref_reflog_entries_is_none_when_the_repo_kept_no_log. No migrations added or renumbered. * fix(node): walk the reconcile backlog on a keyset cursor (#26 split 1/4) The multi-pass reconcile looped but never advanced. Each pass re-issued the same cursor-less `LIMIT n` query, and unlike the drain — which deletes every row it finishes, so its next page is always new work — the reconcile leaves every row it cannot promote exactly where it was. One unprovable row at the head of the ordering therefore pinned page one on every pass, and the backlog behind it was never examined at all, which is the condition the multi-pass loop was added to remove. Rows keep ageing toward MAX_RECONCILE_AGE while they wait, so "picked up next restart" can mean "never recovered". Pages now resume strictly after the `(created_at, id)` of the last row EXAMINED, via `list_pending_ref_transitions_prepared_or_uncertain_after`. A keyset cursor rather than an OFFSET because rows leave the set underneath the walk when they are promoted: an absent row simply does not appear on a later page, and it can never shift an unvisited row into a page that was already read. Pass budget, per-pass limit, short-page exit and the residual-backlog warning are unchanged. This matters more with the reflog gate in place: a landing in a repo that keeps no reflog is PERMANENTLY unprovable, so before this it would jam page one on every startup rather than once. Tests: reconcile_all_walks_the_backlog_past_the_first_page, reconcile_all_advances_past_an_unprovable_row (whose blocker is a no-reflog row, the class the gate introduces). * fix(node): bound the reflog read to a recent tail (#26 split 1/4) `ref_reflog_entries` slurped the whole reflog. A reflog grows one line (~150 bytes) per ref update and how often a ref is updated is PUSHER-controlled, so that was an attacker-sized allocation, taken once per stranded row, at startup — the moment the node can least absorb it. Every other attacker-influenced read in this crate is bounded (read_body_capped, MAX_SIGNATURE_ENTRIES, the pin/walk budgets); this one should be too. The bound costs nothing in proving power, which is why it is a ceiling rather than a tradeoff: the landing gate only ever accepts an entry stamped at or after `created_at - REFLOG_CLOCK_SKEW`, and reflogs are append-ordered oldest-first, so every entry that could possibly qualify is at the tail. The read now seeks to the last REFLOG_TAIL_BYTES (256KB, ~2000 recent updates to one ref) and `take`s that many bytes, so the allocation stays bounded even if the file grows between the stat and the read. A window into the middle of a file starts mid-record, so the bytes before its first newline are discarded: without that, a boundary landing inside a record's old SHA leaves a suffix that still tokenizes and yields a truncated SHA as a bogus `old -> new` pair — and landing proof is an exact pair match, so a bogus pair is a fabricated proof. An entry older than the window reads as absent, which is the safe direction: the caller treats "no matching entry" as unproven and leaves the row where it is. `Ok(None)` for a missing file, and the absence-of-evidence contract, are unchanged. Tests: ref_reflog_entries_reads_whole_records_from_the_recent_tail (lays the file out so the boundary lands 20 bytes into a record's old SHA — the alignment that would otherwise parse), an_entry_older_than_the_reflog_tail_reads_as_unproven. * fix(node): read the report through both side-band frames The report-status is framed twice whenever the client negotiates side-band-64k, which `git push` over smart HTTP does: the outer side-band envelope carries a stream that is itself pkt-line encoded. One strip pass left the first line as `000eunpack ok`, so the unpack check failed and parse_report_status returned None — read by the caller as "no report", which keeps every declared ref. The per-ref landing gate was therefore inert for exactly the pushes it exists to filter. The regression test is a byte-for-byte capture from git 2.50.1 rejecting one ref of a two-ref push, because a hand-written single-framed fixture parses under the old code and hides the case. * fix(node): address round-4 reviewer findings on #26 split 1/4 * fix(node): rustfmt round-4 reviewer fixes * fix(node): remove empty line after doc comment * fix(node): drop redundant reflog gate; implicit-ok on exit-zero no-report * fix(node): address round-5 reviewer findings on #26 split 1/4 * fix(db): v30 migration — add receive_pack_requests table and ordinal column The reviewer's round-5 finding: the push event identity was encoded into a mutable per-ref column (first_ref_name) whose correct value is knowable only after git, and the correction was not committed atomically with the per-ref outcomes. A crash between git updating a later ref and the rewrite left the durable rows naming the rejected ref, and derive_one's row.ref_name == row.first_ref_name guard then meant no push event ever landed for the accepted child. Per the state-transition model at .gravirei/plans/state-model-durable-post-receive.md, this migration introduces the request-level record that owns the push event and the trust score, and extends the per-ref child with an ordinal column the drain and the effect executor walk together. first_ref_name is dropped; the push event id is keyed on (request_id, accepted_ordinal) and lives on the request row. The next commit (handler rewrite) wires the live path and the recovery drain against the new tables. BREAKING CHANGE: pending_ref_transitions.first_ref_name is dropped. Callers that read or write that column must move to receive_pack_requests.accepted_ordinal. * fix(node): rewrite receive-pack handler against the request-level model (#26 split 1/4 step 2) The v30 migration added receive_pack_requests and dropped first_ref_name, but no Rust code used either. The handler still ran a four-branch per-ref state flip plus a first_ref_name rewrite after git returned, which is the mixed-outcome bug a push with the first ref rejected exposes: the live path and the drain computed different push-event ids. - Insert a receive_pack_requests row in state `received` BEFORE git runs (insert_receive_pack_request), carrying the request bytes and the SHA-256 of the body. Crash between intent and git return is now recoverable via the reconcile step. - Drop the first_ref_name rewrite at api/repos.rs:2454. The push event identity is now (request_id, accepted_ordinal); accepted_ordinal is computed once at the per-ref state flip. - Re-key push_event_id_for and ref_cert_id_for on (request_id, ordinal). Anchor job id stays per-transition. - Transition the request row to outcomes_committed (with parsed_report and accepted_ordinal) or rejected_at_git (no report + non-zero exit) in the same handler tail as the per-ref state flip. The drain (step 3) picks up outcomes_committed rows; today the live path also runs the per-ref effects inline. - Stub the step-3 surface (mark_request_effects_pending, complete, list_receive_pack_requests_due, update_request_attempt) with #[allow(dead_code)] + contract-pin comments so the rewrite is bisectable: reverting this commit restores the pre-rewrite live path. The U5 gate (replication tail spawn inside push_succeeded, before guard.release) is preserved; inv22_replication_tail_spawns_at_the_ durability_boundary stays green. * fix(node): use raw bytes for request_bytes_hash (#26 split 1/4 step 2) The v30 migration defined request_bytes_hash as BYTEA but the handler was binding a hex-encoded String. Postgres rejected the insert with "column request_bytes_hash is of type bytea but expression is of type text" and the handler shed the push with a 503. Switch the handler to bind the raw 32-byte digest and update the ReceivePackRequest struct field type to Vec<u8> to match. The drain-side test fixture already used Vec<u8> and needs no change. Pinned by the bin test suite going from 24 failures (all the receive-pack-cap tests that send a 4-byte body) to 0 regressions on the new model. * fix(node): factor out apply_request_effects; drain walks receive_pack_requests (#26 split 1/4 step 3) The step-2 commit moved the request row to be the unit of work but left the per-ref effects fan-out inline in the handler and the drain walking pending_ref_transitions per-ref. Step 3 factors the effects fan-out into apply_request_effects(state, request_id) and rewrites the drain to walk receive_pack_requests. - apply_request_effects lives in durable_outbox.rs; the live handler in api/repos.rs and the drain both call it. Idempotent on (request_id, accepted_ordinal). Returns EffectsOutcome::{Done, Nothing, Retry}. - Drain switches from list_pending_ref_transitions_applied + derive_one to list_receive_pack_requests_due + apply_request_effects. derive_one and the per-ref drain entry points are deleted. - The step-3 stubs in db/mod.rs (mark_request_effects_pending, mark_request_complete, list_receive_pack_requests_due) lose their #[allow(dead_code)] annotations; every stub has a caller in this PR. - Reconcile (reconcile_prepared_from_disk_all) is unchanged. The crash window between intent durable and outcomes commit still routes through per-ref reflog proof. - Drain tests rewritten to stage receive_pack_requests rows; the per-ref fixtures (make_row, lookup_accepted_ordinal) are gone. - New inv26_step3_live_and_drain_share_apply_request_effects assertion pins the live/drain sharing and the per-request drain walk. * fix(node): bounded retirement purge for terminal request rows (#26 split 1/4 step 4) The v30 partial index idx_receive_pack_requests_completed_at exists but no code reads it. Step 4 wires a periodic purge task that deletes terminal `complete` and `rejected_at_git` rows older than the retention window, along with their per-ref children. `quarantined` (a step-5 state) is never purged by the timer. - `purge_completed_receive_pack_requests(older_than, limit)` — deletes parent requests using the v30 partial index. - `purge_completed_pending_ref_transitions(older_than, limit)` — deletes children of purged parents, gated on the child's own applied_at / cancelled_at. - `purge_request_queue(db, retention_days, limit)` — orchestrator in durable_outbox.rs that calls both and returns the totals. The deletion order (parents first, then children) is the contract; a crash mid-purge leaves orphaned children that the next pass will pick up. - `spawn_queue_lifecycle_sweep` in main.rs — periodic task on the same detached pattern as `spawn_legacy_cid_sweep`, 24-hour interval, shutdown-aware. The interval matches the spec's "one per cluster per day" target. - Config knobs `queue_retention_days` (default 7, range 1..=365) and `queue_purge_batch` (default 1000, matches DRAIN_PER_PASS_LIMIT). - 3 new `durable_outbox::drain_tests::purge_*` tests pin the contract: only old terminal rows are deleted, the second pass is a no-op, and a row inside the retention window survives. - New `inv26_step4_queue_lifecycle_purge_is_wired` gate asserts the wiring (main.rs calls the purge, the DB helpers exist, the Config knobs exist). * fix(node): reference-transaction marker + quarantined state + failure matrix (#26 split 1/4 step 5) Steps 2-4 gave the request row the unit of work, the shared executor, and the bounded retirement policy. Step 5 closes the evidence gap: the reconcile now requires a per-request marker ref (refs/gitlawb/requests/<id>) whose value matches request_bytes_hash. A missing or mismatched marker quarantines the request; an operator reclassifies it. - v31 migration: adds `quarantined` to the state vocabulary and a partial index for operator queries. - Handler writes the marker ref before git-receive-pack; the marker is causally bound by being in the same async task as the receive-pack call. The marker's value is content-addressed (git hash-object of the request bytes), so the gate compares consistent SHAs on both sides. - git::store::read_ref reads a single ref's value, returning Ok(None) for absent refs. Used by the marker gate. - git::store::marker_value_for computes the content-addressed marker value; both the live handler and the reconcile use it so the write and the read agree. - Reconcile gains a marker gate between the age check and the reflog proof. Mismatch or absent ⇒ mark_request_quarantined + mark_children_rejected_for_quarantined_parent. - effects_max_attempts bound (config knob, default 8) flips retry-stuck requests to `quarantined` after N attempts, closing the infinite-retry DoS window. - New failure_matrix_tests submodule covers the spec's outcome × ref-kind × exit-point × recovery-scenario matrix (6 cells). - New inv26_step5_marker_quarantine_and_bound_are_wired gate asserts the marker gate, the bound check, the handler's pre-receive-pack ordering, and every load-bearing helper. - Existing 7 reconcile tests updated to stage a marker ref via the new `stage_marker` test helper. * fix(node): converge durable post-receive to singular request-level lifecycle Unify producer, evidence, aggregate outcome, executor, retry, and retirement on the request row: atomic intent and outcome commits, synthetic normalized reports for implicit-ok/reconciled paths, reconcile parent promotion with fail-closed deletions and competing-claimant guard, retry progression from both executable states with backoff/quarantine, terminal-parent-gated purge with marker cleanup and hidden refs, minimal digest-only intent plus request-level RFC9421 proof (v32). * fix(node): harden request occurrence lifecycle, proof, retry, and retirement Reject incomplete report framing and omitted refs as indeterminate, preserve uncertain children on completion, terminalize all-rejected, bind reconcile with landing history, durable proof with ack gate, occurrence-keyed anchors, background due loop, deletion quarantine with operator resolve, children-before-parent purge with marker tombstones, recovery prereq upgrade check, and bounded marker runner. * fix(node): keep recovery prereq upgrade best-effort on push path Refusing pushes when git config upgrade fails broke fake-git and non-repo disk-path tests with 503. Reconcile already fails closed on missing reflogs, so downgrade to warn-and-proceed. * fix(node): close reviewer round on outcome authority and lifecycle ownership Gate effects on unpack_ok with cleared ordinal and terminal no-effects, intersect executor with applied children, treat absent report-status as indeterminate regardless of exit, require landing history before child delete, claim due requests with recoverable leases, dedupe webhooks by occurrence ledger, ack-gate proof retention with terminal disposition, fair marker tombstones with dead-letter, kill_on_drop marker bounds, and loud migration version collision guard. * fix(node): load-bearing coverage for intent, guards, and delivery ledger Handler intent test plus inv22 gate so disabling the durable insert turns red; unpack-false, mixed ok/ng, divergent cancelled, partial sibling, and proof-ack tests pin each executor guard; webhook deliveries record sent_at with stale-pending reclaim; stale parsed_report comment corrected. * fix(node): centralize live retry, bound outcome commit, close retention orphans Route live Retry/Err through schedule_request_retry_or_quarantine so attempt progression matches the drain; retry post-git outcome commits 3x before restart repair; block terminal purge while non-terminal children exist; add v27 upgrade-path test. * fix(node): document attended-restart commit policy and pin executor retry arms Track outcome-commit success, skip metrics/touch on failure, correct the lease-expiry comment to startup reconcile; add restart-repair test for stuck received parents plus RED tests for landing-history and proof-ack failure paths. * fix(node): correct marker prereq comment to warn-and-proceed * fix(node): single partial-completion model with disposition gate Split apply_request_effects into phase-1 bundle (push, cert, anchor, history, webhook, proof ack before child deletion) and phase-2 gate that retries while non-cancelled siblings remain; Nothing only when nothing owed. Gate tail, Tigris, and effects on PostGitDisposition with documented replication carve-out; add partial-sibling webhook and disposition tests; fix stale comments. * fix(node): close review round on outbox ordering, marker visibility, and repair reachability Address the P1/P2 review findings: await the webhook delivery claim before accepted-child deletion; exempt the marker namespace from the fail-closed ref peeling guard so visibility walks tolerate blob-pointing request markers; keep request marker deletion bounded in the purge path; promote cross-page siblings via idempotent report superset; run stuck-parent repair when the prepared queue is empty; bound the durable intent write while admission permits are held; remove dead proof verification scaffolding. * fix(node): close review round on pre-git refusal lifecycle, namespace gate, and promotion merge * fix(node): close review round on namespace gate, intent lifecycle, and promotion atomicity Namespace gate hardening: - Refuse non-UTF-8 ref-update lines (git accepts bytes ≥0x80; gate never sees them) - Catch bare 'refs/gitlawb' name (starts_with missed it) - Add deny test for both holes plus refusal wiring verification Intent lifecycle guards: - Add IntentDropGuard: terminalize stranded intents on dropped futures - Bound refuse_receive_pack_before_git to 10s (no statement_timeout on pool) - Guard mirror fetch: hideRefs blocks non-exempt refs/gitlawb/* from +refs/*:refs/* Promotion merge atomicity: - Take row under SELECT FOR UPDATE so merge is atomic with state read - Prevents racing due worker from stranding newly applied children outside report * fix(test): correct inv22 namespace gate assertions to match actual implementation The test was looking for the wrong patterns: - UTF-8 check: error message string, not pre-loop condition - Namespace check: `*r == "refs/gitlawb"`, not `ref_name ==` - Visibility exemption: `&&` logic (both false to exclude), not `||` * chore: cargo fmt * fix(node): phase-aware intent guard, mirror import prune, terminal-state merge * fix(node): guard cleanup retry with fates, fail-closed mirror prune, refusal coverage * fix(node): bound guard attempts, sibling-wait outcome, fail-closed prune enumeration * fix(node): sibling-wait outcome, no-report path, bounded prune and guard retry * fix(node): no-report durability, cert landing order, bounded reconcile git, flush checks * fix(node): gate trust-score bump on a new push event row * chore: update rustls 0.23.37 → 0.23.45 to resolve RUSTSEC-2026-0285 * fix(node): surface git's report-status when a stdin EPIPE follows child exit * fix(node): cap receive-pack ref-update count before per-ref work and intent * fix(node): cap ref-update parsing in-loop and dedupe identical commands * docs(node): correct round-4 comment claims on client outcome and gate cost * fix(node): single replication tail, proven marker taxonomy, claimed webhook delivery (#558) - One post_receive_replication_tail per push: the carve-out spawn and the durability-bound spawn are mutually exclusive positions on the same proven-landed ref set; inv22 now fails on any unfiltered or ungated spawn. - Marker reconcile: bounded-read errors (git Err/deadline) stay prepared and retry instead of quarantining as 'missing marker ref'; only a successfully read marker proves missing or mismatch. Err-injection test pins no-quarantine on unproven reads. - Claimed webhook delivery: the synchronous claim phase returns the claimed Webhook rows so the send task never re-lists; a list failure after claim can no longer drop claimed deliveries. Source gate pins it. * docs(node): name claimed-hooks variant in fire_event_occurrence doc * fix(node): order pending_ref_transitions create before outbox alters, fill v29 (#558) * fix(node): proven-missing marker taxonomy, v38 upgrade test, report-ok tail fixtures (#558) * fix(node): read marker refs via rev-parse so present refs return their oid (#558) --------- Co-authored-by: Gravirei <[email protected]> Co-authored-by: Kevin Codex <[email protected]>	2026-11-09 23:40:38 +00:00
beardthelionandcarlitos973	3c1ee27b60	fix(node): make creation-route rate limit configurable; fix(gl): surface real errors on pr create failure (#560) ... - gitlawb-node: rate limiter was hardcoded to 10 req/hour per DID across all creation routes (repo/issue/PR create, register, fork). Adds GITLAWB_RATE_LIMIT_MAX and GITLAWB_RATE_LIMIT_WINDOW_SECS config fields (clap env-var pattern matching the rest of Config), defaults unchanged. Lets node operators tune this for real agent/bot usage without recompiling. - gl pr create: previously called resp.json() before checking response status, so any non-2xx response with an empty or non-JSON body crashed with a generic "invalid JSON" error instead of surfacing the real cause. Now reads the raw response body first and includes it in the error message on failure. Co-authored-by: carlitos973 <[email protected]>	2026-11-09 23:02:05 +00:00
beardthelionandKRATOS	209618a2fe	feat(node): add scheduled inbound mirror worker (#555) ... * feat(node): persist upstream mirror transition state Origin-Session: local-d6a143 | Codex | 12 prompts * fix(node): reject null mirror transition state Origin-Session: local-d6a143 | Codex | 14 prompts * fix(node): address mirror state review findings Origin-Session: local-d6a143 | Codex | 16 prompts * fix(node): close mirror review blockers Origin-Session: local-d6a143 | Codex | 20 prompts * fix(node): align peer URL validation message Origin-Session: local-d6a143 | Codex | 20 prompts * fix(node): close remaining mirror review findings Origin-Session: local-d6a143 | Codex | 21 prompts * feat(node): add scheduled inbound mirror worker Origin-Session: local-d6a143 | Codex | 18 prompts * fix(node): harden scheduled mirror fetches Origin-Session: local-d6a143 | Codex | 24 prompts * fix(node): close inbound mirror review blockers Origin-Session: local-d6a143 | Codex | 24 prompts * fix(node): reject special-use upstream literals at configure time (#555) validate_mirror_upstream_url used only the peer public-host classifier, so an IP literal the fetch worker's stricter permit denies (192.0.2/24, 198.18/15, 2001:db8::/32, ...) could be stored and then hard-fail on every scheduled scan. The IANA special-use permit moves to peers:: mirror_upstream_ip_permitted, shared by configure validation and the resolve-path check; the negative test set now covers the denied literals. * test(node): scrub inherited git env in mirror fixture spawns (#555) The repository_local_* fixture tests ran plain Command::new("git") and inherited any GIT_DIR/GIT_CONFIG_* state from an outer checkout, so a fixture spawn could act on the wrong repository. A shared fixture_git helper now sets the cwd and removes git environment vars; all five spawn sites use it. * fix(node): bound mirror preflight probes separately, align operator docs (#555) * chore(node): retrigger checks after code-scanning alert triage (#555) * chore(node): retrigger checks after code-scanning alert triage (#555) --------- Co-authored-by: KRATOS <[email protected]>	2026-11-09 22:48:47 +00:00
beardthelion	a00d7d5c48	fix(gl): pin secret-file modes at creation instead of chmod after (#354) (#557) ... * fix(gl): pin secret-file modes at creation instead of chmod after (#354) identity.pem and ucan.json were written with fs::write and then chmod'd, leaving the private key world-readable between the two syscalls, and ucan.json was never chmod'd at all. A shared helper now opens with mode(0o600) so no permissive window exists, adds O_NOFOLLOW so a pre-planted symlink is refused rather than written through, and re-pins the mode after write so a pre-existing loose file is tightened. The containing directories are created 0700 via DirBuilder and re-pinned the same way. gitlawb-node's own key write gets the same treatment. * fix(gl): tighten secret handling before, not after, replacing contents (#459) Address PR review feedback: - secret_file::write and the node key write now open without truncate, pin mode 0600 on the descriptor, then set_len(0) and write, so a pre-existing permissive file never holds new secret contents while still group/world-readable. - Directory re-pins skip symlinked paths instead of chmodding through them to the link target. - The node's existing-key read opens with O_NOFOLLOW and reads from the descriptor instead of read_to_string on the path. - An existing permissive key parent dir is re-pinned to 0700. * fix(gl): adopt writable --dir leaves the user cannot chmod (#557) create_dir re-pinned every existing final directory to 0700. On a leaf the caller can write but does not own (group 0775, ACL grant, a system dir like /tmp), fchmod fails EPERM and the whole command aborted, a regression over create_dir_all's no-op. The re-pin now applies only when the euid owns the opened directory; residual EPERM on an owned leaf adopts rather than aborts. Regression test proves /tmp is adopted with its mode unchanged.	2026-11-09 20:13:51 +00:00
5 people	4cd0a6a306	fix(node)!: gate agent-task reads behind visibility rules (#553) ... * fix(node): gate agent-task reads behind the same visibility rules as repo data list_tasks and get_task had no authorization at all: any anonymous caller could enumerate every task on the node, including another party's repo-less task, its ucan_token, and its payload (#268). Add task_visible, mirroring the repo read-visibility gate already used by the ref-updates feed: the delegator and assignee can always read their own task, a repo-scoped task follows that repo's normal visibility rules, and a task naming no repo (or a repo this node doesn't host) is visible only to its delegator/assignee. Both REST and GraphQL now route through the same collect_visible_tasks/get_visible_task collectors so the two surfaces cannot drift, and neither read path echoes ucan_token back, since the holder already received it via the create/claim response. Fixes #268 * fix(node): query the task limit-clamp test as the delegator tasks_limit_ceiling_clamped_to_200 seeded 201 repo-less tasks and read them back anonymously, expecting all 200. That read is exactly the enumeration #268 closes, so the new visibility gate correctly returns none of them and the test went red. The clamp ceiling is what this test pins, not the gate, so query as the tasks' delegator, who can legitimately see all 201 rows. Refs #268 * fix(node): scope task visibility lookups to the page's repos collect_visible_tasks loaded every repo on the node and every visibility rule in order to gate at most 200 tasks, so an anonymous request paid for the whole node's repo and rule set. Narrow both lookups to the repo ids the fetched page actually names, and skip them when no task names a repo. The deduped repo snapshot stays the source of truth for resolving a repo_id: it collapses mirror and canonical pairs and omits quarantined repos, and an id missing from it has to keep failing closed. Resolving ids straight from the repos table would surface exactly those withheld rows. Add GraphQL denial tests as well. Nothing pinned that the task resolvers delegate to the shared collectors, so a resolver that queried the database directly would not have gone red. Refs #268 * Preserve authorized task reads across shipped clients. Refs #268 * Bound denied task history scans. Refs #268 * Signal incomplete task scans with recoverable cursors and map read errors to AppError. Refs #268 * Stop disclosing denied task rows in the scan-wall cursor and add GraphQL pagination state. Refs #268 * Normalize task cursors and gate mutation endpoints behind visibility. Canonicalize RFC 3339 timestamps in parse_after_cursor to handle URL-decoded spaces, reject mixed cursor alias families, gate complete_task and fail_task behind get_visible_task so unreadable tasks 404 instead of leaking existence with 403, and only flag incomplete when hitting candidate ceilings on full SQL batches. Refs #268 * Update complete_task test in test_support to exercise both 404 on unreadable and 403 on non-assignee tasks. Refs #268 * Bind task to repo id in complete_task test in test_support. Refs #268 * Preserve timestamp fractional width and anchor continuation tests on legitimate cursors. Refs #268 * Match on Value::Object to extract task ID in GraphQL continuation test. Refs #268 * Remove unused mut binding on visible_newer in GraphQL query test. Refs #268 * Hide unauthorized claim and keep assigned tasks from being stolen Gate REST and GraphQL claim behind the same visibility check as complete and fail, refuse claim when another assignee already holds the task, and only broadcast publicly visible task events. Treat a full list page as incomplete when more candidates remain. Surface HTTP errors from CLI and MCP claim and complete helpers. Refs #327 Co-Authored-By: cairn-code <[email protected]> * Keep task-list incomplete for the scan ceiling only. A full visible page was flagged incomplete whenever the SQL batch was full, so the first page of any list with more than 200 candidates looked stalled. Align the GraphQL claim test with the visibility gate's not-found message. Refs #268 * fix(node): pin claim guards and map mutation errors through AppError Review required tests that go red if the pre-assigned claim predicate or the anonymous announce gate is deleted, and incomplete must not stay true when the candidate stream is exhausted at the scan ceiling. Route claim, complete, and fail through AppError so closed-pool outages stay 503 and 404s match the read envelope. Refs #268 * fix(node): match bare and did:key assignee forms in claim and list create_task stores the supplied assignee unchanged, so a raw SQL equality check drops a designated assignee who presents the other did:key form. Compare the normalized key so claim and filtered list agree with did_matches. Refs #268 * fix(node): check task creation status and add assignee expression index - Add error_for_status() to cmd_create and task_create MCP tool - Update test_create_task_server_error to assert failure on 500 - Add migration v18 creating expression index idx_agent_tasks_assignee_key matching ASSIGNEE_DID_CASE_SQL - Add did:web:z6Mkfoo single-residual shape to parity boundary matrix Refs #327 # Conflicts: # crates/gitlawb-node/src/db/mod.rs * fix(node)!: Replace task cursors with opaque continuation tokens The task read path treated visibility, pagination, and error vocabulary as separate edits, so each one broke where they met. Rework them as one contract. A raw (created_at, id) cursor forced a choice between two broken options: it could name the last visible row, and then a denied window longer than the 1,000-candidate scan budget was unpageable forever; or it could name the last examined row, and then a denied read leaked the id and timestamp of a task GET /tasks/{id} otherwise 404s. Continuation tokens remove the choice. They carry the last examined candidate, so paging always advances a full scan budget per request, and they are encrypted and authenticated under a node-derived key, so the caller learns nothing from one and cannot forge one naming a row of their choosing. Encryption is a synthetic-IV construction over the hmac/sha2 pair already used for webhook signatures, so it adds no dependency and needs no randomness source. Making the token the only accepted cursor also gives the ordering key one domain. agent_tasks.created_at is TEXT and compared as TEXT, so a caller-typed '...Z' and '...+00:00' denote one instant but sort differently, and a client could silently skip or repeat same-time rows. The token carries the stored string verbatim, so the value compared is always one the server wrote. The raw after_*/cursor_* pairs are removed rather than kept alongside it, since a second domain is the bug. Separate the two facts the old single incomplete flag conflated: has_more says candidates remain, incomplete says this page is short only because the authorization scan hit its ceiling. Both REST and GraphQL now return has_more, incomplete, and next_cursor from the shared collector, and REST echoes the limit it actually applied so a clamped request is visible as clamped. Have gl task list and MCP task_list follow next_cursor instead of issuing one request: --limit 500 returned a successful but silently truncated 200 rows. Following is bounded by a page cap and a no-progress guard, and a run stopped by either reports an explicit incomplete result with a resume cursor. Route claimTask, completeTask, and failTask through the same task_write_conflict classifier the REST handlers use, via curated helpers in the graphql module so the map_err source guard still holds. A claim race or stale finish reached GraphQL clients as a generic database error while REST clients got an actionable conflict; genuine sqlx faults stay opaque on both. Refs #327 * fix(node): End the candidate stream only after the batch is examined A short SQL batch means no rows exist past it, not that every row in it was examined. When the page filled mid-batch the collector treated the two as the same, marked the stream ended, and suppressed the continuation, so every row after the one that filled the page was unreachable. The equal-timestamp paging tests caught it: three rows with a limit of one returned only the first. Track how much of each batch was consumed and end the stream only when the whole of a short batch has been examined. Otherwise leave `has_more` to the probe row, which resumes from the last examined candidate. Refs #327 * fix(gl): Reject a non-positive task-list limit and fault the write under test A `--limit 0` reached the node, which clamped it to zero and answered with an empty page marked complete, so an invalid request read as proof that no tasks exist. Reject a non-positive limit in `fetch_tasks()`, the helper the CLI and MCP share, so the guard cannot drift between the two surfaces. `task_write_sql_faults_stay_opaque` did not exercise what it named. Dropping `updated_at` also broke the SELECT in `get_task()`, so the fault surfaced from the `get_visible_task()` pre-check through `graphql_app_err` and never reached `graphql_claim_conflict`. A `BEFORE UPDATE` trigger keeps every read valid and faults only inside `Db::claim_task`, and the test now also asserts that a write-time fault is not reclassified as a claim race. Refs #268 * fix(node): Bind task cursors to the caller and brake the task read routes A continuation token names the last candidate a scan examined, not the last row it returned, so it encodes how far that scan got under one caller's visibility. The MAC bound the page filter but not the presenting identity, so resuming a token as a different caller started the scan past rows that caller was entitled to read and dropped them from the answer with nothing to signal the loss. Bind the caller's normalized DID into the MAC, with anonymous flagged absent rather than encoded as empty. Normalization goes through normalize_owner_key so the two spellings of one did:key identity bind identically, matching did_matches on the read path: a caller who presents the other form of their own DID keeps their own page. A mismatched token renders the existing single rejection message, so this adds no oracle. GET /api/v1/tasks and GET /api/v1/tasks/{id} are anonymously reachable, and the visibility gate costs a task lookup plus deduped-repo and visibility-rule queries before it can return the opaque 404. An unauthenticated prober therefore pays nothing while the node pays per request, whether or not the id exists. Attach the per-IP limiter already used on /ipfs/{cid}, configurable through GITLAWB_TASK_READ_RATE_LIMIT and swept by the periodic task like every other per-key limiter. Refs #268 * fix(node): Brake anonymous GraphQL task reads on the same per-IP bucket The per-IP brake added for the task read routes covered only /api/v1/tasks*, so an anonymous caller reached the same collect_visible_tasks and get_visible_task gate over /graphql with no bucket at all. The fence had an open lane beside it. Carry the brake as GraphQL request data and debit it in the tasks and task resolvers rather than layering rate_limit_by_ip onto the GraphQL router: /graphql is one endpoint for every operation, so a router layer would charge unrelated queries and every mutation against the task-read bucket. Debiting per resolved field also prices an aliased query honestly, since ten aliased tasks fields run the gate ten times. Extract RATE_LIMIT_MESSAGE so the GraphQL surface, which cannot return a 429 status inside a 200 envelope, refuses with the same text the REST routes use. /graphql/ws serves the query root as well and stays unbraked; closing it needs a WebSocketUpgrade handler and is left for a follow-up. Refs #268 Refs #327 * fix(gl,node): parse typed task pages, track cursor cycles, and normalize assignee filter MAC * fix(node): Cap aliased GraphQL task reads per request and derive has_more from visible rows - Cap aliased GraphQL task read fields per request using an atomic counter on TaskReadBrake (MAX_GRAPHQL_TASK_READS_PER_REQUEST = 5). - Derive has_more in collect_visible_tasks by scanning for bounded_limit + 1 visible rows, eliminating the un-gated keyset probe that could leak the presence of trailing denied tasks. - Add regression tests covering aliased GraphQL capping and trailing denied task has_more privacy. Refs #327 * fix(node): Check database continuation when scan budget ends on exact batch boundary When candidate scanning reaches MAX_TASK_SCAN_CANDIDATES without finding a target_visible row and the final batch was full, probe the database for rows beyond the scan position so an exhausted candidate stream is not erroneously marked incomplete. Refs #327 * fix(node): Document and pin the bound on the task scan-ceiling probe The scan-ceiling branch of collect_visible_tasks settles has_more with an un-gated LIMIT 1 probe, so a caller can learn whether any row - readable or not - trails the position the scan stopped at. Withholding the probe does not remove that bit: enumeration past a denied window longer than one scan budget requires handing back a continuation, and following that continuation returns the same terminal page one round trip later. State what the probe discloses (one bit, only at server-chosen positions a full scan budget apart, reachable only through a MAC'd cursor, never a denied row's id, payload or ucan_token) and pin it end to end. Also correct the comment above the branch, which claimed has_more never comes from an un-gated probe while the code below it did exactly that. Refs #327 * Index task keyset pages so the scan ceiling bounds database work. Optional IS NULL predicates kept the planner from using a created_at/id order, so every list_tasks_keyset batch could sort a growing match set before LIMIT. Dedicated per-domain SQL plus v28 indexes make the candidate ceiling a database bound. Refs #327 * Reject oversized task pages so list never returns more than the requested limit. fetch_tasks asked for the remaining total, then appended every row on a valid-shaped page. A remote that sent more tasks than want could make gl and MCP expose more than --limit. Treat that page as protocol-invalid before any extra row is kept. Refs #327 * Enforce task page limits on legacy responses and drop redundant single-column indexes. Refs #327 * Address review feedback on task-read authorization gating and client safety. - Wire TaskReadBrake into /graphql/ws subscriptions and verify per-request field caps and per-IP rate limiting over WebSocket connections. - Define open-claim eligibility separately from read visibility so unassigned tasks on readable/unscoped domains can be claimed without making task bodies enumerable. - Propagate identity errors on explicit key directories in CLI and MCP instead of silently falling back to anonymous mode. - Enforce response byte limits before deserializing task pages, validate row schema and page-local uniqueness before row commit, and sanitize continuation cursors in terminal diagnostics. - Add GraphQL denial assertions for repo-less tasks and token isolation. Refs #268 Refs #327 * Address review feedback on task-read authorization gating, WebSocket budgets, and test framing. Refs #395 * Address review feedback on quarantined task claims, WebSocket auth, and MCP limit parsing. Refs #395 * Reject forged signatures during WebSocket authentication upgrade. Refs #395 * Accumulate forged WebSocket handshake response until headers terminate. Refs #395 * Withhold quarantined-repo tasks on all read surfaces before party checks. Refs #395 * docs: Align task-read documentation with enforced visibility and rate limits. Refs #395 * docs: Distinguish task list and get behavior in rate-limit guidance. Refs #395 * fix(node): Batch task quarantine checks to bound read costs. Refs #395 * docs(node): clarify the shared task-read budget Document that GraphQL task queries over HTTP and WebSocket debit the same per-IP hourly budget as REST task reads. Refs #395 * fix(node,gl): bound scoped repo lookup and validate MCP cursor Bound repository access in list_repos_deduped_by_ids to requested groups via idx_repos_owner_key_name, leaving dedup_cte unscoped. Strictly validate MCP task_list cursor argument before network dispatch. Refs #396 * test(db): verify scoped lookup work in prepared plans Measure discarded rows and verify actual custom and generic prepared-plan counters, including the previous full-index-scan regression. Fixup for a0ea4c78e4. Refs #396 * docs(db): remove stray dedup comment escapes Restore readable root-path notation in the deduplicated listing documentation. Fixup for a0ea4c78e4. Refs #396 * fix(node): address #464 review findings - Hold migration 27/28 numbering: reservation comment now names the competing Gitlawb/node#384 range (27-35) and states the numbering is on hold pending #384. No renumber. - Restore crates/gl/src/identity.rs to LF endings so the diff shows only the added load_optional_keypair function. - Drop the dead single-column idx_agent_tasks_assignee_key build from v27 (v28 builds the keyset-ordered variants directly); adjust the v27 test to assert the raw index is dropped and the expression index is never built. - Align task_claimable with claim_task's conditional write: drop the delegator short-circuit and correct the docstring. - Pin error_for_status on the gl task list path with a 403 mock that serves a well-formed empty page, so the test goes red if the call is removed. * test(node): pin the delegator deny on a pre-assigned claim - Add claim_task_delegator_cannot_claim_preassigned_task: pins opaque 404 for delegator claiming a pre-assigned task (regression guard for P2) - Fix test_list_tasks_server_error_surfaces regex: /api/v1/tasks\? -> /api/v1/tasks\? (escaped ? for literal question mark) - Assert actual reqwest StatusCode::FORBIDDEN instead of matching "403" in error string (avoids spurious pass from random mock port) * test(node): exercise streamed byte-budget guard with chunked body Use with_chunked_body instead of with_body so mockito sends no Content-Length, pinning the no-length streamed accumulation guard. Assert the "exceeded" message to distinguish from the declared-length check. * fix(node): pin declared-length byte-budget arm in oversized content-length test Addresses beardthelion review finding (2026-09-26): test_read_task_page_json_oversized_content_length now asserts on the "(declared" arm the way the chunked twin asserts "(exceeded", so the Content-Length pre-check at crates/gl/src/task.rs:211 is genuinely pinned. Verified the pin by neutering the pre-check (test goes red), then restored. fmt clean. * fix(node): fail closed on claim for unresolvable repo task_claimable returned true for an unassigned task whose repo_id is slash-form (mirror row) or names a repo this node does not host, while task_visible fails closed on the same rows. A signed stranger could claim such a task even though reads 404. Return false for a Some(repo_id) the node cannot resolve to a locally-hosted repo, matching task_visible's fail-closed check; keep the None arm open (repo-less tasks claimable by design). Adds a regression test asserting a stranger's claim 404s and leaves the assignee unchanged for both row classes. * fix(node): pad sealed task cursor to fixed width The scan-ceiling token seals the last examined row's (created_at, id) as variable-length JSON under a length-preserving keystream, so token length leaked the withheld row's field widths: a 32-char to_rfc3339() rendering mints a 148-char token where a 35-char one mints 161 chars. Pad the serialized payload with trailing JSON whitespace to a fixed 128-byte bucket before the SIV tag is computed; decode tolerates the padding (serde_json ignores trailing whitespace) and tokens minted before this change still verify. REST and GraphQL share task_cursor::encode, so one fix covers both. Adds token_length_does_not_vary_with_position_width: asserts identical token length across narrow/wide timestamp positions and that the padded token still decodes to the exact stored position. * fix(node): address beardthelion round-six P3 findings on task cursor - Pin the encode docstring compat claim: add unpadded_legacy_token_still_verifies test minting a token via the pre-padding recipe (siv + apply_keystream with no padding) and asserting decode returns the position. - Pin the PAYLOAD_BUCKET round-up claim: add oversized_payload_rounds_up_to_next_bucket test with a 140-char id crossing the 128-byte bucket, asserting round-trip at two buckets. - Correct timestamp-width figures: to_rfc3339() renders 25 chars with no fractional seconds (not 32); 32 is the microsecond width. * fix(node): address beardthelion round-seven P3 findings on task cursor - Correct wire-form docstring to v1.<iv>.<body> - Fix payload width figures: 101 bytes total, to_rfc3339() precision ladder - Tighten oversized-payload test to assert exactly 2*PAYLOAD_BUCKET - Pin legacy back-compat test with a frozen v1 token fixture * test(node): address beardthelion round-eight P3 findings on task cursor - Pin the one-bucket width: round_trips_position_verbatim now asserts the decoded body of a normal token is exactly PAYLOAD_BUCKET, catching a pad that skips next-multiple or a drifted bucket constant. Every other width assertion here is bucket-relative, so only an absolute check goes red. - Assert the frozen fixture is the unpadded shape: unpadded_legacy_token_still_verifies decodes the body segment and asserts raw.len() < PAYLOAD_BUCKET, so a literal re-minted under the padded recipe would retire the back-compat pin loudly instead of passing silently. - Narrow the recipe-pin claim: the docstring now names what the frozen literal actually binds (domains, MAC input, keystream, segment layout, field names) instead of claiming padding and serialization changes break the test. The padding-width claim stays with oversized_payload_rounds_up_to_next_bucket. * fix(node): trim stored assignee on every task gate; align SECURITY.md task-read wording * test(node): pin mirror-row list-vs-get asymmetry; fix plan-test bind (#553) - EXPLAIN helper binds normalize_owner_key(assignee) to match the production list_tasks_keyset bind. - Pin the deliberate asymmetry jatmn asked about: a delegator sees a slash-form (mirror-hosted) task in a list, while get fails closed for every caller because the row has no local repo backing. Notes in get_visible_task and a regression test keep it from drifting. - Repair test-mod drift from the main merge: build_schema gains its TaskCursorKey arg at three sites and list_tasks calls move to list_tasks_keyset. * test(node): reconcile tasks complexity contract with TaskPageType (#553) The four GraphQL task/brake tests failed on this branch: contract tests still queried the flat tasks { id } shape that TaskPageType replaced, and the alias-brake tests ran default-size lists whose six aliases exceed GRAPHQL_MAX_COMPLEXITY before the brake can debit. - Contract tests now select through items { ... } with refreshed cost arithmetic. - Brake tests use tasks(limit: 1), keeping six aliases under the budget so the 6th field reaches TaskReadBrake and fails RATE_LIMIT_MESSAGE. Complexity stays the outer gate (#552); the brake stays the per-request field-count gate. * docs: note mirror-hosted task list-vs-get asymmetry (#553) Option 3 for slash-form mirror repo_id rows was pinned in code and test at 6c0926de; this adds the operator-facing half jatmn asked for. README and SECURITY task-read bullets now state that mirror rows list for party readers while get-by-id and GraphQL task fail closed for all callers. * ci: retrigger codeql scan after alert 86 dismissal --------- Co-authored-by: euxarist <[email protected]> Co-authored-by: cairn-code <[email protected]> Co-authored-by: cairn-intern <[email protected]> Co-authored-by: Marlowe <[email protected]>	2026-11-09 19:02:54 +00:00
3 people	0919cf7c62	fix(core): ignore invalid signatures in satisfies_threshold (#349) (#554) ... * fix(core): ignore invalid signatures in satisfies_threshold (#349) RefUpdateCert::satisfies_threshold called verify_all, which fails closed on the first malformed or invalid signature entry. Signature entries live outside the signed body, so any third party can append a junk entry with no key material — appending one denied an otherwise valid certificate, a DoS on the threshold check. Add a lenient valid_signers() helper that skips entries failing to parse, decode, or verify, and have satisfies_threshold count distinct valid signers from it. verify_all stays strict (fail-closed) so its API and the tampered_signature_fails_verify_all test are unchanged. A forged entry naming a real maintainer DID with a bad signature is skipped, so it cannot inflate the count either. Adds tests: appended junk entries do not deny a valid 2-of-2 cert, and a forged maintainer signature is not counted toward the threshold. Closes #349 * fix(core): bound the tolerant signature scan before cryptographic work satisfies_threshold verified every entry, so a peer could append arbitrarily many syntactically valid entries with bogus signatures and buy attacker-controlled CPU and allocation on every threshold check. Bound each stage ahead of the loop: reject a list over MAX_SIGNATURE_ENTRIES loudly (a prefix scan would let junk starve real signatures out of the window), refuse to base64-decode any encoded signature that is not exactly the 86 chars of a 64-byte Ed25519 signature, skip non-maintainer and already-counted signers on a string compare before any curve work, and return as soon as the threshold is met. * docs(cert): correct ENCODED_SIG_LEN length-gate comment (#554) The comment claimed anything longer cannot decode to a valid signature. The same 64 bytes appear in padded or non-url-safe encodings; the gate is a canonical-form DoS bound for the tolerant scan, not a crypto impossibility. --------- Co-authored-by: andreolf <[email protected]> Co-authored-by: Kevin Codex <[email protected]>	2026-11-09 17:42:13 +00:00
beardthelionandRapyBus S.A.C.	05afd68e41	bootstrap-peers: add moonwake-ledger with a dialable p2p multiaddr (#559) ... Co-authored-by: RapyBus S.A.C. <[email protected]>	2026-11-09 17:21:54 +00:00
beardthelion	6c971e7ea0	ci(audit): ignore RUSTSEC-2026-0459, unlinked libsqlite3-sys advisory (#564) ... The advisory covers bundled SQLite in libsqlite3-sys 0.30.1, which sits in Cargo.lock only because sqlx resolves sqlx-sqlite unconditionally. sqlx builds postgres-only, so the bundled C source is never compiled or linked. The fix needs libsqlite3-sys >= 0.39 but sqlx-sqlite 0.8.6 pins ^0.30.1; the waiver is documented to retire if the sqlite feature ever enters the build.	2026-11-09 16:54:05 +00:00
beardthelion	10d5b98b2b	fix(gl): report quickstart registration failure when the node returns no UCAN (#355) (#556) ... A 2xx with a non-JSON or ucan-less body still printed "Registered successfully / UCAN saved to ..." while nothing was written, and left a stale ucan.json from another node in place. Print the success lines only when a UCAN was actually persisted.	2026-11-09 16:11:08 +00:00
3 people	a2262d92b2	fix(node): implement reconciliation sweep as durability backstop (#218) (#551) ... * feat(node): implement reconciliation sweep as durability backstop (#218) Reconciliation sweep: periodic background worker that re-derives pin/seal sets and fills gaps so a dropped replication job never means data loss. - visibility_pack.rs: all_object_paths (two-phase ls-tree + cat-file --batch-all-objects), allowed_blob_tree_sets_bounded, ObjectPath/BlobTreeSets type aliases - db/mod.rs: atomic policy transactions, get/set_node_state, has_ipfs_cid, filter_ipfs_pinned_oids, filter_pinata_pinned_oids, repo_policy_epoch, migration v27 (node_state table + policy_epoch column) - ipfs_pin.rs: PolicyFence struct, dispatch fence re-check before POST - pinata.rs: dispatch fence re-check before POST - encrypted_pin.rs: dispatch fence re-check - reconciliation.rs: full sweep worker with keyset cursor, PolicyFence - api/ipfs.rs: effective_cid fallback in list_pins - main.rs: spawn reconciliation sweep worker - .cargo/audit.toml: RUSTSEC-2026-0258 h2 ignore (pending #368) * fix: add missing parameters to pinning functions and adjust timeout settings * fix(tests): update CID assertions to handle Option type and adjust parameters in pinning functions * fix(node): close review findings on reconciliation sweep (#218) Reviewer-1 P1 / Reviewer-2 P1: v27/v28/v29 were referenced by tests but never added to the MIGRATIONS array, so every existing deployed node would fail to boot (node_state, repos.policy_epoch, pinned_cids.cid NOT NULL → NULL all unreadable). Forward-only migrations: v27 pinata_only_clear_legacy_equal_cid: ALTER cid DROP NOT NULL, UPDATE cid = NULL WHERE cid = pinata_cid v28 node_state_key_value: new key/value table for the sweep cursor v29 repos_policy_epoch: BIGINT NOT NULL DEFAULT 0 Reviewer-2 P1: visibility_pack.rs:542 phase 1 used ls-tree -rz, which under -r recurses into blobs and never emits tree entries. Trees then arrived only from the phase-2 catch-all with empty path and the fail-closed filter denied every one — the sweep could repair a non-flat repo's git graph. Switched phase 1 to ls-tree -r -t -z and add a per-commit rev-parse <commit>^{tree} that registers the root tree at "/". 48 visibility_pack unit tests pass. Reviewer-1 P2 / Reviewer-2 P2: list_pinned_cids called is_raw_cidv1 on the cid column before decoding it as Option<String>, so SQL NULL failed the decode and the new Pinata-only rows never reached the handler. Decode cid as Option first, drop only the both-NULL case. The is_raw_cidv1 filter was removed entirely: the new contract lists every row that has something to advertise, and the handler at api/ipfs.rs:list_pins is the seam that decides what to do with a legacy-shape row (the resolver 404s on a mismatched key, per #173 U4). Updated list_pinned_cids_omits_unrepaired_legacy_row to match. Reviewer-1 P2: two new sqlx::test cases pin the gate order: sweep_skips_quarantined_repos_before_scan (SQL filter, plus a defense-in-depth per-row re-check) and sweep_skips_private_repos_before_scan (per-repo listable_at_root). Both use expect(0) on the mock IPFS so any future reorder that moves the scan ahead of the gate fails the build. Also: record_pinned_cid ON CONFLICT now rewrites cid (R1-P2: a stale wrong CID from a previous push is repaired by a subsequent one). record_pinata_cid ON CONFLICT clears cid to NULL when the existing row has the legacy cid = pinata_cid shape; on INSERT, stores cid = NULL when raw_cid == pinata_cid (the Pinata-only signal). * feat: implement per-backend continuation offset for missing OIDs - Enhance `missing_oids` function to accept a `start_after` parameter, allowing for rotation of the missing set based on the last attempted OID. - Update `run_pass` to load and save the continuation offset for both IPFS and Pinata backends, ensuring that previously attempted OIDs are retried last in subsequent passes. - Introduce tests to validate the behavior of the new offset mechanism, ensuring deterministic ordering and proper handling of gaps in the missing set. - Ensure that the offset is cleared when no missing OIDs are found, allowing for fresh starts in subsequent passes. * fix(node): trace provenance/structural invariants through every consumer The v30 local_ipfs_provenance column was set by the writers but only consulted by has_ipfs_cid and filter_ipfs_pinned_oids. The pin loop's "is this already done" check still used row existence (is_pinned), so a Pinata-only row hit the early-return branch without ever reaching the local writer, and the flag stayed FALSE forever. Three other seams carried the same shape-vs-provenance confusion. P1 (ipfs_pin.rs:1843): pin loop's skip keys on has_ipfs_cid (writer-owned local_ipfs_provenance = TRUE), not on row existence. A Pinata-only row now falls through to the local writer path, which calls record_pinned_cid_with_source and flips the flag on the conflict branch. The Pinata pin loop's skip remains keyed on has_pinata_cid (a separate seam). P1 (db/mod.rs v30 backfill): tighten the WHERE clause to cid IS NOT NULL AND pinata_cid IS NULL. The previous permissive (cid <> pinata_cid) clause marked pre-v30 Pinata-first uploads (where record_pinata_cid deliberately stored cid = raw_cid without a local Kubo push) as local IPFS pins, so the sweep filtered them as already-durable and an operator enabling IPFS later never got a local recovery copy. Ambiguous dual-shape rows stay at FALSE under the strict rule and the sweep re-derives on re-pin (cheap, idempotent). P1 (db/mod.rs record_pinned_cid_with_source ON CONFLICT): restore cid = COALESCE(pinned_cids.cid, EXCLUDED.cid). A Pinata-only row with cid = NULL (the raw_cid == pinata_cid shape) would otherwise be marked complete with local_ipfs_provenance = TRUE and a NULL resolver key. P1b (visibility_pack.rs): remove the synthetic "/" path on root trees and add a structural entry-level check (structurally_safe_root_tree / _for_caller) in both the sweep gate (allowed_blob_tree_sets_bounded) and the per-request tree gate (allowed_tree_set_for_caller_bounded). A tree is admitted only if every direct entry in its serialized bytes is safe at /{filename} and (for tree entries) the child tree is itself in the allow set. The previous path-based "/" check let the root tree of a public repo slip through even when its bytes name a denied subtree entry. P2 (api/ipfs.rs list_pins): add local_ipfs_provenance to PinnedCidRecord, include in list_pinned_cids SELECT, and surface as local_pinned / pinata_pinned in the JSON response. A Pinata-only row previously returned the raw resolver key as local_cid, making it indistinguishable from a real dual-backed row. The new fields let a gl consumer distinguish local-only, remote-only, and dual rows without re-inferring semantics from nullability. local_cid is preserved for backward compatibility. Tests: - migration_v30_backfills_local_ipfs_provenance_heuristically: flipped both-distinct expectation to FALSE under strict rule. - migration_v12_makes_cid_nullable_and_preserves_classification and migration_v27_clears_legacy_equal_cid: both-distinct / distinct row expectations flipped to FALSE. - sweep_never_pins_root_tree_naming_withheld_subtree (new): assert root tree + secret subtree tree absent from pinned_cids. - allowed_tree_set_gates_withheld_subtree_tree: root tree assertion inverted to EXCLUDED for anon. - list_pins_reports_writer_owned_provenance_for_all_shapes (new): assert 4 row shapes produce correct local_pinned / pinata_pinned JSON. Scope discipline preserved: PolicyFence capture order, _pin_permit reuse for the seal phase, fresh per-arm rederive_budget, keyset repo cursor with lookahead, mirror-row skip, Kubo 2xx-without-Hash refusal, v27 provenance-clearing migration all untouched. v31 schema unchanged. * fix(node): structural tree gate at every depth + non-commit ref tolerance The prior round installed a `local_ipfs_provenance` column and applied a structural tree gate only to root trees. The fresh review pass found two structural gaps the writer contract didn't propagate to: P1 (visibility_pack.rs:1519): the path-based tree admission admitted a tree whenever its path was allowed, regardless of whether its serialized bytes named a denied subtree. Concrete failure: a public repo with `/public/secret/file` and a `/public/secret/**` deny admitted the `/public` tree (its path is `/public`, allowed) but `/public` still contained a `secret/` entry pointing at the denied subtree — anyone who obtained `/public`'s CID could `cat-file -p` it and read the subtree's name and child OID. The prior round's post-pass structural check ran on root trees only. The fix: replace the prior round's `structurally_safe_root_tree` / `_for_caller` / `root_tree_oids` helpers with one recursive `tree_structurally_safe` that walks every tree's entries at every depth, recursing on tree entries. Bundled the rule/path args into a `TreeCheckCtx` struct to fix the clippy `too_many_arguments` warnings the prior round introduced. A tree is admitted iff every direct entry is safe at its path AND (for tree entries) the child tree is itself structurally safe there. A denied-subtree tree fails at its level; the parent fails because the denied-path child entry fails; the propagation continues up to the root. The `admitted` set memoizes trees proven safe at some path so the recursion short-circuits on cycles and on trees reachable at multiple allowed paths. P1 (visibility_pack.rs:533): `all_object_paths` called `assert_all_refs_are_commits`, which bailed on any ref that didn't peel to a commit (tag-of-tree, tag-of-blob). The sweep (`run_pass`) converted that error into a full-scan failure that skipped the whole repo. A repo with a pushable tag-of-tree never got its missing public pins repaired by the new hourly worker, even though its commit-reachable graph was otherwise safe to classify (`ipfs_cid_tree_served_despite_non_commit_ref` is the in-repo example of that supported shape). The fix: remove the `assert_all_refs_are_commits` call from `all_object_paths` (and the analogous one in `blob_paths`, used by the encrypted recovery path — the same reasoning applies). `git rev-list --all` already silently skips non-commit refs, so the commit-reachable object set is what the sweep needs. The unused `assert_all_refs_are_commits` helper is removed. Unclassifiable ref targets still fail closed at a different layer: the cat-file catch-all enumerates them with no path, and the path-based allow filter drops empty-path entries. Test inversions and new regressions: - `migration_v12_makes_cid_nullable_and_preserves_classification` and `migration_v27_clears_legacy_equal_cid` — both-distinct / distinct row expectations flipped to FALSE under the v30 strict backfill (prior round's tightening is re-affirmed here; the v30 backfill is `cid IS NOT NULL AND pinata_cid IS NULL`, the only unambiguous local-IPFS shape). - `ipfs_cid_gate_withholds_blob_from_unauthorized` — root tree assertion inverted to NOT_FOUND (the recursive structural gate denies root trees whose entries name a denied subtree). - `ipfs_cid_legacy_provider_cid_repaired_on_repush` — legacy-shape row insert now sets `local_ipfs_provenance = TRUE` to reflect that the bytes ARE on local IPFS (the legacy CID is just the wrong key). - `ipfs_cid_tree_served_despite_non_commit_ref` — root tree assertion inverted (same reason as the gate test). - `fails_closed_on_annotated_tag_of_a_blob` and `fails_closed_when_a_ref_cannot_be_traversed` — renamed to `skips_*`, assertions inverted from `Err` to `Ok` (a ref pointing at a non-commit object is no longer a walk error; the blob falls out of the gap set because `git rev-list --all` skips it). - `ipfs_cid_walk_error_fails_closed` — removed (relied on the now-removed `assert_all_refs_are_commits` to force an error; the fail-closed arm is still covered by `ipfs_cid_commit_tag_walk_error_fails_closed`, which uses a different trigger — a nonexistent ref object makes `rev-list --all` fail). - `sweep_does_not_publish_public_ancestor_tree_naming_withheld_subtree` (new) — recursive depth test: a `/public/secret/**` repo asserts root tree, `/public` tree, and `/public/secret` subtree are all absent from `pinned_cids` while `/public/visible.txt` is IPFS-pinned. - `sweep_repairs_commit_reachable_object_in_repo_with_tag_of_tree` (new) — tag-of-tree repo with a separate `mktree`'d tree (not commit-reachable); asserts the commit-reachable blob is IPFS-pinned and the unclassifiable tag-of-tree is not. Scope discipline preserved: PolicyFence capture order, `_pin_permit` reuse for the seal phase, fresh per-arm `rederive_budget`, keyset repo cursor with lookahead, mirror-row skip, Kubo 2xx-without-Hash refusal, v27 provenance-clearing migration, v30 strict backfill, v31 schema, per-(repo, backend) reconciliation offset, and the `local_ipfs_provenance` writer/reader contract are all untouched. * test(node): ignore pre-existing RED replication-tail test on CI `api::repos::tests::receive_pack_tail_survives_a_disconnect_during_release` documents an open bug in `api/repos.rs` (the replication tail must be spawned above `release` and gated on `receive_result.is_ok()`; today it's spawned below, so a disconnect during `release` drops the pins, recovery copy, and announce). The test's `assert!` message starts with "RED:" — this is a TDD-style "keep the design intent visible in the code" test, not a working-behavior assertion. The test was added 2026-07-29 by `beardthelion` (PR #215 lineage), well before the #218 sweep work, and is unrelated to this PR. CI is failing on it under load (passes in isolation) because the open bug hasn't been fixed yet. `#[ignore]` until the open bug is fixed. The test's design intent and the fix description stay in the doc comment and the assert message; only the CI failure is suppressed. Run the test explicitly with `cargo test -- --ignored` to verify the bug is fixed when the tail-spawn moves above `release`. * fix(node): align encrypted manifest anchor log with one-shot contract The "will retry next pass" log at reconciliation.rs:1034 was false: plan_seal returns SkipUnchanged on every subsequent pass once the recipients tag matches, so sealed stays empty and the anchor block never runs again. A transient Irys outage at the moment of a fresh seal therefore LOSES that anchor permanently — the next pass has no delta to anchor and no retry fires. This is intentionally best-effort: re-anchoring an unchanged manifest would burn Irys writes for no recovery benefit. Factored the warn message into ENCRYPTED_MANIFEST_ANCHOR_FAILED_MSG and added a unit test that pins the constant; the test goes RED if "will retry" reappears in the log. * fix(node): address eight reviewer findings on #218 reconciliation sweep (round 3) * fix(node): close audit.toml array and drop inline comment * fix(node): repair the stale for-each-ref fake-git fixture (round 8 P1) `upload_pack_shares_one_deadline_across_walk_and_filtered_serve` — the #174 shared-deadline guard for the FILTERED serve arm — was red on both `test (stable)` and `test (beta)`. The fake git answered `for-each-ref` with the single token `refs/heads/main`, a ref NAME, where `blob_paths` phase 2 asks for `--format=%(objectname) %(objecttype)`. The phase-2 parse fails closed on a line it cannot read, so the walk returned an error and the request surfaced as a generic 500 instead of being reaped into a 504. That silently repurposed the test from "the filtered serve shares one deadline" into "the walk errors" — the guard proved nothing while it was red. The fixture is what was stale, not the production path: the arm now emits the column shape the walk parses. A commit tip has no peeled referent, so two columns is the whole line. The 1.2s walk cost stays on `ls-tree` and the serve's on `pack-objects`, so the two phases remain independently attributable. * fix(node): peel annotated tags in the blob_paths ref walk (round 8 P1) `%(objecttype)` reports the type of the ref's OWN object, which for an annotated tag is `tag`. The phase-2 `if kind == "blob" || kind == "tree"` arms therefore never saw the referent, and a blob reachable ONLY through an annotated tag escaped the withheld set — while `rev_list_keep`'s `git rev-list --objects --all` does peel tags and served it. Phase 2 closed the leak for a direct ref-to-blob and for that shape only. The walk now asks for the peeled atoms `%(*objectname) %(*objecttype)` and inserts the referent with an empty path, alongside the direct blob/tree tips it already handled. Line parsing moves to a field-count match: two columns for a non-tag tip (the peeled atoms expand to empty), four for a tag tip, anything else still fails closed. Measured against git 2.50, the `*` atoms peel the WHOLE chain — a tag-of-a-tag-of-a-tag-of-a-blob reports the blob — so a `tag` peeled type does not occur on stock git. That arm is kept anyway, because `push_delta.rs`'s ref-type guard documents `%(*objecttype)` as a one-level peel: under such a git it finishes the chain with `rev-parse <tip>^{}` plus a `cat-file -t` probe rather than bailing and 500ing every clone of a repo that merely carries a nested tag. Both children are bounded by the walk's shared deadline and are unreachable on the common one-line-per-ref path. * fix(node): one empty-path decision for both blob_paths consumers (round 8 P1) `blob_paths` has two consumers and only one of them carried the empty-path policy. `withheld_from_pairs` (the deny side, what the smart-http serve filter excludes) withheld a phase-2 empty-path entry from everyone but the owner; `allowed_blob_set_for_caller_bounded` (the allow side, what `GET /ipfs/{cid}` hands over) called `visibility_check(..., "")` directly. On a public repo no glob matches the empty path, so that returns `Allow`: the serve filter withheld the OID while the IPFS gate served the bytes — the leak phase 2 exists to close, reopened one layer over. The policy moves into `pair_decision`, which both consumers now call, so the two gates cannot disagree and a future change to the empty-path rule moves both at once. `withheld_blob_recipients_bounded` routes through it too: an unclassifiable blob grants a recovery copy to the owner only, never to a rule's named reader whose grant was written against a path this object does not have. No behavior change for non-empty paths — `pair_decision` delegates to `visibility_check` unchanged. * test(node): bind the phase-2 ref tests to a commit-unreachable blob (round 8 P2) `skips_a_ref_pointing_at_a_blob` and `skips_an_annotated_tag_of_a_blob` were vacuous. Both hung their ref on `fixture()`'s `secret` blob, which is COMMITTED at `secret/b.txt`, so phase 1's per-commit `ls-tree` already produced `(secret, "/secret/b.txt")`, the `/secret/**` rule already denied it, and `withheld.contains(&secret)` passed with phase 2 deleted outright. Both now use `orphan_blob`, a blob written straight to the object store that no commit's tree names, so the ONLY route into the withheld set is the `for-each-ref` phase. That is also the real leak shape: `rev_list_keep`'s `git rev-list --objects --all` does follow a ref to such a blob, so an under-withheld one ships in the clone pack. Verified by mutation: with `if false` injected at the phase-2 filter, every test below goes RED (they were green under that mutation before). New coverage: - `ref_only_blob_is_denied_on_the_allow_side_too` — the allow-side denial for the shared `pair_decision`; it also asserts deny and allow agree on one OID, and that the owner IS still admitted, so it cannot pass by denying everything. Reverting the allow side to `visibility_check` turns it red on exactly that assertion. - `skips_a_nested_annotated_tag_of_a_blob` — real-git tag-of-a-tag. - `peels_a_tag_whose_peeled_target_is_still_a_tag` — drives the one-level-peel fallback through the fake-git seam, so that arm is tested rather than dead. * fix(node): advance the reconciliation continuation only for dispatched work (round 8 P2) `ipfs_last` / `pinata_last` were captured from the missing set above the pin permit, both `PolicyFence` captures and both pin loops, then written under nothing but `if ipfs_enabled` / `if pinata_enabled`. Every stage in between can legitimately produce nothing — a fence capture that fails, a quarantine/visibility recheck that says skip, a pin-boundary re-derivation that errors — and each is transient. Advancing past OIDs that were never attempted is not a harmless retry delay: `missing_oids` rotates strictly past the stored offset, so the whole unattempted prefix lands BEHIND the entire backlog on the next pass. For an at-cap repo — the only kind the continuation exists for — the backlog never drains inside one cap window, so those objects are starved indefinitely. That is a durability hole in the durability backstop. The offset now moves only for work actually handed to a backend, tracked at the dispatch boundary as `to_pin.last()` (not the missing set's last: an OID the pin-boundary re-derivation dropped was never offered). It is recorded before the call, so a pin phase that times out mid-batch still counts as dispatched. The write becomes three outcomes rather than two, via one `next_offset_write` helper so the backends cannot drift apart: * work dispatched -> advance to the last dispatched OID * nothing missing, query OK -> None, marking the row done * nothing dispatched, or the missing-set query failed -> write NOTHING, leaving the resume point a capped pass already paid for `*_scan_ok` is what separates the last two: an empty missing set means "everything is pinned" or "the filter query failed and we know nothing", and only the first should mark the row done. Test: `sweep_leaves_offset_untouched_when_nothing_was_dispatched`, using a `#[cfg(test)]` seam on the pin-boundary re-derivation — that stage cannot be starved from the outside, because the mid-scan re-filter runs first on the same rules and budget and makes the sweep `continue` well before the offset write. It asserts the stored offset comes back byte-identical after a declining pass, then that the same repo does advance it once dispatch happens, so the assertion cannot pass on a dead write site. Restoring the old semantics turns it red. * fix(gl): render the Pinata provider CID in `gl ipfs list` (round 8 P2) The server-side wire change landed without its client half. `cmd_list` read `pin["cid"].as_str().unwrap_or("?")` as the entry heading and nothing in the CLI read `pinata_cid` at all, so a Pinata-only row printed as a bare `?` — the listing showed nothing usable for an object that is in fact durably stored. `cid` is legitimately null for such a row: it is the key `GET /ipfs/{cid}` resolves against, and the node stopped aliasing a Pinata provider CID into it (round 3) precisely because that CID 404s there. So the fix is on this side. Rendering moves into `render_pin`, which leads with the node-resolvable CID when there is one and otherwise the provider CID, labelled so nobody feeds it back to this node's resolver. A dual row keeps both visible. A `backends:` line names where the bytes are, read from the writer-owned `local_pinned` / `pinata_pinned` booleans rather than re-inferred from CID shape — which is what the node's own comment warns against. Older nodes send neither flag; those `cid`-only rows fall back to CID presence and render as they always did (`render_pin_handles_a_legacy_cid_only_row`). * fix(node): implement third fence check for IPFS pinning and enhance visibility rules handling * fix(node): enhance documentation and add tests for ProgressState and helper functions * fix(node): address round-9 reviewer findings on #218 reconciliation sweep * fix(node): correct consumer-4 recipient test for call-invariant set * fix(git): fail closed on non-UTF-8 tree listing in walk_tree_oids_inner (#218 round 10 P1) walk_tree_oids_inner previously returned Ok(()) on a non-UTF-8 ls-tree -z listing, inserting only the tree OID and skipping every child blob/tree OID. A direct tree ref (or an annotated tag peeling to a tree) is valid Git input; git rev-list --objects --all still enumerates the tree and every descendant, so the keep-side removed the tree from the served set but passed the child blob OIDs to pack-objects, exposing their bytes to an anonymous clone. Phase 1 already bails on the same input at the blob_paths walk; this fix puts the tree-walk on the same fail-closed outcome. The new fails_closed_on_non_utf8_tree_tip test pins the invariant for both a direct refs/tags/direct-tree ref and an annotated tag-of-tree (the two non-commit shapes round 9 added tolerance for), confirming the walker bails rather than returning Ok with a partial withheld set. * fix(recon): run encrypted recovery even when public list is empty (#218 round 10 P1) A path-scoped repo whose only reachable object is a direct blob or direct tree ref yields an empty public list (the anonymous public classifier removes the only object from the served set) while withheld_blob_recipients_bounded still assigns that object to the owner recovery set. The early `continue` on object_list.is_empty() (and the second one after the mid-pass refilter) suppressed encrypted recovery too, and a lost or failed encrypted copy was never repaired. Track empty-public-work as a flag and gate the public phase on it. Encrypted phase 2 runs regardless. The backend enable flags and `_pin_permit` move out of the gated block so phase 2 can read them when the public phase did not run. * fix(db): rename migration v12 test to v32 (#218 round 10 P3) The test name and its docstring/comments still say "v12", but the cid-nullable migration is at v32 (line 1163). The test itself is correct — it drives the real path — so this is naming drift rather than dead coverage. A future reader looking for the migration would otherwise chase a v12 that no longer exists. * fix(git): index OID membership before quadratic classifier scan (#218 round 10 P2) all_object_paths was doing O(O×P) work in two places: the catch-all cat-file branch checked `blob_set.iter().any(|o| o == oid)` and `tree_set.iter().any(...)` for every reachable object, and the phase 1 loop also did a per-commit unused `git rev-parse <commit>^{tree}` probe whose result was discarded (the root-tree OID is recomputed by `root_tree_oids`). On the 50k-object repos the sweep exists for, the quadratic scan and the redundant per-commit child process can consume each authorization deadline before any object reaches a backend, producing a permanent hourly skip. Maintain a separate `blob_oids` / `tree_oids` HashSet alongside the path-pair sets so the membership check is O(1). Drop the unused per-commit rev-parse probe. * fix(git): bound tree walk by ls-tree invocation count, not just depth (#218 round 10 P2) walk_tree_oids_inner stopped at MAX_TREE_WALK_DEPTH and the shared deadline. A wide shallow tree reachable from N ref tips or from multiple parents still spawned one ls-tree child process per subtree well inside the depth cap, and the wall-clock bound could not stop that — expiry was the only stop. The ceil was wall-clock only. Add a structural invocation cap (MAX_TREE_WALK_INVOCATIONS) the walker fails closed at, and a memo of walked tree OIDs so the same tree reachable from multiple ref tips is walked once. Round 10 P2. * fix(pinata): suppress push when DB record is not durable (#218 round 10 P2) pin_new_objects pushed (sha, cid) on provider POST success regardless of whether record_pinata_cid persisted the row. A closed/failed DB write or an explicit-transaction source record timeout meant the next sweep queried the DB, found no row, and re-offered the same gap — with reconciliation having counted a fill that never happened. Track a `db_record_durable` flag. On hard failure of either record_pinata_cid (non-timeout) or record_pin_source (any failure, since it's a multi-statement transaction), set the flag false and skip the `pinned.push`. The `BoundedDbError::Elapsed` autocommit case stays as the previous comment's "may have committed" reasoning: the push fires so the reconcile still sees the fill. Round 10 P2. * fix(node): serialize policy narrow with irreversible upload via in-process mutex (#218 round 10 P1) The PolicyFence's epoch read was a one-way read: a visibility narrow (rule insert, quarantine set) could commit between the fence's epoch read and the upload's HTTP POST, and the upload's DB-record INSERT's epoch check would see the new epoch and reject — but the provider already accepted the object. Encrypted envelopes had no fence at all. Add a per-repo in-process mutex that PolicyFence acquires at capture and releases at drop. The narrow paths (set_repo_quarantine, set_visibility_rule, remove_visibility_rule) acquire the same lock for the duration of their DB transaction. A narrow therefore blocks until the in-flight batch finishes, and an in-flight batch waits for the narrow to commit before it even reads the epoch. Multi-process / multi-node is a known gap: a Postgres advisory lock per repo would extend the same guarantee across processes. Deferred to a follow-up. * fix(pinata): update post-upload stalled record test for round 10 P2 contract The previous test asserted that a successful upload with a timed-out post-upload source record still returned the (sha, cid) pair. That assertion pinned the buggy behavior the reviewer called out: the reconcile would count a fill that was not durable, then re-offer the same gap. The test name and the `record_pin_source` arm of `pin_new_objects` are unchanged; the assertion now reflects the new contract: when the post-upload source record times out (its `mark_pin_sources_incomplete` already sets the source set to incomplete, so the next pass re-offers), the pair is suppressed so the reconcile sees the gap as not-filled. Round 10 P2 follow-up. * fix: clippy allow + rustfmt on round 10 commits clippy::too_many_arguments on walk_tree_oids_inner (8 args after the round 10 memo + invocation counter threading). rustfmt re-ran on the if/else blocks added in the has_public_work refactor (round 10 P1 #42). No behavior change. * fix(node): address round-11 review findings on #218 reconciliation sweep P1: remove the whole-batch policy mutex. PolicyFence no longer holds a per-repo lock from capture through drop and the narrow paths (set_visibility_rule, remove_visibility_rule, set_repo_quarantine) no longer acquire it, so a visibility narrow commits immediately and the batch aborts on its next is_current check instead of the narrow blocking behind the sweep. Accepted residual is a single in-flight object; the fenced DB record still refuses to land a raced row as durable. encrypt_and_pin_stops_sealing_when_reader_removed_mid_batch is green again. PolicyMutexes is deleted, resolving the key-leak P2 by removal. P2: cover the round-10 has_public_work branch with sweep_seals_withheld_blob_when_public_list_is_empty: a direct-blob-ref repo yields an empty public list with a non-empty owner recovery set; asserts no public gaps/fills, no cleartext pin, exactly one POST (the seal envelope), and a recorded encrypted copy. P3: peel-case isolation in the non-UTF-8 tree-tip test (delete the direct-tree ref before the second call so the peel arm must run); v12->v32 rename drift in the migration test (boundary, comments, and the stale (2)->true doc line); accurate comments for the pre-refilter flag and the post-refilter fall-through. * fix(node): address merge-readiness findings on #218 reconciliation sweep Reworks the sweep around explicit state-machine contracts instead of adding conditionals: backend results carry last_attempted plus confirmed durability separately (PinBatchOutcome), provider permits scope to provider effects only, discovery pages by commit window with a persisted cursor, legacy provenance migrates as unknown, and the pin list advertises only validated raw local keys. - Empty-path owner recovery without path rules: phase 2 drops the has_path_scoped_rule gate and runs on the actual recipient result, with a dedicated recovery recheck (no anonymous-listability requirement); matrix test over blob/tree/tag refs x zero/root rules, including recreate-after-delete. - Pinata Elapsed is non-durable unless verify_pinata_record proves the exact row and fence epoch; loop test with row-locked repos. - list_pins gates cid/local_cid on is_raw_cidv1 plus confirmed provenance; response-shape matrix uses real CIDs with a legacy row and GET /ipfs follow-through; docs match the nullable wire contract. - Permits acquired per provider effect and dropped after; no nesting at pool size 1 (test renamed to the new shape). - Scan discovery pages SCAN_COMMIT_WINDOW commits oldest-first with a node_state cursor (advance on drained/empty/attempted, delete on cover); refilters and phase-2 recipients run on the same window; walk-layer paging/invocation/union tests plus a 1050-commit cursor-to-completion sweep test. - v35 migrates legacy rows as unknown (no TRUE backfill); v35 test asserts unknown + preserved Pinata history; sweep re-derivation test re-pins instead of filtering. - Drained completions DELETE continuation rows (no tombstones); FK cascade on repo delete; row-count regression test. - Audit: lru waiver gets an explicit retirement owner plus a weekly drift guard; revocation model recorded as a product decision on PolicyFence (prompt revocation, single-in-flight residual). * fix(tests): configure git identity in bare clone for tag fixture The windowed-union test creates its annotated tag in the bare clone, which carries no config. It passed locally only via the developer's global git identity; CI has none, so 'git tag -a' failed there. Verified with an empty GIT_CONFIG_GLOBAL/SYSTEM. * fix(tests): ignore pre-existing hung-git pid race on CI read_object_bounded_returns_by_deadline_with_a_hung_git reads the fake's pid file with no readiness handshake; a slow scheduler yields 'No such file or directory' (stable-only red, identical beta green). Unrelated to #218 (store.rs untouched); re-enable with a readiness signal from the fake. * fix(node): address round-12 review findings on #218 reconciliation sweep - P1 roots: admit structurally safe root trees at "/" into the allow set and candidates, so safe snapshots fully replicate; roots naming a withheld subtree stay excluded via the recursive structural check. - P1 per-pass WalkBudget on sweep enumeration (entries / output bytes / git invocations) with fail-closed budget-exhaustion; serve paths keep the uncapped budget. - P2 recovery lane: independent recovery cursor pages unlistable history across restarts instead of sharing the scan window. - P2 gap-hold: failed gap queries hold discovery advance but not the healthy backend, via test-only failure seams. - P2 nested tags: collect full tag chains so nested tag objects pin without an inner ref. - P2 RepoStore: thread the Tigris-backed store into the sweep with an acquire timeout and shutdown select; restore cache-miss and skip missing. - P2 Pinata source: keep confirmed pairs when the primary already covers the repo; suppress push for stalled post-upload records without coverage. - Update large-history sweep expectation to blob+root+commit per round. * fix(node): address consolidated review findings on #218 reconciliation sweep P1: wire WalkBudgetExceeded through the sweep cursor logic. Both the public scan and the recovery walk now dispatch the typed budget-exhaustion signal at the sweep boundary: a budget-hit window advances past content the sweep refuses to classify partially (scan cursor save/delete via the existing persist site; recovery cursor likewise), while git/policy/panic/timeout failures keep the cursor for retry. visibility_pack documents the obligation; the match arms are the handler, with a one-line pattern note for future typed walk errors. P2: run encrypted recovery when the public scan fails. Scan walk/panic/timeout arms now fall through with an empty public list instead of continuing the whole repo iteration, so phase 2 runs on the recovery lane's own cursor like the unlistable-repo path. The scan cursor stays untouched for retry. Refilter/recheck-public failures remain fail-closed whole-iteration continues by design. Includes a repo-scoped test seam (process-global set, safe across spawn_blocking and parallel tests via fresh UUIDs). P2: bump policy_epoch only when remove_visibility_rule deletes a row, mirroring set_repo_quarantine's rows-affected guard, so a typo'd or double-deleted glob cannot abort fenced batches. Also adds the repo-loop outcome matrix (every early exit × scan cursor / recovery cursor / phase 1 / phase 2) and deny tests: budget-hit advances both lane cursors with zero pins/seals (real 250k-entry trip, early-trip fixture runs in ~30s), scan failure still seals the recovery copy, delete-missing-rule leaves the epoch unchanged. Each deny was verified by temporarily restoring the old branch. * fix(node): address jatmn review findings on reconciliation sweep re-home * fix(node): reload persisted sweep cursor between passes * fix(node,gl): pinata offset warn wording and boundary-safe pin timestamp --------- Co-authored-by: Gravirei <[email protected]> Co-authored-by: Kevin Codex <[email protected]>	2026-11-09 15:35:18 +00:00
3 people	c805e35f40	fix(node): bound GraphQL query cost (#552) ... * fix(node): bound GraphQL query cost * fix(node)!: bound repo names so emitted cursors always parse The reposPage cursor embeds base64(JSON(owner_did, name)) while parse_repo_cursor rejects cursors over 4096 bytes, but repo names were unbounded on every write path. A single long-named visible repo made the server emit an endCursor it then rejected as "invalid repository cursor", dead-ending pagination for every caller past that row. Bound names to MAX_REPO_NAME_LEN (100 bytes) at the create_repo and fork_repo handlers via a shared validate_repo_name helper (bad charset is still a 400), and reject overlong peer-supplied names in Db::upsert_mirror_repo (the sync caller already drops the error and skips the mirror). Also in this round: - docs/graphql-pagination.md states the 200 page-size maximum and corrects the over-budget remedy: a smaller limit does not reduce the reposPage or repos price, which is charged at the full MAX_VISIBLE_REPO_PAGE_SIZE scan regardless of limit. - Tests pin refUpdates(limit: -1) clamping to zero rows, the parse_repo_cursor wrong-shape-JSON arm with its exact "invalid repository cursor" message, the longest-admissible-name cursor round-trip, and the mirror write-path rejection. BREAKING CHANGE: the GraphQL `repos` field now returns an error instead of the full list when more than MAX_VISIBLE_REPO_PAGE_SIZE (200) repositories are visible; larger lists must use `reposPage` with `limit`/`after`. Repository names are additionally limited to 100 bytes at creation, fork, and mirror-registration time. * fix(node): address beardthelion P3 findings on name bound and input messages - Assert each rejection's own message in the reposPage invalid-inputs loop: the loop no longer accepts either message for every query, so a misattributed message (limit arm returning the cursor message) goes red. - Pin the create/fork name-bound wiring through the handler: new test drives create_repo with an overlong name via test_state_lazy and asserts the 400; deleting the validate_repo_name call site would proceed to the lazy DB pool and fail with a connection error instead. - Document the API-side vs disk-side validate_repo_name divergence on the new function, and tie the store's literal 100 to MAX_REPO_NAME_LEN so the two validators cannot drift apart on length. --------- Co-authored-by: Marlowe <[email protected]> Co-authored-by: cairn-intern <[email protected]>	2026-11-08 23:28:09 +00:00
3 people	a4026e84ff	fix(node): honor pre-assigned task assignee on claim (#550) ... * fix(node): honor pre-assigned task assignee on claim Stop strangers from overwriting a reserved assignee_did (and receiving the UCAN). Bind finish_task to the stored assignee; return 403 on reserved steals; keep exact stored DID forms; normalize blank assignees; pin production claim/finish SQL race guards with FOR UPDATE tests; trim broadcast by_did; and tighten blank/SQL re-check regressions. * test(git-remote-gitlawb): wait for shim before real_git_fetch on Windows Windows CI can race: git fetch connects before the shim accept loop is polling, yielding connection aborted (os error 10053) with 0 POSTs. Probe the listener after spawn so multi-round and withheld-path tests start only once the shim is ready. * test(git-remote-gitlawb): retry transient Windows fetch connection errors Windows CI can still abort localhost connections (os error 10053) after the shim is ready. Retry fetch_with_helper up to three times on transient connection failures before asserting. * test(git-remote-gitlawb): revert Windows fetch workarounds The retry-on-10053 and wait-for-shim probes papered over the accepted-socket inheriting the listener's non-blocking state. #312 fixed that at the source (normalize_accepted_stream) and is merged, so take the workarounds back out and let the real fix carry the Windows lane. * test(git-remote-gitlawb): pin the fetch retry's absence with an exact GET count The POST assertions are floors, so a reintroduced fetch-level retry could satisfy them with a second full fetch while masking the withheld shape. A retry cannot avoid re-advertising, so count GET /info/refs in the shim and assert exactly one per fetch in both the multi-round and withheld tests. The merge from main also brings #312's normalize_accepted_stream into this head, closing the round's rebase ask. * fix(node): mirror assignee normalization across GraphQL task mutations GraphQL createTask returned the pre-insert input, so a whitespace-only assignee came back as a reservation the store never kept; complete and fail compared the caller against the raw stored assignee where REST and Db::finish_task trim; and task-event broadcasts reported the caller string rather than the stored assignee form REST publishes. All three paths now mirror the REST and DB contract: the create response is the stored row, finish gates compare the trimmed stored assignee, and by_did on claim/complete/fail broadcasts derives from the returned row. --------- Co-authored-by: Ayush7614 <[email protected]> Co-authored-by: Kevin Codex <[email protected]>	2026-11-08 21:38:53 +00:00
3 people	38bab4191b	fix(node): version ref_certificates for forward compat (#26 split 3/4) (#548) ... * 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]>	2026-11-08 21:38:02 +00:00
3 people	d84c445f94	fix(gl): derive View: URL from node instead of hardcoding gitlawb.com (#370) (#549) ... * fix(gl): derive View: URL from node instead of hardcoding gitlawb.com (#370) gl repo create and gl mirror print a View: link that was unconditionally pointed at gitlawb.com, which 404s for repos on self-hosted nodes. Make the node advertise an optional web_url on GET / (sourced from the new GITLAWB_WEB_URL env var). The CLI fetches GET / after repo creation and only prints View: when the node supplies web_url. Nodes without the config skip the line entirely, which is better than printing a broken link. * fix(gl): validate web_url response and trim trailing slashes (#370) Address review feedback: - Check GET / response status before parsing; only omit View: on success responses that lack web_url - Trim trailing '/' from web_url to avoid double-slash in the URL - Skip the line when web_url is empty after trimming * fix: address review feedback on View-URL PR (#370) - Extract fetch_node_web_url() helper for testability; add 6 unit tests covering: web_url present, trailing-slash trim, absent, empty string, whitespace-only, and server error - Reject empty/whitespace-only GITLAWB_WEB_URL at boot in Config::validate() - Trim whitespace from web_url in the CLI helper (not just trailing slashes) - Document GITLAWB_WEB_URL in .env.example alongside GITLAWB_PUBLIC_URL - Mirror uses shared fetch_node_web_url() helper * fix: validate web_url format and surface node info failures (#370) Node side: GITLAWB_WEB_URL must now parse as an absolute http(s) URL — scheme-less hosts and other garbage fail at boot instead of serving a View link no browser can follow. CLI side: a non-success GET / status or a malformed advertised web_url now warns on stderr (with the status/value) instead of silently dropping the View link; successful responses lacking the field still omit it quietly, as self-hosted nodes without a web front-end are expected to. * fix: reject query/fragment web_urls and warn on node info errors (#370) Both web_url validators now reject URLs carrying a query string or fragment: the CLI builds View links by string-appending /{owner}/{repo}, so "?a=1" would swallow the repo path into the query. The CLI also no longer swallows transport-level failures on GET / — a refused or failed connection warns on stderr like the other degraded-link paths. * fix(gl): cap node info body and reject control-byte web_urls (#370) Treat GET / like every other caller-chosen node reply: bound the body read at 8 KiB and parse from the cap (peer.rs precedent). A web_url containing control or bidi characters — which passes URL parsing but would reach the terminal verbatim through the View: line — is now rejected as malformed, with a defanged preview in the warning. * chore: bump h2 to 0.4.19 for RUSTSEC-2026-0258 h2 0.4.13 is vulnerable to unbounded empty DATA frames (remote DoS); 0.4.16+ carries the fix. Lockfile-only bump, no manifest change. * fix(gl): close merge regressions on #370 branch and pin View: with integration test The a6113fd8 merge onto #370 left the workspace red: a missing closing `}` in `web_url_rejects_non_absolute_or_non_browser_urls` so the next test nested inside it (config.rs), a duplicate `url = "2"` workspace dep (Cargo.toml, one from the redirect-rule PR, one from #370), and `fetch_node_web_url` derefing `CappedBody` as `&str` instead of `.text` (repo.rs, regression in the same merge). All three are straightforward to repair; the brace was the only one blocking CI. The new `cmd_create_view_url_tracks_node_advertisement` integration test drives the real `gl repo create` binary against a mockito node that answers both `POST /api/v1/repos` and `GET /`, and asserts the printed `View:` line tracks the advertised web_url — present with the advertised origin when set, absent when the node omits it. The helper-level `fetch_node_web_url_*` tests would still pass if `cmd_create` reverted to a hardcoded URL; this one would not, so it is the load-bearing check for #370. * fix(gl,node): normalize stored web_url and reject userinfo (#370 review) Two P2s from the #370 review addressed at the configuration boundary. `Config::validate()` now rewrites `self.web_url` to the canonical `url::Url::to_string()` after `validate_web_url` succeeds, so the regular and degraded `GET /` handlers advertise the same string that was actually validated instead of the operator-typed one. The CLI trims a second time in `fetch_node_web_url`, which hid the divergence on the View: line, but any other consumer parsing `web_url` directly would see a non-canonical base. `validate` is now `&mut self` to carry the write; the one non-test caller already binds `mut` and the two test bindings are updated to match. Both `validate_web_url` (node) and `fetch_node_web_url` (CLI) now reject URLs with a non-empty username or a password: the configured value is published on public `GET /` and reprinted on the View: line, so an accidentally configured `https://user:[email protected]` would land in public metadata, terminal scrollback, and CI logs. New regression tests cover user-only and user+password forms on both sides, plus a normalization test that the stored value matches the canonical form (with and without operator padding). * fix(gl,node): classify capped node-info bodies and reject encoded bidi in web_url An oversized or half-read GET / body previously fell through JSON parse failure into the same silent-None arm as a node with no web front-end. The classify step now consults the capped body's truncated and read_failed flags and warns before omitting the View link. web_url validation on both sides also scanned only literal bytes for control and bidi characters, so a percent-encoded reordering character (%E2%80%AE) passed boot validation and CLI ingest even though a browser decodes it into a bidi spoof. Both validators now check the decoded host and path (INV-6). Also fixes the control-byte test, which served invalid JSON and passed without reaching the check it pinned. --------- Co-authored-by: Gravirei <[email protected]> Co-authored-by: Gravirei <[email protected]>	2026-11-08 18:54:05 +00:00
beardthelionandAyush7614	42cc893fb3	fix(gl): make doctor exit non-zero on Fail-class checks (#357) (#547) ... * fix(gl): make doctor exit non-zero on Fail-class checks (#357) gl doctor printed per-row status and 'Some checks failed' prose, but its exit code was always 0. A diagnostic whose exit status cannot express failure is a trap waiting for the first 'gl doctor && ...' to land somewhere it should not (#357). Three changes: 1. Re-tier the 'GITLAWB_NODE unset' check from Fail to Warn. The CLI's --node flag defaults to https://node.gitlawb.com (PUBLIC_NODE) and the CLI works fine without the env var, so an unset env is advisory, not a failure. Without this re-tier the obvious 'return non-zero on any failure' change would have flipped gl doctor to exit 1 on a stock working install, which is exactly the regression the issue warns against. 2. Route the exit-code decision through a new has_failures(&[Check]) helper: exit 1 if and only if at least one row is Fail-class. Warn-class rows (iCaptcha offline, version drift, shell-alias shadowing, GITLAWB_NODE unset) keep the process at exit 0 because those conditions are still printed to the user and the obvious '&& pipeline' pattern stays valid. Use std::process::exit(1) rather than returning Err so anyhow's error frame does not duplicate the user-facing summary that already prints. 3. Three unit tests pin the new predicate against the three regimes: all-Ok, warn-only, and a single Fail row tripping the exit. The warn-only case is the regression guard for the re-tier: if anyone flips GITLAWB_NODE back to Fail, exit_predicate_is_false_for_warn_only fails too, so the two halves of the fix cannot drift apart. Verified end-to-end against the public gitlawb node: * unset GITLAWB_NODE, missing identity/registration -> exit 1 * unset GITLAWB_NODE, full healthy install -> exit 0 (warn) * missing git-remote-gitlawb -> exit 1 * fix(gl): pin doctor exit wiring and GITLAWB_NODE Warn tier - Extract GITLAWB_NODE env classification into gitlawb_node_env_check() so unit tests can pin the Warn tier directly; the previous has_failures-only tests built their own Check::warn rows and could not see a re-tier back to Fail. - Add three unit tests for the extracted helper (unset/empty is Warn, set is Pass, loopback is Pass). - Add integration probe crates/gl/tests/doctor_exit.rs that drives the real binary with a temp --dir and --node http://127.0.0.1:1 and asserts exit 1. Deleting the std::process::exit(1) gate keeps the unit suite green but breaks this probe, closing the gap reviewer noted. Co-authored-by: review fix for #391 * fix(gl): align GITLAWB_NODE operator copy with PUBLIC_NODE * fix(gl): dial the public node when GITLAWB_NODE is empty Check 3 groups an empty env with unset (Warn), but clap still binds args.node to "" and the connectivity probe dialed it verbatim, so a GITLAWB_NODE="" install showed a Fail row and exited 1. effective_node_url normalizes empty or whitespace-only input to PUBLIC_NODE before the probe so empty matches unset on check 4. --------- Co-authored-by: Ayush7614 <[email protected]>	2026-11-08 04:42:15 +00:00
3 people	0189990ac9	fix(node): bound federated repository aggregation (#546) ... * fix(node): bound federated repository aggregation * fix(node): propagate federated peer truncation * fix(node): Bound federation latency and preserve truncated peer pages. * fix(node): complete federation review follow-ups Register the bounded peer-query fixture in the writer ledger, name federation bounds, and share the state-owned limiter across routers and the cleanup sweep. Cover exhausted-bucket rejection before database access. * fix(node): pin federated response contract end to end (#466 review) Address beardthelion's P2: the federated envelope was only exercised at unit level, so hardcoding "truncated": false, deleting the failed-peer truncated flag, or dropping node_did from enrich_federated_repo each left all nine federated tests green. Add four route tests driving build_router and asserting on the real HTTP 200 body: - envelope fields (count, nodes_queried, truncated) and local annotations (node_url, node_did, local) for a seeded public repo - truncated == true when the peer table overflows MAX_FEDERATED_PEERS (unreachable peer URLs, no live peer needed) - truncated == true on the failed-fetch path with the table under the bound - peer annotations (node_url, node_did, local: false) via a mockito peer * fix: strengthen federated response bound tests per review - db/mod.rs: add writer-ledger entries and disposition docs for the three federated route tests touching the peers table - api/repos.rs: seed the envelope test repo under a non-node owner DID so node_did must equal the actual node DID; pin the outbound peer query to exact limit=200&offset=0; make the peer-table-overflow test use a reachable stub for every peer so the count bound is the only possible reason for truncated=true, and assert nodes_queried --------- Co-authored-by: euxarist <[email protected]> Co-authored-by: Marlowe <[email protected]>	2026-11-08 02:24:53 +00:00
beardthelion	61d8a4f8f1	fix(core): compute created-skew in checked arithmetic (#358) (#545) ... created= is attacker-supplied i64 with no range clamp. now - created overflows on i64::MIN (panic in debug, wrap in release), and the wrap case made abs() return i64::MIN, which is not > 300, so a skew of 2^63 seconds read as fresh. Treat an unrepresentable difference as the largest possible skew.	2026-11-07 23:24:17 +00:00
4 people	b33562f1a9	fix(node): bound Identify-derived Kademlia addresses (#544) ... * fix(node): bound Identify-derived Kademlia addresses. * fix(node): preserve refreshed Identify addresses. * fix(node): Log rejected known-peer addresses for diagnosis. * fix(node): bound Identify report processing and refresh eviction Avoid retaining peers with empty or rejected address reports, bound canonicalization to a fixed input prefix, and move refreshed addresses behind stale addresses in global eviction order. Add regressions for each bound and below-cap growth. * fix(node): address CodeRabbit majors on Identify address bounds - Reject Identify address admissions once the global address budget is full instead of evicting another peer's entry. Cross-peer eviction let a flood of new identities push honest peers' addresses out of the book and, when the evicted address was their last one, out of the Kademlia routing table. Per-peer eviction of the reporting peer's own addresses is unchanged. - Re-add refreshed Identify addresses to Kademlia, not just newly added ones. Kademlia may have dropped a retained address after a failed dial or never kept it for a full k-bucket, and a TTL refresh alone would never re-admit it. * fix(node): address #462 review findings from beardthelion - remove the write-only insertion_order bookkeeping (field, remove_insertion_token, and its call sites) and collapse the repeated peers.get/get_mut lookups in IdentifyAddressBook::update into a single entry binding - drop a peer row at the end of update() when every candidate was rejected, so zero-address rows no longer linger until the expiry tick - add a regression test proving per-peer eviction still applies at full global budget (fails if the global check is hoisted above eviction) - extract the explicit-address retention predicate into the tested is_explicit_address helper used by both Kademlia removal sites * fix(node): pin row-removal and expiry guards in Identify address book Address beardthelion's round-two P3 findings on #462: - Add test that a fully-rejected fresh peer at full global budget leaves no peers row (pins the empty-row drop in update()). - Add test that update() past IDENTIFY_ADDRESS_TTL reports the stale address in changes.removed (pins expiry reporting). - Add test that expire() drops rows emptied by expiry (pins the sibling drop in expire_peer). - Distinguish per-peer from cross-peer eviction in the saturation test: insert a sentinel peer before the victim and assert its address survives, so a global-oldest eviction policy would fail. * fix(node): pin refresh predicate negative direction per beardthelion round 3 --------- Co-authored-by: euxarist <[email protected]> Co-authored-by: Marlowe <[email protected]> Co-authored-by: cairn-intern <[email protected]>	2026-11-07 22:49:33 +00:00
beardthelionandGravirei	d3845ced69	fix(node): gate /ipfs/pins and /arweave/anchors behind authentication (#121) (#543) ... * fix(node): gate /ipfs/pins and /arweave/anchors behind authentication (#134) Add auth rejection to list_pins and list_anchors handlers. The pin index spans the entire node and would expose metadata for every object ever pushed; anonymous callers must not see it. - Require AuthenticatedDid extension in both handlers, return 401 when absent - Add server.rs regression test for anonymous rejection through build_router - Fix closed-pool tests to pass authenticated requests (test 503 path) * fix(node): enforce authentication on /api/v1/ipfs/pins and /api/v1/arweave/anchors routes * fix(node): format arweave routes for consistency in build_router function * fix(node): gate scoped /api/v1/arweave/anchors reads on repository visibility Closes the review follow-up on #134: a signed non-reader could pass the auth-layer check on `?repo=<owner>/<name>` and obtain private-repo anchor metadata (ref names, old/new SHAs, CIDs, irys tx ids). The auth check proved only that the signature was valid; it did not bind the request to the canonical repo-read policy. * Parse the slug through `validate_repo_slug` (same helper the sync path uses) and call `authorize_repo_read` before the SQL query. Missing repos, quarantined mirrors, and signed non-readers all collapse to the standard 404 (`repo_not_found`). * The unscoped global listing stays auth-only (#121 narrowing; the wider permissionless-identity enumeration is the #136 class, out of scope). * Clamp the limit to `[0, 200]` so `?limit=-1` no longer reaches Postgres as `LIMIT -1`. Five new `build_router` regressions cover the full production contract through the real `optional_signature` middleware: scoped unsigned 401, scoped owner 200, scoped non-reader 404 with no anchor metadata leaked in the body, scoped missing-repo 404 indistinguishable from non-reader, and `?limit=-1` clamped to an empty 200. * fix(node): keep /api/v1/ipfs/pins out of the CID resolver's per-IP flood brake Follow-up to #134: the previous reordering put `/api/v1/ipfs/pins` and `/ipfs/{cid}` under the same `rate_limit_by_ip` + `IpRateLimiter` layer stack, so both routes shared the resolver's flood brake bucket. That bucket is documented as the once-per-request brake for `GET /ipfs/{cid}` because the resolver can drive bounded-but-expensive repo walks. The pin listing is a single `list_pinned_cids()` call and has no reason to share the bucket. Sharing meant /ipfs/{cid} traffic could exhaust the bucket and 429 the pins endpoint, and signed pin polling could exhaust the bucket and 429 legitimate CID reads. Compose `ipfs_routes` from two sub-routers with independent policies, then merge: * `/ipfs/{cid}` keeps `optional_signature` + `rate_limit_by_ip` + the `IpRateLimiter` extension. Same behavior as before #134. * `/api/v1/ipfs/pins` gets only `optional_signature`. Auth-required (handler denies with 401), but no rate limit — it has nothing to flood. Two new `build_router` regressions exercise the bucket independence through `TrustedProxy::XForwardedFor`: * 5 signed pin requests against a size-2 CID bucket all 200, then a `/ipfs/{cid}` from the same IP also 200 (pins did not debit the bucket). * A `/ipfs/{cid}` exhausts a size-1 CID bucket (200, then 429), then a signed pin from the same IP is 200 (the bucket does not gate pins). All 65 tests in `server::tests::` + `test_support::tests::ipfs_*` still pass, including the existing 429 tests for the CID resolver. * fix(node): normalize scoped anchor SQL filter to stored repo slug After authorize_repo_read succeeds, the SQL filter used the raw ?repo= string (format!("{owner}/{name}")). Anchor rows are stored with normalize_owner_key(owner_did)/name (short key), so a caller using ?repo=did🔑.../name passed authz but got anchors: [] — a false empty page that broke callers that legitimately pass the full DID form. Build the filter from record.owner_did normalized and record.name instead, matching the shape the seeder uses. * All three findings addressed on bug_fix_2: P2 — signed_scoped_anchors_full_did_form_matches_stored_slug_through_build_router now seeds a second repo under a different owner and asserts irys-other-repo-tx is absent. Negative control: flipping the production filter to None made the test go RED with count: 2 showing the cross-repo leak. P3 — let short = owner_did.split(':').next_back().unwrap() replaced with let short = crate::db::normalize_owner_key(&owner_did).to_string(). Reader and writer now share one convention. P3 — PR #134 description updated: pins bullet now describes the actual pre-merge layering (and cites the axum-layer-vs-merge-pitfall memory entry), and round 2's stored-slug normalization is added as its own bullet with the regression's coverage. 5/5 anchor-related tests pass, cargo fmt --check clean. * fix(node): pass SignatureBodyLimit state to optional_signature layers * test(node): complete listing-denial regressions for the new gates --------- Co-authored-by: Gravirei <[email protected]>	2026-11-07 21:46:53 +00:00
beardthelionandGravirei	bac49f150a	fix(node): owner-gate create_task repo_id against hosted repos (#541) ... * fix(node): owner-gate create_task repo_id against hosted repos (#496) * fix(node): address review on #496 gate — opaque lookup errors, no-persist asserts, GraphQL arm pins, honest oracle comment * fix(node): address P3 review on #496 gate — shared denial text, honest oracle wording, db-fault pins - Give the REST and GraphQL copies of the create_task owner gate one denial sentence: add REPO_OWNER_ONLY_MESSAGE, used by require_repo_owner and the REST forbidden() envelope, and pin it from both surface tests. - Carry the corrected one-bit-oracle wording into the GraphQL gate comment and the REST test docstring (drop the "oracle-free" claim). - Fault-inject the gate's repo lookups: drop machine_id (get_repo_by_id) for REST and GraphQL, and quarantined (is_repo_quarantined) for REST, asserting the fixed opaque db_error envelope and no schema text. * docs(node): stop claiming caller-scoped task reads in #496 gate comments The create_task gate is a write gate. Unknown/quarantined repo ids are stored verbatim, and the REST/GraphQL task-list paths are caller-independent on this base, so the comments must not imply those ids are read-scoped to the delegator/assignee. Describe only the enforced ownership check; note the one-bit oracle split without asserting a read contract. * fix(node): mirror AppError's 503 split and opaque INSERT in create_task - db_error now sends pool-class failures to 503 db_unavailable like AppError::Db (db_unavailable made pub(crate)); query faults stay the opaque 500. Unit tests cover the split and context-wrapped sqlx errors. - Route the create_task INSERT through db_error instead of serializing e.to_string(); a REST fault-injection test pins the opaque envelope. - The deny/allow matrix reads back the unknown and quarantined tasks to prove the id is stored verbatim, and drops no-leak asserts on the fixed forbidden envelope that could never carry the repo id. * test(node): pin stored-verbatim repo_id on GraphQL gate arms The REST deny/allow matrix reads each accepted task back and asserts the stored repo_id is the supplied id; the GraphQL unknown/quarantined arms only asserted no errors, so a GraphQL regression that dropped, normalized, or rebound the id stayed green. Read those tasks back by the mutation's returned id and assert the stored label matches. * test(node): pin createTask repo-ownership marker in the GraphQL authz fence --------- Co-authored-by: Gravirei <[email protected]>	2026-11-07 21:31:50 +00:00
beardthelionandGravirei	b4a3122500	fix(core): reject zero threshold, canonicalize did:key, document replay (#509) (#540) ... * fix(core): reject zero threshold, canonicalize did:key, document replay (#509) * test(core): drop error echoes from assert messages flagged by CodeQL --------- Co-authored-by: Gravirei <[email protected]>	2026-11-07 15:48:05 +00:00
beardthelionandeuxarist	0cdb4bcc7c	fix(api,sync): Delimit positional arguments in git clone mirror invocations (#539) ... * fix(api,sync): Delimit positional arguments in git clone mirror invocations Pass "--" before positional path/URL arguments in Command invocations for "git clone --mirror" across repos.rs, sync.rs, and gl mirror.rs. Fixes #374 * style(gl): Format args array in mirror.rs Fix cargo fmt line-length break for git clone --mirror arguments. Refs #376 * fix(node): Pin the git clone delimiter against remote-argument injection The delimiter change had no test, so nothing stopped it being reverted as cosmetic. What it guards is real: git parses a remote beginning with a dash as an option, and --upload-pack=<cmd> is one git hands to a shell. No path reaches that today. Peer URLs are gated by is_public_http_url, so the origin clone_repo composes can only begin with http:// or https://; fork paths are joins under repos_dir; and clap rejects a leading-dash positional in gl mirror. The delimiter is what removes git's dependence on those three unrelated gates staying correct. Assert git reports the whole argument as a repository it cannot find, which is only true once the delimiter forces it to be a path. Undelimited, git consumes it as an option and fails against the destination instead, never naming the injected string, so the assertion is red before the fix and green after. Refs #374 * fix(sync,gl): Delimit positional arguments in fetch_repo and gl clone Carry the positional argument delimiter into fetch_repo when setting the remote origin URL and into setup_partial_clone across all clone branches. Correct the error string quoted in the clone regression test docstring. Refs #374 * test(gl,node): Pin the argv delimiter on every git sink this PR changed The delimiter contract is duplicated across five direct `git` argv constructions, but only `sync::clone_repo` had a test that went red when `--` was deleted. Removing or misplacing the delimiter in any other sink left the suite green, so the hardening rested on review rather than on the tests. Each changed sink now has a subprocess-facing regression driven through real git: the three `setup_partial_clone` shapes (plain, `--branch`, and the sparse/promisor arm), `fetch_repo`'s `remote set-url`, `gl mirror`, and the fork clone. Two of these could not carry the obvious assertion: - `git_global` and `git_run` both format failures as `git {args:?} failed: {stderr}`, so the injected value is already in the message via the debug-printed argv. Asserting that the error merely contains it passes with `--` deleted. The clone tests assert on git's own single-quoted `repository '<value>'` instead, which only appears when git read the value as a repository. - `remote set-url` does not fail on a delimited option-shaped value, it stores it. The `fetch_repo` test asserts the stored URL rather than an error: undelimited, git exits 129 with `unknown option` and leaves the previous URL in place. `gl mirror` and `fork_repo` needed their argv extracted to be testable. `mirror::run` loads a keypair and contacts a node before reaching the clone, and bails with a message that interpolates the source whether or not the delimiter is present. The extraction keeps the production `.status()` call, so a large mirror clone still streams git's progress to the terminal instead of being captured. Every one of the five was confirmed red with its delimiter removed and green with it restored. Refs #374 * fix(gl): Route the last three clone warnings through emit_warning `emit_warning` sanitizes a line before writing it, which strips ANSI escape and bidi characters from untrusted text. Three warning sites in the blob-recovery paths still used a bare `eprintln!` while interpolating `oid`, which comes from node JSON or an Arweave gateway manifest, so those escapes reached the user's terminal raw. Refs #374 * style(gl): elide the lifetime in mirror_clone_args * test(gl,node): pin LC_ALL=C on git-subprocess stderr assertions --------- Co-authored-by: euxarist <[email protected]>	2026-11-07 12:38:49 +00:00
3 people	0e5a1608e2	feat(node): persist upstream mirror configuration and transition state (#538) ... * feat(node): persist upstream mirror transition state Origin-Session: local-d6a143 | Codex | 12 prompts * fix(node): reject null mirror transition state Origin-Session: local-d6a143 | Codex | 14 prompts * fix(node): address mirror state review findings Origin-Session: local-d6a143 | Codex | 16 prompts * fix(node): close mirror review blockers Origin-Session: local-d6a143 | Codex | 20 prompts * fix(node): align peer URL validation message Origin-Session: local-d6a143 | Codex | 20 prompts * fix(node): close remaining mirror review findings Origin-Session: local-d6a143 | Codex | 21 prompts * test(node): track the Db pool field change in the mirror-state migration tests * fix(node): revalidate stored webhook URLs at delivery and cover the subdomain repeated-dot case * docs(node): document the invalid_url webhook delivery outcome --------- Co-authored-by: KRATOS <[email protected]> Co-authored-by: Kevin Codex <[email protected]>	2026-11-07 03:50:15 +00:00
beardthelionandGravirei	37947208c5	docs: state that GITLAWB_ENFORCE_OWNER_PUSH defaults to true (#537) ... * docs: state that GITLAWB_ENFORCE_OWNER_PUSH defaults to true PR #330 flipped the code default to true and updated .env.example, the README env-var table, the AWS guide, the macOS app, and docs/RUN-A-NODE.md, but two prose mentions in operator-facing docs still describe the opposite. SECURITY.md line 57 ("Repository write authorization defaults" section) and README.md line 69 (Known limitations) both say the flag "defaults to `false`", which contradicts the live runtime and the rest of the same files. Reviewer 2 closed #224 with this as the P2 finding to fix in the config-and-policy split. The correct text already exists in the README env-var table; the prose restatements were missed. Bring the prose into line with the code. - SECURITY.md: rewrite the section to lead with the default being true and describe `false` as the explicit rolling-upgrade override. - README.md: rewrite the known-limitations bullet to state the default is true and the `false` opt-out is a rolling-upgrade only. - Add `crates/gitlawb-node/tests/owner_push_default_docs.rs`: a regression guard that reads the four surfaces (clap default, .env.example, README known-limitations prose, README env-var table, SECURITY.md) and asserts each one says the default is `true`. Reverting either doc edit turns one named test red; flipping `default_value_t` back to `false` turns the env example test red. Each surface is checked independently so a stale doc lies alone. No code change. No new migration. No dependency change. Compiles clean, clippy clean under `-D warnings`. Refs: closes the P2 finding from the review of #224. * fix(docs): clarify default value consistency for GITLAWB_ENFORCE_OWNER_PUSH * docs(readme): drop the default-open roadmap line and the em-dash punctuation --------- Co-authored-by: Gravirei <[email protected]>	2026-11-07 00:38:41 +00:00
beardthelion	5ed3381a6b	fix(gl): truncate node-supplied display strings at a char boundary (#350) (#536) ... * fix(gl): truncate node-supplied display strings at a char boundary (#350) Every timestamp/id/SHA the CLI prints was cut with &s[..N] or &s[..N.min(s.len())]. The first panics on a short value, the second still panics when byte N is inside a multi-byte char. A shared text::truncate cuts at a char boundary and covers all sites, including two the issue did not name (node events, agent did) and the same shape in bounty, changelog, ipfs pin listing, and the merge-commit print. * fix(gl): bound truncate by bytes, not chars (#460) Address PR review feedback: char_indices().nth(max) capped characters, so a multibyte char near the cut could exceed the byte limit. Start at max.min(len) and walk back to the nearest char boundary instead. * fix(gl): bound the bounty leaderboard did suffix at a char boundary	2026-11-06 20:38:05 +00:00
beardthelion	9b915b09a7	fix(attest): bound did:key method-id length before base58 decode (#360) (#535) ... gitlawb-core added the same guard in 73bf132c; the second copy of the parser in gitlawb-attest never got it, so an untrusted signer string of arbitrary length reaches a decoder whose cost is quadratic in input.	2026-11-06 18:14:06 +00:00
3 people	77ac1037cb	fix(icaptcha-client): parse GITLAWB_ICAPTCHA_INSECURE as truthy only (#227) (#533) ... * fix(icaptcha-client): parse GITLAWB_ICAPTCHA_INSECURE as truthy only Presence checks treated =0/=false/empty as enabled. Only explicit 1/true now enable loopback HTTP trust relaxation (#227). * fix(icaptcha-client): address #246 review (trim test, fmt, lock) Pin trim with a whitespace truthy case, recover from a poisoned ICAPTCHA_ENV_LOCK, document trim + warn visibility, and cargo fmt. * fix(icaptcha-client): do not log raw ICAPTCHA_INSECURE value The non-truthy warn path previously recorded the full trimmed env value. That is arbitrary deployment input; report only that the setting is invalid (#246 review). * fix(icaptcha-client): pure insecure flag helpers for #246 review Warn on present non-Unicode GITLAWB_ICAPTCHA_INSECURE (fail closed). Pass insecure mode into is_https_with / resolve_solver_url_with so tests no longer mutate process-global env (removes InsecureEnv race). * fix(icaptcha-client): cover real env read + fix warn spacing Add a serialized test that drives icaptcha_insecure_enabled() against the process env so presence-check / typo / always-true mutations fail. Collapse the mid-sentence space run in the non-truthy warning (#246). * fix(icaptcha-client): put the non-truthy warning on one literal line --------- Co-authored-by: Ayush7614 <[email protected]> Co-authored-by: Kevin Codex <[email protected]>	2026-11-06 17:26:16 +00:00
3 people	db98b3957a	fix(attest): make verify_all lenient under RequireAll (#359) (#532) ... * fix(attest): make verify_all lenient under RequireAll (#359) Registry::verify_all propagated every per-attestation error via ?, so a single malformed or unknown attestation aborted the whole batch. Under Policy::RequireAll that is a DoS vector: any third party can attach an attestation, so an unrelated attacker/spam/v1 could block an otherwise valid cert. Under RequireAll, drop entries that fail to verify instead of aborting; the required_types check remains the gate and only counts fully_verified entries, so a required type present only as a malformed attestation still fails with RequiredMissing. AcceptKnown and RejectUnknown stay strict. Adds tests for both: a malformed extra is dropped without aborting, and a malformed-only required type is still rejected. Closes #359 * test(attest): lock the strict batch policies with an error-propagation test AcceptKnown and RejectUnknown were only exercised per-attestation, so a cleanup applying RequireAll's filter_map(Result::ok) shape to the strict arms would fail open with every test green. Drive both policies through verify_all with a wrong-cert-hash member and assert the batch surfaces the underlying error. Also correct the RequireAll doc: unknown valid types are retained as unverified, not dropped — only real Err paths are. --------- Co-authored-by: andreolf <[email protected]> Co-authored-by: Kevin Codex <[email protected]>	2026-11-06 16:54:42 +00:00
beardthelionandAyush7614	8c9e4adc2d	fix(node): canonicalize and 404-on-no-match in remove_label (#344) (#530) ... * fix(node): canonicalize and 404-on-no-match in remove_label (#344) add_label stored labels trimmed and lowercased, but remove_label passed the path segment through as-is, so a label stored as 'bug' did not match DELETE /repos/{owner}/{repo}/labels/Bug. Worse, remove_label's DB call discarded the row count and the handler always returned 200 with {removed: true}, including when the delete matched nothing — the no-op-rendered-as-success shape the rest of the API rejects. Move trim+lowercase and charset/length validation into a shared canonicalize_label helper used by both handlers, and have db::remove_label return the affected row count so the handler can return 404 when no label was deleted. add_label's behavior is unchanged (idempotent semantics preserved); the only externally observable change is that mixed-case and surrounding-whitespace path segments now match the stored label, and a delete that matches no row returns 404 instead of {removed: true}. Tests: * remove_label_canonicalizes_and_404s_on_no_match exercises the mixed-case path, the trimmed path, the empty-after-trim rejection, the disallowed-charset rejection, and the no-op-returns-404 case. * remove_label_denies_non_owner re-pins the 403 owner-gated shape (matches AGENTS.md: owner-gated mutations return 403 after the repo lookup, distinct from the not-found shape). * test(node): pin len>50 arm in remove_label canonicalize The helper's length branch had no coverage — 51 chars must 400 and 50 chars must pass validation (404 when absent). This pins the arm lifted into canonicalize_label and decouples the no-match 404 from the double-delete above, as requested in review of #390. --------- Co-authored-by: Ayush7614 <[email protected]>	2026-11-06 15:08:34 +00:00
beardthelion	8eedb935c3	fix(node): gate bounty mutations on repo read before status checks (#529) ... * fix(node): gate id-keyed bounty mutations on repo read before status checks submit, approve, cancel, and dispute on /api/v1/bounties/{id}/* looked the bounty up and ran status and participant checks without gating on the bounty's repo read visibility. A signed stranger holding an id got a distinguishable 400/403 on a private-repo bounty where an absent id gets 404, so the routes were an existence and lifecycle oracle. Insert the same authorize_repo_read block get_bounty and claim_bounty already carry, right after the lookup, so a denied caller sees the not-found shape either way. Participant checks are unchanged; on a public repo a stranger still reaches them. Closes #341 * Address PR review feedback (#448) - assert the exact not_found body shape for both held and absent bounty ids in the mutation-gate helper, so a denial carrying lifecycle or participant detail fails the test	2026-11-06 15:06:01 +00:00
beardthelion	47c6e6ce22	fix(node): fail pin_git_object when the add response carries no Hash (#528) ... * fix(node): fail pin_git_object when the add response carries no Hash A 200 from /api/v0/add without a parseable Hash fell back to the caller-computed raw CID, recording an address the backend never confirmed stored. For payloads above the backend's chunk threshold (262144 bytes on Kubo) that raw CID is not even a stored block, so every later /api/v0/cat on it fails permanently: get_encrypted_blob errors on every fetch and mirrors retry the envelope on every sync. A Hash-less 200 is now an add error carrying a truncated body excerpt. The delaying_endpoint test fixture, which relied on the fallback by returning an empty 200 body, now answers with a conformant NDJSON line. * fix(node): reject a non-CID Hash in the add response Address PR review feedback (#453): a string-valued Hash that does not parse as a CID would be recorded into encrypted_blobs.cid and fail on every later cat, the same failure class as a missing Hash. Validate the returned Hash before accepting it and cover the malformed case with a regression test. * test(node): return real CIDs from the add-response test mocks The pin_git_object CID validation rejects non-CID Hash strings, and a dozen test_support fixtures answered 200 with placeholder values like "bafyprovider" or "x". Replace them with a valid CID so the fixtures model a healthy backend; no test asserts on the literal value. * fix(node): bound the invalid Hash echoed in the add error	2026-11-06 06:32:13 +00:00
beardthelionandMystic	317a1550c2	fix(node): scope bounty aggregates to anonymously-listable repos (#477) (#527) ... * fix(#477): scope bounty aggregates to anonymously-listable repos bounty_stats and agent_bounty_stats ran unfiltered aggregates over all bounties, leaking private-repo bounty activity to anonymous callers. Mirror the stats() pattern from server.rs (#104): 1. Batch-load all deduped repos + visibility rules (2 SQL round-trips) 2. Filter to listable_at_root(rules, is_public, owner_did, None) 3. Pass visible (owner, name) pairs into SQL aggregates via EXISTS/unnest with owner key normalization on both sides (BOUNTY_OWNER_CASE_SQL). 4. Normalize claimant DID in agent_bounty_stats_visible and group by normalized claimant in bounty_leaderboard_visible (BOUNTY_CLAIMANT_CASE_SQL). 5. Remove orphaned unfiltered aggregate methods to pass dead_code lints. 6. In tests, seed completed bounty claimant state via the real claim/approve path, and verify root-rule-decided visibility overrides. Aggregates are intentionally anonymous-scoped for all callers (matching the #104 stats pattern). Fail-closed: DB errors collapse the visible set to empty, so all counts return 0 — an under-count never leaks existence. Fixes #477 * style(node): apply rustfmt to the bounty visibility change * test(node): pin bounty aggregate visibility for mirror rows * docs(node): record why mirror rows are handled in the bounty visibility filter * perf(node): keep the claimant index usable in per-agent bounty stats * fix(node): return DB errors from bounty stats instead of zero counts --------- Co-authored-by: Mystic <[email protected]>	2026-11-06 03:06:15 +00:00
JATMNandClaude Opus 5.5	30637193a6	fix(node): resolve issue ids from git refnames, not request input (#526) ... * fix(node): resolve issue ids from git refnames, not request input resolve_issue_id answered an exact match by echoing the request-path id back to callers, so get_issue and close_issue spliced caller input into `git cat-file blob refs/gitlawb/issues/<id>`. #521 restricted the input to [A-Za-z0-9-], but the value handed to git was still the request string (CodeQL rust/command-line-injection #83). Resolve exact and prefix lookups through one for-each-ref listing and return the refname git reports. An exact refname still wins over longer ids that share it as a prefix; unique-prefix, ambiguous, not-found, and revision-syntax behavior is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(node): keep a dangling exact issue ref from winning over longer ids for-each-ref lists a ref even when its blob is missing, so the exact-id precedence added in the previous commit picked a dangling exact ref over a readable longer sibling: resolve_issue_id returned the dangling id where main reported an ambiguous prefix, get_issue then returned None, and close_issue hit its expect(). Let the exact id win only when `git cat-file -e` finds its object, run on git's own refname so no request input returns to the argv. Otherwise fall through to the unchanged prefix rules, matching main for every layout. Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: Claude Opus 5.5 <[email protected]>	2026-11-06 03:04:20 +00:00
JATMNandClaude Opus 5.5	a23c81d75a	fix(deps): bump follow-redirects and override ws in scripts/ (#525) ... Resolve Dependabot alerts for the ANS-104 golden-vector tooling in scripts/: - follow-redirects 1.16.0 -> 1.16.2 (lockfile only, within axios's ^1.16.0 range): GHSA-mrwp-f3q8-3g9p, GHSA-q3jh-wrc6-jgq8, GHSA-8r9p-f939-6h3c. - ws 8.18.0 -> 8.22.1 via an npm override. @ethersproject/providers 5.8.0 (latest v5) pins ws to exactly 8.18.0, so a lockfile bump alone cannot move it: GHSA-96hv-2xvq-fx4p, GHSA-58qx-3vcg-4xpx. elliptic (GHSA-848j-6mx2-7j84, low) has no patched release and is left as is. Co-authored-by: Claude Opus 5.5 <[email protected]>	2026-11-06 01:00:41 +00:00
beardthelion	5bb61cc97f	ci(release): verify release provenance for backfill publishes (#523) ... * ci(release): gate publish jobs on the release environment, anchor the tag check, pin npm Closes #234. workflow_dispatch runs the selected ref's YAML and npm trusted publishing matches on the workflow filename, not the ref, so a dispatched non-main ref could reach the OIDC publish with no approval. Declare environment: release on all three publish jobs (npm-publish, docker, docker-manifest) so the environment's deployment-branch rule gates every publish surface, not just the OIDC one. Also close the two residuals the issue carries: the release-tag case glob was right-unanchored (accepted v1.2.3.4.5, v1_x, v1-any-branch-name, bare v1), now an extglob v+([0-9]).+([0-9]).+([0-9]); and the OIDC publish ran npm@^11.5.1, a floating range, now pinned to [email protected].1. The environment itself must exist with a deployment-branch rule limited to main (and required reviewers if desired) for the gate to bite; npmjs.com can also scope each package's trusted publisher to environment "release" for a registry-side check that survives a workflow edit. * ci(release): qualify tag checkouts and keep moving tags forward-only Closes #271, closes #238. actions/checkout resolves an unqualified ref as a branch before a tag, so a branch named v0.7.1 shadows the tag and the release builds unreviewed content. Qualify all four release-tag checkouts as refs/tags/..., which takes checkout's unconditional tag path. docker-manifest applied :latest and :X.Y unconditionally, so a backfill of an older tag repointed them at older code. The manifest now always publishes the immutable :X.Y.Z and applies the moving tags only when the resolved version is not older than the registry's latest release. The same shape existed on the npm side: publish moves the latest dist-tag to whatever it publishes, so a backfilled older version would have stolen it; older backfills now publish under the backfill dist-tag instead. The manifest also asserted nothing about the digest set it stitches, so a wrong-arity or misnamed artifact would ship under :latest. Assert two digests of 64 lowercase hex chars before imagetools create. Extend the resolver test to pin both publish steps byte-for-byte and to reject any unqualified tag-checkout ref, so these guards cannot regress silently. * ci(release): gate every job on the release environment and verify tag provenance Two residuals from review of the dispatch-publish surface: - environment: release only covered the three registry-publish jobs. Declaring it on all eight jobs means a dispatch of unmodified YAML on a non-main ref is denied by the environment's deployment rules before any step runs. - The resolver shape-checked vX.Y.Z but did not prove the tag was produced by the release flow. A write-access user could plant a v99.99.99 tag on unreviewed content and dispatch a backfill against it. In CI (GH_TOKEN present) the resolver now requires a GitHub release for the tag and a main...tag compare status of identical or behind, so the tag's commit must already be reachable from main. * ci(release): qualify the provenance compare and fail closed on npm lookup errors Second adversarial pass found two residuals: - The provenance check compared main...<tag> with an unqualified refname, the same branch-shadows-tag ambiguity the refs/tags/ checkout prefix was added to avoid. Resolve the tag through commits/refs/tags/<tag> to a commit SHA and compare that instead. - npm view ... || true mapped every lookup failure to "no latest", so a registry outage during an older backfill would publish under --tag latest and move the dist-tag backward. E404 still means first publish; any other failure now aborts before a dist-tag is chosen. * ci(release): check release author, decide npm dist-tag per package, close the test gap Third adversarial pass: - Provenance now also requires the release to be authored by the release automation account, so a hand-created release on a valid tag does not qualify for backfill. - The npm dist-tag decision is per package. A partial publish can leave platform packages ahead of the wrapper, and a single lookup on @gitlawb/gl would then downgrade an ahead package's latest tag. - The environment gate test now emits one line per job and marks jobs with no declaration MISSING, so a job added without environment: release can no longer pass silently. * ci(release): anchor docker moving tags to the registry floor, not the release pointer GitHub's latest-release pointer is operator-mutable and is not a bound on what :latest serves. List the package's versions on ghcr and refuse to move :X.Y or :latest below the newest X.Y.Z tag there. A 404 means the package does not exist yet, so the first publish applies all tags; any other lookup failure aborts. The version filter is grouped with || true so an empty list does not trip pipefail. * ci(release): require backfill release assets to be automation-uploaded The npm backfill republishes GitHub release assets, which are mutable for anyone with contents: write. Require every asset on the release to have been uploaded by github-actions[bot], matching the release-author check. Not a hard bound, but it removes the direct edit path and leaves a review signal when a hand-uploaded asset blocks a backfill. * ci(release): pin the verified commit and the exact asset bytes end to end The provenance checks ran at resolve time but the consumers re-resolved mutable names afterward, leaving a move-the-tag or replace-the-asset race: - Checkouts now use the commit SHA the resolver emitted (tag_commit), falling back to the qualified tag ref. - The resolver captures the release's name/id asset pairs once, checks all uploaders against them, and the lay-in step downloads by asset id. An asset deleted and reuploaded between resolve and download gets a new id and fails closed. - The npm-publish comment now names the third trusted-publisher component (environment), which is what binds the OIDC token to the gate. * ci(release): run the resolver on the producer path and hash homebrew inputs locally - release-binaries and web-sync now resolve the release tag and check out the verified commit SHA, closing the tag-move race on the job that produces the release assets, not just the jobs that consume them. - homebrew-bump resolves the tag, downloads the tarballs by captured asset id, and computes sha256 locally instead of trusting the mutable *.tar.gz.sha256 sidecar assets. - The step pin now covers the lay-in, formula, and release-tag checkout blocks so edits to the trust boundary force a deliberate fixture update. * ci(release): bind the tag to the commit its release was created against Reachable-from-main alone accepts a tag moved to any other main commit. The release object records its creation-time target_commitish, so require tag_commit == release.targetCommitish: a moved tag now fails closed, and the SHA emitted for checkouts is provably the released commit. * ci(release): force bash for the release-binaries resolver step The matrix includes windows-latest where run: defaults to pwsh, and the resolver body is bash. * ci(release): base64 release-asset names in the resolver map Release asset names are attacker-controlled text. A crafted name carrying whitespace could smuggle a fake uploader into the third positional field and pass the automation-only check. Emitting names base64-encoded keeps each record in three real fields and makes the name-to-id lookup immune to injection. * Address PR review feedback (#447) - floor the docker minor-line tag at its own X.Y max so an in-line-newest backfill advances :X.Y without moving :latest * Address PR review feedback (#447) - serialize all release runs under one concurrency group; the ref-keyed group let dispatches from different refs interleave * ci(release): freeze docker moving tags on dispatch, retire the registry floor * ci(release): make the resolver test's gh stub discriminate on the full call * ci(release): assert the pinned npm version took effect * ci(release): surface missing-asset errors from the formula sha() subshell * ci(release): consume resolver outputs instead of release-please outputs post-resolve * ci(release): document the concurrency group's pending-slot replacement * ci(release): share the image output and bound the step scanners to jobs The docker-manifest rel step now emits image= the same way the docker job's does, and both manifest steps consume steps.rel.outputs.image instead of recomputing the lowercase ghcr path. The three whole-workflow awk scanners now ignore everything before jobs: and flush at job boundaries, so an `on.push.branches` list item can no longer open a pseudo-step that swallows top-level YAML, and a step record can no longer absorb the next job's header lines. * ci(release): close the review-found evasions in the release checks Review of the follow-up diff found the structural assertions could be satisfied or evaded without the behavior they guard: - the gh stub now pipes a fixture response through real jq with the caller's own -q on every arm, so a resolver that hardcodes its expected fields is detected - the moving-tag scan keys on the outcome (a push mechanism plus a mutable-tag reference in decommented step text) and requires the event+ref guard on the carrier's if: line, instead of binding one argv spelling and accepting comment-borne guard text - a new assertion pins the on.push.branches / workflow_dispatch triggers, which no step-level check could see - the npm check detects install/i and -g/--global variants, requires a non-empty install set, and reads the pin comparison from code, not comments - the stale-reads scan covers first step lines, job-level env/if lines, and jobs without a resolver step - the resolver test asserts the GHAE heredoc terminator, clears ambient GH_TOKEN/GITHUB_REPOSITORY on non-stub calls, and prints the captured resolver stderr when a pass-expected case fails - sha() exits on a failed download, since errexit is off inside its command substitution and a truncated file would otherwise be hashed * ci(release): floor the moving tags against the releases list The event gate alone cannot bound a stale run: a re-run of an older push run replays its stored needs outputs under the original event and ref, and would move :latest and :X.Y backward to that run's version. The gated step now derives the newest published release from the live releases list (contents:read, drafts and prereleases excluded) and refuses when this run's version is not the maximum. The check reduces to VERSION == max, because the run's own release already exists by the time the step runs. The all-steps assertion now requires the carrier to carry the floor as well as the event+ref guard, and the red-check spec gains a gated-but-floorless carrier mutation to prove the clause binds. * ci(release): pin the rebranded Homebrew formula text in the resolver-steps test * ci(release): keep npm stderr out of the dist-tag lookup The Publish step captured `npm view ... dist-tags.latest` with 2>&1, so a warning on stderr from a successful lookup became part of registry_latest and could make a new release compare as older than the registry latest, publishing it under the backfill dist-tag instead of latest. Send stderr to a temp file and read it only when the lookup fails. E404 still means the package was never published, and any other failure still fails closed. The pinned copy of the block in the resolver test is updated to match, and a stubbed-npm test covers a warning on success, a clean lookup, a genuine backfill, E404 and a non-E404 failure. * ci(release): bound the moving tags against every release page The Move floating tags step floored a stale re-run against the semver maximum of the first page of the releases list only. That list is ordered by creation, not semver, so once history passes one page the true newest version could sit on a later page and a stale re-run would move :latest and :X.Y backward. Read every page with gh api --paginate and take the semver maximum in bash. The gh call stays its own command substitution so a failure on any page still fails closed, and an empty list still yields no floor. The pinned copy in the resolver test is updated to match, and a stub-gh test serving two pages covers a stale run whose newer release is only on page 2, the run that is the newest, a gh failure, and an empty list. * ci(release): fail closed when a release asset has no uploader login The resolver ran its automation-only asset check only when the collected uploader list was non-empty. A null uploader login made jq emit an empty third field, the list collapsed to nothing, and provenance passed although no asset was shown to be automation-uploaded. Substitute a sentinel for a missing login so every asset row carries a value and a null, missing, or login-less uploader fails the existing comparison. The base64 name and id map is unchanged, and a release with no assets behaves as before. The resolver test covers an all-null uploader, a null login, a missing uploader key, a bot asset mixed with a null one, and the all-bot release.	2026-11-05 21:51:35 +00:00
beardthelionandGravirei	2c341f71ac	fix(node): ANS-104 codec and recovery policy types (#26 split 2/4) (#524) ... * fix(node): ANS-104 transport, three-outcome probe, verify endpoint (#26 split 2/4) Reviewer 2 closed PR #224 on 2026-08-28 with a directive: split into four narrow PRs. This is Split PR 2 (Arweave transport and verification). The P1 finding the reviewer assigned to this split: the legacy `anchor_item_present` mapped `BAD_REQUEST` and `GONE` to `Ok(false)`, and the recovery code interpreted false as permission to reclaim the row and pay for another upload. Neither 400 nor 410 proves a previously paid item was never accepted: 410 can describe an artifact that existed but is no longer served, and 400 can be produced by gateway, proxy, or routing failure. The concrete failure sequence the reviewer named: bundler accepts the item, the response or terminal DB write is lost, recovery probes the persisted item_id, the gateway returns 400 or 410, and the node pays for a second immutable item for the same transition. Fix: model the probe result as three outcomes, and authorize a re-upload only on a trustworthy, protocol-defined absence. NEW MODULE crates/gitlawb-node/src/ans104.rs - DataItem: the on-wire ANS-104 shape (signature, owner, target, anchor, tags, data — all base64url-encoded except target/anchor). - deep_hash: the canonical signing input per the spec. The signature is over a SHA-256 mix of the dataitem/list/map discriminators, the signature type, owner, target, anchor, the recursively-hashed tag list, and the data payload. A hand-rolled sha256 of the JSON body would produce a hash no Arweave gateway would recognize; the deep-hash is the only correct form. - sign_data_item: Ed25519 over deep_hash. - verify_data_item: parses the owner, checks the signature against an expected pubkey, and reports a specific failure reason on each branch. - 7 unit tests covering round-trip, flipped signature, wrong expected key, mutated data, wire-shape round-trip, deep-hash stability, and the empty-tags edge case. NEW MODULE crates/gitlawb-node/src/arweave_v2.rs - ProbeOutcome: Present (2xx, body parses as ANS-104, sig verifies), DefinitivelyAbsent (404 with a known JSON body shape), Indeterminate (everything else). The reviewer's three-outcome model. - ProbeOutcome::permits_reupload: the recovery policy. ONLY DefinitivelyAbsent authorizes a paid re-upload. Indeterminate keeps the outbox non-terminal; Present skips re-payment. - probe_anchor_item: the gateway probe. 2xx bound to a different item id is Indeterminate, not Present. 2xx with a body that does not parse as ANS-104 is Indeterminate. Oversized bodies are Indeterminate (defense against body-stuffing). - read_capped_body: enforces PROBE_MAX_BODY_BYTES (1 MiB) on the full body, with a Content-Length fast-path that rejects before reading. - verify_anchor: the full path the public verify endpoint takes. Fetches the data item, parses it, verifies the Ed25519 signature against the persisted node_did, decodes the data payload as JSON, and reports the result. - 15 unit tests pinning each classification boundary: 400/410/5xx/transport/2xx-non-json/2xx-bad-sig/2xx-different- owner are all Indeterminate; 404 with empty or known-JSON body is DefinitivelyAbsent; 2xx with a valid signed item is Present; the re-upload policy is exhaustive. NEW DB METHOD crates/gitlawb-node/src/db/mod.rs - get_arweave_anchor_by_item_id: looks up an existing arweave_anchors row by its id (which is the Irys tx id, the same value the Arweave gateway uses to serve the data item). Used by the public verify endpoint to fetch the persisted node_did so the envelope signature can be verified against it. NEW CONFIG crates/gitlawb-node/src/config.rs - GITLAWB_ARWEAVE_GATEWAY_URL: the Arweave gateway the probe reads from. Defaults to https://arweave.net; operators can point it at a private mirror. NEW HTTP ENDPOINT crates/gitlawb-node/src/api/arweave.rs - GET /api/v1/arweave/anchors/verify/{item_id}: the public verify endpoint. Returns {item_id, status, verified, owner_did, data_payload, error} where status is one of "verified" / "definitively_absent" / "indeterminate". - 3 endpoint-level tests covering the verified, 404, and 400 shapes via the live router, asserting the public surface surfaces the three outcomes. WIRE-UP crates/gitlawb-node/src/server.rs - The verify route is registered alongside the existing /api/v1/arweave/anchors list route. NOT IN THIS SLICE (the bundler upload, the next commit): - The handler that builds a DataItem from a ref-cert and POSTs it to the bundler is not in this PR. The ANS-104 module is ready to be called by it; the cert/push-event flow in PR 1 persists the row that the bundler upload would consume. - PR 1's recovery drain produces an `anchor_jobs` row keyed on the (repo, ref, old, new) tuple; the next slice reads that row, builds the ANS-104 data item, POSTs to the bundler, and writes the resulting tx id to arweave_anchors. Compiles clean, 1113 tests pass with 0 regressions, clippy clean under -D warnings, fmt clean. Cross-PR overlap (declared in the PR description): - #134 (anchors auth): composes. The verify endpoint reads from arweave_anchors; #134's auth layer applies unchanged. The probe model is server-side and unauthenticated by design. - #285 (advisory-lock session affinity): independent. The probe is read-only. - #306 (Content-Digest on signed requests): independent. The probe does not sign requests. - #314 (small-order Ed25519): composes. The verify path calls Did::to_verifying_key which already enforces the small-order check from #314. - #324 (libp2p keypair persistence): independent. - #325 (gossip ref-update auth): independent. The verify path is for the public Arweave gateway, not gossip. - #382 (replication withheld-subtree trees): independent. * fix(node): address reviewer findings on #26 split 2/4 (transport) - P1 (DB lookup): get_arweave_anchor_by_item_id now filters on irys_tx_id (the externally-routable item id) rather than the internal UUID primary key. The production writer stores a fresh UUID in `id` and the Irys response / ANS-104 derived id in `irys_tx_id`; the old filter would always 404. New integration tests exercise the production writer path with distinct values for the two columns. - P1 (verify-gate): verify_anchor now gates on repo read via authorize_repo_read. The route is split in server.rs: the public list_anchors stays ungated (#134 tracks that surface), the verify route gets optional_signature per the team memory `axum-layer-vs-merge-pitfall.md`. Denials on row-lookup, gate, and missing repo all collapse to the same opaque 404 so the public endpoint does not leak anchor-row existence. - P1 (ANS-104 deep-hash): rewritten to the verified SHA-384 recursive list/blob algorithm (matches `arbundles` JS reference). deep_hash returns [u8; 48]; signature is over the raw 48-byte digest. Three reference vectors from `Irys-xyz/arbundles/src/__tests__/deepHash.spec.ts` are bit-exact-asserted as the interop canary per the team memory `self-roundtrip-tests-do-not-prove-interop.md`. - P1 (artifact-identity): v2 verify now derives the protocol id via DataItem::id() (base64url(SHA256(signature))) and requires equality with the requested URL item_id. A node key signs many data items, so a valid signature only proves who signed the response — not that the served item is the one the caller asked to verify. The team memory `verify-against-artifact-id-not-signer.md` is the policy. - P1 (verifier can't read this head's writes): verify_anchor now accepts BOTH the v2 ANS-104 format and the v1 raw-JSON shape the live path on this branch actually writes. The v1 path does a field-equality check on repo, ref_name, old_sha, new_sha, node_did against the persisted row (no signature in v1; Irys storage plus the JSON parse are the integrity guarantee). - P2 (ProbeOutcome in result): AnchorVerifyResult now carries outcome: ProbeOutcome. The HTTP handler maps the structured field to the status string instead of parsing the human- readable error message — any future wording change in arweave_v2.rs can no longer silently reclassify a DefinitivelyAbsent as Indeterminate. - P2 (empty 404 body): classify_404 returns Indeterminate for bodyless 404s. The team memory `distinguish-unknown-from-empty.md` is the policy: a bodyless 404 from a proxy is not a proof of absence, and the recovery policy (permits_reupload -> true on DefinitivelyAbsent) authorizes a paid, irreversible re-upload. - P2 (streaming cap): read_capped_body no longer uses resp.bytes() (which buffers the full response before checking size). It now reads chunk-by-chunk and aborts the moment the cumulative byte count crosses the cap, so a chunked response without Content-Length cannot force unbounded allocation. * fix(node): address three reviewer findings on #26 split 2/4 (round 2) * fix(node): address five reviewer findings on #26 split 2/4 (round 3) * fix(node): address reviewers round-4 on #26 split 2/4 (interop) * fix(node): address coderabbit on #26 split 2/4 (raw tags, vint overflow, golden key) * fix(node): enforce artifact-id in probe_item on #26 split 2/4 * fix(node): require expected owner for probe Present on #26 split 2/4 * fix(node): narrow #26 split 2/4 to codec and policy types * fix(node): silence dead-code for deferred slice API on #26 split 2/4 * fix(node): doc and bounds-check cleanup in ans104 on #26 split 2/4 * docs(node): point tests doc at Ed25519 golden script on #26 split 2/4 * fix(node): strict owner lengths, sign-before-hash on #26 split 2/4 * fix(node): keep ans104 tags and field widths consistent on hash and encode DataItem keeps both the decoded `tags` and the original Avro `raw_tags`, both public. After a parse, changing `tags` left deep_hash and verify_data_item using the stale raw bytes, and to_binary wrote a header tag count that no longer matched the payload, so the frame failed to parse back. Take the tag block for hashing and encoding from one helper that decodes `raw_tags` and requires it to equal `tags`, and fail closed otherwise. The header count comes from the same validated pairs. Parsed multi-block and size-prefixed items still re-encode byte-exact and keep their signatures. deep_hash now also rejects a non-empty target or anchor that is not 32 bytes, as to_binary already does, so sign_data_item fails instead of signing an item that cannot be encoded. The legacy non-base64url anchor fallback is kept. The module and signature-type docs now say binary parsing accepts sigtypes 1 to 4 while signing and verifying are Ed25519 only on this slice. * build(scripts): declare the golden script's direct npm dependencies ans104_golden_ed25519.mjs imports bs58, @noble/ed25519 and base64url, and ans104_golden.mjs imports base64url, but package.json listed only arbundles, so they resolved only through hoisting. Declare them at the versions already locked, update the lockfile root entry (no resolved version or hash changes), and note the regeneration command in the golden output file. --------- Co-authored-by: Gravirei <[email protected]>	2026-11-05 21:12:46 +00:00
beardthelion	911be4d9fe	fix(node): validate ref-update SHAs are hex, not just length 40 (#433) ... * fix(node): validate ref-update SHAs are hex, not just length 40 parse_ref_updates accepted any 40-byte string as an object id, so a peer-supplied pkt-line with non-hex content was forwarded into RefUpdateEvents on gossip, branch_cid rows, and sync-notify JSON as a canonical-looking identifier. Require [0-9a-fA-F] on both fields. Closes #398 * chore: refresh PR head	2026-11-05 16:57:42 +00:00
beardthelion	df87b150fd	fix(gl): pass the mirror temp path to git as an OS string (#434) ... * fix(gl): pass the mirror temp path to git as an OS string The clone destination went through to_str().unwrap(), so a TMPDIR with non-UTF-8 bytes panicked before git started. Pass the Path directly and add a Unix regression test cloning into a non-UTF-8 directory name. Closes #417 * test(gl): cover a non-UTF-8 TMPDIR end to end via a child process * chore: refresh PR head	2026-11-05 16:52:25 +00:00
beardthelion	0366f18458	fix(node): reconcile public-read messaging with per-repo enforcement (#439) ... * fix(node): reconcile public-read messaging with per-repo enforcement GITLAWB_PUBLIC_READ is read nowhere except a startup warning, while read enforcement actually happens per repository through is_public and path-scoped visibility rules. The flag's help text, the startup warning, .env.example, and the readiness audit all still claimed private-read enforcement was unwired. The help and warning now state the flag is reserved and inert, and point at the real per-repo control. A config test asserts the rendered help neither claims enforcement is missing nor omits that the flag does nothing. Closes #338. * docs: drop the stale private-read priority entry * chore: refresh PR head	2026-11-05 16:46:47 +00:00
beardthelion	c2e175d222	fix(git-remote): stop waiting on EOF for a delete-only push (#442) ... * fix(git-remote): stop waiting on EOF for a delete-only push * fix(git-remote): consume negotiated push-options and push-cert sections * fix(git-remote): frame signed pushes correctly, cert before options and no command list * chore: refresh PR head	2026-11-05 16:39:29 +00:00
beardthelionandKevin Codex	cd27756134	ci(triage): match test as a name segment, not just a whole component (#277) ... * ci(triage): match test as a name segment, not just a whole component #202 landed the namespaced-attribute fix, so main already detects #[tokio::test] and #[sqlx::test]. What it still misses is a third-party harness whose final path component carries `test` as an underscore segment. #[test_case(1)] and #[wasm_bindgen_test] both fall through and wrongly earn the PR a needs-tests label. Widen the final component to (?:[A-Za-z0-9]+_)*test(?:_[A-Za-z0-9]+)*, keeping the leading ::, the r# form, and the whitespace tolerance #202 added. Differential over 972 real file-patches from 400 commits: no verdict changes either way, so this is a forward-looking net for harnesses the tree does not use yet, not a fix for a current mislabel. Segment rather than substring, on purpose. addsInlineTest suppresses the label when it matches, so a false positive is the quiet failure: #[contest] or #[latest] would clear needs-tests on a PR that added no tests and nobody would notice. The cost is the harness names with no underscore, such as #[rstest], which stay unmatched and produce the loud, author-correctable failure instead. The segment classes are [A-Za-z0-9]+ rather than \w+ so that `_` is only ever the literal delimiter. \w+ contains `_`, which makes the repetition ambiguous and backtracks exponentially. That matters here specifically because the workflow triggers on pull_request_target, so f.patch is fork-controlled text on a privileged runner: with \w+, `#[test` followed by a long `_a` run took 71ms at 22 characters and roughly quadrupled every two more, which stalls the triage job for any contributor who can open a PR. The character-class form measures linear across every adversarial input tried. * fix(ci): allow Rust token separators before test attribute delimiters The needs-tests inline detector required ] or ( immediately after horizontal whitespace, so valid spellings like #[test /* rationale */] or a path split across added patch lines still triggered needs-tests. Match block comments, line breaks, and split-line closers instead. * ci(triage): scan attribute closers token-aware, bound to adjoined added lines The two-regex fallback matched a path-only attribute line and a ]/( line anywhere else in the patch, so unrelated records could complete each other and silently clear needs-tests; the mandatory separator regex also dropped legal //-to-newline and nested block comments, relabeling real tests. Replace both with one forward token scan that starts on the attribute's added line, continues only through immediately adjoined added lines under a hard bound, and answers 'no inline test' on anything uncertain. Fence the detector for extraction and add scripts/test-pr-triage-detect.mjs, run by a new pr-checks job, since pull_request_target executes the target branch's workflow and can never exercise the proposed body. * fix(ci): skip test attributes inside raw strings, block comments, and strings patchAddsInlineTest had no Rust lexical state, so a #[test_case] inside a raw string (r#"..."#), block comment, or regular string literal, followed by ] or ( on an adjoining added line, falsely cleared needs-tests. That is the silent failure direction the detector is supposed to fail loud against, and it is newly reachable because the underscore-segment regex introduced by this branch matches forms the old test\b did not. A pre-pass now walks every patch line (context and added, not removals) tracking raw strings (r#*", br#*", cr#*"), block comments (/* */ with nesting), regular strings (which can span newlines in Rust), and line comments. Added lines that start inside any of these are skipped as candidates. The pre-pass resets at hunk headers, is O(n) in patch length, and only sees what the patch shows: a non-code region opened before the first context line of a hunk is invisible, and the candidate is still scanned (conservative, fails toward the loud label). Also corrects the failure-direction comment in test-pr-triage-detect.mjs: false positives suppress needs-tests (they set touchedTests, which clears the label), while false negatives apply it (the loud, corrigible direction). The previous text had them backwards. Adds 7 regression cases: raw-string, block-comment, no-hash raw-string, and multiline regular-string false positives (reject), plus raw-string, block-comment, and regular-string closing before a real test attribute on the next added line (accept). Revert-check confirmed the pre-pass is load-bearing for both the nonCodeStart skip and the inString carry: disabling either makes the corresponding reject cases go RED, restoring makes all 31 pass. * fix(ci): recognize char literals so their contents cannot open string state The lexical pre-pass in patchAddsInlineTest missed real tests when an earlier added line contained a double-quote char literal like '"' or b'"'. The scanner saw the " inside the char token and set inString, which persisted to the next line and suppressed a following #[test]. Add bounded char-literal recognition: on seeing ', look ahead 2-4 chars (or to } for \u escapes) for a closing '. If found, skip the whole literal. If not, it is a lifetime or label, and the apostrophe is skipped as a regular char. Not a skip-to-next-' rule; lifetimes and labels fall through unchanged. * fix(ci): bound the \u{ escape scan to prevent quadratic stall The char-literal pre-pass scanned to the next } with no bound when it saw \u{, so a fork-controlled patch repeating '\u{ thousands of times made the triage job quadratic (21s for 30000 reps). Bound the scan to 20 chars past the { (a valid \u{...} holds at most 6 hex digits). Add a runtime probe for the malformed-escape shape. * chore: refresh PR head --------- Co-authored-by: Kevin Codex <[email protected]>	2026-11-05 16:33:22 +00:00
beardthelion	5479fd59b8	fix(icaptcha): return unsolved instead of blocking or overflowing (#444) ... * fix(icaptcha): return unsolved instead of blocking or overflowing interactive_prompt waited on stdin().read_line even when stdin was an open, silent non-TTY (an orchestrator holding the pipe open), which blocked obtain_proof forever. Check IsTerminal first and return None so the caller surfaces the "couldn't auto-solve" error. The deterministic solvers evaluated prompts with unchecked i64 arithmetic; service-controlled literals could push add/sub/mul/div/abs past the range, panicking in debug builds or wrapping to a wrong answer in release. Use checked arithmetic throughout and return None on overflow. Fixes #345 * chore: refresh PR head	2026-11-05 16:01:31 +00:00
beardthelion	131bb95c68	fix(node): scope the refs listing by the canonical repo slug (#388) ... * fix(node): scope the refs listing by the canonical repo slug `list_refs` built its `branch_cids` filter from the raw URL path segments while the only production writer stores the canonical `normalize_owner_key(owner_did)/name` slug. `get_repo` resolves both `did:key:zX` and bare `zX` to the same record, so a request in the full DID form passed the read gate and then matched zero rows, returning 200 with an empty ref list. A denial shaped like a success is worse than an error: a caller cannot tell an empty repository from one it queried under a form the table does not carry. Bind the record `authorize_repo_read` already returns and build the filter from it, so the queried resource is the one that was authorized. This is the existing pattern in `api/events.rs` and in this file's own push path. Two regressions, each proven to fail without the line it guards. The first drives both owner-DID forms through `build_router` and asserts they return the same refs, with a second repo whose refs must not appear; widening the query to match any owner turns that half red. The second covers the deny direction, which the endpoint had never had: a private repo answers an anonymous caller with the same 404 a missing repo gets and puts no CID in the body. Without it, removing the read gate outright left the suite green. * chore: refresh PR head	2026-11-05 15:40:50 +00:00
beardthelion	0303222eae	fix(node): gate replica register/unregister on repo read visibility (#445) ... * fix(node): gate replica register/unregister on repo read visibility list_replicas already applied authorize_repo_read, but the two mutations looked the repo up directly: a non-reader of a private repo could register or remove their own replica and receive the replica count in the response. Route both handlers through authorize_repo_read so a denied caller gets the same 404 as a missing repo and nothing is written. Self-registration and self-removal are unchanged for authorized readers. Fixes #435 * test(node): assert denial bodies leak no replica or repo data CodeRabbit nitpick: the new replica-mutation denial tests only checked the 404 status. The path instructions for this file require non-leaking body assertions on denied responses, matching the sibling list_replicas_is_read_visibility_gated test. Assert the denied PUT and DELETE bodies contain neither the submitted replica URL nor the repo id, in both the handler-level test and the real-signature e2e. * chore: refresh PR head	2026-11-05 15:20:09 +00:00
beardthelion	2450bbd9e9	fix(core): cap the blinded recipient scan in open_blob (#419) (#440) ... * fix(core): cap the blinded recipient scan in open_blob * fix(core): reject over-cap recipient lists at seal too	2026-11-05 05:24:52 +00:00