22 Commits

Include renames

Author	SHA1	Message	Date
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]>	2026-11-11 02:49:14 +00:00
3 people	a2262d92b2	fix(node): implement reconciliation sweep as durability backstop (#218) (#551) ... * feat(node): implement reconciliation sweep as durability backstop (#218) Reconciliation sweep: periodic background worker that re-derives pin/seal sets and fills gaps so a dropped replication job never means data loss. - visibility_pack.rs: all_object_paths (two-phase ls-tree + cat-file --batch-all-objects), allowed_blob_tree_sets_bounded, ObjectPath/BlobTreeSets type aliases - db/mod.rs: atomic policy transactions, get/set_node_state, has_ipfs_cid, filter_ipfs_pinned_oids, filter_pinata_pinned_oids, repo_policy_epoch, migration v27 (node_state table + policy_epoch column) - ipfs_pin.rs: PolicyFence struct, dispatch fence re-check before POST - pinata.rs: dispatch fence re-check before POST - encrypted_pin.rs: dispatch fence re-check - reconciliation.rs: full sweep worker with keyset cursor, PolicyFence - api/ipfs.rs: effective_cid fallback in list_pins - main.rs: spawn reconciliation sweep worker - .cargo/audit.toml: RUSTSEC-2026-0258 h2 ignore (pending #368) * fix: add missing parameters to pinning functions and adjust timeout settings * fix(tests): update CID assertions to handle Option type and adjust parameters in pinning functions * fix(node): close review findings on reconciliation sweep (#218) Reviewer-1 P1 / Reviewer-2 P1: v27/v28/v29 were referenced by tests but never added to the MIGRATIONS array, so every existing deployed node would fail to boot (node_state, repos.policy_epoch, pinned_cids.cid NOT NULL → NULL all unreadable). Forward-only migrations: v27 pinata_only_clear_legacy_equal_cid: ALTER cid DROP NOT NULL, UPDATE cid = NULL WHERE cid = pinata_cid v28 node_state_key_value: new key/value table for the sweep cursor v29 repos_policy_epoch: BIGINT NOT NULL DEFAULT 0 Reviewer-2 P1: visibility_pack.rs:542 phase 1 used ls-tree -rz, which under -r recurses into blobs and never emits tree entries. Trees then arrived only from the phase-2 catch-all with empty path and the fail-closed filter denied every one — the sweep could repair a non-flat repo's git graph. Switched phase 1 to ls-tree -r -t -z and add a per-commit rev-parse <commit>^{tree} that registers the root tree at "/". 48 visibility_pack unit tests pass. Reviewer-1 P2 / Reviewer-2 P2: list_pinned_cids called is_raw_cidv1 on the cid column before decoding it as Option<String>, so SQL NULL failed the decode and the new Pinata-only rows never reached the handler. Decode cid as Option first, drop only the both-NULL case. The is_raw_cidv1 filter was removed entirely: the new contract lists every row that has something to advertise, and the handler at api/ipfs.rs:list_pins is the seam that decides what to do with a legacy-shape row (the resolver 404s on a mismatched key, per #173 U4). Updated list_pinned_cids_omits_unrepaired_legacy_row to match. Reviewer-1 P2: two new sqlx::test cases pin the gate order: sweep_skips_quarantined_repos_before_scan (SQL filter, plus a defense-in-depth per-row re-check) and sweep_skips_private_repos_before_scan (per-repo listable_at_root). Both use expect(0) on the mock IPFS so any future reorder that moves the scan ahead of the gate fails the build. Also: record_pinned_cid ON CONFLICT now rewrites cid (R1-P2: a stale wrong CID from a previous push is repaired by a subsequent one). record_pinata_cid ON CONFLICT clears cid to NULL when the existing row has the legacy cid = pinata_cid shape; on INSERT, stores cid = NULL when raw_cid == pinata_cid (the Pinata-only signal). * feat: implement per-backend continuation offset for missing OIDs - Enhance `missing_oids` function to accept a `start_after` parameter, allowing for rotation of the missing set based on the last attempted OID. - Update `run_pass` to load and save the continuation offset for both IPFS and Pinata backends, ensuring that previously attempted OIDs are retried last in subsequent passes. - Introduce tests to validate the behavior of the new offset mechanism, ensuring deterministic ordering and proper handling of gaps in the missing set. - Ensure that the offset is cleared when no missing OIDs are found, allowing for fresh starts in subsequent passes. * fix(node): trace provenance/structural invariants through every consumer The v30 local_ipfs_provenance column was set by the writers but only consulted by has_ipfs_cid and filter_ipfs_pinned_oids. The pin loop's "is this already done" check still used row existence (is_pinned), so a Pinata-only row hit the early-return branch without ever reaching the local writer, and the flag stayed FALSE forever. Three other seams carried the same shape-vs-provenance confusion. P1 (ipfs_pin.rs:1843): pin loop's skip keys on has_ipfs_cid (writer-owned local_ipfs_provenance = TRUE), not on row existence. A Pinata-only row now falls through to the local writer path, which calls record_pinned_cid_with_source and flips the flag on the conflict branch. The Pinata pin loop's skip remains keyed on has_pinata_cid (a separate seam). P1 (db/mod.rs v30 backfill): tighten the WHERE clause to cid IS NOT NULL AND pinata_cid IS NULL. The previous permissive (cid <> pinata_cid) clause marked pre-v30 Pinata-first uploads (where record_pinata_cid deliberately stored cid = raw_cid without a local Kubo push) as local IPFS pins, so the sweep filtered them as already-durable and an operator enabling IPFS later never got a local recovery copy. Ambiguous dual-shape rows stay at FALSE under the strict rule and the sweep re-derives on re-pin (cheap, idempotent). P1 (db/mod.rs record_pinned_cid_with_source ON CONFLICT): restore cid = COALESCE(pinned_cids.cid, EXCLUDED.cid). A Pinata-only row with cid = NULL (the raw_cid == pinata_cid shape) would otherwise be marked complete with local_ipfs_provenance = TRUE and a NULL resolver key. P1b (visibility_pack.rs): remove the synthetic "/" path on root trees and add a structural entry-level check (structurally_safe_root_tree / _for_caller) in both the sweep gate (allowed_blob_tree_sets_bounded) and the per-request tree gate (allowed_tree_set_for_caller_bounded). A tree is admitted only if every direct entry in its serialized bytes is safe at /{filename} and (for tree entries) the child tree is itself in the allow set. The previous path-based "/" check let the root tree of a public repo slip through even when its bytes name a denied subtree entry. P2 (api/ipfs.rs list_pins): add local_ipfs_provenance to PinnedCidRecord, include in list_pinned_cids SELECT, and surface as local_pinned / pinata_pinned in the JSON response. A Pinata-only row previously returned the raw resolver key as local_cid, making it indistinguishable from a real dual-backed row. The new fields let a gl consumer distinguish local-only, remote-only, and dual rows without re-inferring semantics from nullability. local_cid is preserved for backward compatibility. Tests: - migration_v30_backfills_local_ipfs_provenance_heuristically: flipped both-distinct expectation to FALSE under strict rule. - migration_v12_makes_cid_nullable_and_preserves_classification and migration_v27_clears_legacy_equal_cid: both-distinct / distinct row expectations flipped to FALSE. - sweep_never_pins_root_tree_naming_withheld_subtree (new): assert root tree + secret subtree tree absent from pinned_cids. - allowed_tree_set_gates_withheld_subtree_tree: root tree assertion inverted to EXCLUDED for anon. - list_pins_reports_writer_owned_provenance_for_all_shapes (new): assert 4 row shapes produce correct local_pinned / pinata_pinned JSON. Scope discipline preserved: PolicyFence capture order, _pin_permit reuse for the seal phase, fresh per-arm rederive_budget, keyset repo cursor with lookahead, mirror-row skip, Kubo 2xx-without-Hash refusal, v27 provenance-clearing migration all untouched. v31 schema unchanged. * fix(node): structural tree gate at every depth + non-commit ref tolerance The prior round installed a `local_ipfs_provenance` column and applied a structural tree gate only to root trees. The fresh review pass found two structural gaps the writer contract didn't propagate to: P1 (visibility_pack.rs:1519): the path-based tree admission admitted a tree whenever its path was allowed, regardless of whether its serialized bytes named a denied subtree. Concrete failure: a public repo with `/public/secret/file` and a `/public/secret/**` deny admitted the `/public` tree (its path is `/public`, allowed) but `/public` still contained a `secret/` entry pointing at the denied subtree — anyone who obtained `/public`'s CID could `cat-file -p` it and read the subtree's name and child OID. The prior round's post-pass structural check ran on root trees only. The fix: replace the prior round's `structurally_safe_root_tree` / `_for_caller` / `root_tree_oids` helpers with one recursive `tree_structurally_safe` that walks every tree's entries at every depth, recursing on tree entries. Bundled the rule/path args into a `TreeCheckCtx` struct to fix the clippy `too_many_arguments` warnings the prior round introduced. A tree is admitted iff every direct entry is safe at its path AND (for tree entries) the child tree is itself structurally safe there. A denied-subtree tree fails at its level; the parent fails because the denied-path child entry fails; the propagation continues up to the root. The `admitted` set memoizes trees proven safe at some path so the recursion short-circuits on cycles and on trees reachable at multiple allowed paths. P1 (visibility_pack.rs:533): `all_object_paths` called `assert_all_refs_are_commits`, which bailed on any ref that didn't peel to a commit (tag-of-tree, tag-of-blob). The sweep (`run_pass`) converted that error into a full-scan failure that skipped the whole repo. A repo with a pushable tag-of-tree never got its missing public pins repaired by the new hourly worker, even though its commit-reachable graph was otherwise safe to classify (`ipfs_cid_tree_served_despite_non_commit_ref` is the in-repo example of that supported shape). The fix: remove the `assert_all_refs_are_commits` call from `all_object_paths` (and the analogous one in `blob_paths`, used by the encrypted recovery path — the same reasoning applies). `git rev-list --all` already silently skips non-commit refs, so the commit-reachable object set is what the sweep needs. The unused `assert_all_refs_are_commits` helper is removed. Unclassifiable ref targets still fail closed at a different layer: the cat-file catch-all enumerates them with no path, and the path-based allow filter drops empty-path entries. Test inversions and new regressions: - `migration_v12_makes_cid_nullable_and_preserves_classification` and `migration_v27_clears_legacy_equal_cid` — both-distinct / distinct row expectations flipped to FALSE under the v30 strict backfill (prior round's tightening is re-affirmed here; the v30 backfill is `cid IS NOT NULL AND pinata_cid IS NULL`, the only unambiguous local-IPFS shape). - `ipfs_cid_gate_withholds_blob_from_unauthorized` — root tree assertion inverted to NOT_FOUND (the recursive structural gate denies root trees whose entries name a denied subtree). - `ipfs_cid_legacy_provider_cid_repaired_on_repush` — legacy-shape row insert now sets `local_ipfs_provenance = TRUE` to reflect that the bytes ARE on local IPFS (the legacy CID is just the wrong key). - `ipfs_cid_tree_served_despite_non_commit_ref` — root tree assertion inverted (same reason as the gate test). - `fails_closed_on_annotated_tag_of_a_blob` and `fails_closed_when_a_ref_cannot_be_traversed` — renamed to `skips_*`, assertions inverted from `Err` to `Ok` (a ref pointing at a non-commit object is no longer a walk error; the blob falls out of the gap set because `git rev-list --all` skips it). - `ipfs_cid_walk_error_fails_closed` — removed (relied on the now-removed `assert_all_refs_are_commits` to force an error; the fail-closed arm is still covered by `ipfs_cid_commit_tag_walk_error_fails_closed`, which uses a different trigger — a nonexistent ref object makes `rev-list --all` fail). - `sweep_does_not_publish_public_ancestor_tree_naming_withheld_subtree` (new) — recursive depth test: a `/public/secret/**` repo asserts root tree, `/public` tree, and `/public/secret` subtree are all absent from `pinned_cids` while `/public/visible.txt` is IPFS-pinned. - `sweep_repairs_commit_reachable_object_in_repo_with_tag_of_tree` (new) — tag-of-tree repo with a separate `mktree`'d tree (not commit-reachable); asserts the commit-reachable blob is IPFS-pinned and the unclassifiable tag-of-tree is not. Scope discipline preserved: PolicyFence capture order, `_pin_permit` reuse for the seal phase, fresh per-arm `rederive_budget`, keyset repo cursor with lookahead, mirror-row skip, Kubo 2xx-without-Hash refusal, v27 provenance-clearing migration, v30 strict backfill, v31 schema, per-(repo, backend) reconciliation offset, and the `local_ipfs_provenance` writer/reader contract are all untouched. * test(node): ignore pre-existing RED replication-tail test on CI `api::repos::tests::receive_pack_tail_survives_a_disconnect_during_release` documents an open bug in `api/repos.rs` (the replication tail must be spawned above `release` and gated on `receive_result.is_ok()`; today it's spawned below, so a disconnect during `release` drops the pins, recovery copy, and announce). The test's `assert!` message starts with "RED:" — this is a TDD-style "keep the design intent visible in the code" test, not a working-behavior assertion. The test was added 2026-07-29 by `beardthelion` (PR #215 lineage), well before the #218 sweep work, and is unrelated to this PR. CI is failing on it under load (passes in isolation) because the open bug hasn't been fixed yet. `#[ignore]` until the open bug is fixed. The test's design intent and the fix description stay in the doc comment and the assert message; only the CI failure is suppressed. Run the test explicitly with `cargo test -- --ignored` to verify the bug is fixed when the tail-spawn moves above `release`. * fix(node): align encrypted manifest anchor log with one-shot contract The "will retry next pass" log at reconciliation.rs:1034 was false: plan_seal returns SkipUnchanged on every subsequent pass once the recipients tag matches, so sealed stays empty and the anchor block never runs again. A transient Irys outage at the moment of a fresh seal therefore LOSES that anchor permanently — the next pass has no delta to anchor and no retry fires. This is intentionally best-effort: re-anchoring an unchanged manifest would burn Irys writes for no recovery benefit. Factored the warn message into ENCRYPTED_MANIFEST_ANCHOR_FAILED_MSG and added a unit test that pins the constant; the test goes RED if "will retry" reappears in the log. * fix(node): address eight reviewer findings on #218 reconciliation sweep (round 3) * fix(node): close audit.toml array and drop inline comment * fix(node): repair the stale for-each-ref fake-git fixture (round 8 P1) `upload_pack_shares_one_deadline_across_walk_and_filtered_serve` — the #174 shared-deadline guard for the FILTERED serve arm — was red on both `test (stable)` and `test (beta)`. The fake git answered `for-each-ref` with the single token `refs/heads/main`, a ref NAME, where `blob_paths` phase 2 asks for `--format=%(objectname) %(objecttype)`. The phase-2 parse fails closed on a line it cannot read, so the walk returned an error and the request surfaced as a generic 500 instead of being reaped into a 504. That silently repurposed the test from "the filtered serve shares one deadline" into "the walk errors" — the guard proved nothing while it was red. The fixture is what was stale, not the production path: the arm now emits the column shape the walk parses. A commit tip has no peeled referent, so two columns is the whole line. The 1.2s walk cost stays on `ls-tree` and the serve's on `pack-objects`, so the two phases remain independently attributable. * fix(node): peel annotated tags in the blob_paths ref walk (round 8 P1) `%(objecttype)` reports the type of the ref's OWN object, which for an annotated tag is `tag`. The phase-2 `if kind == "blob" || kind == "tree"` arms therefore never saw the referent, and a blob reachable ONLY through an annotated tag escaped the withheld set — while `rev_list_keep`'s `git rev-list --objects --all` does peel tags and served it. Phase 2 closed the leak for a direct ref-to-blob and for that shape only. The walk now asks for the peeled atoms `%(*objectname) %(*objecttype)` and inserts the referent with an empty path, alongside the direct blob/tree tips it already handled. Line parsing moves to a field-count match: two columns for a non-tag tip (the peeled atoms expand to empty), four for a tag tip, anything else still fails closed. Measured against git 2.50, the `*` atoms peel the WHOLE chain — a tag-of-a-tag-of-a-tag-of-a-blob reports the blob — so a `tag` peeled type does not occur on stock git. That arm is kept anyway, because `push_delta.rs`'s ref-type guard documents `%(*objecttype)` as a one-level peel: under such a git it finishes the chain with `rev-parse <tip>^{}` plus a `cat-file -t` probe rather than bailing and 500ing every clone of a repo that merely carries a nested tag. Both children are bounded by the walk's shared deadline and are unreachable on the common one-line-per-ref path. * fix(node): one empty-path decision for both blob_paths consumers (round 8 P1) `blob_paths` has two consumers and only one of them carried the empty-path policy. `withheld_from_pairs` (the deny side, what the smart-http serve filter excludes) withheld a phase-2 empty-path entry from everyone but the owner; `allowed_blob_set_for_caller_bounded` (the allow side, what `GET /ipfs/{cid}` hands over) called `visibility_check(..., "")` directly. On a public repo no glob matches the empty path, so that returns `Allow`: the serve filter withheld the OID while the IPFS gate served the bytes — the leak phase 2 exists to close, reopened one layer over. The policy moves into `pair_decision`, which both consumers now call, so the two gates cannot disagree and a future change to the empty-path rule moves both at once. `withheld_blob_recipients_bounded` routes through it too: an unclassifiable blob grants a recovery copy to the owner only, never to a rule's named reader whose grant was written against a path this object does not have. No behavior change for non-empty paths — `pair_decision` delegates to `visibility_check` unchanged. * test(node): bind the phase-2 ref tests to a commit-unreachable blob (round 8 P2) `skips_a_ref_pointing_at_a_blob` and `skips_an_annotated_tag_of_a_blob` were vacuous. Both hung their ref on `fixture()`'s `secret` blob, which is COMMITTED at `secret/b.txt`, so phase 1's per-commit `ls-tree` already produced `(secret, "/secret/b.txt")`, the `/secret/**` rule already denied it, and `withheld.contains(&secret)` passed with phase 2 deleted outright. Both now use `orphan_blob`, a blob written straight to the object store that no commit's tree names, so the ONLY route into the withheld set is the `for-each-ref` phase. That is also the real leak shape: `rev_list_keep`'s `git rev-list --objects --all` does follow a ref to such a blob, so an under-withheld one ships in the clone pack. Verified by mutation: with `if false` injected at the phase-2 filter, every test below goes RED (they were green under that mutation before). New coverage: - `ref_only_blob_is_denied_on_the_allow_side_too` — the allow-side denial for the shared `pair_decision`; it also asserts deny and allow agree on one OID, and that the owner IS still admitted, so it cannot pass by denying everything. Reverting the allow side to `visibility_check` turns it red on exactly that assertion. - `skips_a_nested_annotated_tag_of_a_blob` — real-git tag-of-a-tag. - `peels_a_tag_whose_peeled_target_is_still_a_tag` — drives the one-level-peel fallback through the fake-git seam, so that arm is tested rather than dead. * fix(node): advance the reconciliation continuation only for dispatched work (round 8 P2) `ipfs_last` / `pinata_last` were captured from the missing set above the pin permit, both `PolicyFence` captures and both pin loops, then written under nothing but `if ipfs_enabled` / `if pinata_enabled`. Every stage in between can legitimately produce nothing — a fence capture that fails, a quarantine/visibility recheck that says skip, a pin-boundary re-derivation that errors — and each is transient. Advancing past OIDs that were never attempted is not a harmless retry delay: `missing_oids` rotates strictly past the stored offset, so the whole unattempted prefix lands BEHIND the entire backlog on the next pass. For an at-cap repo — the only kind the continuation exists for — the backlog never drains inside one cap window, so those objects are starved indefinitely. That is a durability hole in the durability backstop. The offset now moves only for work actually handed to a backend, tracked at the dispatch boundary as `to_pin.last()` (not the missing set's last: an OID the pin-boundary re-derivation dropped was never offered). It is recorded before the call, so a pin phase that times out mid-batch still counts as dispatched. The write becomes three outcomes rather than two, via one `next_offset_write` helper so the backends cannot drift apart: * work dispatched -> advance to the last dispatched OID * nothing missing, query OK -> None, marking the row done * nothing dispatched, or the missing-set query failed -> write NOTHING, leaving the resume point a capped pass already paid for `*_scan_ok` is what separates the last two: an empty missing set means "everything is pinned" or "the filter query failed and we know nothing", and only the first should mark the row done. Test: `sweep_leaves_offset_untouched_when_nothing_was_dispatched`, using a `#[cfg(test)]` seam on the pin-boundary re-derivation — that stage cannot be starved from the outside, because the mid-scan re-filter runs first on the same rules and budget and makes the sweep `continue` well before the offset write. It asserts the stored offset comes back byte-identical after a declining pass, then that the same repo does advance it once dispatch happens, so the assertion cannot pass on a dead write site. Restoring the old semantics turns it red. * fix(gl): render the Pinata provider CID in `gl ipfs list` (round 8 P2) The server-side wire change landed without its client half. `cmd_list` read `pin["cid"].as_str().unwrap_or("?")` as the entry heading and nothing in the CLI read `pinata_cid` at all, so a Pinata-only row printed as a bare `?` — the listing showed nothing usable for an object that is in fact durably stored. `cid` is legitimately null for such a row: it is the key `GET /ipfs/{cid}` resolves against, and the node stopped aliasing a Pinata provider CID into it (round 3) precisely because that CID 404s there. So the fix is on this side. Rendering moves into `render_pin`, which leads with the node-resolvable CID when there is one and otherwise the provider CID, labelled so nobody feeds it back to this node's resolver. A dual row keeps both visible. A `backends:` line names where the bytes are, read from the writer-owned `local_pinned` / `pinata_pinned` booleans rather than re-inferred from CID shape — which is what the node's own comment warns against. Older nodes send neither flag; those `cid`-only rows fall back to CID presence and render as they always did (`render_pin_handles_a_legacy_cid_only_row`). * fix(node): implement third fence check for IPFS pinning and enhance visibility rules handling * fix(node): enhance documentation and add tests for ProgressState and helper functions * fix(node): address round-9 reviewer findings on #218 reconciliation sweep * fix(node): correct consumer-4 recipient test for call-invariant set * fix(git): fail closed on non-UTF-8 tree listing in walk_tree_oids_inner (#218 round 10 P1) walk_tree_oids_inner previously returned Ok(()) on a non-UTF-8 ls-tree -z listing, inserting only the tree OID and skipping every child blob/tree OID. A direct tree ref (or an annotated tag peeling to a tree) is valid Git input; git rev-list --objects --all still enumerates the tree and every descendant, so the keep-side removed the tree from the served set but passed the child blob OIDs to pack-objects, exposing their bytes to an anonymous clone. Phase 1 already bails on the same input at the blob_paths walk; this fix puts the tree-walk on the same fail-closed outcome. The new fails_closed_on_non_utf8_tree_tip test pins the invariant for both a direct refs/tags/direct-tree ref and an annotated tag-of-tree (the two non-commit shapes round 9 added tolerance for), confirming the walker bails rather than returning Ok with a partial withheld set. * fix(recon): run encrypted recovery even when public list is empty (#218 round 10 P1) A path-scoped repo whose only reachable object is a direct blob or direct tree ref yields an empty public list (the anonymous public classifier removes the only object from the served set) while withheld_blob_recipients_bounded still assigns that object to the owner recovery set. The early `continue` on object_list.is_empty() (and the second one after the mid-pass refilter) suppressed encrypted recovery too, and a lost or failed encrypted copy was never repaired. Track empty-public-work as a flag and gate the public phase on it. Encrypted phase 2 runs regardless. The backend enable flags and `_pin_permit` move out of the gated block so phase 2 can read them when the public phase did not run. * fix(db): rename migration v12 test to v32 (#218 round 10 P3) The test name and its docstring/comments still say "v12", but the cid-nullable migration is at v32 (line 1163). The test itself is correct — it drives the real path — so this is naming drift rather than dead coverage. A future reader looking for the migration would otherwise chase a v12 that no longer exists. * fix(git): index OID membership before quadratic classifier scan (#218 round 10 P2) all_object_paths was doing O(O×P) work in two places: the catch-all cat-file branch checked `blob_set.iter().any(|o| o == oid)` and `tree_set.iter().any(...)` for every reachable object, and the phase 1 loop also did a per-commit unused `git rev-parse <commit>^{tree}` probe whose result was discarded (the root-tree OID is recomputed by `root_tree_oids`). On the 50k-object repos the sweep exists for, the quadratic scan and the redundant per-commit child process can consume each authorization deadline before any object reaches a backend, producing a permanent hourly skip. Maintain a separate `blob_oids` / `tree_oids` HashSet alongside the path-pair sets so the membership check is O(1). Drop the unused per-commit rev-parse probe. * fix(git): bound tree walk by ls-tree invocation count, not just depth (#218 round 10 P2) walk_tree_oids_inner stopped at MAX_TREE_WALK_DEPTH and the shared deadline. A wide shallow tree reachable from N ref tips or from multiple parents still spawned one ls-tree child process per subtree well inside the depth cap, and the wall-clock bound could not stop that — expiry was the only stop. The ceil was wall-clock only. Add a structural invocation cap (MAX_TREE_WALK_INVOCATIONS) the walker fails closed at, and a memo of walked tree OIDs so the same tree reachable from multiple ref tips is walked once. Round 10 P2. * fix(pinata): suppress push when DB record is not durable (#218 round 10 P2) pin_new_objects pushed (sha, cid) on provider POST success regardless of whether record_pinata_cid persisted the row. A closed/failed DB write or an explicit-transaction source record timeout meant the next sweep queried the DB, found no row, and re-offered the same gap — with reconciliation having counted a fill that never happened. Track a `db_record_durable` flag. On hard failure of either record_pinata_cid (non-timeout) or record_pin_source (any failure, since it's a multi-statement transaction), set the flag false and skip the `pinned.push`. The `BoundedDbError::Elapsed` autocommit case stays as the previous comment's "may have committed" reasoning: the push fires so the reconcile still sees the fill. Round 10 P2. * fix(node): serialize policy narrow with irreversible upload via in-process mutex (#218 round 10 P1) The PolicyFence's epoch read was a one-way read: a visibility narrow (rule insert, quarantine set) could commit between the fence's epoch read and the upload's HTTP POST, and the upload's DB-record INSERT's epoch check would see the new epoch and reject — but the provider already accepted the object. Encrypted envelopes had no fence at all. Add a per-repo in-process mutex that PolicyFence acquires at capture and releases at drop. The narrow paths (set_repo_quarantine, set_visibility_rule, remove_visibility_rule) acquire the same lock for the duration of their DB transaction. A narrow therefore blocks until the in-flight batch finishes, and an in-flight batch waits for the narrow to commit before it even reads the epoch. Multi-process / multi-node is a known gap: a Postgres advisory lock per repo would extend the same guarantee across processes. Deferred to a follow-up. * fix(pinata): update post-upload stalled record test for round 10 P2 contract The previous test asserted that a successful upload with a timed-out post-upload source record still returned the (sha, cid) pair. That assertion pinned the buggy behavior the reviewer called out: the reconcile would count a fill that was not durable, then re-offer the same gap. The test name and the `record_pin_source` arm of `pin_new_objects` are unchanged; the assertion now reflects the new contract: when the post-upload source record times out (its `mark_pin_sources_incomplete` already sets the source set to incomplete, so the next pass re-offers), the pair is suppressed so the reconcile sees the gap as not-filled. Round 10 P2 follow-up. * fix: clippy allow + rustfmt on round 10 commits clippy::too_many_arguments on walk_tree_oids_inner (8 args after the round 10 memo + invocation counter threading). rustfmt re-ran on the if/else blocks added in the has_public_work refactor (round 10 P1 #42). No behavior change. * fix(node): address round-11 review findings on #218 reconciliation sweep P1: remove the whole-batch policy mutex. PolicyFence no longer holds a per-repo lock from capture through drop and the narrow paths (set_visibility_rule, remove_visibility_rule, set_repo_quarantine) no longer acquire it, so a visibility narrow commits immediately and the batch aborts on its next is_current check instead of the narrow blocking behind the sweep. Accepted residual is a single in-flight object; the fenced DB record still refuses to land a raced row as durable. encrypt_and_pin_stops_sealing_when_reader_removed_mid_batch is green again. PolicyMutexes is deleted, resolving the key-leak P2 by removal. P2: cover the round-10 has_public_work branch with sweep_seals_withheld_blob_when_public_list_is_empty: a direct-blob-ref repo yields an empty public list with a non-empty owner recovery set; asserts no public gaps/fills, no cleartext pin, exactly one POST (the seal envelope), and a recorded encrypted copy. P3: peel-case isolation in the non-UTF-8 tree-tip test (delete the direct-tree ref before the second call so the peel arm must run); v12->v32 rename drift in the migration test (boundary, comments, and the stale (2)->true doc line); accurate comments for the pre-refilter flag and the post-refilter fall-through. * fix(node): address merge-readiness findings on #218 reconciliation sweep Reworks the sweep around explicit state-machine contracts instead of adding conditionals: backend results carry last_attempted plus confirmed durability separately (PinBatchOutcome), provider permits scope to provider effects only, discovery pages by commit window with a persisted cursor, legacy provenance migrates as unknown, and the pin list advertises only validated raw local keys. - Empty-path owner recovery without path rules: phase 2 drops the has_path_scoped_rule gate and runs on the actual recipient result, with a dedicated recovery recheck (no anonymous-listability requirement); matrix test over blob/tree/tag refs x zero/root rules, including recreate-after-delete. - Pinata Elapsed is non-durable unless verify_pinata_record proves the exact row and fence epoch; loop test with row-locked repos. - list_pins gates cid/local_cid on is_raw_cidv1 plus confirmed provenance; response-shape matrix uses real CIDs with a legacy row and GET /ipfs follow-through; docs match the nullable wire contract. - Permits acquired per provider effect and dropped after; no nesting at pool size 1 (test renamed to the new shape). - Scan discovery pages SCAN_COMMIT_WINDOW commits oldest-first with a node_state cursor (advance on drained/empty/attempted, delete on cover); refilters and phase-2 recipients run on the same window; walk-layer paging/invocation/union tests plus a 1050-commit cursor-to-completion sweep test. - v35 migrates legacy rows as unknown (no TRUE backfill); v35 test asserts unknown + preserved Pinata history; sweep re-derivation test re-pins instead of filtering. - Drained completions DELETE continuation rows (no tombstones); FK cascade on repo delete; row-count regression test. - Audit: lru waiver gets an explicit retirement owner plus a weekly drift guard; revocation model recorded as a product decision on PolicyFence (prompt revocation, single-in-flight residual). * fix(tests): configure git identity in bare clone for tag fixture The windowed-union test creates its annotated tag in the bare clone, which carries no config. It passed locally only via the developer's global git identity; CI has none, so 'git tag -a' failed there. Verified with an empty GIT_CONFIG_GLOBAL/SYSTEM. * fix(tests): ignore pre-existing hung-git pid race on CI read_object_bounded_returns_by_deadline_with_a_hung_git reads the fake's pid file with no readiness handshake; a slow scheduler yields 'No such file or directory' (stable-only red, identical beta green). Unrelated to #218 (store.rs untouched); re-enable with a readiness signal from the fake. * fix(node): address round-12 review findings on #218 reconciliation sweep - P1 roots: admit structurally safe root trees at "/" into the allow set and candidates, so safe snapshots fully replicate; roots naming a withheld subtree stay excluded via the recursive structural check. - P1 per-pass WalkBudget on sweep enumeration (entries / output bytes / git invocations) with fail-closed budget-exhaustion; serve paths keep the uncapped budget. - P2 recovery lane: independent recovery cursor pages unlistable history across restarts instead of sharing the scan window. - P2 gap-hold: failed gap queries hold discovery advance but not the healthy backend, via test-only failure seams. - P2 nested tags: collect full tag chains so nested tag objects pin without an inner ref. - P2 RepoStore: thread the Tigris-backed store into the sweep with an acquire timeout and shutdown select; restore cache-miss and skip missing. - P2 Pinata source: keep confirmed pairs when the primary already covers the repo; suppress push for stalled post-upload records without coverage. - Update large-history sweep expectation to blob+root+commit per round. * fix(node): address consolidated review findings on #218 reconciliation sweep P1: wire WalkBudgetExceeded through the sweep cursor logic. Both the public scan and the recovery walk now dispatch the typed budget-exhaustion signal at the sweep boundary: a budget-hit window advances past content the sweep refuses to classify partially (scan cursor save/delete via the existing persist site; recovery cursor likewise), while git/policy/panic/timeout failures keep the cursor for retry. visibility_pack documents the obligation; the match arms are the handler, with a one-line pattern note for future typed walk errors. P2: run encrypted recovery when the public scan fails. Scan walk/panic/timeout arms now fall through with an empty public list instead of continuing the whole repo iteration, so phase 2 runs on the recovery lane's own cursor like the unlistable-repo path. The scan cursor stays untouched for retry. Refilter/recheck-public failures remain fail-closed whole-iteration continues by design. Includes a repo-scoped test seam (process-global set, safe across spawn_blocking and parallel tests via fresh UUIDs). P2: bump policy_epoch only when remove_visibility_rule deletes a row, mirroring set_repo_quarantine's rows-affected guard, so a typo'd or double-deleted glob cannot abort fenced batches. Also adds the repo-loop outcome matrix (every early exit × scan cursor / recovery cursor / phase 1 / phase 2) and deny tests: budget-hit advances both lane cursors with zero pins/seals (real 250k-entry trip, early-trip fixture runs in ~30s), scan failure still seals the recovery copy, delete-missing-rule leaves the epoch unchanged. Each deny was verified by temporarily restoring the old branch. * fix(node): address jatmn review findings on reconciliation sweep re-home * fix(node): reload persisted sweep cursor between passes * fix(node,gl): pinata offset warn wording and boundary-safe pin timestamp --------- Co-authored-by: Gravirei <[email protected]> Co-authored-by: Kevin Codex <[email protected]>	2026-11-09 15:35:18 +00:00
3 people	c805e35f40	fix(node): bound GraphQL query cost (#552) ... * fix(node): bound GraphQL query cost * fix(node)!: bound repo names so emitted cursors always parse The reposPage cursor embeds base64(JSON(owner_did, name)) while parse_repo_cursor rejects cursors over 4096 bytes, but repo names were unbounded on every write path. A single long-named visible repo made the server emit an endCursor it then rejected as "invalid repository cursor", dead-ending pagination for every caller past that row. Bound names to MAX_REPO_NAME_LEN (100 bytes) at the create_repo and fork_repo handlers via a shared validate_repo_name helper (bad charset is still a 400), and reject overlong peer-supplied names in Db::upsert_mirror_repo (the sync caller already drops the error and skips the mirror). Also in this round: - docs/graphql-pagination.md states the 200 page-size maximum and corrects the over-budget remedy: a smaller limit does not reduce the reposPage or repos price, which is charged at the full MAX_VISIBLE_REPO_PAGE_SIZE scan regardless of limit. - Tests pin refUpdates(limit: -1) clamping to zero rows, the parse_repo_cursor wrong-shape-JSON arm with its exact "invalid repository cursor" message, the longest-admissible-name cursor round-trip, and the mirror write-path rejection. BREAKING CHANGE: the GraphQL `repos` field now returns an error instead of the full list when more than MAX_VISIBLE_REPO_PAGE_SIZE (200) repositories are visible; larger lists must use `reposPage` with `limit`/`after`. Repository names are additionally limited to 100 bytes at creation, fork, and mirror-registration time. * fix(node): address beardthelion P3 findings on name bound and input messages - Assert each rejection's own message in the reposPage invalid-inputs loop: the loop no longer accepts either message for every query, so a misattributed message (limit arm returning the cursor message) goes red. - Pin the create/fork name-bound wiring through the handler: new test drives create_repo with an overlong name via test_state_lazy and asserts the 400; deleting the validate_repo_name call site would proceed to the lazy DB pool and fail with a connection error instead. - Document the API-side vs disk-side validate_repo_name divergence on the new function, and tie the store's literal 100 to MAX_REPO_NAME_LEN so the two validators cannot drift apart on length. --------- Co-authored-by: Marlowe <[email protected]> Co-authored-by: cairn-intern <[email protected]>	2026-11-08 23:28:09 +00:00
beardthelion	0366f18458	fix(node): reconcile public-read messaging with per-repo enforcement (#439) ... * fix(node): reconcile public-read messaging with per-repo enforcement GITLAWB_PUBLIC_READ is read nowhere except a startup warning, while read enforcement actually happens per repository through is_public and path-scoped visibility rules. The flag's help text, the startup warning, .env.example, and the readiness audit all still claimed private-read enforcement was unwired. The help and warning now state the flag is reserved and inert, and point at the real per-repo control. A config test asserts the rendered help neither claims enforcement is missing nor omits that the flag does nothing. Closes #338. * docs: drop the stale private-read priority entry * chore: refresh PR head	2026-11-05 16:46:47 +00:00
beardthelion	c97695a259	Add a completeness fence over the GraphQL mutation surface (#219) ... * test(node): add a completeness fence over the GraphQL mutation surface Gate the GraphQL mutation surface the way REST is gated, as a SEPARATE guard with its own bucket table (a guard over a heterogeneous set must classify per member, INV-1). every_graphql_mutation_has_its_gate asserts the mutation set is complete (every mutation classified) and that each carries its gate marker. Completeness is derived from async-graphql's own introspection: the field names come from the engine as structured data, so no source-text or SDL-format shape (a brace or async fn in a comment or string, a whitespace/generic/skip variant, a brace in a rustdoc description) can hide a live mutation. A new unregistered mutation, including a repo-write, fails the build. The marker check (does each resolver body call its gate) is a best-effort static backstop, bounded to each resolver's own comment-masked, string-aware body. It is explicitly not the completeness guarantee and not load-bearingness: a marker inside a string literal is a documented vacuity residual, and the adversarial sqlx tests in graphql/mutation.rs are the real proof the gates reject. Verified by execution: an unregistered mutation reddens (unclassified), a removed did_matches gate reddens (missing marker), the clean tree is green; no DB, no feature flag. * docs(node): reconcile GraphQL mutation-auth claims with the shipped state README's known limitations, the OSS-readiness live-network blockers, and the maintainer roadmap still listed GraphQL mutation auth as an open item, which contradicts #87: GraphQL mutations already require a verified signer bound to the acting DID. Reconcile each to the shipped reality while keeping the genuinely-open nuance: the GraphQL mutation surface is the agent-task queue, repo writes stay REST-only, and no owner-gated repo-write mutation is exposed over GraphQL. The every_graphql_mutation_has_its_gate fence forces any new mutation to be classified in its table; owner-gate and REST parity stay the INV-1 review check. * fix(node): source the GraphQL mutation fence from SDL, not introspection Resolves jatmn's #219 P1. The every_graphql_mutation_has_its_gate completeness fence enumerated mutation fields via async-graphql introspection, which filters out `#[graphql(visible = false)]` fields. Those fields stay executable, so a future hidden mutation would need no registry row and no marker while both set-equality checks stayed green -- defeating the fence's guarantee that no live mutation goes unclassified. (Confirmed by execution: a synthetic visible=false mutation is absent from introspection, executable, and present in schema.sdl().) Source completeness from the engine's own SDL instead. A new pure helper mutation_field_names_from_sdl parses schema.sdl() with async-graphql's parser AST (async_graphql::parser::parse_schema) and reads the mutation type's fields; SDL includes visible=false fields and the AST parse is immune to a brace in a field description. Introspection is kept as a cross-check: any field present in SDL but hidden from introspection now fails named as a forbidden hidden mutation. The stale doc comments (which claimed introspection could not hide a field) are corrected. A self-checking version-safety test builds a synthetic schema with a visible=false mutation and asserts the SDL-sourced set includes it while introspection omits it -- so if a future async-graphql ever filters SDL by visibility, that test goes RED rather than the gap reopening silently. Load-bearing verified by execution: stubbing the SDL enumeration to empty turns four tests RED, including the fence itself; restoring returns green. Only api/mod.rs changed; the mutation resolvers are untouched. `cargo test -p gitlawb-node --bins` green (495); clippy and fmt clean. * fix(node): make the SDL mutation-fence wiring load-bearing; admit deprecated An adversarial review of the SDL switch found three gaps: - The fence's completeness source (SDL) and its hidden-mutation cross-check were not load-bearing: because the real MutationRoot has no visible=false field, reverting `fields` back to introspection and dropping the cross-check left the whole suite green (INV-21). Extract the sourcing + cross-check into a generic `assert_mutation_fence<Q,M,S>` that the real fence AND a synthetic hidden schema both drive. A new #[should_panic] test (fence_rejects_a_hidden_mutation) feeds a visible=false mutation that IS registered, so only the cross-check can fire; reverting the SDL source or deleting the cross-check now changes/removes that panic and turns the test RED (verified). - The introspection cross-check used bare `fields { name }` (GraphQL default includeDeprecated:false), so a `#[graphql(deprecation = "...")]` mutation -- executable and present in SDL -- would false-red as a hidden visible=false mutation. Use `fields(includeDeprecated: true)`; a new test (fence_admits_a_deprecated_mutation) pins it, going RED without the flag. - schema_lists_exactly_the_registered_mutations still sourced from introspection and its comment called introspection "the source of truth", contradicting the fix. Retargeted at the SDL set with a corrected comment. Also folds the two introspection navigations into one generic helper (introspection_mutation_fields) and points the version-safety test at it, so there is a single introspection query. cargo test -p gitlawb-node --bins green (497); clippy and fmt clean. Only api/mod.rs changed. * fix(node): rebase #219 and require signer gate in the GraphQL fence Rebase onto current main and skip the obsolete 0.5.1 lockfile commit. The mutation fence now requires both require_signer(ctx)? and did_matches( in every registered resolver, and each_mutation_requires_and_binds_signer covers all four task mutations for unsigned and DID-mismatch denial. * docs(node): reconcile GraphQL fence docs with SDL discovery Restore the historical OSS audit blocker text and add a dated status update for #219. README and the maintainer roadmap now describe the SDL-sourced fence and signer-plus-binding markers. * ci: trigger the PR checks on a new head	2026-11-05 04:43:52 +00:00
JATMNandClaude Opus 5.5	e2ae21795a	chore: rebrand user-facing gitlawb references to Twigpine (#520) ... * chore: rebrand user-facing gitlawb references to Twigpine Rename the product name in docs, README, CLI help/output, MCP tool descriptions, macOS app strings, installers, npm metadata and license headers from gitlawb/Gitlawb to Twigpine. Point GitHub links at the renamed Twigpine org (and fix the release workflow's repository guard, which still matched Gitlawb/node), and switch the public node hosts and website/install URLs to twigpine.com. Protocol and operator identifiers are intentionally unchanged (did:gitlawb, gitlawb://, signing domains, GITLAWB_* env vars, crate and binary names, X-Gitlawb-* headers, icaptcha and peer-network URLs); they need a follow-up migration. Co-Authored-By: Claude Opus 5.5 <[email protected]> * docs: point image pulls at ghcr.io/twigpine/node and fix macOS DMG name The release workflow publishes the container image to ghcr.io/${GITHUB_REPOSITORY,,}, which is ghcr.io/twigpine/node since the org rename; the old ghcr.io/gitlawb/node path no longer resolves for anonymous pulls. Point the RUN-A-NODE docker run example, the OSS readiness audit's claim about it, and the macOS app's generated compose file at the published path. The README listed the macOS DMG as "Twigpine Node.dmg", but build-macos-app.sh writes GitlawbNode-macOS.dmg (kept on purpose), so document the real file name. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(infra): keep the AWS security group description to avoid replacement `description` on aws_security_group is ForceNew, so renaming it forced a replace of the in-use group on the next apply of an existing deployment. Restore the original text and note why it must stay until a separate infra migration. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(macos): regenerate the app's compose file instead of treating it as a repo's Config.repoPath fell back to the data directory when no repo clone was found, and DockerCompose then picked up the compose file it had generated there as if it belonged to a repo. The generator therefore only ran on first launch, so upgraded installs kept their original compose, including the old ghcr.io/gitlawb/node image, which no longer serves anonymous pulls. repoPath now returns nil when there is no clone, and the app regenerates its compose file whenever no repo compose is in use. Project directory and named volumes are unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(infra): default the AWS node image to ghcr.io/twigpine/node release.yml publishes to ghcr.io/${GITHUB_REPOSITORY,,}, which is now ghcr.io/twigpine/node; ghcr.io/gitlawb/node denies anonymous pulls, so the default made new deployments fail to start and the upgrade command fail on existing ones. On an existing deployment `terraform apply` only updates the upgrade SSM document in place (user_data drift is ignored); the new image is pulled on the next upgrade_command. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(macos): keep operator edits to the generated compose, migrate only the image e04efe4 regenerated the app's compose file on every access, overwriting edits operators make to pass settings the generated environment block omits (e.g. GITLAWB_OPERATOR_STRICT_MODE). Generate the file only when it is missing; when it exists, keep it and rewrite only references to the retired ghcr.io/gitlawb/node image, keeping tags and digests. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(macos): only migrate complete ghcr.io/gitlawb/node references Without a left boundary the migration also rewrote the retired path when it appeared inside a longer reference, such as a registry mirror (registry.example.com/ghcr.io/gitlawb/node), pointing it at a path the mirror may not have. Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: Claude Opus 5.5 <[email protected]>	2026-11-04 21:58:29 +00:00
Kevin Codex	73fd7473af	Merge pull request #330 from Gitlawb/fix/default-enforce-owner-push ... fix(node)!: enforce owner-only push by default	2026-08-25 22:06:12 +08:00
Kevin Codex	21f8cb6135	Merge pull request #173 from Gitlawb/fix/issue-135-ipfs-cid-tree-gate ... fix(node): gate GET /ipfs/{cid} tree objects so a withheld subtree's structure can't leak (#135)	2026-08-25 22:04:14 +08:00
Vasanthdev2004	de4d932609	docs(infra,macos): make the opt-out reachable and stop the AWS guide contradicting itself ... All four are in the deployment half added last round; none is in the Rust. The AWS guide told operators two different things -------------------------------------------------- "Changing configuration" still said editing `image_tag` does not affect a running instance and routed to `-replace`, while the upgrade section thirty lines above now reinstalls the rendered compose, which carries `image_tag`. The same paragraph told operators to edit `.env` and restart — which the new "Why the upgrade rewrites the compose file" section explains is silently ineffective for any key the installed compose does not already name. Split by which file carries the change rather than deleted, because the distinction is real: values rendered into `compose.yaml` (`image_tag`, ports, `domain_name`, `db_host`, `icaptcha_*`) roll out with `apply` + `upgrade_command`; values user-data writes into `.env` at first boot (`public_url`, `bootstrap_peers`, integrations) do not propagate from tfvars at all and need an edit on the instance or a replacement. The third case is the one that bites: a setting the installed compose does not yet name is read from `.env` and dropped, so it needs the upgrade first. `--remove-orphans` stopped being inert --------------------------------------- The flag is not new, but until the upgrade rewrote `compose.yaml` it reconciled against the instance's own file, so the only orphans were hand-started containers. It now reconciles against a fresh rendering whose service set is conditional — `postgres` only when `db_host` is empty, `caddy` only when `domain_name` is set. Setting `use_rds` or clearing `domain_name` and then running what the README calls a pull-and-restart stops and removes the local postgres or the TLS terminator. That is what those variables mean, but it happens on the upgrade rather than on apply, so it is now stated next to the Terraform-owned warning. Data survives either way; both bind-mount under /mnt/data. The macOS setting had no writer -------------------------------- `enforceOwnerPush` was persisted, loaded, emitted into `.env`, and forwarded by the generated compose — and nothing set it. `SettingsWindow` had no state and no assignment, so the only way to reach the opt-out on that launch path was hand editing config.json in Application Support, which no document mentions. Editing `.env` does not work there either: `writeEnvFile()` rebuilds it from `Config` on every start. It now has a Toggle beside "Sync repos from peers", following the pattern already in that view. Three lines the change made untrue ----------------------------------- The handler test asserted "with no configuration at all" while setting `enforce_owner_push` explicitly — the right thing for the test, the wrong claim for the message. The cross-tenant docstring still described three distinct pusher DIDs after A and B were given `n1`'s owner. And OSS-READINESS-AUDIT listed unprotected branches not enforcing owner checks as a live blocker; owner checks now cover every branch, and only the UCAN half of that sentence still stands. Also dropped the `#[allow(dead_code)]` on `test_state_with`, redundant since the same commit added its call site.	2026-08-15 11:47:36 +05:30
anandh8x	0b47e1fc08	docs: clarify visibility limits	2026-08-15 10:32:24 +05:30
Vasanthdev2004	cdbda19c29	fix(node)!: enforce owner-only push by default ... A `did:key` signature is authentication, not authorization: the method is self-certifying, so any party can generate a keypair, derive its DID and sign. With `GITLAWB_ENFORCE_OWNER_PUSH` defaulting to `false`, `owner_push_rejection` short-circuited and every `git-receive-pack` carrying any valid signature was accepted — including pushes to a repository the signer does not own, and including private ones, on every branch not explicitly protected. The gate itself already worked when switched on. What was missing is that no reachable default configuration switched it on, so a node started with nothing configured accepted a push from anyone. That default is what this changes. The argument now takes a value so there is a way back: `--enforce-owner-push false` and `GITLAWB_ENFORCE_OWNER_PUSH=false` both disable it, while the bare `--enforce-owner-push` form still means `true`. Without this the flag would be presence-only (`ArgAction::SetTrue`), which with a `true` default would leave operators no way to opt out during a rolling upgrade. Three concurrency tests pushed as non-owners in order to reach the shedding logic they actually assert on. They now push as the repo owner, so they keep exercising the shipped default rather than opting out of the gate. Docs updated in README.md, .env.example and docs/RUN-A-NODE.md, including the delegated-key caveat below. README also carries a dated cutover for `GITLAWB_REQUIRE_SIGNED_PEER_WRITES`, which stays `false` for now so live peers can finish upgrading. BREAKING CHANGE: `git-receive-pack` now rejects a push whose authenticated DID is not the repo owner, returning 403 before any ref update is applied. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so an agent pushing under its own DID cannot push while this is on. Set `GITLAWB_ENFORCE_OWNER_PUSH=false` during a rolling upgrade, or have automation push as the repo owner, until scoped collaborator / UCAN-delegated push rights land.	2026-08-14 17:22:23 +05:30
beardthelion	f4f0f4bff0	Merge remote-tracking branch 'origin/main' into fix/issue-135-ipfs-cid-tree-gate	2026-08-13 09:44:35 -05:00
Gravirei	9f340eac5e	Merge upstream/main into fix/issue-210-stable-advisory-lock-key ... Resolve conflicts: keep the SHA-256 advisory-lock key change alongside upstream's connection-affine RepoWriteGuard rework and its F1/F3b/F4/U8 test suite; RUN-A-NODE.md keeps both the updated Public URL row and the rolling-upgrade advisory-lock row.	2026-08-10 21:31:38 +06:00
beardthelion	8f3a52175a	docs(node): correct the walk-cap and db-pool operator docs ... Two doc drifts jatmn flagged on #173, both docs-only. The /ipfs walk cap is min(internal ceiling, GITLAWB_IPFS_MAX_REPOS_WALKED), where the internal ceiling is MAX_PIN_SOURCES + 1 = 17. config.rs, README, and .env.example all described a max() floor instead, so an operator reading them would think the default of 64 was the effective ceiling and that raising the knob bought headroom. Neither is true. The runtime is not changed: two tests set the knob to 1 and depend on the cap actually being 1, so wiring the documented floor would make the knob inert and break them. db_max_connections has defaulted to 48 for a while and four comments still said 20. Two of them did arithmetic on it. main.rs and repo_store.rs justified the separate advisory-lock pool by "20 is below 32", which is now false and made the separation look like a consequence of the defaults; it is structural, since a push holds its connection for the whole receive-pack at any pool size. Also adds a RUN-A-NODE troubleshooting entry for the boot failure an in-place upgrade hits when it is still configured with the old pool size of 20.	2026-08-10 01:24:46 -05:00
mike	547f9aed10	fix(node): preserve legacy peer compatibility	2026-08-09 23:09:55 -04:00
Gravirei	89e10d0ff1	fix(repo_store): use SHA-256 for stable advisory-lock key (#210) ... DefaultHasher is not frozen by the Rust standard and has shifted across toolchain versions, so the same (owner_slug, repo_name) produced a different i64 advisory-lock key on different builds and broke cross-machine write-exclusion for the same repo. Hash with SHA-256 instead and pin the result with a golden-value test, plus a one-axis-at-a-time differential test so a regression that drops either parameter is caught. Document the SHA-256 re-keying caveat on acquire_write and in the RUN-A-NODE.md operator checklist (drain writes or cut over single-node during a rolling upgrade).	2026-08-05 19:26:44 +06:00
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>	2026-07-08 08:44:55 +08:00
3 people	0a15e763d2	feat(node): owner-only push enforcement behind GITLAWB_ENFORCE_OWNER_PUSH (#31) (#68) ... * feat(node): owner-only push enforcement behind GITLAWB_ENFORCE_OWNER_PUSH (#31) A valid did:key signature on git-receive-pack is authentication, not authorization: any party can generate a key, derive its DID, sign, and push to an unprotected branch. Owner checks only ran on protected branches, so a non-owner could write to unprotected branches of an arbitrary repo. Add an opt-in flag GITLAWB_ENFORCE_OWNER_PUSH (default false, mirroring GITLAWB_REQUIRE_SIGNED_PEER_WRITES) that requires the authenticated pusher to be the repo owner on every branch. Default-off keeps live nodes unaffected. Design follows the maintainer's review on #31: - Authorize on the canonical AuthenticatedDid injected by require_signature, via Extension<AuthenticatedDid> on the handler — not a re-parse of the headers. The legacy branch-protection check is left as-is. - Fail closed: an absent identity or a non-owner caller is rejected. The policy lives in a pure fn (owner_push_rejection) so it is unit-tested without a DB. - The gate runs before branch protection, so one rejection yields one error body. - Factor the duplicated owner-match into auth::is_repo_owner, reused by receive-pack, protect.rs, and visibility.rs (each keeps its own message). events.rs / peers.rs are left out — they aren't owner checks. - Name the push gate by intent (caller_authorized_to_push) so the planned Phase 2 UCAN git/push capability is a pure addition, not a rewrite. Tests: owner (full + short DID) allowed, non-owner rejected, missing-DID rejected, flag-off legacy. Docs: .env.example + docs/RUN-A-NODE.md. Co-Authored-By: Claude Opus 4.8 <[email protected]> * fix(node): address review on #68 — 403 status, finish owner-match consolidation Per @beardthelion's review: - Return 403 Forbidden (not 400) for authorization denials: the owner-push gate, the branch-protection denial, and the owner checks in protect.rs / visibility.rs. 400 reads as a malformed request and some git/CI clients retry it. - Finish the is_repo_owner consolidation: route the branch-protection check and the repos list filter through the helper. The branch-protection check now uses the verified AuthenticatedDid (auth.0) instead of re-parsing the headers. - Drop the second identity source on the push path entirely: the post-push event recording also uses auth.0, so extract_did_from_auth (and the handler's HeaderMap param + import) are removed. - Tests assert the Forbidden variant (so a future status regression fails loudly) and add direct coverage for is_repo_owner / caller_authorized_to_push. - Docs: warn that enabling the flag blocks delegated and CI agents until the Phase 2 UCAN git/push capability is honored. Co-Authored-By: Claude Opus 4.8 <[email protected]> --------- Co-authored-by: FrozenRaspberry <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]>	2026-06-24 11:03:28 +08:00
Kevin CodexandOpenClaude	9e6e832ec5	Infra reorg (#27) ... * refactor: move fly.toml into infra/fly/ Deployment configs now live under infra/, organized per target. Dockerfiles and docker-compose.yml intentionally stay at the repo root (shared by CI, build scripts, and the macOS app). Deploy with: fly deploy -c infra/fly/fly.toml Co-Authored-By: OpenClaude <[email protected]> * feat(infra): Terraform for single-EC2 AWS deployment infra/aws/ provisions a production-leaning single-node deployment: - t4g.small (AL2023 arm64) running the published ghcr.io/gitlawb/node image + postgres:16 via docker compose - separate encrypted EBS data volume (prevent_destroy) with daily DLM snapshots; survives instance replacement - postgres password via random_password -> SSM SecureString, fetched at boot by instance profile (never in user-data); optional secrets (operator key, Pinata JWT, S3 secret) follow the same path - SSM Session Manager access (no SSH by default), IMDSv2 required, metrics port closed unless explicitly opened - SSM command document for image upgrades (user-data runs once) Verified: terraform fmt/init/validate clean; rendered compose passes docker compose config; rendered user-data passes bash -n. Co-Authored-By: OpenClaude <[email protected]> * fix(infra): address CodeRabbit review feedback - api_url output now honors the public_url override (local.public_url) - disable xtrace around SSM secret fetch + .env write so secret values never reach the bootstrap log - add retry/timeout flags to the docker compose download - validate snapshot_retain_count (1-1000) at plan time - add language tags to fenced tree blocks (MD040) Co-Authored-By: OpenClaude <[email protected]> * fix(infra): address PR #27 review feedback - security group now derives its VPC from the selected subnet, so subnet_id overrides into non-default VPCs work - s3_access_key_id moves to SSM SecureString (same path as the secret key) instead of being embedded in user-data and state - ignore user_data drift on the instance: it only runs at first boot, so re-rendering caused pointless stop/starts; README documents the -replace workflow for config changes - DLM snapshot targeting is stack-specific (Snapshot + Name tags), not any Snapshot=true volume in the account - comment in fly.toml that it targets the shared test instance - user-data compose up gains --remove-orphans (consistent with the upgrade SSM document) - node service gets an HTTP /health healthcheck in compose Co-Authored-By: OpenClaude <[email protected]> * feat(infra): optional customer-managed KMS key for SSM secrets Add ssm_kms_key_id (default null = AWS-managed aws/ssm key). When set, all five SecureString parameters are encrypted with the CMK and the instance role gains kms:Decrypt scoped to that key — without it the boot-time secret fetch would fail. Co-Authored-By: OpenClaude <[email protected]> * fix(infra): scope CMK kms:Decrypt to this stack's SSM parameters The grant on the customer-managed key was unconditioned, allowing the instance role to decrypt any ciphertext under the same CMK. Restrict it with kms:ViaService = ssm.<region>.amazonaws.com and kms:EncryptionContext:PARAMETER_ARN limited to the stack's parameter ARNs. Co-Authored-By: OpenClaude <[email protected]> --------- Co-authored-by: OpenClaude <[email protected]>	2026-06-05 22:38:53 +08:00
Vasanth T	0b056f3ad0	[codex] fix Dependabot Rust alerts (#15) ... * fix(deps): resolve dependabot rust alerts * docs: clarify dependabot validation	2026-05-28 16:44:05 +08:00
Vasanth T	d366fc6b97	[codex] chore: add live-safe OSS readiness pass (#14) ... * chore: add live-safe OSS readiness pass * fix(node): verify optional peer signatures during rollout * docs: rewrite README for node roadmap	2026-05-28 15:34:47 +08:00
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]>	2026-04-29 16:34:27 +08:00