mirror of
https://github.com/Twigpine/node.git
synced 2026-11-13 06:51:46 -10:00
main
43
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 beardthelion
|
6f990197ea |
feat(node): per-IP write-surface rate brake + purge-spam admin tool (#196)
* feat(node): add per-IP write-surface rate brake and apply to write_routes
Introduces a dedicated write_rate_limiter bucket (GITLAWB_WRITE_RATE_LIMIT,
default 600/hr, 0 disables), separate from the creation brake so a write flood
cannot drain the creation budget (KTD-1). Attaches rate_limit_by_ip to
write_routes, mirroring creation_routes. Per-DID deliberately not paired (a DID
farm never trips it).
Tests (route-level, execution-verified against Postgres): a write_routes sink
(PUT /star) is 429'd before auth; an exhausted write bucket does not throttle
repo creation (separate-bucket property).
* feat(node): extend the per-IP write brake to the remaining write sinks
Attaches rate_limit_by_ip (write bucket) to task_write_routes,
issue_write_routes, bounty_write_routes, profile_write_routes, and the /graphql
POST/GET surface (KTD-5 — an HTTP-layer brake counts every /graphql request;
/graphql/ws subscriptions stay unbraked). write_ip_limiter is now built once and
shared. Every non-git, non-creation authenticated write group is braked.
Tests (execution-verified against Postgres): /graphql POST is 429'd, and a
representative REST write sink (issue comment) is 429'd, by the write brake.
* test(node): adversarial TrustedProxy verification through the write brake
Middleware-level tests complementing the client_key unit coverage: a spoofer
varying the client-controlled leftmost X-Forwarded-For hop cannot rotate its
bucket key (keyed on the trusted rightmost hop); distinct trusted client hops
get distinct buckets (no cross-starvation); and a request with no trusted header
in a proxy mode falls back to the socket-peer key — the documented collapse,
asserted by name so any change is deliberate.
Operator-warning on missing-header deferred deliberately: a per-request warning
is a log-noise risk and the security-load-bearing fallback (peer key, never
skip) is what these tests lock down.
* feat(node): purge-spam admin subcommand for the empty-burst cleanup
Adds a purge-spam subcommand (clap #[command(subcommand)] split; no-subcommand
path still runs the node unchanged). Candidate selection is signature-based, not
per-DID: a repo qualifies only if owned by the named burst DID AND verified empty
(zero git refs) per repo. A hard exclusion gate runs BEFORE the empty check, so
an empty repo owned by the content-bearing user or the mirror-bot DID is never a
candidate. Dry-run is the default; deletion needs an explicit --execute flag and
operates per-repo, never 'all repos of DID X'.
Tests (RED->GREEN, exclusion gate confirmed load-bearing by targeting an excluded
DID directly): empty target listed; target-with-refs spared; empty excluded/
intern repos spared (must-assert); dry-run deletes nothing; execute removes only
the empty target on disk.
* fix(review): make purge-spam empty-check reject git-discovery reads
P1 from code review: on_disk_ref_count shelled 'git for-each-ref' with only the
repo path as cwd and no --git-dir, so a path that exists but is not a bare repo
let git discover a parent .git (repos_dir may sit inside the operator's checkout)
and read a DIFFERENT repo's refs — possibly 0, deleting a real repo. Now require
the bare-repo markers (HEAD file + objects/ dir) before trusting any count;
anything else fails closed (skipped).
Tests: a non-git dir under a git ancestor (zero-ref) now fails closed — verified
RED without the guard (read the ancestor's 0 refs); target-DID-never-excluded
invariant pinned; write_flood test anchored to a genuinely-drained bucket so its
assert_ne can't pass vacuously.
* fix(review): normalization-consistent purge, brake helper, CLI + coverage
Addresses the remaining code-review findings:
- purge-spam DID matching now uses did:key normalization on both sides
(list_repos_by_owner_did via OWNER_KEY_CASE_SQL, is_excluded and the target
scope via normalize_owner_key), so short/full form is consistent and an
excluded identity is protected in either form. Adds a short-form exclusion test.
- Extract the per-IP write brake into a chainable WriteBraked helper so a future
write group can't silently ship unbraked (kept separate from add_auth_layers
by design; documented why).
- Mark --repos-dir / --database-url global so admin subcommands accept them after
the subcommand (was an 'unexpected argument' parse error); verified.
- Document GITLAWB_WRITE_RATE_LIMIT in .env.example incl. the per-IP NAT-aggregate
caveat so operators can size it.
- Coverage: adoption-floor (under-limit write passes), write-limit=0 disables,
task/bounty/profile route-level 429s, /graphql/ws stays unbraked.
Full suite 927 pass / 0 fail; fmt + clippy -D warnings clean.
* test(review): close the reasoned-not-run gaps by execution
- purge-spam execute now re-verifies emptiness immediately before delete (TOCTOU:
a push between selection and delete no longer risks deleting a now-non-empty
repo) and removes the on-disk bare repo dir so DB and disk stay consistent; a
per-repo delete failure warns and continues rather than aborting the batch.
- Tests (RED-verified where they cover new behavior): partition_for_delete skips
a repo no longer empty; execute removes the on-disk dir (RED without the
removal); list_repos_by_owner_did finds a short-form burst row (RED without the
SQL normalization); every braked write group passes under-limit (per-group
grant path, not just the star representative).
* fix(node): reject path-traversal repo names in purge-spam before remove_dir_all
A peer-mirror row skips API name validation, so a burst-owned repo can carry
a '../' name; repo_disk_path joined it verbatim and the emptiness marker check
passed on the traversed target, so purge-spam selected and remove_dir_all'd a
repo OUTSIDE repos_dir. Validate the name via repo_store::validate_repo_name at
selection (fail closed -> non-candidate) and assert canonical containment inside
repos_dir immediately before the destructive remove. Adversarial RED->GREEN: a
victim bare repo reachable via '../../victim' is deleted before, survives after;
symlink-escape covered by path_within.
* fix(node): delete child rows transactionally in delete_repo_by_id
delete_repo_by_id was a bare DELETE FROM repos with no cascade and the schema
has no FKs, so every child row (ref_certificates, pull_requests + pr_reviews/
pr_comments, webhooks, push_events, protected_branches, repo_stars, repo_replicas,
repo_labels, visibility_rules, encrypted_blobs, repo_icaptcha_proofs, agent_tasks,
and the slug-keyed branch_cids/sync_queue/received_ref_updates/arweave_anchors)
was orphaned and could re-join a later repo reusing the id. Wrap parent+child
deletes in one transaction, resolving the owner_short/name slug for the repo-TEXT
children. bounties (financial) and issue_comments (unmappable) are left intact by
design. RED->GREEN test seeds a repo_id child, a slug child, and a PR grandchild.
* fix(node): reap write_rate_limiter in the periodic cleanup loop
The PR's new write_rate_limiter was the one limiter omitted from main()'s
hand-maintained cleanup list, so its key map never shed expired entries until
the 200k cap. Move the sweep into AppState::cleanup_rate_limiters (co-located
with the limiter fields, single source of truth) and call it from the loop, so
a newly-added limiter is reaped rather than silently dropped. Guard test seeds a
reclaimable entry into write_rate_limiter and asserts the sweep reaps it — RED
if write_rate_limiter is dropped from cleanup_rate_limiters (verified).
* fix(node): hold the per-repo advisory lock across purge recheck+delete
purge-spam took no write lock, so a concurrent receive-pack could land a ref
between the emptiness recheck and the delete, silently losing a live push.
Add RepoStore::try_lock_repo — a lock-only, non-blocking counterpart to
acquire_write (no Tigris I/O) whose guard holds a dedicated pooled connection so
the advisory lock and its unlock run on the same session. purge now locks each
candidate, rechecks emptiness UNDER the lock, deletes, and releases; a repo held
by a live writer is skipped, not force-deleted. RED->GREEN: a locked empty repo
survives purge, then deletes once the writer releases (neuter-check confirms the
lock is load-bearing).
* fix(node): count purge-spam disk-removal failures separately from deletes
The Ok(n) arm incremented the deleted total before remove_dir_all, whose failure
only warn!'d, so the summary reported N deleted even when the on-disk dir
survived (DB/disk drift shown as clean success). run_purge_spam now returns a
PurgeSummary and counts a failed (or containment-refused) on-disk removal in
disk_failed, never folding it into deleted; the summary line surfaces
'M on-disk removal(s) failed'. RED->GREEN: a read-only parent dir forces the
removal to fail; deleted=1 AND disk_failed=1 with the dir surviving (neuter-check
confirms the counter is load-bearing).
* fix(node): warn on unparseable rate-limit env vars instead of silently defaulting
GITLAWB_CREATE/WRITE/PUSH_RATE_LIMIT parsed with .ok().and_then(parse).unwrap_or,
so a typo (e.g. 5O with a letter O) silently reverted to the default and left the
operator believing a stricter cap was in force. Route all three through
rate_limit_from_env, which distinguishes absent/empty (default, silent) from
present-but-unparseable (default, WARN). Pure resolve_rate_limit is unit-tested
across absent/empty/valid/zero/unparseable — 0 stays valid (its own ==0 warn
handles the disable case).
* docs(node): document GITLAWB_IPFS_API/TIGRIS_BUCKET/METRICS_ADDR/SHUTDOWN_GRACE_SECS in .env.example
Four config knobs read in config.rs were absent from .env.example, so operators
had no template for them. Add each in its matching section with the config
default (INV-24: operator docs match the config the code reads, in the same PR
that touches the file).
* test(node): exercise the dry-run guard with a real on-disk candidate present
dry_run_deletes_nothing ran against an empty repos_dir, so select_spam_candidates
found nothing and run_purge_spam hit the no-candidate early return — the
if !execute { return } guard was never exercised with candidates present (a
vacuous guard, INV-21). Materialize a real empty bare repo so a genuine candidate
exists, then assert dry-run leaves both the row and the on-disk dir. Verified
load-bearing: removing the !execute guard turns the test RED (dry-run deletes).
* style(node): cargo fmt for the #196 fix set
* fix(node): pin the writer's advisory-lock connection in RepoWriteGuard
acquire_write took the session-scoped advisory lock through the shared
pool and returned the lock-owning connection to the pool, so release
unlocked on a different connection (silent no-op) and a concurrent
try_lock_repo could be handed the idle lock-owning connection and
reentrantly re-grab the lock, defeating the purge tool's mutual
exclusion. Pin a dedicated PoolConnection for the guard's lifetime and
unlock on it, mirroring RepoLockGuard.
Load-bearing contention test (single-connection pool to force the
reentrant grab deterministic): RED (Ok(Some)) before, GREEN after.
* refactor(node): introduce ObjectStore trait seam over TigrisClient
Extract an async ObjectStore trait (exists/upload/download/delete) that
TigrisClient implements, and hold Option<Arc<dyn ObjectStore>> in
RepoStore instead of Option<TigrisClient>. A dyn async trait needs
async-trait (added as a direct dep; already transitively present), which
E0038 would otherwise reject. Pure refactor — no behavior change; full
suite stays green. Enables an in-test fake for the purge Tigris work (U3).
Cargo.lock also picks up the pre-existing 0.5.0->0.5.1 version sync
(#185: the committed lock was stale vs the manifests) that any cargo run
regenerates.
* fix(node): make purge-spam Tigris-authoritative and archive-deleting
purge-spam built its RepoStore with no object store, so on a Tigris
deployment it rechecked emptiness against possibly-stale local disk
(deleting a repo with live remote refs) and left the archive behind (to
be re-downloaded into a later same-owner/name repo). Give the admin
command the configured object store, refresh the local copy from the
authoritative archive under the per-repo lock before the recheck
(fail-closed on a store error via a new skipped_store_error), and delete
the archive on a successful purge (failures counted in a new
archive_failed, never folded into success). When no bucket is configured
the None path is byte-for-byte unchanged.
Three load-bearing guards verified RED->GREEN on the Tigris-blind purge:
remote-refs-not-deleted, archive-deleted, fail-closed-on-unreachable.
* fix(review): don't hold/leak the pool connection in acquire_write
Two regressions the connection-pinning fix (
|
||
|
beardthelionandAyush7614
|
4fc041a399 |
fix(node): encrypt webhook HMAC secrets at rest and redact from Debug (#575)
* fix(node): encrypt webhook HMAC secrets at rest and redact from Debug Webhook secrets are signing keys and were stored in cleartext, exposed by the Webhook struct's derived Debug, and reachable through JSON serialization. - Introduce a WebhookSecret newtype whose Debug and Serialize redact to "***", so the secret never reaches a log line or an API body. - Seal the secret at rest with a node-held key (HKDF-derived from the identity, domain-separated as gitlawb/webhook-secret/v1) via XChaCha20-Poly1305; the column holds enc:v1:<nonce||ciphertext>. - Open the secret only at delivery-signing time; legacy plaintext rows still sign and are re-encrypted once at startup by Db::reencrypt_legacy_webhook_secrets. Closes #506 * test(node): derive webhook test keys from the OS CSPRNG CodeQL flagged the hard-coded [0x42u8; 32] / [0x43u8; 32] helper keys as a hard-coded cryptographic value. Derive them with OsRng.fill_bytes instead, matching gitlawb-core's scan_token::new_key, so no hard-coded key material sits in the tree. A wrong key still fails XChaCha20-Poly1305 authentication with overwhelming probability. * test(node): draw webhook test keys via OsRng.gen to clear CodeQL flag CodeQL's 'Hard-coded cryptographic value' query treats the [0u8; 32] zero-init flowed through OsRng.fill_bytes as a hard-coded key, so the CSPRNG fix still tripped the gate. Generate the key array directly with OsRng.gen() so no constant byte array reaches the key sink. * fix(node): withhold unsigned webhook deliveries, classify sealed rows by open() - delivery path now decides via SigningOutcome: a hook whose stored secret cannot be opened (foreign or rotated key, truncated ciphertext) is withheld with a warn and the secret_unopenable metric label, instead of posting unsigned and marking the event sent; a hook with no secret still delivers unsigned as before - reencrypt_legacy_webhook_secrets treats "already sealed" as "opens under the node key", not "has the enc:v1: prefix": a plaintext secret that literally begins with the prefix, or a value sealed elsewhere, is re-sealed raw instead of being skipped and wedged. The UPDATE is compare-and-set on the value read so a concurrent secret change is not clobbered, and prefixed-but-unopenable rows log a warning - WEBHOOK_SECRET_KEY_INFO doc now says a version bump renders every sealed row unopenable until a re-seal path exists, rather than advertising rotation the code cannot do - shared hkdf_sha256 helper in state.rs: derive_scan_token_key and derive_webhook_secret_key were byte-identical HKDF bodies - new pins: signing_decision across all four outcomes, reveal on a sealed value, the prefixed-plaintext re-encryption wedge * fix(node): leave foreign ciphertext alone in webhook re-encrypt, count CAS outcome - has_seal_shape splits "prefixed plaintext or corrupt blob" from "well-formed ciphertext": a sealed value that fails open() under the node key is left unchanged so delivery keeps withholding, instead of re-wrapping the outer blob and signing with it as a fake plaintext - rewritten counts rows_affected, not iterations; a lost CAS race is logged and not counted - derive_webhook_secret_key rustdoc now agrees with WEBHOOK_SECRET_KEY_INFO: a version bump orphans sealed secrets, it is not a rotation - the signing_decision test key comes from OsRng.gen() like the other webhook test keys, clearing the hard-coded-key CodeQL alert - pins: foreign ciphertext survives the pass untouched and reveal still returns None for it * chore: re-trigger CodeQL check evaluation --------- Co-authored-by: Ayush7614 <[email protected]> |
||
|
beardthelionandVasanthdev2004
|
3a0d91a801 |
feat(node)!: anchor the UCAN proof chain and honour delegated git/push (#567)
* feat(core)!: verify_chain returns the proof chain's root issuer verify_chain returned Result<()>, so a caller learned that a chain was internally consistent and nothing else. `did:key` is self-certifying: anyone can mint a keypair, self-issue `*` on `*`, and produce a token that verifies. That is correct UCAN behaviour rather than a defect — a token with no proofs is its own root — and it still verifies after this change. But it does mean "the chain verifies" is not an authorization answer on its own, and without the root identity no caller could ask the question that actually decides a push: does this chain rest on someone this repository trusts? Returning the root means a caller cannot accept a chain without being handed the identity it rests on. Callers that legitimately do not care — the node middleware checking that a bootstrap token is well-formed, for one — discard it explicitly. The doc comment now states outright that this establishes internal consistency and not trust, and names what a caller has to compare the root against. Multi-proof chains are refused: more than one proof means more than one root, and nothing says which root authorized a given capability, so returning any single one would be unsound. The old loop followed every proof but returned nothing, so a two-proof chain verified silently; the new test pinned that by observing the pre-fix code accept one. Ucan::delegate only ever writes one proof, so no token this codebase produces is affected. BREAKING CHANGE: Ucan::verify_chain returns Result<Did> instead of Result<()>. * fix(node): gate two Unix-only tests so the crate compiles on Windows Two tests build a fake git as a `/bin/sh` script, mark it executable through `std::os::unix::fs::PermissionsExt`, and reap the hung `rev-list` with `libc::kill(SIGKILL)`. None of that exists on Windows, and neither test was cfg-gated, so `cargo test -p gitlawb-node` failed to compile there with six errors before running anything. A Windows checkout could not run a single test in the crate, including the ones that are platform-independent. Gate both with `#[cfg(unix)]`. The attribute is a no-op on Linux, so CI keeps running them exactly as before; only the Windows build changes, from "does not compile" to "runs the platform-independent tests". Refs #228 * feat(node): carry the verified UCAN and its root into the request require_ucan_chain validated a presented token and discarded it, so no handler could read the result and Ucan::can had no call site anywhere in the node. A UCAN could only ever fail a request, never authorize one. The middleware's own behaviour is unchanged — an absent header passes through, an invalid chain is still 401. It now also parks the verified token and the root issuer its chain rests on in request extensions, so an authorization decision downstream can use them. The root is stored rather than recomputed so the chain is walked once per request. Holding a VerifiedUcan is deliberately not an authorization decision. did:key is self-certifying, so a chain that verifies proves only internal consistency; the caller has to compare the root against an identity it trusts for the resource being touched. The next commit adds that comparison for git/push. * feat(node): add ucan_grants_push, anchored at the repo owner The predicate that decides whether a delegated capability authorizes a push. The chain must root at the repo owner — data the node holds independently of the token, which is what a self-minted chain cannot forge — and the leaf must carry git/push for this repo. Only the leaf is examined for the capability. verify_chain has already established that each leaf capability is attenuated by its proof, transitively to the root, so a surviving leaf capability is no broader than what the root granted; re-walking the chain would duplicate that guarantee. Resource matching is structural rather than a string compare because owner_did is stored full on canonical rows and bare on mirror rows; a literal match would deny valid delegations for every mirror, which is a defect that would have looked like a permissions bug rather than a parsing one. A capability carrying nb authorizes nothing while constraints stay uninterpreted, so an owner who writes a ref restriction never accidentally grants repo-wide push. Not yet wired into the push path — the next commit does that — so a non-test build still reports VerifiedUcan::ucan as unread. * feat(node): honour a delegated git/push capability on the push path caller_authorized_to_push becomes owner || delegated, exactly the Phase 2 its own doc comment described. The owner check is unconditional and runs first, so the UCAN path can only ever turn a 403 into a 200, never the reverse. A push carrying no X-Ucan reaches the same owner-only decision it did before. git_receive_pack takes the verified token as Option<Extension<VerifiedUcan>>; axum extracts an absent extension as None rather than rejecting, so the optional header stays optional. The denial message is deliberately identical whether a delegation was absent, expired, or named a different repository. A behavioural test asserts the two bodies are byte-identical, because a difference would turn the refusal into an oracle for which capabilities exist. The behavioural test drives both auth layers with a real RFC 9421 signature and a real invocation, and discriminates on status: 500 means the request passed require_signature, passed require_ucan_chain, cleared the owner gate, and reached git on a repo with no disk backing. A bare `!= 403` would let a 401 regression through. It needs no fake-git shim, so unlike the rest of the push path it is not cfg(unix) and runs on every platform. Both new tests were verified to fail: with the delegation branch removed the behavioural test reports 403 where it wants 500, and the unit test's assertion fires. A test written after its implementation proves nothing until it has been watched failing. * feat(gl): gl ucan import stores a delegation for the push helper The git remote helper needs a delegation on disk in a location it can derive from a gitlawb:// URL alone. Files key on the bare base58 owner key because did:key contains a colon, which Windows will not accept in a filename, and the same identity appears in both full and bare form across this codebase — storing under one and looking up by the other would silently miss. Import decodes the token so a malformed delegation fails here, where the error is actionable, rather than surfacing as an unexplained 403 in the middle of a git push. A delegation naming no repository is rejected with the resources it did carry, since that is almost always a wrong --cap argument. * feat(git-remote): send a delegated push capability as X-Ucan Wraps a stored delegation into an invocation (iss=agent, aud=node, prf=[delegation]) and attaches it to the receive-pack POST, which is what lets a CI or delegated key push once owner-only push is enforced. Only on the push: a fetch is gated by read visibility, not by git/push. Capabilities are copied from the delegation unchanged, so an invocation can never be broader than what was delegated. No expiry is set on the invocation itself — the delegation`s own exp still bounds the chain because verify_chain checks each proof`s expiry as it recurses. A test proves that rather than asserting it: an already-expired delegation still fails the chain after wrapping, so an expired grant cannot be laundered into an open-ended one. The whole path is best-effort. A missing delegation, an unreachable node, or an unreadable stored token all send the request without the header. The node decides whether one was required, and a node denial has to reach the user instead of being pre-empted by a local guess. delegation_path duplicates gl`s six-line version because gl is not a dependency of this crate; both carry a pointer to the other. * test(git-remote): cover pack-POST URL splitting split_pack_post_url decides which repo a delegation is looked up for, so a parsing slip silently means no delegation was found, surfacing as a confusing 403 rather than a visible error. Covers the optional .git suffix, a repo genuinely named x.git, and the malformed shapes that must not parse. * build: sync Cargo.lock for the git-remote-gitlawb dependencies Adding serde_json and chrono to crates/git-remote-gitlawb/Cargo.toml updated Cargo.lock locally, but the lockfile lives at the repo root and was never staged. Every CI job builds with --locked, so all seven compiling jobs failed immediately with "cannot update the lock file ... because --locked was passed". * fix(core,gl,git-remote): close two review findings on delegated push Both are mine, both were found in review, and both are load-bearing. 1. Stripping `nb` mid-chain escalated a constrained delegation. `Capability::is_attenuated_by` compared only `with` and `can`, so the holder of a capability constrained by `nb` could re-delegate the same resource and action with the constraints removed and `verify_chain` still accepted the chain. The node's push gate refuses constrained capabilities at the LEAF, so it then saw an unconstrained one and granted repo-wide push — guarding the leaf guarded the wrong end of the chain. Constraints now participate in attenuation: an unconstrained parent permits anything, an identical child is attenuated, and both a differing child and a child that drops the constraints are refused. `nb` has no interpreted semantics yet, so "different" cannot be shown to be narrower and is refused with the stripping case. Tests cover the forged strip, the legal unchanged case, and adding constraints under an unconstrained parent. 2. `gl ucan import` wrote to a path taken from an untrusted token. `repo_from_resource` split the resource with `rsplit_once('/')`, so the owner half could carry separators, `..`, or an absolute prefix. `Path::join` with an absolute component discards the base entirely, so a resource of `gitlawb://repos/C:/Windows/System32/x` did not merely climb out of the delegations directory — it replaced it. The value then reached `std::fs::write`. The resource must now be exactly `gitlawb://repos/<owner>/<repo>`, with each half an allow-listed component: alphanumerics plus `.`, `-`, `_` and `:` (a DID carries colons), never `.` or `..` or anything containing `..`. An allow-list rather than a deny-list of separators, which would miss whichever ones the next platform introduces. The same shape existed in the helper's `split_pack_post_url`. There the value comes from the remote URL and the path is only read, but a traversing owner would read an arbitrary file and send its contents to the node as `X-Ucan`, so it carries the same guard. Also: the node-DID probe now uses a 5s timeout instead of inheriting the shared client's 300s. It is best-effort metadata, and a stalled node should not delay every delegated push by five minutes before falling back to sending no header. * fix(core,node,gl,git-remote): close the second review round on delegated push Each finding was reproduced against the code before being changed; two were confirmed with the mutation the reviewer described. A delegation could be perpetual -------------------------------- `exp` is optional, `is_expired` reports false when it is absent, `gl ucan delegate` defaulted to no expiry, and there is no revocation path. So the default flow minted a permanent push grant: once the token leaked, the owner's only remedy was rotating the DID the repo is keyed on. The PR body claimed the damage window was the token's `exp`, which was simply untrue when `exp` was `None`. `Ucan::chain_lifetime_is_bounded` walks every link, and `ucan_grants_push` requires it. It is deliberately NOT enforced inside `verify_chain`: an unbounded token is well-formed and may suit a read-only capability; whether an unbounded grant is acceptable is the consumer's policy, not the format's. `gl ucan delegate` now defaults to 720 hours with an explicit `--no-expiry` opt-out, and the helper carries the delegation's expiry onto the invocation so the leaf is bounded too. The recursion to the root was untested --------------------------------------- Every chain in the suite was depth two, where the immediate proof IS the root, so nothing distinguished walking to the true root from returning the proof's issuer. Confirmed by mutation: keeping full recursive validation but returning `proof.payload.iss` left gitlawb-core at 92 passed, the node's UCAN tests at 17, and the e2e green. A three-link owner -> lead -> agent test now pins it, with `assert_ne!` against the middle issuer as well as `assert_eq!` against the root — without the former, returning the middle would still pass. A path-prefixed node base broke delegated push entirely -------------------------------------------------------- `GITLAWB_NODE` may carry a path (`https://host/gitlawb` behind a proxy), and `repo_base` passes it through verbatim. `split_pack_post_url` read the FIRST two segments as owner/repo, so the prefix became the owner: the delegation lookup missed, the DID probe hit the wrong URL, no `X-Ucan` was sent, and a valid delegate got a 403 — silently, because every failure on that path is best-effort. It now strips the known trailing `<owner>/<repo>/<service>` instead, which is correct at any prefix depth, and prefix segments carry the same allow-list so a `..` cannot redirect the probe. A wildcard delegation grew without limit ----------------------------------------- `with: "*"` covered every repo the owner created AFTER signing, a scope nobody chose. `build_invocation` now narrows to the concrete repo, which `is_attenuated_by` accepts under a `*` parent, so a captured invocation is worth one repo rather than all of them. Constraints are copied from the covering capability rather than dropped, since dropping them is a widening. Branch protection and the owner gate now disagree, on purpose -------------------------------------------------------------- A delegate clears the owner gate and is still refused on a protected branch. That is the intended policy: a protected branch is the owner's explicit marker that even routine writes should stop, and if a delegation overrode it, issuing any capability would weaken every protection already set. The comment claiming non-owners never reach that loop is corrected, and `delegated_push_is_still_refused_on_a_protected_branch` pins it — asserting the body names the branch, so the refusal is provably branch protection rather than the owner gate. Smaller items ------------- The denial body no longer says "only the repo owner may push", which stopped being true once a delegation could authorize one. It stays a single unconditional message: varying it by whether a delegation was presented, expired, or named another repo would turn the refusal into an oracle for which capabilities exist. `gl ucan import` writes the delegation 0600, matching the sibling identity key. The token alone cannot push — the node requires `iss` to equal the request signer — but it discloses the delegation graph. `docs/RUN-A-NODE.md` told operators not to enable owner-push until every pusher was the owner, which is the workflow this change adds. It now documents the delegation flow, the four requirements the node enforces and why, that a delegation does not override branch protection, and that withdrawal is by expiry only. `README.md` no longer describes UCAN as a future workflow. * fix(gl): restrict the delegations directory, and state the contract The imported delegation was written 0600 on Unix and left to inherit the directory ACL elsewhere. Tightening only that one file would have been theatre: identity.pem sits in the same directory under exactly the same assumption, and its disclosure is strictly worse. The directory itself is now 0700 on Unix, and the comment states the contract that already governed the private key — gitlawb_dir accepts any path, std::fs has no portable ACL API, so on other targets the caller must supply a user-private directory, which a per-user profile provides by default. * fix(gl,git-remote): make a delegation issued the documented way actually work Two silent failures, both of which end as a 403 telling the delegate to obtain the delegation they are already holding. The narrowing fix broke the documented issuing form ---------------------------------------------------- `build_invocation` built `gitlawb://repos/{owner}/{repo}` from the push URL and compared it to the delegation's `with` with `==`. The URL always carries the BARE owner, since `parse_gitlawb_url` takes the last colon-delimited segment, while `docs/RUN-A-NODE.md` tells the owner to issue `--cap gitlawb://repos/<owner-did>/<repo>` — the full DID. Those strings never match, `find` returns None, and because every failure in `delegation_header` is best-effort the push goes out with no `X-Ucan`. This was introduced by the previous round: copying `att` through unchanged worked because the node normalizes both forms in `did_matches`. Narrowing to a URL-derived string did not. The owner segment is now compared on the bare key, and the parent's `with` is kept VERBATIM whenever it already names this repo — `is_attenuated_by` compares `with` by exact equality, so re-emitting the bare form under a full-DID parent would fail attenuation at the node and trade one silent refusal for another. Only a `*` parent uses the URL-derived resource, which is the case narrowing exists for. Both combinations are now tested; neither side exercised them before. The two halves disagreed about where delegations live ------------------------------------------------------ `git-remote-gitlawb` resolves its store from `resolve_key_path().parent()`, which honors `GITLAWB_KEY`. `gl ucan import` wrote under `gitlawb_dir(None)`, which was always `~/.gitlawb`. With `GITLAWB_KEY=/data/keys/identity.pem` — the shape `.env.example` documents — the import stored the token in one directory and the helper read another, empty one. `gitlawb_dir` now falls back to the parent of `GITLAWB_KEY` before `~/.gitlawb`, so both halves derive the store from the same setting. Also: `.env.example` still said a push from a non-owner DID is rejected, which is the behaviour this branch removes. * fix(gl): refuse a relative or non-UTF-8 GITLAWB_KEY when deriving the store Two defects in the previous round's fix, both of which put the delegation somewhere the helper does not look and end as an unexplained 403. A relative GITLAWB_KEY resolved differently in each half. `gl ucan import` and `git-remote-gitlawb` do not share a working directory, so `keys/identity.pem` sends the import to one `delegations` directory and the lookup to another. A one-component value is worse: `parent()` yields "", so the store becomes `./delegations` relative to whatever directory happened to be current. It is now refused with a message that says why, rather than silently resolved. `std::env::var` returns Err for a non-UTF-8 value, which the code treated as unset — indistinguishable from having no GITLAWB_KEY at all, and silently selecting ~/.gitlawb instead of the operator's real key directory. `var_os` keeps the OsString, so a valid non-UTF-8 path now works and an empty one is still treated as unset. * fix(identity): one resolver for GITLAWB_KEY across gl and the helper `gl` and `git-remote-gitlawb` each carried their own answer to "where is my identity?", and so did five call sites inside `gl` itself. The round-three fix hardened one of them — `gitlawb_dir` — and left the rest, which is why the same misconfiguration kept surfacing in a new place each round. The rules now live in `gitlawb-core::identity_path`, the crate both binaries already depend on: identity_key_path() $GITLAWB_KEY, else ~/.gitlawb/identity.pem identity_dir() its parent — the delegation store read through `var_os` (so a non-UTF-8 path is refused rather than folded into "unset" by `var`), with `~/` expanded on the first path component, a relative value refused, and a bare `~`, `~/`, or `/` refused rather than resolved to a directory whose parent is not where anything lives. Both take the home directory as an argument rather than looking it up: core is held to an explicit dependency allowlist and both callers already carry `dirs`, so the rules can be shared without widening core's tree. It also lets every case be tested against a fixed home instead of the machine's. Call sites moved onto it: gl identity::gitlawb_dir delegates to identity_dir gl identity::load_keypair_from_dir was ~/.gitlawb even after `gl identity new` wrote elsewhere, so `gl ucan delegate` signed with a stale DID — or found nothing — for exactly the operators who moved their key. Every `load_keypair_from_dir(None)` caller (register, repo, pr, clone, mcp) is fixed with it. gl doctor::run the one command whose job is to explain a broken setup was reporting on a directory the setup does not use gl init (ucan.json, generate_identity) gl mcp ucan_show gl ucan_cmd::cmd_show helper resolve_key_path / the delegation store behind delegation_header The helper's version was the loosest of the set: `env::var`, a literal `"~/"` prefix, `HOME` falling back to `"."` (never set on Windows), and no absolute check at all. Also in this commit: - `gl ucan import` creates the store and the token at their final mode. `create_dir_all` then chmod leaves 0755 under the usual umask, and `fs::write` then chmod leaves 0644, both readable by any local user until the second call lands. `DirBuilder::mode` and `OpenOptions::mode` close the window; the trailing `set_permissions` now only matters for a store an older `gl` left behind. - `gl ucan import` refuses a delegation the push path cannot use. It filtered on resource shape only, so a `pr/open` token printed "Stored delegation for owner/repo" and was then dropped by `build_invocation` behind a `tracing::warn`, surfacing as a 403 with nothing connecting it to the earlier success. Import now applies the same push-class filter the helper does. - README's write-authorization limitation is narrowed to what is actually missing (revocation, `nb` interpretation, non-push capabilities) rather than claiming delegated push is not implemented. Every new guard was verified by disabling it and watching the matching test go red: the absolute-path check, the bare-`~` refusal, the `load_keypair_from_dir` routing, and the import action filter each own a failing test. * test(git-remote): cover delegation_header end to end `delegation_header` was the one piece of the delegated push path with no test. Its parts each had one — `split_pack_post_url`, `build_invocation`, `delegation_path` — but nothing checked they compose, and this is the function where a regression is silent by construction: every failure inside it returns `None` and the push goes out without `X-Ucan`, so a break surfaces as a 403 at the node with nothing locally to connect it to. Five cases, driven against a mockito node with the store seeded where `gl ucan import` would have left it: - the delegated push: a stored token becomes an invocation issued by the agent, addressed to the node's DID, rooted at the repo owner, with every link bounded - the owner's own push: no store read, no node round-trip. The comparison has to survive the form mismatch — the keypair holds `did:key:z…` while the URL carries the bare key — so the mock asserts zero hits - no usable delegation: an empty store, a token that does not decode, and a `git/fetch` capability that carries nothing to wrap. All three yield no header rather than an error - an unreadable node DID: a 500, a JSON body with no `did`, a proxy's HTML error page, and a `did` that does not parse. The push loses its header, never aborts - a path-prefixed node base: the DID probe must go to `/gitlawb`, not `/`. `split_pack_post_url` is unit-tested for the prefix, but nothing checked the probe followed it; the mock on `/` asserts zero hits Each case was watched failing before it was kept, against five separate mutations: the owner short-circuit removed, the prefix dropped from the node base, `build_invocation`'s push-class filter widened, the node-DID lookup given a fallback, and the invocation addressed to the agent instead of the node. * fix(gl): finish the identity-path sweep on the write side The previous commit unified where `gl` READS the identity from and stopped there. Four resolvers were left holding their own `~/.gitlawb`, and one of them is a write: register.rs ucan_path the bootstrap UCAN from POST /api/register quickstart.rs the wizard dir identity generation AND the UCAN it stores name.rs identity_dir reads identity.pem node_stake.rs load_did reads identity.pem `register.rs` is the one that actually breaks a working setup. With `GITLAWB_KEY=/data/keys/identity.pem` — the shape `.env.example` documents — `gl register` loaded the key from `/data/keys/identity.pem` and then wrote `ucan.json` to `~/.gitlawb/`. Registration reported success; `gl doctor`, `gl ucan show`, `gl init`, and `gl mcp ucan_show` all read `ucan.json` from the key's directory, found nothing, and reported an unregistered identity. Split storage, no error anywhere. `quickstart` had the same shape end to end: it generates the identity and stores the bootstrap token, so an operator who ran the wizard under `GITLAWB_KEY` got both in a directory nothing else consults. `name.rs` and `node_stake.rs` only read, but both fell back to `PathBuf::from(".")` when the home directory could not be determined, which makes the identity path depend on the working directory — the same defect the helper's `resolve_key_path` carried before it moved to the shared resolver. All four now go through `crate::identity::gitlawb_dir`. Afterwards the only `dirs::home_dir()` calls left in `gl` are the one inside `gitlawb_dir` itself, one test expectation, and `doctor`'s shell-rc scan, which is unrelated to the identity directory. `gl register` also no longer prints a path it did not use: the success line was hardcoded to `~/.gitlawb/ucan.json`, so an operator with `GITLAWB_KEY` set was told to look somewhere the file was not. It now prints the real destination, and says so plainly when the node returned no token at all. The `GITLAWB_KEY` test guard moves to `identity::test_env` so the resolver's own cases and the new register case share one lock. Two suites each holding their own mutex over the same process-global variable would not serialise against each other. Verified by reverting `ucan_path` to the hardcoded form and watching `register_saves_the_bootstrap_ucan_beside_the_key` fail on exactly that assertion. * fix(gl,core): close round five — lazy home, envelope reads, honest register Five findings from the round-five review, four of them introduced by round four's own centralisation. resolve_key_path and resolve_identity_dir opened with `home_dir()?`, but identity_key_path only needs a home when the key is unset, empty, or `~/`-prefixed. On a host where `dirs::home_dir()` returns None — no HOME and no passwd entry, an ordinary container shape — a perfectly valid absolute GITLAWB_KEY was discarded and the push went out unsigned, blaming the home directory for a setting the operator had got right. The pre-round code resolved an absolute key without consulting home at all, so this was a regression, not an inherited gap. The core resolvers now take `Option<&Path>` and demand a home only where the value being resolved needs one. `gl ucan show` called Ucan::decode on the whole of ucan.json, but register, init, and quickstart all write an envelope — {"ucan", "node", "did", "saved_at"} — and doctor and quickstart already read it as one. Ucan is {payload, s}, so show failed with "missing field `payload`" immediately after a successful `gl register`. It now reads the envelope like every other reader, and still accepts a bare token so files written by an older gl stay readable. The predating mismatch belongs to this round because this round rerouted cmd_show and claimed to unify the identity flow. `gl register` printed "You are now a verified agent" even when the node returned no bootstrap UCAN — the exact shape the previous commit message said it existed to remove, one line below the fix. The closing line is now inside the branch that actually stored a token. The MCP `ucan_show` tool called gitlawb_dir(None) while every sibling tool in call_tool honours the server's --dir, so one session read two identity directories. The parameter was already in scope. Six `--dir` help texts still promised "default: ~/.gitlawb" after the resolver stopped having that default. Both behavioural fixes were verified by mutation: demanding a home unconditionally turns an_absolute_key_resolves_without_a_home_directory red, and dropping the envelope branch turns the_register_envelope_decodes red. * fix(node,gl,git-remote): enforce repo narrowing at the node, honour the key basename Round seven. Two P1s, and the first changes an authorization outcome. The node accepted a leaf resource of `*` for delegated push. Narrowing a wildcard delegation to the concrete repository happened only in `git-remote-gitlawb`, which is one client's behaviour and not a protocol boundary: a delegate can sign an otherwise valid `agent -> node` invocation that keeps `with: "*"`, it passes attenuation because its proof is `"*"` too, and the node then authorised a push to any repository owned by the root DID — including repositories created after the delegation was issued. `docs/RUN-A-NODE.md` promised exactly the opposite, and named the growth it was preventing, while resting that promise on the helper. `repo_capability_matches` now requires a concrete `gitlawb://repos/<owner>/<repo>`. Wildcard delegations still work through the normal path, because the helper narrows before signing; only a hand-built wildcard leaf is refused. `honours_the_resource_wildcard_and_repo_admin` asserted the old behaviour, so it is now `refuses_a_resource_wildcard_and_honours_repo_admin`, and it separates the two axes: a `*` ACTION on a concrete repository is still fine, since attenuation bounds it. `a_wildcard_delegation_cannot_reach_a_second_repository` covers the attack directly. `gl` discarded the basename of `GITLAWB_KEY`. The shared resolver returns the configured PEM path, but `gl` kept only its parent and re-appended `identity.pem`, while the helper opens the configured path. With `GITLAWB_KEY=/data/keys/ci-agent.pem` the CLI wrote and read `/data/keys/identity.pem` while every push loaded `/data/keys/ci-agent.pem`, so the two either disagreed on identity or the helper found no key — and both owner enforcement and the delegation proof key off that identity. This was mine, from last round's centralisation: a directory-level contract substituted where callers needed a key-file-level one. `key_path_for` now serves both, and the seven production sites that still hardcoded the basename go through it: `identity` (load/new/backup/restore), `doctor`, `init`, `name` (x2), `node_stake`, `quickstart` (x2). An explicit `--dir` keeps `<dir>/identity.pem`. Also: - `gl ucan import` binds the delegation's audience to the local identity and verifies its chain, expiry, and bounded lifetime before storing. It previously accepted a token addressed to a different agent, reported success, and left the push to fail on proof linkage with nothing pointing back. - `build_invocation` selects only a capability the node can authorise. It took the first push-class capability, including a constrained one, while the node refuses constrained leaves outright — so a delegation holding both a constrained and an unconstrained grant succeeded or failed purely on their order in `att`. - Delegation files are replaced atomically: written to a sibling temp at 0600, fsynced, then renamed. `truncate(true)` on the live path emptied a working delegation before the replacement was written, so an interruption left the helper with an unreadable token and pushes that silently drop `X-Ucan`. - `gl quickstart` no longer reports "UCAN saved to ..." when the node returned no token. Both reviewers flagged this independently; it is the same false success fixed in `register` last round, one file over. Every fix was verified by mutation: restoring the node's wildcard acceptance, restoring `<parent>/identity.pem`, disabling the audience check, and dropping the helper's constraint filter each turn the matching test red. * fix(gl): align the MCP ucan_show tool with gl ucan show The two surfaces answer the same question and answered it in different shapes: `gl ucan show` decodes the envelope and prints fields, while the MCP tool returned the file verbatim. That was the round-seven non-blocking note. Aligning them removes a second problem the note did not name. The file contains the bootstrap token, so the MCP tool handed the caller the credential itself — and an MCP response travels further than a terminal does, into a model's context, transcripts, and logs. Nothing needs the token to answer "am I registered, with what, until when", so the tool now returns issuer, audience, version, capabilities, expiry, expired, signature validity, and the path it read. The tool description said "Show the saved bootstrap UCAN token"; it now says what is returned and that the token is not. Verified by mutation: restoring the raw-file return turns `ucan_show_returns_decoded_fields_and_withholds_the_token` red on the assertion that the token is absent. * fix(gl): drop a needless borrow in quickstart's unix key write `generate_identity` now takes `path: &Path` rather than deriving it from an owned directory, so `set_permissions(&path, ..)` became a double borrow. Clippy refuses it under `-D warnings`. It sits inside `#[cfg(unix)]`, which is why the local gate passed: this machine is Windows, so that block is never compiled or linted here. Only CI sees it. The sibling `fs::write` on the line above was already corrected; this one was missed because the same pass could not fail locally. The other `&path` / `&dest` / `&tmp` borrows in the unix blocks this PR touches are on owned `PathBuf`s, where the borrow is required — `quickstart` is the sole place a parameter changed from owned to reference, and so the sole place the lint applies. * fix(node,gl,git-remote): fix a delegation's scope at issuance Round eight. The round-seven wildcard fix closed the wrong half, and I confirmed that by execution before changing anything: PROBE ucan_grants_push = true An owner issues `with: "*"`, the delegate narrows it to a concrete leaf for a repository created afterwards, `verify_chain` passes, and the node authorised the push. Refusing a wildcard LEAF did nothing, because `is_attenuated_by` accepts a concrete child under a `*` parent — and that narrowing is exactly what `build_invocation` performed, so the first-party helper was the working mint path. The round-seven tests never saw it: they injected a self-issued `*` leaf with no `prf`, so they never touched a proof chain at all. The contract is now that a delegation's scope is fixed when it is issued, which is what both operator documents already promised: - The node requires EVERY link to name the repository, not just the leaf (`proofs_name_repo` walks `prf` after `verify_chain` has established the chain). - `git-remote-gitlawb` no longer selects or narrows a `*` capability, so a stored wildcard fails closed locally with a clear message instead of minting an invocation the node would refuse. - `gl ucan delegate` refuses to issue a push-class wildcard at all. Also, all from round eight: - `gl quickstart` loads the exact key it selected. It resolved `pem_path` through `key_path_for` but then loaded through `Some(&dir)`, which re-derived the basename as `identity.pem` — so with `GITLAWB_KEY=/data/keys/ci-agent.pem` the existence check and the load disagreed, and the Err arm regenerated ONTO `ci-agent.pem`, destroying a working key and changing the DID that repository ownership, registrations, and delegations all key off. - `gl ucan import` binds every capability's owner segment to the VERIFIED ROOT. `verify_chain` proves a chain is internally valid; it says nothing about which repository it applies to. Without this, any key holder could issue a valid bounded token naming `gitlawb://repos/<victim>/repo` and displace the working delegation for a repository they have no authority over. The whole set is validated before anything is written, so a later bad capability cannot leave a multi-repository import half applied. - Import applies the same push-class rule the helper and node use, including `constraints.is_none()`. A constrained-only grant used to import cleanly and then authorise nothing. - Delegation replacement stages to a per-call path (`create_new`, pid + counter) rather than a shared `.<name>.tmp`, so two concurrent refreshes cannot write the same inode and publish each other's bytes. The non-Unix path stages and renames too; it previously truncated the live file, so a failed refresh destroyed a working token on that platform. Three tests that were weaker than they read, all mine: - `a_refused_reimport_leaves_the_stored_delegation_intact` never reached `write_private_file` — it stopped at the audience check. Renamed to say what it covers, and `a_failed_write_leaves_the_stored_delegation_intact` now forces a real write failure against a read-only store. - The MCP token-withholding assertion used `!out.contains(&token)`, which passes whether or not the token leaked: the token is itself JSON, so embedding it escapes every quote. It now asserts on the signature, a bare base64 string that survives escaping. - `build_invocation_narrows_a_wildcard_to_the_pushed_repo` asserted the behaviour this commit removes; it is now `build_invocation_refuses_a_wildcard_delegation`. * fix(gl): drop an unused import in the unix-only refresh tests `refresh_atomicity_tests` moved from `token_for_agent` to `owned_token` when import began requiring the resource owner to equal the verified root, and the import was left behind. The module is `#[cfg(all(test, unix))]`, so it does not compile on this Windows host and no local clippy run could see it. Cross-linting is not available either: `cargo clippy --target x86_64-unknown-linux-gnu` fails in ring's build script for want of a C cross-compiler. CI is the only linter for cfg(unix) code from here, which is worth recording rather than rediscovering. The sibling `import_binding_tests` still uses both helpers; only the unix module's import changed. * fix(node): pass the UCAN extension at the last two receive-pack call sites Two `git_receive_pack` calls in `#[cfg(unix)]` test blocks still supplied six arguments. Like the rest of that class they do not compile on this Windows host, so neither the local build nor clippy could see them; CI found them one at a time. Rather than wait for a third round, every call site was audited by grep instead of by compiler: 25 calls, 2 missing, both fixed, none others. The other signatures this branch changed — `caller_authorized_to_push`, `identity_key_path`, `identity_dir`, `key_path_for` — were audited the same way and are consistent. Both sites push as a synthetic non-owner DID under an owner-push-disabled config, so `None` is correct: no delegation is involved in what they test (write-lock release on disconnect, and on push success). * fix(core,node,gl,git-remote): one definition of a usable push capability Round nine. Both reviewers named the same pattern independently: the rule for "can this token push to this repository?" was reconstructed at every boundary — issuance, import, invocation construction, node authorization — and each round found a token one boundary accepted and the next refused. This round's own instance: the node walked the whole proof chain, import checked only the leaf, so a concrete leaf on a wildcard proof imported cleanly and 403'd on every push. The rule now lives once, in `gitlawb_core::ucan::push`, the crate all four already depend on: did_key_eq the node's exact did:key collapse rule — bare and full forms are one identity; a bare id never matches across methods (`did:gitlawb:X` is not `X`) is_push_action git/push, "*", repo/admin parse_repo_resource gitlawb://repos/<owner>/<repo>, exactly two segments is_push_wildcard a push-class action on a "*" resource Capability::grants_push_to(owner, repo) push-class, unconstrained, names exactly this repo Ucan::chain_grants_push_to(owner, repo) the leaf AND every proof behind it, depth-bounded, failing closed on undecodable or multi proofs and every boundary calls it: node ucan_grants_push = root anchored && bounded && chain_grants_push_to. `repo_capability_matches` is a thin wrapper; `proofs_name_repo` and `push_class_names_repo` are gone. `did_matches` delegates to did_key_eq so there is one collapse rule, not two. import the same walk, before any write. A leaf naming the repo on a "*" proof is refused with a message that names the proof. Everything is validated before anything is written. helper `build_invocation` refuses a stored token whose chain does not grant push to this repo, with a message that says why, before selecting a leaf — instead of minting an invocation the node is certain to refuse. issuance `gl ucan delegate` and the MCP `ucan_delegate` tool share is_push_wildcard. The MCP arm had no guard at all. Two tests that failed CI, both invisible on this Windows host because they are cfg(unix): - `import_creates_the_store_and_token_owner_only` still issued against a synthetic `z6MkAbc` owner, so import's owner-root binding refused it before the mode assertions. Now built from `owned_token`, like its siblings. - `a_failed_write_leaves_the_stored_delegation_intact` chmod'd the store to 0500 and expected the staging create to fail. It did not: `create_private_dir` repairs the store to 0700 before every write, so the second import succeeded and the test failed on `is_err()` — and a privileged runner would have made the arrangement pass anyway. Failure is now injected through a test-only seam (`fault::FailStagingWrites`) that fires after the bytes are written and before anything is published, on every platform, and the test also asserts no staging file is left behind. The module is no longer unix-only. Also: the node's rustdocs for `repo_capability_matches` and `ucan_grants_push` described the pre-round-8 behaviour (helper narrowing; leaf-only coverage) and now match the code. An orphaned docblock above `staging_path` — the old `write_private_file` doc, left behind in round 8 and claiming "not create_new" when the writer uses exactly that — is removed and the writer has a correct one. Verified by mutation, each restored byte-for-byte afterwards: core walks no proofs → core 3-link, node 2-link, node 3-link, import, and helper wildcard tests all red core walks ONE proof only → ONLY the two 3-link tests red; every 2-link test stays green — the recursion is load-bearing MCP guard removed → MCP deny test red, MCP named-resource test green staging seam disconnected → atomicity test red import chain check removed → import deny test red, delegate deny test green fmt, clippy -D warnings, check --locked --workspace --all-targets clean. gitlawb-core 122, gl 389, git-remote-gitlawb 65, node auth:: 23, and the DB-backed delegated-push tests pass alongside main's owner-push test. * fix(node): drop repo_capability_matches, orphaned by the shared push rule Round nine routed the node's authorization through Ucan::chain_grants_push_to, which parses the resource and compares owners with did_key_eq itself. The thin wrapper left behind had no callers, and CI's -D warnings build rejects dead code. Nothing else in the crate referenced it. * fix(core,gl): bound every proof-chain walk and make verify report the chain Round ten of #331. `verify_chain` and `chain_lifetime_is_bounded` recursed with no depth guard while `chain_grants_push_to` carried one. The node verifies the chain on the `X-Ucan` header of any signed request before the scope walk runs, so the bound protected nothing: a deep chain met the unbounded walk first. Both now stop at `MAX_CHAIN_DEPTH`, which moves to the module root since all three walks share it, and a chain at the bound still validates end to end. The MCP `ucan_delegate` tool issued an unbounded token when `expiry_hours` was omitted, which import and the node both refuse for push. It now defaults to the CLI's 720 hours and refuses a non-positive value. `gl ucan verify` and the MCP `ucan_verify` tool answered from the leaf alone, so a good signature on a broken proof chain reported valid and then failed at import and at the node. Both report through one `VerifyReport`, which walks the chain; `valid` requires it. The non-Unix `write_private_file` removed the live delegation before `rename`, on the belief that Windows refuses to replace an existing destination. std maps `rename` to `MoveFileExW` with `MOVEFILE_REPLACE_EXISTING`, so the remove only opened a window with no delegation at all. Dropped; a refresh over an existing token is now tested on every platform. Also: `cmd_import` and `cmd_verify` share one token-argument reader that reports an unreadable path as a file problem and caps the file size, instead of falling through to a decode error on the path string; and the MCP `ucan_delegate` description says import is CLI-only. * docs: state the two gates a delegated push crosses, and finish the sweep Round eleven of #331, documentation half. A `git push` crosses two independent gates: read visibility on the `info/refs` advertisement, and push authorization on the receive-pack POST. This PR implements the second and left the first as it was, and a UCAN grants no read access — so on a private repository a delegate must also be a reader. Nothing said so. RUN-A-NODE now states the contract once, in the owner-push section, and the delegation section carries a "Private repositories" note with the `gl visibility set` invocation and a "One repository per token" note matching what import enforces. The rest is the drift the earlier doc edits left behind: the README env-var table still said a `git/push` capability was "verified but not yet honored", the known-limitations bullet still said enforcement defaults to `false`, and the RUN-A-NODE section opened with a blanket 403 for every non-owner two paragraphs above the text that says a delegate can push. All three now describe the same node. * fix(gl): store one repository per imported delegation; pin the two-gate composition Round eleven of #331, code half. `gl ucan import` validated every capability up front and then wrote one file per repository, and `write_private_file` is atomic per file, not across files: a token naming two repositories could publish the first delegation, fail on the second, and report failure for both. `gl ucan delegate` and the MCP tool issue one capability per token, so such a token is hand-built; import now refuses it with the supported shape in the message. Several capabilities on one repository are still one file. `delegated_push_on_a_private_repo_needs_read_visibility_too` pins the composition the docs now describe, through the middleware production gives each route: on a private repository a valid delegation is refused at the `info/refs` advertisement (404) until the owner names the agent as a reader, clears the receive-pack POST on its own, and a reader with no delegation is still refused on the push. Making the visibility check admit everyone reddens the advertisement assertion. * fix(gl,core): sanitize token-derived output, prove the publish step, add MCP import Round eleven of #331, second half. Every string a token carries is chosen by whoever built it: `Did` deserializes any string (only `FromStr` validates), and a capability field is free text. `gl ucan verify`, `gl ucan show`, and the import refusal messages printed them raw, so an ANSI or OSC sequence in `with`, `can`, `iss` or `aud` — or in the error text `verify_chain` quotes them into — reached the operator's terminal. The strip is now one definition in core, `sanitize::strip_terminal_controls` (both halves of INV-6), and `gl`'s existing `sanitize_node_msg` is that plus its length cap. In `ucan_cmd`, `shown()` wraps every token-derived value on its way out. The refresh-replacement test could not tell the current non-Unix writer from the one that removed the live file before renaming, because a rename that succeeds ends in the same state either way. A second seam, `fault::FailPublish`, fails the rename after the staged bytes are complete; the new test requires the old token to survive that, and reinstating the remove reddens it on the platform that had it. The MCP server could issue and verify a delegation but not store it where `git-remote-gitlawb` looks. `cmd_import` is now a printer over `import_delegation`, and the `ucan_import` tool is the same function with a JSON answer: one set of checks, one store. * test(gl): count the MCP tool list at 41 with ucan_import `e3314c6` added the tool and left the count test at 40, so `test (windows)` went red first. The test also carried "42" in its name while asserting 40; it is now named for what it checks, and the names test covers `ucan_import` alongside the other two UCAN tools. * fix(node,gl): reader gate on receive-pack POST, GITLAWB_KEY in optional load, sanitize + bound UCAN surfaces (#331) * test(gl): pin GITLAWB_KEY honouring in load_optional_keypair (#331) * fix(gl): drop needless borrow caught by clippy * fix(gl): doctor reports refused resolvers, sanitize MCP UCAN JSON, atomic key write (#567) * fix(ucan): match push capabilities against the canonical repo name A capability written gitlawb://repos/<owner>/<repo>.git passed import and filed under the suffixed name, but the helper looks the store up by the canonical name and the node compared the resource verbatim against record.name, so the delegation missed at every hop and the push died at the owner gate with a bare 403. Introduce ucan::push::canonical_repo_name - the same at-most-one-.git strip smart_http_repo_name applies - and run it everywhere a push capability is issued (gl ucan delegate, MCP ucan_delegate), imported (repo_from_resource now returns the canonical segment, so the file lands where the helper looks), loaded (split_pack_post_url), and matched (grants_push_to, hence chain_grants_push_to and the node gate). At-most-once preserves the literal foo.git repo, reachable as foo.git.git. Document the canonical form in RUN-A-NODE.md. * fix(ucan): refuse key clobber on unreadable identity, fix doc gate sites and message escapes - quickstart: bail on an existing-but-unreadable identity instead of regenerating over it, and write the fresh key with create_new so a file that appears mid-setup is not clobbered (same rule gl init uses) - RUN-A-NODE: gate 1 read visibility applies on both GET info/refs and the git-receive-pack POST (same 404 for a client that skips the advertisement); visibility set example carries an explicit --mode - ucan delegate: replace literal \n escapes in the wildcard refusal messages (CLI and MCP) with line continuations so the error prints as one sentence; MCP expiry bound uses the shared MAX_EXPIRY_HOURS - MCP ucan_show: run the token version field through shown() - helper tests: restore GITLAWB_KEY via a drop guard so a panicking case cannot leak a tempdir path, and run the advertisement signature test under the shared lock with a deterministic empty store * docs(node): gate 1 allow-list semantics, fix comment-strip order in fn_body - RUN-A-NODE: a `/` rule is an allow-list that replaces the public flag, so "public repo = open to everyone / reader step unnecessary" only holds when no `/` rule exists; gate-1 bullet, the both-gates summary, and the private-repositories section now tell that one story - authz_guard fn_body strips // line comments before /* */ blocks so a line comment containing /* cannot open a range that swallows a gate * fix(node,gl): fail closed on protection-lookup errors, surface push readiness in verify - git-receive-pack: a non-owner whose is_branch_protected lookup errors is now refused instead of treated as unprotected; the owner path is unchanged (skip the check outright) - VerifyReport gains bounded_lifetime and push_ready so `gl ucan verify` and MCP verify show what import and the node would do with an unbounded push chain instead of a bare valid:true - RUN-A-NODE.md: the owner-push bullet names the delegation exception, and the delegation walkthrough now states the GITLAWB_KEY absolute-path prerequisite (import --dir must land in the same store the helper reads) - pins: delegate push refused when the protection lookup errors; an unbounded push-shaped delegation verifies but is not push_ready * fix(gl): judge push readiness with the shared push predicates in ucan verify - push_class now uses the same is_push_action rule import and the node apply, so repo/admin and * actions classify as push-shaped instead of omitting the Push-ready line - push_ready is a stored verdict, not just bounded+valid: every concrete gitlawb://repos/... push capability on the leaf must pass chain_grants_push_to, so a wildcard proof behind a concrete leaf verifies cryptographically yet reports not push-ready, matching what import and git-receive-pack would do - pins: repo/admin and * actions are push-ready; a wildcard-proof chain verifies but is not push-ready * fix(git-remote): gate delegation_header on import's push predicates and the file cap - a stored delegation that is expired or has an unbounded link is now ignored instead of attached: the node would refuse it at middleware with a distinguishable invalid_ucan response, and omitting the header keeps the denial on the unified owner-push path - the store file is size-checked against the same 1 MiB cap MAX_TOKEN_FILE_BYTES applies at import; an oversize entry is ignored like an unreadable one rather than read into memory on every push - pins: an expired on-disk delegation produces no X-Ucan (and no node probe); an oversize store entry produces none either --------- Co-authored-by: Vasanthdev2004 <[email protected]> |
||
|
|
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]> |
||
|
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. |
||
|
|
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]> |
||
|
|
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
|
||
|
beardthelion
|
86c544a577 |
fix(core): reject small-order Ed25519 keys at DID resolution and before X25519 (#314)
* fix(core): reject small-order Ed25519 keys at DID resolution and before X25519 A did:key can encode a small-order (weak) Ed25519 point. VerifyingKey::from_bytes only decompresses, so such a key resolved cleanly, and x25519_public converted it to the all-zero Montgomery u. X25519 against u = 0 is the all-zero shared secret for every scalar, so the per-recipient wrap in seal_blob could be rebuilt with no secret at all. Since one content key is wrapped per recipient, a single weak recipient exposed the blob for every recipient. Reject in Did::to_verifying_key, which is the choke point every consumer that resolves a DID shares, so a weak key becomes unresolvable and the existing fail-closed recipient handling catches it without a new code path. Guard x25519_public as well so the primitive is safe for a caller that obtained the key some other way. The attacker test performs the real attack rather than asserting an absence: with either guard removed it recovers the plaintext and fails printing it. * fix(attest): mirror the small-order key rejection in its own did:key parser gitlawb-attest parses did:key itself in verifying_key_from_did_key and never routes through gitlawb-core's Did. It cannot: gitlawb-core is a dev-dependency here, so the choke-point rejection is structurally unreachable from this crate and has to be mirrored rather than inherited. Signature verification is already strict, so this is defense in depth against a future consumer of this parser that does not verify strictly, not the closing of an exploitable path. One consequence worth naming: verify_rejects_weak_key_signature now observes Error::Did rather than Error::Signature, because the weak signer is refused at resolution before verify_strict is reached. That test is retargeted accordingly. It no longer exercises verify_strict, which remains the second layer for malleability that does not involve a weak public key (a small-order R under an honest key); this fixture cannot construct that case. * test(node): pin that a weak recipient DID falls into the existing fail-closed arm Because a small-order did:key no longer resolves, it lands in the unresolved set and plan_seal returns the #47 SkipUnresolvable arm that already refuses to seal to a partial recipient set. Nothing is sealed for the blob and the existing bounded operator warning fires. No new branch, no new SealPlan variant, no new log site. The must-not-over-reject control asserts an all-legitimate set still seals. * fix(gl): stop telling a did:key that did:key is unsupported did_resolve reported every resolution failure as "only did:key is supported without a resolver". That is right for a method with no local resolver and misleading for a did:key that parsed fine and failed for its own reason, which now includes a small-order key. Split the message on whether the DID is a did:key and surface the underlying error when it is. Extracted as a pure helper so both branches are unit-testable without an MCP client harness. * fix(review): restore verify_strict coverage the parser guard displaced Refusing a small-order key at DID resolution means a weak signer can no longer reach attestation.rs's verify_strict through verify_signature, which left that line with no test at all: downgrading it to the non-strict verify kept the whole attest suite green. Split the signature check into verify_sig_with_key so a test can drive it directly with a weak key. Check order in verify_signature is unchanged; only the seam is new. Also from review: - auth/mod.rs sent a did:key that failed on its own key material away with "only did:key is supported in alpha", the same misleading hint the gl MCP tool was just fixed for. Made it conditional on is_did_key so the two parallel surfaces say the same thing. - The guard comments claimed a small-order point converts to the all-zero Montgomery u. That holds for order 1 and 2 only; order 4 and 8 are annihilated by the scalar clamping instead. is_weak covers the whole torsion set, and mixed-order points are correctly still accepted. - did.rs doc comment now states the new rejection. - encrypt.rs: extracted the repeated weak-key fixture into a test helper and switched to the file's existing Err(anyhow!(..)) rejection idiom. - gl tests derive the did:key discriminator from a real parsed Did instead of hardcoding the bool, so inverting it at the call site now fails. * fix(attest): cap the did:key method-id before decoding, mirroring core The parser here mirrors gitlawb-core's small-order rejection because it cannot call into it (gitlawb-core is a dev-dependency of this crate). It was mirroring only one of the two guards that sit three lines apart in the function being copied: core also caps the method-id at 64 bytes BEFORE the multibase decode, because base58 decoding is quadratic in its input. signer comes off an attacker-supplied attestation and verify_signature reaches this parser after computing the JCS bytes, so an oversized method-id bought a large decode for a short request. An ed25519 did:key method-id is a fixed 48 characters, so the bound is slack rather than a behavior change. The test asserts the length error specifically rather than any error: a multibase error there would mean the cap ran after the decode and bought nothing. * fix(node): surface stored envelopes sealed to a now-unresolvable recipient plan_seal short-circuited on a matching stored tag before resolving the recipient set, so a blob sealed to a small-order did:key before the choke-point guard landed kept its old CID and was silently treated as unchanged. Resolve the set on the match and surface a now-unresolvable reader as SkipUnresolvableStored so the operator sees the exposure. * fix(attest): anchor VerifiedAttestation.signer to the key that verified Registry::verify discarded the VerifyingKey returned by verify_signature and copied the artifact's signer string into the result. The parser is canonical so the strings agree for honest artifacts and a mismatched field already fails verification, but the consumer-facing value now provably comes from the key that verified rather than a raw artifact field. * fix(node): bound the echoed DID in the unresolvable-did error and pin the hint branches The keyid is unauthenticated request input and was reflected into the response message at full length. Bound the echo to 96 chars and extract the two-branch hint selection into a testable helper with both branches pinned. * test(gl): build the weak did:key fixture from its identity-point bytes The hardcoded WEAK_DID_KEY constant trips secret scanners, which cannot tell a public DID from an API key. Deriving the DID from the compressed identity point states why the fixture is weak and matches the house pattern used in the core, attest, and node crates. ed25519-dalek is added as a dev-dependency (it was already a workspace dependency). * fix(core): skip a low-order ephemeral on the envelope open side open_blob built the X25519 box from the header-supplied eph with no small-order check, so a crafted entry forced the all-zero shared secret and unwrapped for any reader. Skip entries whose eph decompresses to a small-order point; honest entries in the same envelope still open. * fix(core): test the exchange result, not the ephemeral's encoding The open-side guard decompressed the ephemeral to Edwards and asked is_small_order, treating a failure to decompress as safe. That reports safe for any low-order input that is not a valid Edwards encoding. Measured across the seven standard low-order encodings: six caught, u = p-1 missed. Its exchange with any reader is still the all-zero shared secret, so an entry built on it unwraps for everyone and the attacker controls the plaintext. Check the result instead: an ephemeral is rejected when the exchange with this reader's own scalar is all-zero. That is the property the guard exists to enforce, and it holds for every low-order input regardless of encoding. The new must-not test needed two attempts. The first passed with the guard, without it, and with the original pre-fix guard alike, because a blanket `use chacha20poly1305::aead::Aead` in the test resolved the wrap's encrypt through a different aead version than open_blob decrypts with, building a ciphertext that could never open for any reason. Every call in it now names its trait. * docs(core): anchor open-side guard comments to exchange-result invariant The seal-side comment still named is_low_order_montgomery and the low-order vector test doc claimed a decode-path filter that does not exist. Both now describe yields_all_zero_shared_secret. * test(core): use generated keys in poisoned-envelope fixtures Replace hard-coded test scalars that CodeQL flags as cryptographic material. The all-zero shared-secret attack does not depend on which scalar builds the attacker box. * test(core): use Default::default() for zero-point fixtures flagged by CodeQL CodeQL flags [0u8; 32] literals that flow to a cryptographic key argument, even in test fixtures where the value is the all-zero Montgomery point (a public low-order test vector, not a secret). Default::default() produces the same bytes without a literal, clearing the alert without changing behavior. * test(core): generate body nonces with OsRng in poisoned-envelope fixtures CodeQL flags hard-coded byte arrays used as nonces, even in test fixtures where the value is a fixed test vector. Generate with OsRng instead, matching the production seal path. * chore: sync Cargo.lock after merging main (ed25519-dalek edge for gl) |
||
|
beardthelionandKevin Codex
|
c301a68161 |
fix(node): persist the libp2p identity instead of deriving it from the node DID (#324)
* feat(node): add a persistent libp2p identity key file
Add p2p_key_path (--p2p-key-path / GITLAWB_P2P_KEY, default
~/.gitlawb/p2p.key) with a resolver mirroring resolved_key_path, and
load_or_create_p2p_keypair, which generates an Ed25519 keypair on first
start, persists it 0600, and loads it thereafter. Mirrors the existing
load_or_create_keypair idiom for the node identity PEM.
A corrupt or unreadable key file is a hard error naming the path rather
than a silent regeneration, so a disk problem cannot quietly rotate the
node's network identity.
Not yet wired into p2p::start; that follows.
* fix(node): load the libp2p identity from the persistent key file
p2p::start now takes the Ed25519 keypair loaded by
load_or_create_p2p_keypair instead of computing one from the node DID,
so a node's network identity is generated once from the OS RNG and kept
on disk rather than recomputed from a public value on every start.
The node DID parameter is gone from start; the call site loads the key
first and continues without p2p if the key file cannot be read, matching
how a swarm-start failure is already handled.
The gossipsub message_id_fn is untouched and keeps its own hasher.
* fix(node): create the p2p key with 0600 at creation and reject a loose one
Open the key file with create_new and the mode set at creation, then
fsync, instead of writing it and narrowing the mode afterwards. The
secret is never on disk under a wider mode, an interrupted start cannot
leave it readable, and the exclusive open also refuses a pre-existing
entry at the path and makes a concurrent start take the key that landed
rather than clobber it.
Refuse to load a key file whose mode grants group or other access, and
name the observed mode so the operator can fix it. Report an empty key
file as empty rather than surfacing a protobuf decode error that blames
a missing rsa feature.
Pin GITLAWB_P2P_KEY onto the mounted volume in the Docker and fly
configs and document it, so the key does not depend on home-directory
resolution to land on persistent storage.
* fix(node): publish the p2p key atomically and pin its directory
Write the key to a scratch file in the same directory and hard-link it
onto the final path. The bytes are durable before any name points at
them, so a crash cannot leave a partial key that fails to load on the
next start and takes the node off the network until someone reads the
logs. A concurrent reader can no longer observe a half-written file
either, since the final name appears complete or not at all.
hard_link rather than rename: rename replaces its destination silently,
so refusing to clobber an existing key would depend on a check followed
by a separate rename, and a concurrent start can land in that gap.
hard_link is atomic and refuses an occupied path, including a symlink,
which it does not follow.
Create the key directory 0700 and tighten it when an existing one grants
group or other access. A 0600 key under a writable directory can still be
replaced or unlinked. Tightening rather than refusing to start, because
existing installs already have 0755 there and refusing would take p2p
down on all of them through a path that only warns.
Formatting on the branch is swept up here; it was already failing
cargo fmt --check before this change.
* style(node): comma instead of a dash in the key-load warning
House style avoids em dashes in text we write. The swarm-failure warning
beside it predates this branch and is left alone.
* fix(node): require the p2p key path to name a directory
A bare filename in GITLAWB_P2P_KEY put the key in whatever directory the
process started from, and the directory guard was skipped entirely on that
path: Path::parent returns Some("") for a bare filename, which the caller
filtered out before ever reaching ensure_key_dir. The key file was created
0600 inside a directory that kept whatever mode it already had.
Config::validate now rejects a p2p key path that names no directory, so the
node says so at boot instead of starting with a key it cannot protect. That
placement is the point: an error raised in the p2p start path is logged and
stepped over, leaving the node running without p2p and reporting healthy.
The check is lexical on the tilde-resolved path. canonicalize would fail on a
parent that does not exist yet, which is the shipped ~/.gitlawb default and
every container's first boot, and comparing against the working directory
would reject /data/p2p.key under the image's WORKDIR, an absolute directory
the operator did name.
Three sites answered the parent question differently, which is how the gap
arose: one filtered the empty case out, one already normalized it, and one
opened "" and silently skipped its fsync. They now share key_parent, and
Config::validate calls it rather than adding a fourth answer.
load_or_create_p2p_keypair also refuses a path naming no directory. That is a
backstop behind the config gate, not the gate, so a later caller that skips
validation cannot quietly restore the old behaviour.
* test(node): run the key-permission probe in its own process
The probe zeroes the umask so the assertion means something: under a
restrictive ambient umask the bits are masked to 0600 regardless of whether
the code pins the mode, and the check passes either way.
Zeroing it in the shared test process is the problem. umask is process-global
and cargo runs these tests on threads, so any test creating a file in that
window inherits 000. Measured before this change: an unrelated concurrent
test's file was created 0666.
The probe now runs in a child process, where the zeroed umask cannot reach a
sibling and dies with the child. The parent is an ordinary test that runs
concurrently with everything else. Double-gated with #[ignore] plus an env
check so a bare --ignored sweep does not zero the umask in the shared process
after all.
The parent asserts the child ran exactly one test and that it passed, not
just that it exited 0. A libtest filter matching nothing runs zero tests and
still exits 0, so without that assertion a renamed fixture would read as a
green permission check while asserting nothing. Verified by pointing the
filter at a name that does not exist and watching the parent fail.
* docs(node): scope the p2p key permission claim and note the rotation
The old wording said the key file is "created with owner-only permissions"
without qualification, which is only true on Unix: every permission path in
p2p/mod.rs is cfg(unix), so on other platforms the file inherits whatever the
directory gives it and nothing is enforced. Say what is actually enforced and
where.
Also document what operators now have to do rather than leaving them to
discover it:
- GITLAWB_P2P_KEY must name a directory, since a bare filename is refused at
startup.
- The PeerId rotates once on the first start after upgrading, so a
GITLAWB_P2P_BOOTSTRAP multiaddr pinning a peer's old id with a /p2p/<PeerId>
suffix needs updating or dropping. Suffix-less addresses and the HTTP seed
list are unaffected.
- If the node reports tightening a loose key directory, the key that was in it
should be treated as possibly exposed and deleted so a fresh one is
generated.
* fix(review): close a `..` escape from the p2p key-path check
Two reviewers found the same hole independently: the check rejected a path
naming no directory, but a relative parent that walks back out through `..`
named one and still landed in the working directory. `a/../p2p.key` and
`./keys/../p2p.key` resolve to the cwd itself and `../p2p.key` resolves above
it, so all three put the key exactly where the check exists to keep it out of,
and had ensure_key_dir chmod that directory to 0700 on the way. Verified by
running the paths through the predicate and printing where each parent lands.
The rule is now that a relative key path must name a directory and must not
walk back out: at least one Normal component, no ParentDir. `..` inside an
absolute path stays accepted, since it cannot depend on where the process
started. The predicate moves into names_no_usable_directory next to key_parent,
and the config gate and the load_or_create_p2p_keypair backstop both call it, so
they cannot drift apart.
Also fixes two smaller gaps found in the same pass:
- The permission fixture could report "1 passed" while asserting nothing. Its
env gate returns early, and an early return is a passing test, so a renamed
variable would look green. It now prints a sentinel after its assertions and
the parent requires it. Confirmed by pointing the child at a different
variable and watching the parent fail.
- A GITLAWB_P2P_KEY starting with `~/` is refused when no home directory
resolves, instead of creating a literal `~` directory relative to wherever
the node happened to start.
The backstop had no test, so it has one now, along with a both-directions test
for the predicate. That test cleans up after itself: with the guard removed it
really does write a key next to the source, which broke a later run once.
* fix(review): reject `..` in an absolute p2p key path too
The previous commit closed this for relative paths and exempted absolute ones,
reasoning that an absolute path cannot depend on the working directory. That is
true and it is not the hazard. `key_parent` hands `ensure_key_dir` the lexical
parent, so `/data/keys/../p2p.key` chmods `/data` rather than the `keys`
directory the path appears to name, and `/data/../p2p.key` run as root would
try to tighten `/` to 0700. The exemption also had a test asserting the first of
those was fine, so the gap was written down as intended behaviour.
`..` is now rejected wherever it appears. An absolute path's root counts as
naming a directory, so `/p2p.key` still validates and `/data/keys/p2p.key` is
unaffected.
Found by a second-model review pass after the in-process reviewers had cleared
the relative half.
* fix(node): scrub the serialized p2p private key from memory
Two buffers held the private key in its protobuf form and dropped without
scrubbing: the encoding produced when a new identity is generated, and the file
contents read back on every subsequent start. Both are now Zeroizing, matching
what gitlawb-core already does for its own key material.
Scope worth being honest about: this scrubs our copies of the serialized form,
not the libp2p Keypair itself, which owns the secret for the process lifetime
and exposes no way to zeroize it. The gain is that the encoded bytes do not
outlive the write and the read.
zeroize was already in the tree through gitlawb-core, so this promotes it to a
direct dependency of gitlawb-node and adds no packages; the lockfile change is
the one line recording that.
* fix(node): validate the p2p key path before securing its parent
Reject filesystem-root parents, directory-valued paths, and symlinks before
any directory is created or chmodded. Create missing ancestors at the ambient
mode and pin only the nominated key directory to 0700. Refuse symlink reads
with O_NOFOLLOW, propagate directory durability sync failures, and document
the loose-key rejection path for operators.
* fix(node): satisfy clippy single-element-loop on p2p path test
* fix(node): tighten the p2p key storage contract before mutation
Anchor parent-directory checks on symlink_metadata, reject ~/ escapes and
non-regular existing keys with a bounded read, treat directory AlreadyExists
as a first-boot race, and skip key-path validation when P2P is disabled.
* fix(node): refuse foreign-owned p2p key paths and cap reads on all platforms
Check directory and key ownership before chmod or load, walk ancestors for
foreign control, bound non-Unix reads to MAX_P2P_KEY_BYTES, and gate unix-only
fixture tests without panicking on other targets.
* fix(node): hold the p2p key directory open across its trust checks
Resolve the key path once, then do every check and mutation through an
O_DIRECTORY|O_NOFOLLOW handle on the parent (openat/fstat/fchmod/linkat),
so nothing can be swapped between the ownership check, the tighten, and
the key open. Key reads go through one bounded, non-blocking loader on
every platform; new keys are generated in an unlinked scratch file and
published atomically, with concurrent first boots converging on the
winner's identity. The storage contract is indexed by two matrix tests:
filesystem shapes in p2p (each refusal proven side-effect free against a
recursive snapshot) and path spellings plus the p2p-disabled guarantee
in config.
* fix(node): walk the p2p key ancestor chain descriptor-anchored
Replace the path-following metadata walk (foreign_ancestor_error) and the
post-walk create_dir_all gap with a single descriptor-anchored walk that
verifies and creates the chain together. Missing components are created at
0700 relative to the verified parent and re-verified; AlreadyExists is a race
the winner must pass. Non-unix builds keep create_dir_all.
* fix(node): refuse group-writable and non-directory p2p key ancestors
The ancestor walk now refuses any component with group or world write bits
unless sticky, and any intermediate component that is a symlink or another
object type. Tests drive the walk through real boots: 0770/0775 ancestors
refused before any key IO, 0755 and 1777-sticky ancestors accepted with stable
reload, and an intermediate symlink refused with its target untouched. Both
guards proven load-bearing by neuter revert-checks.
* fix(node): verify the cwd anchor for relative p2p key paths
A bare-relative key path is now walked from a verified working directory:
the cwd is opened as '.' with O_NOFOLLOW|O_DIRECTORY, fstat-verified for
ownership and write bits before any relative component is touched, and a
writable or foreign-owned cwd is refused before anything is created. The
refusal names the working directory path. Child-process fixtures prove both
directions without mutating the shared process cwd.
* test(node): stop rendering uids in p2p assert-message strings
CodeQL flags the uid-interpolating assert messages as cleartext logging to a
console sink. Every assert message in the p2p test module now avoids
interpolating uid values or the uid-bearing error text, while the substantive
assertions (the refusal names both uids, the ownership check fires before the
chmod) stay intact and are verified by the same assertions.
* test(node): add walk rows to the p2p key-storage contract matrix
The matrix now proves the ancestor-walk refusals table-driven: a
group-writable (0775) ancestor, a world-writable non-sticky (0777) ancestor,
and an intermediate symlink component are each refused with the no-mutation
snapshot discipline, while the existing boot rows still reload to a stable
PeerId. The fchmod-on-create path pins newly created ancestors to 0700
regardless of the ambient umask.
* docs(node): note the verified working directory for relative p2p key paths
README, .env.example, and the clap help for GITLAWB_P2P_KEY now state that a
relative key path is anchored at a verified working directory, and that an
unsafe cwd or ancestor is refused with p2p off while HTTP stays up. Also
normalizes two pre-existing em dashes in the README settings table.
* fix(node): pin and verify the mode of every key object this process creates
POSIX applies the process umask to every requested creation mode, so the
0700 passed to mkdirat and the 0600 passed to openat were requests rather
than results. Under a mask that strips owner bits the created directory
landed 0000 and the no-follow reopen failed EACCES before the repairing
fchmod was reached, so that half failed loudly. The scratch key was worse:
it was published unreadable, the boot that created it succeeded on its
already-open descriptor, and only the next boot lost p2p, behind a health
check that still reported healthy.
Every object this process creates now goes through one rule: create, pin
the mode on the object just created, reopen, and verify the achieved mode
by fstat. The directory pin is issued by name off the verified parent
before the reopen, because a directory that landed 0000 cannot be opened
at all, so a pin that waits for the reopen is unreachable in exactly the
case it exists for. A race winner is verified as-is and never chmodded,
which is the rule the ancestor walk already applied.
Pinned's field is private to the pin module and no constructor is
exported, so a descriptor that skipped the pin cannot be turned into a
key-directory handle or reach publication. That is what keeps the rule
from depending on review.
Two adjacent corrections the same defect exposed. The leaf mode predicate
was supersets-only: mode & 0o077 != 0 asks whether anything is granted
beyond the owner, so 0000, 0100 and 0400 all passed it while being
unusable, and an inherited setgid bit was judged on the wrong bit width. A
loose directory is still tightened; an over-closed one is now refused with
its remedy rather than widened, matching how an over-closed key file is
already handled. And an unreadable key or key directory now names its own
cause: every open failure previously reported a refused symlink, which is
what an operator saw for a key masked at creation.
The matrix runs first boot and fresh-process reload across umask 0000,
0022 and 0777, crossed with missing and existing ancestors and leaf, with
concurrent creators and an injected write failure. It runs in child
processes because umask is process global, and it requires at least one
successful concurrent creator so a row where every creator fails cannot
pass. All 21 rows were RED under umask 0777 before this change.
* fix(node): create and publish the node identity key through the pinned path
The node identity PEM had its own storage flow: create_dir_all with no
mode, then write, then set_permissions. That is the exists-then-write-then-
chmod sequence INV-23 prohibits, and it sits in the same ~/.gitlawb the p2p
key uses, so the umask defect the previous commit fixed for one key was
still live for the other.
Measured on the current code, as a non-root uid, all three RED:
umask 0000 directory landed 0777, world-writable, holding the key
umask 0022 directory landed 0755, world-traversable
umask 0777 directory landed 0000 and the write failed with EACCES
The third is the one that mattered most: load_or_create_keypair runs before
the listener binds, so the node exited there and no p2p code was reached at
all. The umask-independence guarantee could not be demonstrated on the
shipped default while this stood.
The directory is now created through the same pin helper, at a verified
0700, and the PEM is published through the same scratch-then-link path at a
verified 0600. An existing directory that grants access beyond the owner is
tightened, which closes the other half of the same gap: a 0600 key inside a
0755 directory is still replaceable by anyone who can write that directory.
An existing key is loaded untouched and never chmodded.
Deliberately not the full ensure_key_dir. That carries the ancestor trust
walk, and importing its refusals onto a path that never had them would turn
an unsafe but currently booting deployment into a boot failure on upgrade.
Only the immediate parent goes through the pin helper, which verifies the
grandparent it is about to chmod a child of; ancestors above that keep the
existing create_dir_all behavior. The one new refusal is a group or world
writable non-sticky grandparent, which is the case where another local user
can replace the node's identity outright.
* fix(node): split p2p key config validation from live storage validation
The key-path validator answered two different questions and its callers
disagreed about what to do with the answer. Bare names, `..`, a trailing
separator and the filesystem root are properties of the configured value,
and Config::validate refuses them before the listener binds. The same
function also stat'd the key path and its parent, so a symlinked parent, a
non-directory parent or an unreadable parent exited the node as invalid
configuration, while the identical class of fault found one layer later in
load_or_create_p2p_keypair only logged and left HTTP serving.
That split was invisible from the outside and the docs described only one
half of it. Measured against the binary before this change: a symlinked
parent, a regular-file parent and an unreadable parent all exited 1 before
bind, which is the opposite of what README and .env.example promise.
Validation is now lexical only, behind a P2pKeyConfigError so the two
domains cannot be confused at a call site. Every live storage fact is left
to the load path, which already re-establishes each one on a descriptor it
opened rather than a pathname it stat'd: O_NOFOLLOW on the key open, a
regular-file check by fstat, and ELOOP or ENOTDIR at the leaf. The verdicts
are unchanged; what changes is that one policy now decides all of them.
Two supporting changes. The whole p2p port gate moves ahead of the database
connect, both arms together, because connect_db_with_retry retries forever
and left the disabled arm unreachable without a database; only p2p::start
still needs the pool. And a failed key load is now logged at error with a
stable event name and mirrored into a gauge, because the policy this commit
settles on is to keep serving HTTP with a green health check while the node
is off the p2p network, and that is only defensible if the outage is
visible to something other than a human reading startup logs.
A new integration test drives the real binary with no database and proves
both domains at the process boundary: nine lexical spellings exit before
binding, ten storage faults serve HTTP with the failure logged, and
GITLAWB_P2P_PORT=0 reaches its disabled log without touching the key tree.
Every row asserts a before/after snapshot, because a refusal that mutates
storage on its way out is its own defect and a returned error cannot show
it. Three of those rows were RED before this change.
A symlinked key directory also reports as a symlink again. Linux returns
ENOTDIR rather than ELOOP when O_NOFOLLOW meets O_DIRECTORY, so the errno
alone cannot separate a symlink from a regular file and the message had
regressed to naming the wrong cause.
* docs(node): state the two-domain key-path policy and how to see a degrade
README and .env.example promised that p2p stays off while HTTP keeps
serving when the key cannot be loaded, including for an unsafe ancestor or
working directory. Measured against the binary, that was false for a
symlinked parent, a non-directory parent and an unreadable parent, each of
which exited before binding. The preceding commit made the code match the
promise; this makes the promise precise.
All three operator surfaces now name the split the same way. A value that
cannot name a securable key file is refused before the node binds. Anything
wrong with the storage itself degrades, and the sentence says how to notice:
the node logs p2p_identity_key_load_failed at error and sets the matching
metric, /health stays green in that state, and fixing the storage and
restarting restores the same PeerId. A node quietly off the network is the
cost this policy accepts, so the docs point at the signal rather than
leaving an operator to find it in startup logs.
Two corrections while here. The key directory is tightened only when it
grants access beyond its owner; one closed too far is refused with a
chmod 700 remedy rather than widened, so the sentence no longer implies the
node will open up a directory an operator locked down. And the existing
advice to delete a key after a tightening warning is now scoped to the
loosening case it was written for: an over-closed directory is refused, not
tightened, and nothing in it was ever exposed, so following that advice
would have meant deleting a safe key and taking a second PeerId rotation
that breaks every pinned bootstrap multiaddr.
* test(node): pin the degrade signal and wait for both startup lines
Two gaps in the boundary test, both found by extending the mutation spec to
cover this commit's own guards.
The degrade rows asserted only the prose message, so renaming the event
field or dropping the level would have passed while every alert built on
the operator docs broke and the node still looked healthy. The rows now
pin the error level and the p2p_identity_key_load_failed name the docs tell
operators to watch.
The rows also waited on a single log line, which is a race: the degraded
server logs "ready" from a spawned task while the p2p gate logs its verdict
from the main task, so whichever the reader stopped at first left the other
uncaptured. It passed when the test binary ran directly and when the row ran
alone, and failed only once other work shifted the scheduling. Reads now
wait for every needle before returning.
The fix stays on the test side on purpose. Logging the p2p verdict before
the degraded server is spawned would also make the order deterministic, but
that is production sequencing changed for a test's convenience.
* fix(node): keep key publication transactional and path-policy split
Bare GITLAWB_KEY filenames publish into the working directory without
chmodding it. A directory this process created is removed if pin or
verify fails. Ancestor walks use search descriptors so a safe 0111
component is traversable. Scratch unlink is part of publish success and
is fsynced. Rotation advice is limited to writable or readable exposure,
not ordinary 0755 tightening.
* fix(node): refuse a writable cwd for a bare identity key
Publishing identity.pem into cwd must not chmod cwd, but a 0600 key is still replaceable if that directory is group or world writable. Apply the same trusted-parent check used for nominated key directories before the handle is returned.
* fix(node): load identity without following pathnames
Create already published through a directory handle. Load still used exists-then-read, which followed a symlink at the key and opened the grandparent by pathname. Open both no-follow, treat /identity.pem like a cwd publish so it reaches the filesystem, and open a search-only grandparent with the walk flags.
* fix(node): parse key paths, leave identity parents, repair special modes
A terminal `.` in GITLAWB_P2P_KEY was accepted then retargeted by Path onto
the parent directory. Identity creation imported p2p's 0700 pin onto any
existing named parent. A setgid-only 2700 key directory skipped repair and
disabled P2P. Darwin ancestor walks still required directory-list permission.
* fix(node): type-check the non-unix p2p scratch writer
The shared scratch-then-link path names pin::Pinned on every target, but
the pin module was unix-only, so gitlawb-node did not compile off Unix.
* fix(node): refuse identity path spellings that Path would retarget
GITLAWB_KEY had the same terminal-dot hole as GITLAWB_P2P_KEY, and its
tilde expansion did not refuse a suffix that escaped home. Bare filenames
stay legal.
* fix(node): create every missing identity parent on one pinned walk
The identity key path split one creation transition across two policies.
`create_dir_all` built every ancestor above the immediate parent at the
ambient umask, then `create_dir_pinned_at` ran `verify_trusted_parent` on
the last of those and refused it. Under umask 0002 that ancestor lands
0775, so the phase that made it handed it to the phase that rejects it:
the node exits before binding and the failed boot leaves the directory
behind at 0775, so the next attempt starts from state the first one made.
Replace the split with one descriptor-anchored walk. Pass 1 walks up from
the key's parent until a component opens; that descriptor is the anchor, and
it is the same fd the next step creates into. Pass 2 walks back down calling
`create_dir_pinned_at` per component, each one pinned 0700 and verified at
its achieved mode before it becomes the parent of the next, so from the
anchor down no pathname is resolved twice. Pre-existing ancestors are
adopted as-is and never chmodded, since GITLAWB_KEY is a file path rather
than a dedicated-directory setting; write-authority is still required on the
directory each component is created in.
A boot that fails past the walk now removes exactly what it created,
deepest first, through unlinkat with AT_REMOVEDIR against the descriptor
each entry was made in. A race-adopted directory is never in that list, and
AT_REMOVEDIR refuses a non-empty directory, so a leaf another boot has
published into survives. The accumulator is an out-parameter so a mid-walk
failure and a post-walk failure reach the same single rollback arm.
Two consequences worth naming. A symlink at the deepest existing ancestor is
refused when the missing suffix reaches it, where `create_dir_all` followed
it. Ancestors ABOVE that anchor are still resolved by pathname and their
symlinks still followed, unchanged from before this walk, because O_NOFOLLOW
binds only the final component of a path-based open; closing that would mean
an openat chain from the root and the full ancestor policy that GITLAWB_KEY
paths deliberately do not get. The doc comment now says so rather than
claiming more than the code does. And a losing concurrent first boot can
remove an intermediate another boot adopted but has not yet written into,
turning that boot's start into an ENOENT failure; both still fail closed.
Also replace the root-adjacent test. It proved a pathname property by
running the full mutating path against the real filesystem root and
assuming the runner lacks permission. Under euid 0, common in build
containers, `/` is accepted (root-owned, 0755, no group or other write
bit), the production path returns Ok, the test's panic arm fires before its
cleanup assertion, and a real private key is left at /identity.pem, after
which the leading exists() check makes every later run a silent skip. The
routing is now asserted lexically through a named predicate, with no
filesystem access and no dependence on euid.
The matrix covers one, two and three missing levels under umasks 0000,
0002, 0022 and 0777, a pre-existing 0755 ancestor that must stay 0755, a
relative multi-level path, a symlinked deepest ancestor, refusal without
partial mutation, and a populated leaf whose removal must be refused and
reported. Created-directory modes are asserted by exact equality, and each
success row reloads in a fresh process and requires the same DID.
* test(node): race real concurrent first boots of the identity key
The rollback added with the pinned walk can remove an intermediate that a
concurrent boot has already adopted, and until now that was reasoned about
rather than executed. Nothing in the suite started two real first boots
against the same key path.
Add a `concurrent-first-boot` layout to the self-exec fixture and a driver
that races several children through `load_or_create_keypair_at` on one
missing multi-level path. A two-phase file rendezvous puts every child
inside a spin loop before any of them is released, so the spread across the
racing call is one loop iteration rather than process-startup jitter. Three
arms: unaided boots, one injected write failure against one unaided boot,
and four injected failures against one, which is what actually opens the
window.
The assertions are properties of every legal outcome, never one
interleaving, so the test is deterministic even though the race is not. A
boot that claims success has a parseable 0600 key on disk, all successes
agree on the DID, surviving directories are exactly 0700 with no scratch
residue, and if no key was published then nothing claimed success and a
retry must still work. Observation counters are printed, not asserted, so a
run that never hits the window says so rather than passing as proof.
Two things the race showed that reading could not. The documented window is
real: a boot was refused with "No such file or directory" on its publish
after a loser rolled back the leaf it had adopted. But the more common shape
is one hop earlier, the loser removing an adopted intermediate so the ENOENT
lands on the next component's creation instead. And under enough concurrent
rollbacks every boot can fail, not merely the losers, so several nodes
starting against a shared first-boot path can all need a restart. Two-child
racing never opened the window at all in 500 iterations.
* fix(node): resolve identity key path components one at a time
`O_NOFOLLOW` binds only the final component of a path-based `open(2)`, which
is all the kernel offers for a multi-component pathname. Three sites resolved
a whole configured path in one open and so guarded only the last position:
the load path, the existing-named-parent fast path, and the walk that creates
missing parents. Every interior symlink was followed, and `verify_trusted_parent`
then attested the directory that was reached rather than the path taken to it.
That is enough to substitute the node's whole identity, not merely misplace
its key. Load and create resolve the configured pathname identically, so
repointing one interior link redirects both. Point it at a directory holding
another PEM and the node boots as that DID while its real key sits untouched
on disk and nothing is logged as wrong. Point it at an empty directory and
the node mints a fresh identity and reports a first boot. Neither needs the
attacker to read anything: ownership and mode checks still pass, because they
run on the object at the end of the path.
Resolve each component with `openat` and `O_NOFOLLOW` against the descriptor
of the one before it, from `/` for an absolute path or the working directory
for a relative one, so a symlink is refused at every position. Interior
components open search-only and the final component keeps the caller's flags,
so a legitimate 0111 ancestor still resolves and the leaf descriptor is still
the one that gets fsynced and chmodded.
Components are adopted with no ownership or mode judgment. The only new
refusal is a symlink on the key path: a shared 0775 volume, a foreign-owned
ancestor and `/etc` all keep working, because `GITLAWB_KEY` is a file path
rather than a dedicated-directory setting. `verify_trusted_parent` stays
exactly where it was, on the descriptor a child is created in. The p2p key
path is untouched; it already walked component by component.
This is pre-existing rather than a regression: the same probe against the
branch point produces the same result. It is fixed here because closing it
replaces the walk this branch just added, so splitting it would mean
reviewing that code twice.
Covered by rows for an interior symlink with the next level present, an
absolute target escaping the base, a two-link chain, the fast path, a
relative configured path, and both load-path variants asserting on the DID
the node ends up presenting rather than on a mode or an error string. The
cases that were already refused are pinned too, including a symlink at the
immediate parent and a dangling link, neither of which had a test before.
---------
Co-authored-by: Kevin Codex <[email protected]>
|
||
|
JATMNandClaude Opus 5.5
|
623b600835 |
fix(deps): bump libp2p to pick up hickory 0.26 and serde_with 3.22 (#518)
hickory-proto/hickory-resolver 0.25.2 were pulled in only by libp2p-dns 0.44. libp2p-dns 0.45 moves to hickory 0.26 but needs libp2p-core 0.44 and libp2p-identity 0.3, so the node's direct libp2p crates move to the matching release line (core 0.44, gossipsub 0.50.1, identify 0.48, identity 0.3, kad 0.49, quic 0.14, swarm 0.48, dns 0.45). multiaddr goes to 0.19 to match libp2p-core and avoid compiling two copies. No source changes were needed. Lockfile: hickory-proto/resolver 0.25.2 -> 0.26.3 (GHSA-q2qq-hmj6-3wpp / RUSTSEC-2026-0119, GHSA-6f2x-v7q7-m7m5), serde_with 3.18.0 -> 3.22.0 (GHSA-7gcf-g7xr-8hxj). With the fix in, drop the RUSTSEC-2026-0118/0119 ignores from .cargo/audit.toml as #76 requires. Closes #76 Co-authored-by: Claude Opus 5.5 <[email protected]> |
||
|
JATMNandClaude Opus 5.5
|
151d688387 |
fix(deps): bump aws-smithy-json to 0.62.7 (GHSA-8ffr-xgwf-xj56) (#516)
Cargo.lock resolved aws-smithy-json 0.61.9 (via aws-sdk-s3 1.119.0) and 0.62.5 (via aws-config and the sso/ssooidc/sts clients). Both are in the range affected by GHSA-8ffr-xgwf-xj56 / CVE-2026-18140 (<= 0.62.6): uncontrolled recursion when skipping unknown keys in deeply nested JSON. 0.62.7 requires aws-smithy-runtime-api ^1.12.3, so the AWS SDK moves as a set to the newest releases that still build on our 1.91 MSRV (they declare rust-version 1.91.1; later releases need 1.94.1): aws-config 1.8.18, aws-sdk-s3 1.137.0 (which itself requires aws-smithy-json ^0.62.7), aws-sdk-sso 1.102.0, aws-sdk-ssooidc 1.104.0, aws-sdk-sts 1.107.0, aws-sigv4 1.4.5, and matching aws-smithy-* crates. aws-sigv4 1.4.5 moves to p256 0.13, which drops crypto-bigint 0.4.9 from the lockfile. Update the RUSTSEC-2026-0387 note in .cargo/audit.toml to match; the ignore is still needed for crypto-bigint 0.5.5. Lockfile and audit-config comment only; no manifest or source changes. Co-authored-by: Claude Opus 5.5 <[email protected]> |
||
|
JATMNandClaude Opus 5.5
|
4d024e01bc |
fix(deps): resolve cargo audit failures on main (#514)
* fix(deps): resolve cargo audit failures on main Update quinn-proto 0.11.19, rustls 0.23.46, rustls-webpki 0.103.16, webpki-roots 1.0.11, zstd 0.13.4, zstd-safe 7.4.0 and zstd-sys 2.1.2 in Cargo.lock to clear RUSTSEC-2026-0338..0342, 0346..0349, 0381, 0382, 0391, 0398, 0403 and 0410. Ignore RUSTSEC-2026-0387 (crypto-bigint): the fix is only in 0.7.x, and no dependent compatible with our tree uses it. alloy-signer (through 2.5.0) pins elliptic-curve 0.13, and aws-sigv4 >=1.5 requires Rust 1.94.1, above the 1.91 MSRV. Co-Authored-By: Claude Opus 5.5 <[email protected]> * docs(audit): document RUSTSEC-2026-0387 reachability and graph-based removal Correct the aws-sigv4 boundary (1.6.4 is the first release on crypto-bigint 0.7; 1.5.x-1.6.3 still need ^0.5.4), mark the ignore as reachable through the k256 PrivateKeySigner and SigV4a signing paths, and tie removal to Cargo.lock having no crypto-bigint below 0.7.6. Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: Claude Opus 5.5 <[email protected]> |
||
|
beardthelion
|
6588590813 | chore: update rustls 0.23.37 → 0.23.45 to resolve RUSTSEC-2026-0285 (#455) | ||
|
Kevin Codex
|
21c74c65d9 |
Merge pull request #223 from Gravirei/fix/issue-220-whoami-registered-false-trust-lookup
fix(gl): only report registered: false on 404 in whoami (#220) |
||
|
Gravirei
|
861cced8f8 | chore: update h2 0.4.13 → 0.4.18 to resolve RUSTSEC-2026-0258 | ||
|
beardthelion
|
ba44a018ec |
fix(core): share the origin-scoped redirect rule with the git helper
The previous commit stopped the CLI carrying request signatures to a host the node redirected it to, and left the other signing client alone. That one is the git remote helper, which is what actually runs on clone, fetch and push, and it had no redirect policy at all. A push signs from its first request, so a node answering 302 handed out a working credential, and on a 307 or 308 the pack body went with it. The rule now lives in core and both clients use it, so the two cannot drift apart again. Three guards were not holding what they claimed. Bounding the retry wait by the remaining deadline could be deleted with every test still green, which the timing shows was four real seconds of overshoot past the bound the docs advertise; the assertion was loose enough to pass either way and is now tight enough to fail. The refusing-gateway warning was asserted through a test-only mirror rather than the write the user sees, so deleting the write kept the test green. And both no-hint exceptions, the definitive not-found and the wrapped scan, were only ever reached without a token in hand, which is the one state where they do nothing. A response whose body failed partway through the read still replayed the held token for every remaining attempt, spending the whole ladder on one position and then reporting that the object might lie further on. A body cut by the read cap was already terminal for that reason; a body cut by a broken connection is the same unreadable state and is terminal now too. A successful fetch no longer buffers the whole object before writing a byte of it, so a node cannot make the client hold an arbitrary amount of memory inside a time limit that was never a memory limit. The remaining silent skips in the recovery loops now say what went wrong, including the manifest fetch and the body reads, so a failure there is reported rather than looking like an absent blob. Also: the redirect chain bound is off by one against reqwest's own, has a test, and its comment no longer claims to prevent something the request timeout already prevents. |
||
|
beardthelion
|
a53c151879 | chore: sync Cargo.lock after rebase onto main | ||
|
beardthelion
|
659ee00d7d |
test(git-remote): tear down the fetch tree with a Job Object on Windows (#192)
run_bounded's clean-exit descendant reap was Unix-only: on Windows a git fetch leader that exits cleanly leaves the git-remote-gitlawb helper blocked in its ~300s HTTP request holding the inherited pipes, and taskkill /T can't reach a descendant once the leader is gone, so the reader joins blew past the promised bound. The child is now assigned to a Job Object at spawn (the analog of the Unix process_group(0)), and the job is terminated on both the timeout and clean-exit paths, which kills the whole tree regardless of leader liveness. KILL_ON_JOB_CLOSE is the safety net. The clean-exit regression is now cross-platform so it exercises this on a future Windows CI lane (#228). Unix tests unchanged and green; the Windows path is compile-verified for x86_64-pc-windows-msvc (runtime coverage lands with #228's CI lane). |
||
|
beardthelion
|
4201ce4d58 |
test(git-remote): reap the whole fetch process group and make the withheld guard load-bearing (#192)
Address jatmn's two P2 review findings on real_git_fetch.rs. F1 (INV-22): run_bounded killed only the top-level `git fetch` on the timeout path, so the git-remote-gitlawb helper descendant kept the inherited stdout/stderr pipe write-ends open and the reader joins blocked on the helper's ~300s HTTP timeout instead of the 30s deadline. Spawn the child with process_group(0) and, on timeout, tear the whole group down (SIGTERM, ESRCH poll, SIGKILL escalation, bounded cap, then reap) before joining, mirroring gitlawb-node/src/git/smart_http.rs. libc is added as a cfg(unix) dev-dep; non-unix keeps the child.kill() fallback so the always-on test still compiles. New regression run_bounded_reaps_descendants_holding_the_pipe: a backgrounded sleep inherits the pipe and outlives a 1s-deadline leader; reverting to a leader-only kill turns it RED (elapsed ~10s). F2 (INV-21): the non-success branch of real_git_withheld_shaped_first_post accepted any `git fetch` failure and only printed the POST count, so a failure before the first POST (broken advertisement, helper lookup, connection) passed as the expected withheld rejection. Assert posts >= 1 before accepting the nonzero exit. Forcing a pre-POST failure (0 POSTs) turns the assertion RED. |
||
|
github-actions[bot]
|
5f445152a8 | chore: sync Cargo.lock with the release version bump | ||
|
euxaristia
|
ee9bc93198 |
chore(deps): bump ruint to 1.20.0 for RUSTSEC-2026-0220
cargo audit fails on ruint 1.17.2 (incorrect overflow flags / truncated shift amounts). Upgrade the lockfile to the patched 1.20.0 release. |
||
|
t
|
f2f99c6684 |
chore: regenerate Cargo.lock for the 0.7.0 manifests after rebase
The branch forked before v0.7.0 merged, so the rebase left main's 0.7.0 manifests against workspace lock records still at 0.6.0. That state fails the gate this PR adds: `cargo metadata --locked` exits 101 with "cannot update the lock file because --locked was passed". Regenerated with `cargo update --workspace`, the same command the new sync-release-lock job runs. Only the 5 workspace crates move 0.6.0 -> 0.7.0; all 252 external dependencies are unchanged, so this is a version resync and not a dependency bump. `cargo metadata --locked` now exits 0. |
||
|
t
|
b8eff30fc2 | chore: sync Cargo.lock with the 0.6.0 manifests | ||
|
beardthelionandt
|
c7427e943d |
test(icaptcha): assert obtain_proof consumes the mocked service (#184)
* test(icaptcha): assert obtain_proof consumes the mocked service Add an integration test that runs obtain_proof against a mockito server and asserts both /v1/challenge and /v1/answer were consumed, so the flow is proven to call the service rather than short-circuit or make a live call. Adds mockito as a dev-dependency. * test(icaptcha): block non-mock egress so the no-live-call guard is real (#184) The mock-consumed assertions only prove the two mock endpoints were hit; they cannot see a request that also leaks the DID, answer, or API key to DEFAULT_URL or any other origin, so the test's stated "no live call" guarantee was untested. Blackhole every non-loopback destination for the flow: NO_PROXY lets the loopback mock through while ALL_PROXY routes any other host to a closed port, so the client contacting anything but the injected mock fails the request. Verified load-bearing: pointing cfg.url at DEFAULT_URL makes obtain_proof fail with a proxy tunnel error (RED), and the loopback-mock path still passes (GREEN). * test(icaptcha): blackhole scheme-specific proxy vars so the no-live-call guard covers https The mock-consumed guard blackholed non-loopback egress with ALL_PROXY only, but reqwest honors HTTPS_PROXY/https_proxy (and the http equivalents) OVER ALL_PROXY for https URLs. On a runner whose environment has a working HTTPS_PROXY, a regression could still leak the DID/answer/API key to the https DEFAULT_URL while both loopback mock calls are consumed and the test stayed green. Override the scheme-specific proxy vars (both cases) to the closed port too; NO_PROXY keeps the loopback mock direct. Single #[test] per process, so the override needs no restore. Verified: a throwaway repro showed reqwest routing an https request to HTTPS_PROXY past the ALL_PROXY blackhole (the gap); with the fix the override sends that egress to the closed port even when a working HTTPS_PROXY is pre-set; the guard still goes RED when cfg.url points off-loopback. --------- Co-authored-by: t <t@t> |
||
|
beardthelionandt
|
ee8d7c03b2 |
fix(node): gate /hooks and sibling read surfaces leaking private repo metadata (#94) (#113)
* fix(node): owner-gate list_webhooks to stop leaking webhook URLs (#94) list_webhooks sat on the optional_signature read group with no auth or ownership check, so any unauthenticated caller who knew a repo's owner/name could enumerate its webhooks (target URL, created_by_did, events; only the secret was redacted). Gate it read-visibility-then-owner, mirroring create_webhook/delete_webhook: headerless -> 401, non-reader of a private repo -> 404 (existence hidden, uniform with the sibling read surfaces), non-owner of a public repo -> 403. Add a list_webhooks row to the authz_guard drift table so the gate can't silently regress. * fix(node): read-visibility-gate list_replicas for private repos (#94) list_replicas sat on optional_signature with no visibility check, so a private repo's replica URLs were enumerable by anyone who knew owner/name. Replica lists are a documented public mirror-discovery surface, so gate on read visibility (authorize_repo_read) rather than owner-only: a public repo stays anonymously listable, a private repo returns 404 to anyone who cannot read it. register_replica registers non-owner DIDs, and a replica operator is not a visibility reader, so a non-owner replica operator of a private repo gets 404 — the intended contract, pinned by the new test. Add the authz_guard row. * fix(node): read-visibility-gate list_protected_branches (#94) list_protected_branches sat on optional_signature with no visibility check, leaking a private repo's branch names to any unauthenticated caller. Gate on read visibility (authorize_repo_read at the root, INV-2 root-listing exception): public repos stay anonymously listable, private repos 404 to a non-reader. Add the authz_guard row. * fix(node): gate list_repo_events for local private repos, keep gossip-only (#94) list_repo_events served a private repo's ref certificates and gossip ref metadata (ref names, SHAs) to any unauthenticated caller. The handler also tolerates a repo it knows only via gossip (no local row) and legitimately serves its events, so a blanket get_repo->authorize_repo_read swap would have 404'd that federation path. Gate only the locally-hosted branch: when get_repo returns a record, run visibility_check on it at the root and 404 on deny before fetching either the cert or gossip events. A gossip-only repo (no local row) keeps its existing public federation-feed behavior; the global /api/v1/events/ref-updates feed remains a separately tracked deferral. Add the authz_guard row (visibility_check marker, since the gate is direct to avoid a second get_repo). * fix(gl): sign the webhook list request now that GET /hooks is owner-gated (#94) `gl webhook list` built its NodeClient with no keypair, so the GET went out anonymously. With the node now owner-gating GET /hooks, that request returns 401. Thread a --dir through the List subcommand and load the keypair, matching `gl webhook create`/`delete`, so the listing is signed as the owner. (Replica and protected-branch listings stay anonymous for public repos under read-visibility gating, so their gl clients need no change.) * fix(review): sign the webhook list GET (get_signed), not plain get (#94) Code review caught that NodeClient::get() never attaches the RFC 9421 signature even when a keypair is loaded — only get_signed/post/put/delete sign. The earlier gl webhook-list fix loaded the keypair but still called get(), so `gl webhook list` (and the MCP webhook_list tool) 401'd against the now-owner-gated endpoint. Switch both to get_signed. Harden the two cmd_list tests to require a Signature/Signature-Input header on the mock (verified: they fail against plain get(), pass with get_signed) — the missing assertion that let the bug through. Add the untested owner-of-private- repo webhooks path (200 + redaction). Fix the stale module doc and the authz_guard bucket comment for list_webhooks. * fix(gl): sign read-visibility GETs so private-repo owners can read their own repos (#94) The #94 server change read-visibility-gates GET /replicas and /branches/protected: public repos stay anonymous, private repos require the owner/reader to authenticate. Four gl read commands sent unsigned GETs and so broke (404) or silently degraded (empty output) for a private repo's owner: gl protect list, gl repo owner (silent empty protected list), gl repo info (silent missing replica count), gl repo replicas (and it had no --dir flag). Add NodeClient::get_maybe_signed — signs when an identity is present, stays anonymous otherwise (mirrors the conditional signing of post/put/delete) — and route the four call sites through it. Add --dir to `gl repo replicas`. Harden the protect-list tests to assert the request is signed (verified: they fail on plain get()). Public-repo anonymous reads are unchanged. * fix(review): sign the primary repo fetch in gl repo info/owner too (#94) Round-2 review caught that cmd_info and cmd_owner loaded a keypair and signed their replica/protected-branch sub-fetch but left the PRIMARY GET /api/v1/repos/{owner}/{name} on plain get(). get_repo is read-visibility-gated, so for a private repo that primary fetch 404'd first and the command bailed before reaching the signed sub-fetch — the half-fix never actually let an owner inspect their own private repo. Route both primary fetches through get_maybe_signed. Add the missing signing guards: harden test_cmd_owner_is_owner to require a signature on both mocks, and add tests for cmd_replicas and cmd_info (both were untested) asserting the request is signed when an identity is present. Out of scope (pre-existing, gated before #94, untouched here): gl repo commits, gl pr list/view/diff, and the MCP repo_get/repo_commits/pr_* tools call other already-gated read endpoints unsigned and are broken for private repos independently of this change. * test(review): execute the previously-reasoned-only paths (#94) Close the vetting gaps from the review: - node: a listed reader (non-owner) passes the read gate but is still refused the webhook list (403), and headerless on an existing-private vs an absent repo both 401 — proving the 401 fires before any lookup, so it is not an existence oracle. - node: the read-visibility surfaces (replicas, protected branches) admit a listed reader who is not the owner (the allow-list branch of visibility_check), while a non-reader stranger still 404s. - gl: get_maybe_signed's anonymous fallback (no identity -> unsigned GET) is now exercised, RED-proven by flipping it to get_signed (which signs and fails the no-signature-header assertion). * test(review): execute the MCP dispatch and gl->node signature seams (#94) Close the last two reasoned-only paths by execution: - MCP webhook_list: a call_tool("webhook_list", ...) dispatch test against a mock node asserts the /hooks request carries a signature (get_signed). RED-proven: flipping the arm back to get() fails the header assertion. - gl->node end-to-end: a real RFC-9421 signature produced exactly as the gl client's get_signed does (gitlawb_core::http_sig::sign_request over GET + empty body) is run through the node's actual optional_signature middleware, which verifies it and authorizes the owner (200 + webhook body). RED-proven: signing over POST while sending GET makes the node reject it. This stitches the gl signing side and the node verifying side in one test rather than mocking one end. * fix(review): fail closed when the repo lookup errors in list_repo_events (#94) `get_repo(...).await.ok().flatten()` collapsed a lookup Err into None, so a transient DB failure skipped the read-visibility gate and the handler served a private repo's gossip ref-update events. Propagate with `?` instead: an error now maps to AppError::Internal (500) and fails closed, while a genuine Ok(None) (repo not hosted locally) stays the intentional ungated pass-through. Add a regression test that forces a get_repo error (drop the column its SELECT reads) and asserts 500 with no secret in the body. The injection is self-verifying: it asserts the lookup succeeds pre-drop and errors post-drop, so the test can't pass vacuously if get_repo's projection changes later. * fix(node,gl): close residual gaps in repo-read gating (#113) Address jatmn's review feedback on the read-surface gate: - gl webhook: resolve owner from the signing identity, not the node DID. cmd_create/list/delete all routed through the local resolve_owner helper, which only ever returned the node root DID, so for a repo owned by the local identity the owner-gated /hooks route returned 404/403. Drop that helper and reuse repo::resolve_owner_repo_pair (accepts owner/repo, else derives from --dir/keypair). Fixed across all three subcommands so create-writes and list/delete-reads stay on the same owner. - gl webhook list: check the HTTP status before reading `webhooks`. A 401/403/404 body has no webhooks field and previously printed "No webhooks" and exited 0, hiding the rejection; now bails with the server message like cmd_create. - visibility::is_owner: delegate to api::did_matches so the read gate and require_repo_owner share one normalization. Fixes the under-match (bare owner stored, full did:key caller, as mirror rows) that denied the owner with the opaque 404, and the cross-method over-match. - authz_guard: pin both halves of list_webhooks (authorize_repo_read and require_repo_owner) so removing the existence-hiding gate fails the drift test. cargo fmt/clippy clean; 644 workspace tests pass. * test(node)+fix(gl): close events reader-coverage gap, align label-list signing Self-review follow-ups on the read-gate work: - list_repo_events: add list_repo_events_admits_listed_reader, pinning the allow-list branch of the events gate (a listed non-owner reader gets 200 with the cert; a non-reader stranger 404s with no leak). Mirrors the read_visibility_admits_listed_reader coverage the sibling replica and protected-branch surfaces already had; the events handler was the only gated read surface missing it. - gl repo: cmd_label_list now loads an identity and uses get_maybe_signed instead of a bare anonymous get(), matching cmd_replicas/cmd_info. Public repos stay anonymously listable; a private-repo owner can read their own labels once /labels is read-visibility-gated, with no silent client drift. fmt/clippy clean; 645 workspace tests pass. * fix(review): pin the label-list signing and the events 404 sha-no-leak Final-review hardening of the prior two follow-ups: - gl repo label-list tests: require the signature/signature-input headers on the signed-branch mocks, and add an anonymous (no-identity) test asserting the header is absent. Without the header assertion the tests passed even with a bare get(), so the signing change was not actually pinned (RED-verified: reverting to get() now fails both signed-branch tests). - list_repo_events_admits_listed_reader: also assert the cert sha (not just the ref name) is absent from the stranger's 404 body, matching the list_repo_events_is_read_visibility_gated leak-check convention. fmt/clippy clean; 646 workspace tests pass. * fix(gl): error on 2xx webhook-list body missing the webhooks array (#113) A 2xx /hooks response without a `webhooks` array could hide a node/API contract regression. cmd_list now errors with context instead of mapping it to "No webhooks". An empty-but-present array stays a valid no-webhooks result. Adds a test for the missing-array case. * fix(gl,node): scope MCP webhook_list owner to keypair, gate list_labels on read visibility (#113) - MCP webhook_list: default owner to the signing keypair's short DID (explicit owner arg still overrides) instead of the node root DID, so the signed, owner-gated /hooks request resolves to the caller's own repo. Mirrors the gl webhook list fix. resolve_owner and webhook_create/delete left untouched. - list_labels: gate on authorize_repo_read so a private repo's label names 404 for non-readers (public repos stay anonymously listable), matching the sibling read surfaces and making the existing client comment true. Added the authz_guard marker and a read-visibility integration test. - gl label list: check HTTP status before reading the body so the now-gated 404 surfaces as an error instead of printing "No labels" and exiting 0, matching the sibling list clients (replicas, protected branches). * fix(gl): surface denied repo info reads instead of fabricating a repo (#113) cmd_info parsed the GET /api/v1/repos/{owner}/{name} body without checking the HTTP status, so a private-repo JSON 404 was rendered as a fake repo summary (owner / ? / public / main) and exited 0. Capture the status and bail on non-2xx, matching the sibling list commands. Also assert the anonymous 404 label-list body does not leak the label name, closing the last unread-body branch in the read-visibility gate test. * test(node): drop #113-gated handlers from KNOWN_UNGATED; sync lockfile to 0.4.0 The repo-scoped egress guard's allowlist (added in #122, merged after this branch was opened) still listed the read surfaces this branch gates. Now that list_labels, list_repo_events, list_webhooks, list_replicas, and list_protected_branches are read-visibility gated, the staleness assert requires removing them from the allowlist. The #120 surfaces (certs, issues, bounties, stars) stay listed; git_info_refs stays until #119 lands. Lockfile versions follow the 0.4.0 release the rebase landed on. * fix(gl): fail the MCP webhook_list tool on non-2xx node responses The MCP webhook_list path deserialized the response body and returned it as a successful tool result without checking the HTTP status, so a 401/403/404 JSON error (missing identity, wrong owner, private or deleted repo) was reported as success with the error body as the result. Check status().is_success() before deserializing and surface the node's message as a tool error, matching the `gl webhook list` path. Covered by a non-2xx test (RED without the check). * fix(gl): read the protected_branches key in repo owner gl repo owner parsed the protected-branch response as a bare array or a "branches" key, but the node returns {"protected_branches":[...],"count":N}, so it always printed "Protected: (none)" even when branches were protected. Extract the parse into parse_protected_branches matching the gl protect list shape, and update the owner tests to mock the real schema instead of bare arrays (which is why the bug went uncaught). * test(gl): cover the deny/negative arms reconciled by the merge The merge changed three gl branches that no existing test executed (they were only reasoned): resolve_owner_did with no local identity, cmd_info on an authenticated-but-denied 403, and get_maybe_signed with no keypair. Add a test per branch asserting the must-not case (error/unsigned, never a fabricated repo card). Verified load-bearing by mutation: disabling the cmd_info status gate turns the 403 test red. --------- Co-authored-by: t <t@t> |
||
|
beardthelionandt
|
d00d89ae5b |
fix(node): gate POST /api/v1/sync/trigger and rate-limit the peer-sync routes (#82) (#161)
* feat(node): add per-client-IP limiters + knobs for the peer-sync routes (#82) Plumb two dedicated per-IP RateLimiters into config/state so the peer-sync enqueue routes can be braked independently: sync_trigger_rate_limiter (tight, GITLAWB_SYNC_TRIGGER_RATE_LIMIT, default 60/hr) and peer_write_rate_limiter (generous, GITLAWB_PEER_WRITE_RATE_LIMIT, default 600/hr). Both are bounded and swept by the periodic cleanup loop, keyed via the node-wide push_limiter_trust. No route is wired to them yet; that lands next. * fix(node): require a signature on /sync/trigger and rate-limit the peer-sync routes (#82) /api/v1/sync/trigger was reachable unauthenticated in the default config and drove an O(peers) outbound fan-out + per-repo sync_queue enqueue — an anonymous amplification/DoS. The same enqueue_sink is reachable via /sync/notify, which also accepts unsigned known-peer requests and had no rate limit. - Move /sync/trigger into its own always-signed route group (add_auth_layers), independent of require_signed_peer_writes, and give it a tight per-IP brake. - Add a separate, generous per-IP brake to the peer-write group (/peers/announce, /sync/notify); notify's signature behavior is unchanged (rolling-upgrade compat), it only gains the brake. Separate buckets so an unsigned notify flood can't drain the signed trigger caller's quota. - Per-IP (not per-DID): a did:key farm self-registers, so a signature/per-DID gate does not cap cost (INV-10); the brake keys on the client IP before auth. Adds an adversarial test matrix (unsigned->401 both modes, IP flood->429, DID-agnostic brake, forged X-Forwarded-For can't bypass, notify flood->429, distinct-IP not throttled, cross-route non-exhaustion, 0-disables-but-sig-holds, announce still accepts unsigned). * fix(gl): surface node denial on sync trigger and sanitize the error body (#82) Now that the node gates /api/v1/sync/trigger behind a signature and rate limit, 'gl sync trigger' must not render a 401/429 as a fabricated '✓ sync triggered / 0 peers' success. Check resp.status() before parsing; on a non-2xx, extract the node's message and bail. Strip C0/C1 control bytes from the node-supplied string before printing (INV-6) — a hostile node could embed ANSI/OSC escapes in the body. Adds mockito tests: 401/429 surface as errors (RED without the status check), control bytes are stripped, and the 200 path is unchanged. * fix(review): bound gl error-body read, strip bidi controls, genericize 429 message (#82) Code-review follow-ups on the #82 change: - gl sync trigger: read the node error body with a capped reader (read_body_capped) instead of resp.text(), so a hostile/broken node can't force an unbounded allocation to surface a denial (INV-6, read half). - sanitize_node_msg: also strip Unicode bidi/format controls (U+202A-202E, U+2066-2069, LRM/RLM/ALM) — is_control() only covers the Cc category, but a right-to-left override can reorder the displayed error line. - too_many_requests(): the shared per-IP 429 middleware now serves the peer-sync routes too, so drop the push-specific wording ('rate limit exceeded'); the push-specific 429 in the receive-pack handler is unchanged. Adds tests for the bidi/length sanitize and the oversized-body path. * test(#82): close reasoned-not-run gaps with load-bearing tests Executes the branches that were previously only reasoned: - read_body_capped byte-cap test (RED against resp.text() — 2MB body -> ≤cap): vets the bounded-read fix, not just the display cap. - sync-route 429 body asserts 'rate limit exceeded' and NOT 'push' (RED before the message was genericized); also finishes genericizing the warn log, which the response-body change had missed. - real RFC 9421 signature through the full router reaches the handler (200) in both config modes — the positive path through the actual gate, not a direct-mount with an injected DID. - /peers/announce flood -> 429 (the peer_write brake covers announce, not just notify). - trigger_counts extraction unit test (happy + malformed -> 0,0), closing the count-formatting gap in the 200 path. No production behavior change beyond the warn-log wording. * fix(#82): require an identity for sync trigger and fix the rollout docs Address review feedback on the always-signed /sync/trigger route. - gl sync trigger now requires a local identity via load_keypair_from_dir and fails locally with "identity not found" before issuing a request, instead of .ok() silently sending an unsigned request that can only 401 remotely. Add a test proving it fails before dialing the node. - Split /sync/trigger out of the staged unsigned peer-write rollout in the README and the OSS-readiness audit; only /peers/announce and /sync/notify keep the rolling-upgrade unsigned behavior. - Reorder the two peer-sync rate-limit vars in .env.example for dotenv-linter. - Bump crossbeam-epoch to 0.9.20 (RUSTSEC-2026-0204), matching main, so cargo audit passes. --------- Co-authored-by: t <t@t> |
||
|
beardthelionandt
|
67ad2b876c |
fix(deps): bump crossbeam-epoch to 0.9.20 for RUSTSEC-2026-0204 (#162) (#163)
RUSTSEC-2026-0204 (invalid pointer dereference in crossbeam-epoch's fmt::Pointer impl for Atomic/Shared) hard-fails cargo audit on every branch, since the advisory's addition to the RustSec db. crossbeam-epoch is a fully transitive dependency (moka -> hickory-resolver -> libp2p-dns), and the advisory's prescribed fix is >= 0.9.20; moka 0.12.15 accepts >=0.9.18,<0.10.0 so the bump resolves with no other graph changes. Lockfile-only, single crate. Verified: cargo audit exits 0 and the workspace builds clean with 0.9.20. Closes #162. Co-authored-by: t <t@t> |
||
|
Kevin CodexandClaude Opus 4.8
|
06388ec26a |
feat(gl): sanctioned iCaptcha client flow + secure git lifecycle (#138)
* feat(gl): sanctioned iCaptcha client flow + secure git lifecycle Builds the client side of the iCaptcha gate so a legitimate agent passes the challenge transparently, with no manual headers and no GITLAWB_ICAPTCHA_PROOF env var. Server enforcement is unchanged. New crate `icaptcha-client` (blocking, shared by gl + the git helper): - deterministic solvers for the computational challenge types (arithmetic, algebra, sequence) matching the iCaptcha generators; anagram/logic/LLM fall back to a solver hook / interactive prompt. - obtain_proof(): POST /v1/challenge (requesterId = caller DID), solve, POST /v1/answer, handle escalation, return the signed proof. Optional bearer auth via GITLAWB_ICAPTCHA_API_KEY. gl (crates/gl): - http.rs: unify post/put/delete through one signed sender that, on a 403 iCaptcha challenge (detected via x-icaptcha-url / x-icaptcha-level headers), solves it and retries the same signed request with the x-icaptcha-proof header (bounded retry absorbs the ~5 min proof TTL). Removes the env hack. Emits an actionable hint on 401 not_an_agent (old-CLI / unregistered). - repo.rs: `repo info` checks status and returns a real 404 instead of a stub card with `?` fields; `repo info`/`clone` resolve the owner from the local identity or an explicit owner/name — never the node's own DID. - doctor.rs: warn on gl-vs-node version drift; add an iCaptcha reachability check. git-remote-gitlawb: same shared client — push stays signed (RFC 9421); on a 403 iCaptcha (safety net; push is signed-only, not gated) it solves and retries. Clone/fetch unsigned; missing repo keeps its clear 404. gitlawb-node: the 403 icaptcha_proof_required response now advertises the service url + required level as JSON fields and x-icaptcha-url / x-icaptcha-level headers (mirroring the human_detected pattern) so clients discover them instead of scraping the message. The sub == authenticated-DID and expiry/level checks are unchanged — enforcement is not weakened. README: documents the full lifecycle (identity -> register -> repo create with auto-iCaptcha -> push -> clone), the requesterId == DID rule, and proof TTL. Tested: solver unit tests; full workspace green against Postgres (node still rejects missing/expired/wrong-subject proofs). * fix(icaptcha-client,gl): address PR #138 review feedback Rebased onto main (resolving the git-remote-gitlawb/main.rs conflict from #119) and addressed the review findings: - [beardthelion P2 / jatmn P2 / CodeRabbit] Removed the dead iCaptcha retry loop from the push path. git_receive_pack never calls verify_request (only create/fork/register are iCaptcha-gated), so a push 403 never carries x-icaptcha-* headers and the loop could never solve. Taking main's push path (via #119) deletes it, which also resolves the pack-clone (retries no longer copy the whole pack) and the interactive-prompt-over-git-stream threads, and restores the sanitized info/refs error path. git-remote-gitlawb no longer depends on icaptcha-client. - [beardthelion P1] Sanitize and length-bound iCaptcha error bodies before they reach the terminal. The origin is only as trusted as the node that advertised it, so its non-2xx bodies (and Failed{reason}) are attacker-influenceable; they went raw into bail! (CWE-150, the #137 class). Now bounded-read + C0/C1-stripped + capped, same shape as #137. - [beardthelion P2] Do not trust or hand credentials to a node-chosen iCaptcha URL. x-icaptcha-url was used as the base with no scheme/host check and the API key was sent to it on the first hop (SSRF + key exfiltration). resolve_solver_url now honors an advertised URL only when https + host-allowlisted (public default or the operator's GITLAWB_ICAPTCHA_URL), else falls back to the trusted origin; the API key is attached only to the operator's own configured origin. - [jatmn P2 / CodeRabbit] gl repo clone with a bare name now derives the short owner key via resolve_owner_did, matching the other commands' gitlawb://z.../name URL shape instead of the colon-bearing full DID. Skipped (with reason): jatmn P3 (doctor GITLAWB_ICAPTCHA_URL) is resolved by the P2 change — that env var is now read by the write path, so the doctor's probe is accurate. CodeRabbit User-Agent-version nit lived in the push code reverted to main. README push wording kept ("signed-only, no per-push challenge") — now accurate since the retry path is gone; the API-key bullet was updated for the new trust model. Tests: added sanitize + resolve_solver_url unit tests; full workspace green. Co-Authored-By: Claude Opus 4.8 <[email protected]> --------- Co-authored-by: Claude Opus 4.8 <[email protected]> |
||
|
beardthelionandt
|
6f36fc07b8 |
fix(node,git-remote): gate receive-pack advertisement, sign client fetch/push (#119)
* fix(node,git-remote): gate receive-pack advertisement, sign client fetch/push
The info/refs ref advertisement was gated only for service=git-upload-pack
(the visibility check sat inside `if service == "git-upload-pack"`), so an
anonymous GET /{owner}/{repo}.git/info/refs?service=git-receive-pack returned a
private repo's branch names and commit tips, bypassing the read gate. Run the
visibility "/" check for both services.
Now that the advertisement is gated for both services, git-remote-gitlawb has to
authenticate its own requests, or a private repo's owner can neither fetch nor
push it. Sign the Phase-1 advertisement GET and the Phase-2 pack POST for both
services over path_and_query when an identity keypair is present; public repos
stay anonymous. This also repairs a pre-existing break where an owner could not
fetch their own private repo, because the upload-pack POST was sent unsigned.
Tests: node-side gate (anon denied, owner-signed cleared, both services); a
real-signature seam proving the client's own signature verifies under the node's
verification primitives; real served-content withholding against an on-disk repo
(the owner gets the ref bytes, a denied caller's 404 omits them); and mockito
coverage that the client attaches and omits signatures correctly.
* docs(node,git-remote): correct auth-contract comments on receive-pack gating (#119)
receive-pack POST is signature-required and separately owner-gated, not
visibility_check-gated; push implies read, not read-subset-of-push.
* test(node): assert unsigned receive-pack POST is rejected (#119)
No test covered the signature-required half of the receive-pack auth
contract. Wire the POST route behind require_signature as production does and
assert an unsigned POST returns 401 before reaching the handler; 401 (not the
handler's 404/500) proves require_signature rejected it pre-handler.
* test(node): RAII-clean the served-refs test's temp dirs on panic (#119)
advertisement_serves_real_refs_only_to_authorized_callers created /tmp/<slug>/<name>.git and /tmp/gl-seam-src-<short> and only removed them after the final assertion, leaking those dirs on the runner when an earlier assertion failed. Wrap each known path in a DirGuard whose Drop calls remove_dir_all, bound right after creation, so cleanup runs on success or panic. tempfile::TempDir does not fit here because repo_store::for_testing fixes the path layout.
* test(node): drop git_info_refs from KNOWN_UNGATED now that #119 gates it
The repo-scoped egress guard's allowlist (added in #122, merged to main
after this branch was opened) still listed git_info_refs as ungated. This
branch makes the info/refs visibility gate unconditional for both
git-upload-pack and git-receive-pack, so the handler is now gated and the
staleness assert requires removing it from the allowlist.
* style(node,git-remote): tidy #119 review nits (em dashes, dead binding, test header)
Post-rebase ce-code-review pass on #119. No behavior change:
- replace em dashes in the added comments with commas/colons/parentheses
- drop the dead `let _ = COVERED_COMPONENTS;` binding (and its import); the
`missing_components().is_empty()` assert already covers the intent
- add a section header above the new git-info-refs test block to match the
module's sectioning convention
* test(node): recurse nested api modules in the egress-gate guard
The completeness scan in every_repo_scoped_handler_is_gated used a flat
read_dir over src/api, so a future api/<module>/mod.rs could add a
repo-scoped handler the scrape never inspects. Walk the api tree recursively
via a new collect_rs_files helper (covered by its own unit test), skipping
only the top-level mod.rs so nested module files are checked too.
* fix(git-remote): keep public fetches anonymous, sign only on a 404 retry (#119)
git-remote-gitlawb signed every git-upload-pack request whenever an identity
existed, disclosing the caller's DID to any node it fetched a public repo from,
even though public repos need no auth. Fetch now sends the advertisement GET
anonymously and escalates to a single signed retry only when the node denies it
with 404 (how it withholds a private repo from an unauthenticated reader); if the
retry was needed, the upload-pack POST is signed too. Push (git-receive-pack) is
unchanged and signs from the first request, since a push can never be anonymous.
Verified against the node by execution: an unsigned private fetch returns 404,
not 401, and a signed retry clears the read gate for the owner and for a granted
non-owner reader, while a signed non-reader stays 404.
Tests drive handle_connect end to end (recording mock): public fetch stays
anonymous for GET and POST, private fetch retries signed once and signs the POST,
a keyless fetch does not retry, a still-denied signed retry surfaces 404 once, and
push signs from the first request.
* fix(node): scan nested api sources by relative path in the egress guard (#119)
The egress-gate completeness scan skipped already-covered files by basename
(listed.contains(fname)), so a nested api/<module>/repos.rs would be skipped
because the top-level repos.rs is listed, letting a future nested module add an
ungated repo-scoped handler the scrape never inspects. Extract the skip logic into
unlisted_source_files and match on the path relative to api_root, so only the exact
top-level files already covered are skipped and a nested file with a colliding
basename is still scanned. Completes CodeRabbit's outstanding request.
---------
Co-authored-by: t <t@t>
|
||
|
GravireiandGravirei
|
6c95592d18 |
fix(node): prefer canonical repo row over mirror row in get_repo (#124) (#141)
* fix(node): prefer canonical repo row over mirror row in get_repo (#124) get_repo matched both mirror rows (bare short-DID, is_public=true, no visibility rules) and canonical rows (full did🔑 DID, correct rules), returning one nondeterministically. When the mirror row won, the visibility gate saw is_public=true + empty rules, allowing unauthenticated read of private/withheld content. Add ORDER BY to prefer canonical rows (UUID id, no slash) over mirror rows (slash-form id), matching the ranking logic already used by DEDUP_CTE and dedupe_canonical_repos. Test seeds mirror FIRST so an unordered fetch_optional returns the mirror row — proving the test locks in the fix. * fix(test): date canonical row after mirror to guard CASE term in get_repo fix (#124) * fix(node): tighten get_repo owner matching to did:key only, not any DID method (#124 P2) * fix(node): refactor owner DID normalization to a dedicated function in create_repo * fix(node): align clone URL and slug construction with tightened did:key matching - Make normalize_owner_key pub(crate) and reuse it in api/repos, api/events, api/peers, and visibility modules. - Add read-gate authorize_repo_read and to_response clone_url slug tests. * fix(node): hoist owner-key CASE into shared const + address #124 review feedback - Hoist 7 verbatim SQL CASE copies into OWNER_KEY_CASE_SQL const (dedup_cte(), get_repo, count_repos_deduped), keeping one literal copy in the v7 migration with a cross-reference comment. - Add 9 unit tests for normalize_owner_key over the full boundary set. - Add LIMIT 1 to get_repo query for single-row intent. - Add non_key_owner_bare_short_does_not_match is_owner regression test. - Update is_owner doc to reflect did:key-only rule. - Document non-key clone_url disk-path constraint in to_response. * test(node): add SQL round-trip test for normalize_owner_key vs OWNER_KEY_CASE_SQL Verifies every boundary value produces identical results from the Rust function and the SQL CASE expression, preventing silent drift. --------- Co-authored-by: Gravirei <[email protected]> |
||
|
Kevin CodexandClaude Opus 4.8
|
adc20f9eff |
feat(node): iCaptcha proof-of-intelligence gate on create_repo + register (#108)
* feat(node): iCaptcha proof-of-intelligence gate on create_repo + register Add an optional gate that requires callers to present an iCaptcha proof (an Ed25519-signed token from icaptcha.gitlawb.com, earned by solving an escalating challenge) on the two spam-prone endpoints. Proofs are verified OFFLINE with ed25519-dalek against the published public key, and bound to the authenticated agent DID so they cannot be shared. Backward-compatible by default: ICAPTCHA_MODE is `off` unless set, so deploying changes nothing. `shadow` verifies and logs would-be rejections but always allows; `enforce` rejects requests lacking a valid, strong-enough proof. If the public key cannot be loaded at startup the gate stays inert (fail safe) so an iCaptcha outage can never break repo creation or registration. - New src/icaptcha.rs: env-driven verifier (OnceLock), /v1/pubkey fetch at startup or ICAPTCHA_PUBKEY override, proof parse + signature + expiry + level + DID-binding checks; mode-aware decide() split out for testing. - Wired into create_repo and register (X-ICaptcha-Proof header). git push (receive-pack) is intentionally NOT gated. - 11 unit tests incl. verification of a real captured proof (JS-signed, Rust-verified), tamper/expiry/wrong-DID/level/missing-header rejections, and the off/shadow/enforce/fail-safe mode matrix. Author: Kevin Codex <[email protected]> * fix(node): address review on icaptcha gate (validate config, bound key fetch) Addresses CodeRabbit and @jatmn review feedback on #108: - parse_mode now returns Option and rejects unrecognized ICAPTCHA_MODE values; init() warns and falls back to off instead of silently disabling the gate on a typo like "enforced". [P2] - ICAPTCHA_REQUIRED_LEVEL: warn on a non-empty unparseable value instead of silently lowering the threshold to the default. [P2] - fetch_key uses a reqwest client with a 5s timeout so a hung /v1/pubkey can never block node startup; still fails inert on timeout/error. [P2] - .env.example: reorder ICAPTCHA_* keys (MODE, PUBKEY, REQUIRED_LEVEL, URL) to satisfy dotenv-linter. [P3] - add parse_mode unit test (rejects junk, accepts documented values). cargo test -p gitlawb-node icaptcha: 12 passed. clippy + fmt clean. Author: Kevin Codex <[email protected]> * fix(node): consume iCaptcha proof jti so a proof can't be replayed Addresses @jatmn P2 on #108: the verifier checked exp/level/sub but never consumed the proof's jti, so one solved challenge was a reusable bearer proof for that DID until expiry, covering every gated create_repo/register in the window. Now, in enforce mode, each verified proof's jti is recorded once and replays are rejected. - db: migration v7 adds icaptcha_consumed_proofs(jti PK, expires_at); new consume_proof_jti (INSERT ON CONFLICT DO NOTHING -> bool) and sweep_expired_proofs. DB-backed so it holds across restarts and instances. - icaptcha: ProofClaims carries jti; verify returns the claims; decide() stays pure and returns Allow | Reject | Consume{jti,exp}; check() is now async and spends the jti via the DB, rejecting replays (enforce only; shadow observes without consuming). - handlers: create_repo/register await check(&state.db, ...). - main: periodic sweep of expired proof rows alongside rate-limit cleanup. - tests: 12 pass incl. decide() yields Consume with the proof's jti. Author: Kevin Codex <[email protected]> * fix(node): consume iCaptcha proof only after request passes validation Addresses CodeRabbit (Major) on #108: check() spent the jti as soon as the proof verified, but create_repo/register ran it before name/DID/existence checks, so a rejected request (bad name, already-exists, wrong DID) permanently burned a valid proof without the action succeeding. Split verification from consumption: - icaptcha: replace check() with verify_request() -> ProofGuard (verifies and rejects invalid/missing proofs early; off/shadow/inert yield a no-op guard) plus ProofGuard::consume(db) which spends the jti right before the write and rejects replays. Guard is #[must_use]. - create_repo/register: verify_request() up front, then consume() immediately before the first write, after cheap validation passes. Replay protection unchanged (consume-before-write). 12 tests pass; clippy clean. Author: Kevin Codex <[email protected]> * fix(node): give iCaptcha rejections a dedicated error code Addresses CodeRabbit on #108: iCaptcha failure paths returned AppError::Unauthorized, which maps to the "not_an_agent" API code and conflates a valid-but-unproven agent with a failed signature. Add a dedicated AppError::IcaptchaProofRequired variant mapped to the "icaptcha_proof_required" code (HTTP 401), and use it for both the missing/invalid-proof and replay paths so clients can distinguish "solve a challenge" from "your signature is invalid". 12 tests pass; clippy + fmt clean. Author: Kevin Codex <[email protected]> * fix(node): gate fork_repo with iCaptcha and cover the proof ledger Address remaining PR #108 review comments: - fork_repo was the third repo-creation entrypoint and bypassed the iCaptcha gate. Verify the proof up front (fail fast on missing/invalid) and consume it only just before the git clone, so cheap rejections (bad name, conflict, withheld subtree) never burn a valid proof — matching create_repo's verify/consume split. - Add #[sqlx::test] coverage for the single-use ledger that the gate's anti-replay value depends on: consume_proof_jti (first-use vs replay vs distinct jti) and sweep_expired_proofs (only expired rows removed, unexpired spent proofs still block replays). - Return 403 Forbidden (was 401) for IcaptchaProofRequired: the caller is authenticated but forbidden without a valid proof. The distinct icaptcha_proof_required code is unchanged. - Recommend pinning ICAPTCHA_PUBKEY in production in .env.example to avoid the startup fetch dependency. The two shadow-mode review comments were already resolved by the earlier verify/consume refactor (decide/verify are IO-free; check_proof_jti gone). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]> |
||
|
beardthelion
|
828dd279a2 |
fix(node): gate repo-listing and stats surfaces on visibility (#97, #99, #101, #104) (#111)
* fix(node): NFC-normalize visibility matcher to close NFC/NFD under-withholding (#101) A deny rule authored in NFC byte-compared unequal to a path committed in NFD (or vice versa), so blob_paths/withheld_blob_oids failed to classify the blob and it shipped in cleartext on the replication/pack path. Normalize both the glob prefix and the candidate path to NFC at the single matcher seam (glob_matches + specificity); NFC not NFKC so compatibility forms are not folded into distinct paths. Live serve is unaffected (git resolves byte-exact; the gate only becomes more restrictive). Adds a matcher unit test and a replication-path integration test with the variant byte inside the rule-covered directory name. * fix(node): fail closed on the full-scan pin path to stop dangling-blob leak (#99) The withheld set is computed over reachable objects (rev-list --all + HEAD), but the full-scan pin fallback (cat-file --batch-all-objects) includes dangling objects the reachable walk never classified. Subtracting the reachable-only withheld set therefore could not catch a private blob orphaned by a force-push/amend, which pinned to IPFS/Pinata in cleartext. Invert the default for blobs on the full-scan path: a candidate replicates only if it is a non-blob (commits/trees are structural) or a reachable, visibility-allowed blob; anything else (a dangling blob, or a withheld one) is dropped. The delta path keeps the cheaper reachable-only subtraction since delta candidates are always reachable, so no extra walk on the hot path. Adds list_all_objects_with_type/all_blob_oids (typed enumeration), replicable_blob_set + replicable_objects_fail_closed, and threads a full_scan flag through resolve_candidates_for_push. Pin functions now take a pre-filtered object list. Regression tests: dangling blob excluded end-to-end, fail-closed unit semantics, and the all-blob universe includes dangling + excludes non-blobs. * fix(node): gate repo-listing surfaces on visibility to stop private-repo enumeration (#97) list_repos, list_federated_repos, and the GraphQL repos query returned every repo row with no visibility check, so an unauthenticated caller could enumerate private repos' name, owner, description, and clone URL — and X-Total-Count leaked the private-repo count even when rows were filtered. Each surface now keeps a row only when visibility_check at "/" allows the caller, the same decision the per-repo content endpoints make — not a bare is_public filter, which would diverge for is_public=false-with-root-reader and is_public=true-with-root-deny. The "/" decision depends on owner short/full-DID matching and JSON reader-DID membership, so it is applied in Rust over an over-fetched set rather than pushed into SQL; the paged path derives X-Total-Count from the filtered set (dropping the SQL window count) and paginates after filtering, so neither the page nor the count leaks. Adds a batch root-rule query (list_visibility_rules_for_repos) to avoid N+1, and converts list_all_repos_paged into list_all_repos_deduped (mirror dedup preserved, pagination moved to Rust). Tests: anonymous hides private repo + count, owner sees own private repo, authorized root-reader sees an is_public=false repo (proving the visibility_check path, not is_public), and federated listing hides private from anonymous. * fix(review): share the listing visibility seam, extract fail-closed pin helper, add listing tests Code-review follow-up on the VIS-subtree fixes (no behavior change to the three shipped fixes; all confirmed sound by review): - DRY the '/' listing gate into one shared visibility::listable_at_root used by list_repos, list_federated_repos, and the GraphQL repos query, so the three surfaces enforce one rule instead of three drifting copies (enforce-shared-rule-on-every-reader-path). - Extract the full-scan fail-closed pin block into fail_closed_full_scan_objects, mirroring replication_withheld_set's degraded-path shape; drop the duplicated comment and the per-site no_rules sentinel (use &[]). - Add the three listing tests the review flagged as missing: anonymous GraphQL repos hides a private repo, the paged X-Total-Count path excludes the private count (the KTD2 exploit shape), and an is_public=true repo under a root deny is not listed (proves the gate is visibility_check, not a bare is_public filter). * fix(review): route the last inline root-visibility check through listable_at_root Round-2 review follow-up. replication_withheld_set still made the announce decision with an inline visibility_check(.., "/") == Allow — the fourth copy of the listing gate the seam refactor was meant to collapse. Route it through crate::visibility::listable_at_root (caller is always None, the anonymous replication audience), so every reader of the '/' decision now shares one seam. Also drop an em dash from the fail_closed_full_scan_objects doc comment. * fix(node): gate the /api/v1/stats repos count through visibility to close the count oracle (#104) The stats endpoint returned count_repos_deduped(): an unfiltered, unauthenticated count of every logical repo including is_public=false and mode-A hide-existence repos. A polling observer could detect that a hidden repo was created (N to N+1), the exact existence signal mode-A suppresses everywhere else. Count only the repos an anonymous caller could list: over-fetch the deduped set, batch-load visibility rules, and keep rows that pass listable_at_root(.., None), mirroring the listing seam in api/repos.rs. The caller is always None (meta_routes carries no auth layer). Fail closed: any DB error collapses the count to 0, since an under-count cannot leak existence. count_repos_deduped is retained as the tested reference for the DEDUP_CTE dedup count (its tests pin the CASE the live list paths share); it has no production caller after this change and is allowed dead outside tests. Tests: bare-private (no-rule branch), hide-existence mode-A with both repos is_public=true (the Some(rule) branch), public-under-root-deny, list-parity, sibling-fields-preserved, and empty-DB. * test(review): add listing visibility regression guards (#97) Three gaps the review flagged as uncovered: - owner-filtered listing (?owner=) still hides a private repo and its count from anonymous (a distinct SQL bind branch from the unfiltered path). - offset past the visible set returns an empty page while X-Total-Count stays the full visible total (guards against deriving the total from the cut page). - a logical repo present as both a canonical row (with a root-deny rule) and a gossip mirror row stays hidden: pins that the DEDUP_CTE picks the canonical survivor so the batch rule lookup finds the deny. If dedup ever picked the mirror (slash-form id, no rule) the gate would fall back to is_public and leak. * fix(review): address CodeRabbit feedback on #111 - visibility_pack NFC/NFD test: accept a SHA-1 or SHA-256 object id (40 | 64) instead of hardcoding 40, matching the fixture guard later in the file, so the test is not SHA-1-only. - federated listing test: also assert the response `count` reflects only the visible repos, guarding against a regression to the pre-filter total. - add an authenticated-caller GraphQL test: an owner sees their own private repo via `repos`, covering the resolver's caller-DID path that the anonymous-only test did not exercise. - stats: count via the no-stars `list_all_repos_deduped()` (same DEDUP_CTE) instead of the stars-joined variant, since the count never needs stars. * fix(review): match owner filter to did_matches for bare-owner mirror rows (#111) Addresses jatmn's P2. The shared listing CTE filtered owners with `owner_did = $1 OR owner_did LIKE '%:' || $1`, so a full `did:key:z...` query did not match a mirror-only row stored with the bare owner `z...`. #97 routed the no-limit owner-filtered path (used by `gl repo list`) through this SQL predicate, regressing the full-DID-against-bare-mirror direction that `crate::api::did_matches` handled in the old Rust path. Normalize the bound owner to its did:key short form and compare it against the row's owner key using the same CASE the dedup grouping already applies, so full and short forms match symmetrically and a non-key DID still matches only exactly. The no-arg `list_all_repos_deduped()` binds NULL and is unaffected. Regression test: a bare-owner mirror-only row is returned when queried by both the full `did🔑` form and the bare short form. Confirmed RED (test fails) before the fix. |
||
|
beardthelion
|
636d3abe17 |
chore(deps): bump tar to 0.4.46 and rand to 0.9.3 to clear advisories (#105)
tar 0.4.45 had a PAX header desynchronization issue (GHSA-3pv8-6f4r-ffg2, fixed in 0.4.46); rand 0.9.2 was unsound with a custom logger using rand::rng() (GHSA-cq8v-f236-94qc, fixed in 0.9.3). Both resolve as lockfile-only bumps with no manifest change. Build and cargo audit pass. lru 0.12.5 (GHSA-rhfx-m35p-ff5j) stays pinned by aws-sdk-s3 (lru ^0.12.2) and the hickory advisories remain accepted risk per .cargo/audit.toml (#76). |
||
|
beardthelion
|
2d4105e479 |
chore(deps): bump quinn-proto 0.11.14 -> 0.11.15 (RUSTSEC-2026-0185) (#83)
quinn-proto 0.11.14 (pulled transitively via libp2p's QUIC transport) is flagged by RUSTSEC-2026-0185. 0.11.15 is a semver-compatible patch that clears it; cargo audit passes clean after the bump. Lockfile-only, no manifest or code change, so it carries no ignore entry. Repo-wide: unblocks cargo audit on main and every open branch. |
||
|
beardthelion
|
803d83efdb |
fix(node): reap leaked git child processes (#53) (#61)
* fix(docker): reap orphan processes via tini + init, cap pids The node runs as PID 1 in the container and never reaps reparented orphans, so git grandchildren (e.g. pack-objects left behind when a served clone disconnects) accumulate as zombies until fork() fails with EAGAIN. Run under tini and set compose init: true so reparented orphans are reaped, and add pids_limit as a blast-radius cap. Refs #53 * fix(node): reap leaked git child processes (#53) Serving git over smart-HTTP could leak zombie processes that, over weeks, exhaust the PID table until fork() fails with EAGAIN and all git operations break until restart. The serve path (run_git_service) now runs git in its own process group and SIGKILLs that group when the request future is dropped (client disconnect) or returns early, so git and its pack-objects child are torn down together instead of orphaning the grandchild to PID 1 as a zombie. A drop guard, disarmed once the request completes cleanly, owns this teardown. create_issue and build_filtered_pack spawned a std Child and dropped it unwaited if the stdin write failed (and build_filtered_pack could panic on a missing stdin handle). Both now always wait the child before surfacing any error, so the failure path cannot leak. Adds libc for the group-kill syscall and three regression tests covering direct-child kill, grandchild reaping via the group, and the disarmed no-kill path. Refs #53 * fix(review): reap-then-disarm on serve-path errors; tini/comment nits Address code-review findings on the #53 fix: - run_git_service no longer early-returns on a stdin-write error. It captures the write result, always reaps the child, then disarms the group guard before surfacing the error. Previously a write/wait error path could drop the still-armed guard after the child was already reaped, so the group SIGKILL could in theory land on a reused pgid. (correctness, reliability, adversarial all flagged this region.) - Drop tini's -g flag: the node manages its own children and graceful shutdown, so group-wide signalling would only disturb in-flight git helpers on shutdown. - Correct the SAFETY comment (kill(2) is not relevant to async-signal-safety here) and the alive() test-helper comment (EPERM does not read as alive). * fix(node): SIGTERM (not SIGKILL) the served-git group on disconnect SIGKILL gave git no chance to run its signal cleanup, so an aborted receive-pack could leave a stale .git/*.lock that blocks the next push. SIGTERM lets git remove its lock files before exiting. Signalling the whole process group still reaches pack-objects directly, so the grandchild is terminated rather than orphaned, and a single immediate signal avoids any delayed-kill pid-reuse window. Refs #53 * docs(node): correct stale SIGKILL comments to SIGTERM in served-git teardown |
||
|
beardthelion
|
7533b74f79 |
ci: harden PR pipeline (audit, MSRV, beta matrix, release + docker smoke) (#57)
* ci: harden PR pipeline (audit, MSRV, release build, beta matrix, docker smoke) Restructure the single stable-only fmt+clippy+test job into separate jobs: fmt+clippy (stable), a test matrix on stable+beta (beta non-blocking), build --release, cargo audit, an MSRV check pinned to Rust 1.91, and a Docker build smoke test mirroring the release workflow. Add concurrency cancellation for superseded PR runs and per-job timeouts. Declare rust-version = "1.91" in [workspace.package] and inherit it in every member crate so the stated MSRV is a real, CI-enforced contract. * ci: disable credential persistence on checkout steps Set persist-credentials: false on all six actions/checkout steps. These jobs run untrusted PR code via cargo and docker build, and checkout's default leaves the GITHUB_TOKEN in the local git config where that code could read it. None of the jobs push or otherwise need git credentials. * fix(deps): bump rustls-webpki to 0.103.13 Clears three advisories flagged by the new cargo audit job: a reachable panic in CRL parsing and two name-constraint bypasses (wildcard and URI). Semver-compatible patch, lockfile only, no manifest or code change. cargo also corrects a stale lockfile reference while rewriting Cargo.lock: data-encoding-macro-internal 0.1.17 (a build-time macro crate) requires syn 1.x, and its dependency entry is re-pinned from syn 2.0.117 to the 1.0.109 it actually resolves to. Unrelated to the bump, unavoidable when cargo touches the lock, and the more-correct state. * ci(audit): suppress no-fix advisories with justification and a re-check The cargo audit job stays hard-fail. The remaining three advisories have no available upstream fix, so they are suppressed in .cargo/audit.toml (the location cargo-audit actually reads; a repo-root audit.toml is not read), each with an inline rationale and removal condition: - RUSTSEC-2026-0118/0119 (hickory-proto 0.25.2, via libp2p-dns 0.44): reachable, since the node wraps QUIC in the system DNS resolver. No fix until libp2p-dns ships hickory 0.26. Accepted, reachable risk. - RUSTSEC-2023-0071 (rsa Marvin): in the lockfile via sqlx's unconditional sqlx-mysql, but not linked (sqlx is postgres-only), so no RSA oracle exists; no upstream fix regardless. Two controls keep the suppression honest. The PR audit job echoes the active ignore list to the job summary, so a green check is not read as advisory-clean. A weekly scheduled job re-runs cargo audit without the ignores for visibility and hard-fails if hickory-proto moves off 0.25.x while the ignores remain, so the suppression cannot silently outlive the upstream fix. * ci(audit): reference tracking issue #76 in the hickory ignores * ci(audit): treat hickory-proto removal as drift too The scheduled drift guard only failed when hickory-proto was present and off 0.25.x, so removing it from Cargo.lock entirely left the RUSTSEC-2026-0118 /0119 ignores in place with the guard still passing. Treat an absent version as drift as well, so dead ignore entries are caught whether the dependency is bumped past 0.25.x or removed. |
||
|
beardthelionandCommandCodeBot
|
5ff7af84fa |
feat: encrypted replication for private subtrees (B1/B2/B3) for #18 (#36)
* feat(core): ed25519 to x25519 key conversion for envelope crypto * feat(core): seal/open per-blob encryption envelopes * feat(node): resolve per-blob encryption recipients (least-privilege) * feat(node): encrypted_blobs table and recipient-scoped queries * feat(node): encrypt-then-pin withheld blobs at push (IPFS) At the push chokepoint, after pinning withheld objects, resolve each withheld blob's recipient DIDs and seal it to their Ed25519 keys with seal_blob, pinning the ciphertext to IPFS and recording it in encrypted_blobs. Best-effort per blob: failures are logged and skipped, never pinned in plaintext. Pinata replication is unchanged; B1 encrypts to IPFS only. Adds ed25519-dalek as a direct dependency of gitlawb-node (it was only declared in the workspace Cargo.toml). * feat(node): authenticated discovery and fetch for encrypted blobs * feat(gl): transparent recovery of authorized encrypted blobs on clone * style: cargo fmt * fix(node): re-seal encrypted blob when recipient set changes Skip re-pinning only when an existing envelope already covers exactly the current recipients. A reader added to a rule after the first pin now gets a re-seal on the next push instead of being permanently locked out. Reader removal stays non-retroactive (the old envelope is already public). * feat(node): unscoped encrypted-blob listing for replication * feat(node): encrypted-blobs/replicate endpoint for peer mirrors * feat(node): route encrypted-blobs/replicate under git_read_routes * feat(node): peer mirrors replicate encrypted withheld blobs (Option B2) * feat(node): anchor_encrypted_manifest for Option B3 Arweave manifests * feat(node): encrypt_and_pin returns the per-push sealed delta * feat(node): anchor encrypted-blob manifest on push (Option B3) * feat(gl): Arweave/IPFS gateway recovery for encrypted blobs (Option B3) * feat(gl): transparent Arweave fallback recovery on clone (Option B3) * style: cargo fmt * fix(gl): bound Arweave recovery gateway requests with a 30s timeout * test(gl): read-path recovery tests for Arweave fallback; quiet promisor present-check Add two hermetic integration tests for recover_from_arweave that drive the full read path over mocked Arweave GraphQL + IPFS gateways: discover the manifest, fetch it, fetch the envelope, decrypt, and install the withheld blob. One covers an authorized recipient (blob installed), the other a non-recipient (nothing recovered). Both simulate origin death by removing the promisor remote and enable uploadpack.allowFilter so the blob is truly withheld over file://. Also harden the local presence check in recover_from_arweave with GIT_NO_LAZY_FETCH=1 and .output() so the expected 'missing object' case does not trigger a wasted promisor fetch or leak git stderr to the user. * fix: surface corrupt recipients JSON and silent recovery I/O failures db: parse_recipients now propagates a descriptive error instead of defaulting corrupt recipients JSON to an empty list, which would have denied authorized readers and handed peers incomplete metadata. gl: clone recovery now warns when the sparse-checkout file cannot be read or written, or when the post-recovery checkout fails, instead of silently discarding those errors and claiming files were recovered. * feat(core): blind recipient identities in withheld-blob envelopes Envelope v2 drops the cleartext recipient public key (kid) from each wrapped-key header entry, so a party holding the public IPFS/Arweave copy can no longer enumerate who is authorized to decrypt a withheld blob. The version is bumped to 2 and v1 envelopes are rejected. open_blob now selects the reader's entry by trial decryption (the AEAD tag authenticates exactly one entry) instead of matching on the public key. Recipient count is still visible; identity is not. Scope is envelope-only, the node DB recipients column and peer replication metadata are unchanged. Co-authored-by: CommandCodeBot <[email protected]> * fix(core): reject malformed envelope nonces instead of panicking open_blob fed attacker-controlled envelopes to GenericArray::from_slice and XNonce::from_slice, which panic on a wrong-length input. Validate both the per-recipient box nonce and the body nonce to 24 bytes before use: skip a recipient entry whose nonce is malformed, and return an error for a malformed body nonce, so the public recovery path surfaces an error rather than panicking. * feat(node): blind recipient identities on the B2 replication surface The encrypted-blobs/replicate endpoint shipped every blob's full recipient DID list to mirrors, which persisted it, so any mirroring peer learned the reader set. The v2 envelope blinding already removed recipient public keys from the pinned envelopes, so the comment justifying this (DIDs are already public) was no longer true. /replicate now returns {oid, cid} only. Mirrors detect a re-seal by the CID changing (the OID is stable across re-seals) instead of comparing recipient sets, and store no recipient identities. Origin-side authz and the at-rest recipients column are unchanged; this blinds only the peer-facing surface. * feat(node): stop anchoring recipient identities to Arweave The B3 encrypted-blob manifest anchored {oid, cid, recipients} to Arweave, a permanent public record of every blob's reader set. The v2 envelope blinding already removed recipient keys from the pinned envelopes, so the comment justifying this (recipient DIDs are already public) was no longer true. The manifest now anchors {oid, cid} only. The gl reader already ignores the recipients field (it recovers by trial decryption), so no reader changes. --------- Co-authored-by: CommandCodeBot <[email protected]> |
||
|
Kevin CodexandClaude Fable 5
|
c7df6cad20 |
Fix/backport prod fixes (#39)
* fix: backport production fixes from monorepo Three fixes that landed on the production node but never made it here: - trigger_sync: 30s timeout with the body read inside it (was 5s covering headers only — large unpaginated repo lists from canonical nodes and transpacific peers aborted mid-body), plus distinct logging for non-2xx vs timeout - announce: reject self-announcements by public URL or own DID; complements the existing boot-time prune_self_peers so self-loop rows never enter the peers table in the first place - list_all_repos_paged: report MAX(updated_at) across each logical repo group instead of the canonical row's own value — gossip pushes touch only the mirror row, so canonical timestamps go stale Monorepo refs: 8c2fe1b, c1dc33e, ef21c75. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(p2p): dual-stack QUIC listen + DNS multiaddr dialing for Fly 6PN Backports the monorepo TCP-era p2p fixes (9f875b6, 267d371) adapted to the QUIC transport: - listen on /ip6/:: as well as /ip4/0.0.0.0 — Fly's 6PN inter-app network is IPv6-only (<app>.internal resolves to AAAA records), so without an IPv6 socket peers on the private network can't reach us - wrap the QUIC transport in libp2p-dns so bootstrap multiaddrs can use /dns6/<app>.internal — dialing peers through the public anycast edge breaks the handshake when the proxy closes the connection mid-stream Co-Authored-By: Claude Fable 5 <[email protected]> * fix(sync): HTTP notify records received_ref_updates like the gossip path A push propagated via the /api/v1/sync/notify HTTP fallback replicated the repo but left no trace in received_ref_updates — only the libp2p gossip path recorded it, so /api/v1/events/ref-updates went blind whenever the mesh wasn't delivering. Insert the same record from the notify handler, with optional provenance fields (pusher_did, old_sha, timestamp, cert_id) that new senders already include. Monorepo ref: 087eeac. Co-Authored-By: Claude Fable 5 <[email protected]> --------- Co-authored-by: Claude Fable 5 <[email protected]> |
||
|
Vasanth TandClaude Opus 4.8
|
2ce4da9cc5 |
feat(node): graceful shutdown + Prometheus metrics endpoint (#22)
* feat(db): versioned schema migrations with idempotent backfill Replace the inline &str SQL array that re-runs on every node startup with a versioned migration system. Each migration is recorded in a new schema_migrations table and applied at most once per node, inside a single transaction. For nodes upgrading from a pre-migration-versioning build, the canonical `repos` table is used as a signal to mark v1 as already applied without re-running its ~140 statements — so existing operators see zero behavior change on first restart after this commit. Closes the gap called out in docs/OSS-READINESS-AUDIT.md:78. - Adds Db::migration_status() returning applied (version, name, applied_at) - Adds 6 unit tests validating the static migration catalogue (versions strictly increasing, names distinct, bodies non-empty, v1 name locked to initial_schema) - cargo fmt + clippy -D warnings + cargo test --workspace all clean * fix(db): make legacy migration backfill safe and lock-guarded Address two robustness gaps in the v1 migration runner on the live network: - Remove the "repos table present => v1 complete" backfill short-circuit. It assumed every existing node already had the full current schema; a node that was behind would be marked v1-applied while still missing newer tables or columns. Every v1 statement is idempotent (CREATE TABLE/INDEX IF NOT EXISTS, ADD COLUMN IF NOT EXISTS), so legacy installs now simply run v1 once: existing objects are no-ops, missing ones get created. - Guard the whole runner with a Postgres advisory lock so two instances against the same database (blue/green or rolling deploy) can't race to apply the same migration and trip the schema_migrations primary key. The lock is released explicitly, and automatically if the connection drops. Also document that per-migration transactions preclude CREATE INDEX CONCURRENTLY in future migrations. Co-Authored-By: Claude Opus 4.8 <[email protected]> * feat(node): graceful shutdown + Prometheus metrics endpoint Two operational-safety changes that the live network needs: 1) Graceful shutdown. A process-wide tokio::sync::watch::Sender<bool> in AppState is flipped on SIGINT (all platforms) or SIGTERM (Unix). Every long-lived loop now selects on the shutdown arm: * axum server (with_graceful_shutdown — drains in-flight) * libp2p swarm (drops Swarm, closes connections) * gossip task (exits between peer announces) * sync worker (exits between batches) * operator heartbeat loop * rate-limit cleanup loop * peer-count metrics poller * the deferred DID-record publish task New GITLAWB_SHUTDOWN_GRACE_SECS (default 30s) controls how long axum waits for in-flight requests before returning. 2) Prometheus /metrics endpoint. New metrics module exposes: * gitlawb_info{version, did} (constant gauge = 1) * gitlawb_pushes_total{repo} (counter) * gitlawb_fetches_total{repo} (counter) * gitlawb_auth_successes_total{route} (counter, helper) * gitlawb_auth_failures_total{route,reason} (counter, helper) * gitlawb_sync_queue_processed_total{status} (counter) * gitlawb_webhook_deliveries_total{result} (counter) * gitlawb_pack_size_bytes (histogram) * gitlawb_peers_connected (gauge) Opt-in via GITLAWB_METRICS_ADDR (e.g. 127.0.0.1:9091). The endpoint is on a separate listener so the public port stays unexposed; bind to localhost or a private interface. 3) Metric instrumentation hooks in api/repos.rs (push/fetch), webhooks.rs (delivery outcomes), and sync.rs (per-item status). The auth helpers are defined but not yet wired — that's a focused follow-up. Tests: * 3 new metrics::tests::* — encode returns valid prometheus text, record_* helpers are no-ops before init(), encode() errors if init() was never called. * cargo fmt + clippy -D warnings + cargo test --workspace all clean. Closes the metrics and graceful-shutdown gaps in docs/OSS-READINESS-AUDIT.md:131 and docs/MAINTAINER-ROADMAP.md:35. --------- Co-authored-by: Claude Opus 4.8 <[email protected]> |
||
|
Achille Wasque
|
924bccd8e5 |
feat(gitlawb-attest): External Attestation v1 for ref-update certs (#20)
Adds an optional `attestations` field on ref-update certs plus a small
crate to sign and verify them. An envelope with no attestations
serializes to identical bytes as a bare cert, so nothing on the wire
changes for nodes that don't opt in.
Each attestation carries a `type` discriminator (`covenant/exec/v1`,
`slsa/v1.0`, `sigstore/dsse/v1`, ...), an opaque payload, a `cert_hash`
that binds it to one specific cert, a `did:key` signer, and a base64url
ed25519 signature over a domain-separated JCS-encoded input.
## Wire shape
cert_hash = SHA-256(JCS(cert_body \ {signatures, attestations}))
encoded as lowercase hex.
signing_input = b"gitlawb-attest-sig/v1\n"
|| JCS({type, payload, cert_hash})
sig = base64url-no-pad(Ed25519(signing_input))
The cert hash is reproducible across implementations because JCS pins
key order, number formatting, and string escaping (RFC 8785). Lowercase
hex is required on the wire: the JCS signing input includes `cert_hash`
verbatim, so accepting mixed case would force every signer and verifier
to normalize before computing the signing bytes — an easy spec rule to
miss across language ports.
The domain tag lives in the byte stream, not in the canonical JSON, so
an attestation cannot be confused with a same-shape JCS document signed
for a different protocol that uses the same Ed25519 key.
Signers are `did:key` over Ed25519 with the multibase base58btc (`z`)
prefix. Non-base58btc encodings of the same public key are rejected on
verify so allowlists and signer-pinning policies stay consistent across
peers.
## Verification
`Registry` maps type to verifier. `Policy::AcceptKnown` (default) lets
unknown types pass without trust so adoption stays incremental.
`Policy::RequireAll` enforces a per-repo allowlist but stays lenient on
unknown types in the batch — anyone can attach an attestation, so
short-circuiting on an unrelated `attacker/spam/v1` would be a DoS
vector. `Policy::RejectUnknown` is strict.
`AttestedRefUpdateCert::verify_attestations(&Registry)` is the one-call
convenience over `registry.verify_all(&env.attestations, env.cert_hash()?)`.
## Canonical test vectors
Ed25519, JCS, and SHA-256 are all deterministic, so a fixed seed plus a
fixed cert body produce exactly the same `cert_hash`, `signer` DID, and
`sig` across implementations. `tests/canonical_vectors.rs` pins these
values so a port to Go, JS, Python, or any other language has a concrete
oracle to check against without reading Rust. If a vector drifts, that
is a wire-protocol break.
## Tests
42 pass: 33 unit + 4 cross-crate integration against the real
`gitlawb_core::cert::RefUpdateCert` + 4 canonical vector pins + 1
rustdoc quick-start doctest.
`cargo fmt --all -- --check` clean.
`cargo clippy --workspace --all-targets -- -D warnings` clean.
`cargo test --workspace` green; the new crate breaks no existing suite.
## Not in this PR
- Node integration: storage, GraphQL, register flow.
- UI surfacing.
- Trust-score weighting policy for typed attestations.
- Rollout ordering: old nodes silently drop the new `attestations`
field (`RefUpdateBody` does not set `deny_unknown_fields`); signers
and verifiers roll out independently.
- wasm / no_std: not targeted; ed25519-dalek and serde_json are
std-only.
## Reference implementation
End-to-end demo at open-covenant/covenant@64ec780,
`agent-os/examples/gitlawb-attest-demo/` (`covenant/exec/v1`).
Closes #7.
|
||
|
Vasanth T
|
0b056f3ad0 |
[codex] fix Dependabot Rust alerts (#15)
* fix(deps): resolve dependabot rust alerts * docs: clarify dependabot validation |
||
|
Kevin CodexandClaude Opus 4.7
|
14a1cdd656 |
chore: bump v0.3.9, fix all clippy warnings, enforce strict CI lint (#12)
* chore: bump v0.3.9, fix all clippy warnings, enforce strict CI lint - Bump workspace version 0.3.8 → 0.3.9 - Fix 22 clippy warnings across 17 files: - Replace .last() with .next_back() on DoubleEndedIterators (8 instances) - Remove useless .into() conversions on anyhow::Error (3) - Remove needless borrows in gl init (3) - Replace map_or with is_some_and/is_none_or (3) - Collapse consecutive str::replace into char array pattern (2) - Change &PathBuf to &Path in function signatures (2) - Remove dead code: unused init_bare_repo function - Collapse if-let into outer match arm - Replace vec![...] with array literal - Introduce RecordAnchorInput struct (10 params → 1 struct) - Fix always-true boolean condition in gossip task guard - Promote clippy from advisory to strict (-D warnings) in CI Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * style: cargo fmt Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> |
||
|
Kevin CodexandOpenClaude
|
6b9c5278d4 |
chore: initial public release
Open-source release of the gitlawb node, CLI, and git remote helper. - gitlawb-node: Axum HTTP server with git smart-HTTP and DID auth - gl: command-line client (identity, repos, PRs, MCP server) - git-remote-gitlawb: git remote helper for gitlawb:// URLs - gitlawb-core: shared crypto primitives (DID, CID, HTTP sigs, UCAN) Dual licensed under MIT or Apache-2.0. Co-Authored-By: OpenClaude <[email protected]> |