The committed lockfile lagged the manifests: workspace crates were bumped to
0.5.1 and icaptcha-client's Cargo.toml declares sha2, but the lock still
carried 0.5.0 and no sha2 entry. Every build regenerated it, leaving
Cargo.lock perpetually dirty. Regenerated offline (no dependency upgrades) so
the tree stays clean.
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.
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.
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.
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.
On a resumed request whose visit budget was already spent by the provenance
phase, the scan's top-of-loop visit arm sealed pager.cursor, which at that
moment is the position the caller just sent. The node returned the caller's own
token, verbatim, rung after rung. Three rungs were observed returning an
identical position.
A token looks like progress, so the client keeps going: gl retries to its
resume cap, and every one of those requests re-runs the full provenance phase,
up to seventeen repo acquires and cat-file subprocesses, advancing nothing
before it errors. That is roughly nine anonymous requests worth of work for
none, and it is worse than shedding nothing, because a caller who is told the
ladder is over stops immediately.
A seal now has to be strictly ahead of where the request itself started: a
different candidate is ahead by construction, since only the gated advance can
name one, and the same candidate needs a row past the start row. A request that
started at the front is before everything, so its seals pass untouched.
When the proposer settled at least one row this rung, the existing ceiling arm
already seals that row, and it is strictly ahead because a resumed scan only
walks rows past its cursor. Only a rung that settled nothing sheds without a
token, and that is honest: the spender is the provenance phase, which runs the
same way every rung, so no retry can do better.
The filter sits at the single mint site, where a future call site cannot bypass
it, and it logs the drop as a boolean. record_scan_truncation has already
logged that a position was sealed by then, and a 503 carrying no token next to
that line is the confusion that log exists to prevent.
The compose reinstall added last round is not safe under overlap, and `main`
never wrote compose from this document, so the race arrives with that commit.
Two executions — a double run, a retry while the first is still pulling, two
operators — shared the fixed `/opt/gitlawb/compose.yaml.new`. The second
truncates the file the first is about to install; and once the first renames,
the second's open descriptor keeps writing into the live `compose.yaml` inode.
Both outcomes stay valid YAML, so the `up -d --remove-orphans` on the next line
accepts a blended or truncated service set and deletes whatever fell off — on a
live node, the local postgres or the TLS terminator. Both reviewers reproduced
it independently; one measured a short file in 12 of 12 trials.
The whole step now runs under `flock -n` on /opt/gitlawb/.upgrade.lock and
exits non-zero if the lock is held, so a second run cannot restart against a
file the first is mid-install. The temp file comes from `mktemp`, so concurrent
writers cannot truncate each other, with a trap to clean it up on failure.
The compose body is base64-encoded rather than written through a nested
heredoc. The rendering carries `$${VAR}` passthroughs that must reach the file
unexpanded, which needs a quoted delimiter, and a shell heredoc nested inside an
indented Terraform heredoc depends on the dedent landing that delimiter at
column 0. Decoding one line has neither failure mode and no delimiter can
collide with the content.
Not validated locally: terraform is not installed on this machine, so this
change has had no `fmt`, `validate`, or `plan` run against it.
A CID that maps to several oids shared one resume slot across every candidate,
and the slot kept the maximum position. An earlier candidate could spend the
probe budget walking past a repo that holds the object for a later one, seal a
position beyond it, and the next rung would resume past a row that candidate
never examined, wrap, and shed tokenless. The client reads an absent token as
the ladder being over, so the object became permanently unretrievable, at stock
config, deterministically on every retry.
The token now names which candidate it is resuming, and the rules that keep
that sound are narrower than they first look:
Only one candidate per request may seal, and which one depends on where the
REQUEST started. On a resumed request it is the resumed candidate alone, since
the shared pager holds only the table suffix from the caller's cursor, so a
later candidate walked a suffix and never saw the front. On a front-started
request it is the first unfinished candidate, since there every candidate walks
from the front and a later candidate's stop is honest coverage. Silencing later
candidates unconditionally would remove the only thing that mints rung 1 when
the first candidate wraps untruncated.
A candidate is finished when its row loop walked every fetched row, or when it
owed no scan at all. Both matter: a properly provenanced candidate never wraps,
so without the second arm the ladder dies every rung. The wrap is witnessed per
candidate at the row loop's own two exits, never by reading the shared pager
flag at the tail, which any short page sets and which would let a candidate that
truncated mid-page look finished and strand the rows it refused.
Finishing a non-final candidate advances the seal to the next one at a front
sentinel and taints, because the tail emits a continuation only when something
tainted; sealing without tainting would suppress the taint and return a
definitive 404 while discarding the token it had just minted.
The keep-the-maximum comparison is gone. With one proposer per request the slot
is written at most once, so an assertion states that directly instead.
The pager stays shared per request. A per-candidate pager would restore the
fan-out the paging exists to remove.
A CID can map to several git oids, and the ladder needs to name which one it is
resuming. The sealed position gains the candidate's oid hex so a rung resumes
that candidate rather than a position in a list: oids_for_cid is a sorted set,
so an ordinal silently repoints at a different candidate when a pin that sorts
earlier arrives between rungs, while an identity degrades safely to "not found,
restart at the front".
The field is length-prefixed and padded to 64, matching the framing the row
fields already use, because production oids are 40 hex, not 64: repos are
created with --object-format=sha1 and only the test fixtures are sha256. A
fixed 64-byte field would fail every seal on a real deployment and shed a
tokenless 503, which the client reads as the ladder being over. Both widths are
exercised, and a zero-length candidate is rejected at decode so it cannot be
confused with the front-of-table sentinel, which is empty row fields with a
real candidate.
VERSION goes to 3 and the plaintext to 527 bytes, so a token minted under the
old layout opens to None and the caller restarts at the front. Nothing has
minted one outside tests.
The slot carrying the position is a struct rather than a widened tuple on
purpose: a 3-tuple would have pulled the hex into the existing keep-the-maximum
comparison, changing behavior this commit is meant to leave alone.
Token length stays invariant across both oid widths, since length would
otherwise be a side channel for the withheld row it names. That is asserted on
a real seal in gitlawb-core, not on the gl fixtures: nothing in gl seals or
opens a token, so its width constant cannot detect a wrong layout.
oids_for_cid ran a bare SELECT with no ORDER BY, so Postgres was free to return
the candidates in physical heap order. get_by_cid walks those candidates under
one shared probe budget, visit budget and pager, so whichever comes back first
is the one that spends the request's budget: two nodes holding identical data,
or one node before and after an unrelated write, could resolve the same CID
differently and one could shed a 503 where the other serves.
The instability is not hypothetical. An unpin and re-pin of a single object,
which is an ordinary production sequence, moves that row to the end of the heap
and rotates the list.
The sibling pin_sources_for_oid already orders its union for exactly this
reason, and the handler comment next to it leans on that determinism.
Code review of the previous commit turned up three things worth fixing in place.
record_scan_truncation centralized the taint and the seal but logged nothing, so
centralizing actually made a truncation less visible than the scattered inline
taints it replaced: only the visit ceiling logged, and it logs from inside the
gate, before the caller decides whether a position gets sealed. Two identical log
lines could therefore mean "the ladder continues" or "the caller is stranded".
One debug line now carries the reason and whether a position was sealed. It logs
only whether one exists, never its value, since the position names a withheld
row's created_at and id.
The doc comment claimed to be "the one site that records a scan truncation" while
eight other taint sites bypass it. The distinction is real but it is not the one
the comment drew: a ceiling stops the scan and owes the caller a position, while
a transient skip refuses one row and the rows behind it are still walked. Says
that now.
The forward-only rule's stated justification was wrong. A later candidate's
position is still ahead of the token the caller echoed, so letting it win would
not move the ladder backwards; it would shrink each rung toward a single row.
Also records that the comparison is Rust byte order while the pager's keyset runs
under the database collation, which can disagree on a non-C collation, and why
that costs a replay rather than a skipped row.
The probe and visit ceilings taint inside gate_and_serve and returned Skip, so
the row they refused and every row behind it were walked past without a verdict
while the resume position was sealed from pager.cursor, the end of the fetched
page. Two ways that stranded content:
- On the final page, `pager.exhausted` breaks ahead of every mint arm, so the
shed carried no continuation at all. A tokenless search_incomplete is the
wrapped-scan answer, which tells `gl ipfs get` its ladder is over, so a holder
on that page was unreachable on every retry.
- Mid-page, the sealed cursor sat past the refused rows, so the resume skipped
them. The shipped ladder tests all set page_rows == ceiling, which puts the
break exactly on a page boundary and hides both.
The ceiling now returns GateOutcome::CeilingStop rather than tainting on its way
out, and the scan loop stops there and seals the row in front of the one that was
refused. record_scan_truncation is the single site that records a truncation:
it taints and seals together, and only ever moves the position forward, so a
later oid candidate re-walking the same rows on a spent budget cannot hand back a
token the caller already echoed. The wrapped-scan tail no longer clears a seal,
since a ceiling can stop a resumed scan part way through the last page.
The lifecycle comment still listed image_tag among the values needing a deliberate instance replacement. It is rendered into compose.yaml, which the upgrade command now reinstalls, so apply plus upgrade rolls it out. Only the values user-data writes into .env at first boot stay replacement-only.
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.
Code review found the doc comments carrying claims the code does not
support and two coupling points documented from one side only.
The floor and the resolver's walk_cap are not textual twins: the floor
reads the constant, the resolver reads the AppState seam, and they agree
only because every construction seeds one from the other. Say that, and
give walk_cap the back-reference it lacked. The lifted node_verifies
helper no longer named the middleware it mirrors, so an edit to
require_signature would not find either copy.
The request-target clause pins @path, not @method or content-digest: a
301, 302 or 303 still rewrites a signed POST to a bodyless GET while the
signature headers ride along. Record that rather than implying the hop is
safe. Add the query-removed and fragment-only matrix rows, and a mutant
pinning that scan-phase walks are not charged to the work bucket, which
nothing covered.
The floor reserved one complete legacy search per window, probes plus page
tolls, but the provenance visibility walk debits the same bucket before the
fallback runs and its cap is charged per phase. With a route limit set below
the floor the provenance phase spent from the budget the search was promised,
the fallback 429d short of its configured reach, and the retry re-paid the
same walk charges, so a readable holder past that point stayed unreachable.
Add the walk term, min(MAX_HISTORY_WALKS_PER_REQUEST, ipfs_max_repos_walked),
which is textually the resolver's own walk_cap so the two move together. The
ladder fixture pins repos-walked to 1 to keep the page toll, not the new term,
as the thing binding it.
The verifier already rebuilds @path from the URI it was sent, which is why
the client-side redirect bug surfaced as a 401 rather than as a bypass.
Nothing pinned that, so a change to the reconstruction could drop the query
half or collapse it to a constant and only the clients would notice.
Drive the production router fixture with a signature made over one path and
a request on another, and again with a query mismatch, plus an identically
signed control. No pre-fix RED is obtainable here by construction, so each
case is proven load-bearing by injecting the defect it names.
The two same-origin follow tests drove exactly the path-changing hop the
predicate now refuses, and their mocks only proved a target was reached,
never that the signature verified there.
Rewrite both into refusal tests and run the real gitlawb-core verification
over the request the target actually received, so the recorded verdict is
what the assertion speaks about. Each refusal is paired with a positive
control that verifies, so an empty verdict slot is attributable to the
refusal rather than to a dead harness.
may_follow compared host, port and scheme only, so a same-origin hop that
normalized a trailing slash or a query was followed. The signature binds
@path as the client sent it and the node rebuilds it from the URI it
received, so that hop left the signature covering a target nobody asked
for and the read 401d.
Add the request-target clause and repoint the seven matrix rows that rode
on a path change, so each keeps pinning the host or port property it
exists for rather than going false on the path alone.
The previous round forwarded GITLAWB_ENFORCE_OWNER_PUSH in three compose
templates. Two of those are not the file their runtime actually reads, so the
documented rolling-upgrade opt-out did not work where it mattered.
Existing AWS instances could not opt out
-----------------------------------------
`aws_instance` ignores user_data drift, so an instance keeps the compose file it
was created with, and the published SSM upgrade only ran `docker compose pull &&
up -d`. Compose passes only the variables named in a service's `environment:`
block, so a key added to the template never reached a running instance: an
operator following our own instructions wrote GITLAWB_ENFORCE_OWNER_PUSH=false
to /opt/gitlawb/.env, upgraded, and got the new default anyway — silently, with
their CI pushers refused.
The upgrade document now reinstalls the rendered compose file before restarting.
It is idempotent (identical bytes on an already-current instance) and it makes
the upgrade a migration rather than a restart. compose.yaml is Terraform-owned
and overwritten; per-instance settings belong in .env, which is untouched.
infra/aws/README.md documents that `terraform apply` must precede the upgrade,
since the SSM document embeds the rendering.
The packaged macOS app read a different compose file
-----------------------------------------------------
The bundled Resources/docker-compose.yml is not what a packaged app runs: with
no source checkout selected, `generateComposeFile()` writes its own template,
whose environment block did not carry the key. `Config.writeEnvFile()` also
regenerates .env on every start, so a hand-added value was discarded. Between
them, a packaged-app user had no way to opt out at all.
`enforceOwnerPush` is now a persisted app setting, emitted into .env on every
start, and `generateComposeFile()` forwards it with the same interpolation as
the bundled resource — one definition, three consistent sites.
A test inherited the environment it was supposed to assert about
-----------------------------------------------------------------
`default_config_refuses_a_non_owner_push` built state through `test_state`, which
parses `Config` from the process environment — 47 fields come from GITLAWB_*
variables. On a host exporting GITLAWB_ENFORCE_OWNER_PUSH=false, which is exactly
what the rolling-upgrade guidance tells operators to set, it constructed a
disabled state, let the push through to git, and failed for a reason unrelated to
the code under test.
Renamed to `enforced_owner_push_refuses_a_signed_non_owner` and given an explicit
config through a new `test_state_with`, so it states the configuration it is
about. Verified by running it both with and without the variable exported. The
shipped default keeps its own env-independent proof off the parser declaration,
in config::tests — a parser question and an authorization question should not
share a failure mode. `test_state` now carries a warning about the hazard, since
any of those 47 fields can skew a test the same way.
Every item below was verified against the code before changing it; two of them
were confirmed by running the probe the reviewer described.
A test the fixture sweep missed, and that could not fail
--------------------------------------------------------
`f1_write_cap_is_inert_without_a_resolvable_source_key` seeded `z6f1none` and
pushed as `z6MkF1NoKeyPusher…`, so the owner gate rejected it before it reached
the per-caller cap the test exists to measure. Its assertion was
`!matches!(r, Err(Overloaded(_)))`, which a `Forbidden` satisfies, so it reported
ok while measuring nothing — and it is not `#[cfg(unix)]`, so it did that inside
every green run. Confirmed by inverting the assertion to expect `Forbidden`: it
passed, proving the request stopped at the gate.
The pusher is now the owner, and the assertion is positive: reaching git on the
missing on-disk repo (`Err(Git(_))`) is proof the request cleared both the gate
and the cap. The sibling `f1_write_cap_key_is_per_source_not_per_repo` carried
the same weak negative in its second assertion and is tightened the same way.
The default was barely tested, and what tested it read the environment
---------------------------------------------------------------------
`Config::parse_from` consults the process environment, so on a host exporting
`GITLAWB_ENFORCE_OWNER_PUSH=false` the old `enforce_owner_push_defaults_to_true`
reported the operator's setting rather than what this crate declares — it failed
outright, which would also break any contributor who sets the variable. It is
replaced by two tests that are env-independent and behavioural:
* `enforce_owner_push_is_declared_true_independent_of_the_environment` reads
the declared default off `Config::command()`.
* `default_config_refuses_a_non_owner_push` drives the handler on an untouched
`test_state` config and asserts 403 — authenticated by a real RFC 9421
signature, so 403 rather than 401 is what makes it a statement about
authorization.
Both were checked by reverting `default_value_t` to `false`: the handler test
reports 500 where it wants 403, and the declared test reports `["false"]`.
The cross-tenant test lost its second tenant
--------------------------------------------
The sweep collapsed `u1_parked_push_does_not_shed_another_pusher_behind_the_same
_ip` onto one identity, leaving the test named for cross-tenant behaviour with a
single tenant and two docstrings describing a scenario it no longer ran. `n2` is
now owned by `z6u1nat2` and its push signs as that owner, so two distinct tenants
share one edge IP again — which is the property under test, since the per-source
key is the resolved peer address and never the DID.
Four comments outlived their premise
-------------------------------------
`repos.rs` (3 sites) and `.env.example` still said owner enforcement defaults
off. The IP-keying rationale they support is unchanged and still correct — a DID
farm defeats a DID key — so only the dead premise is rewritten: the cap is
load-bearing because the rate limiter bounds arrival rate rather than occupancy,
because the gate can be turned off for a rolling upgrade, and because one owner
can open many concurrent pushes to their own repo.
Operators on the container path got none of this
-------------------------------------------------
`GITLAWB_ENFORCE_OWNER_PUSH` is now forwarded in `docker-compose.yml`, the macOS
app's bundled compose, and `infra/aws/compose.yaml.tftpl`, defaulting to `true`
to match the node. Without it the rolling-upgrade opt-out this PR documents in
three places was unreachable for anyone running compose.
A cutover this project never promised
--------------------------------------
The README paragraph announcing a 15 September 2026 flip of
`GITLAWB_REQUIRE_SIGNED_PEER_WRITES` is removed. That flag is not changed here,
the date appears nowhere on main, and a PR about a different flag is not where
that commitment would be made. The table row for `GITLAWB_ENFORCE_OWNER_PUSH`
carries the part that belongs.
A docstring that credited the wrong thing
------------------------------------------
The field and test docs claimed the value-taking action is what makes
`GITLAWB_ENFORCE_OWNER_PUSH=false` resolve to `false`. It is not: the env form
resolved under the presence-only declaration too. What the change buys is
`--enforce-owner-push false` parsing at all, rather than erroring as an
unexpected argument. Both docstrings now say so.
`p2_push` signed every push as one shared `P2_PUSHER` constant, but its two
callers own different repos — `z6p2tail` and `z6p2fail`. Pinning the constant to
either owner leaves the other rejected by the owner-push gate, so the identity
now follows the `owner` argument the helper already takes, the same way the
burst test derives it.
The rejection surfaced as `async fn resumed after completion` rather than a 403:
a denied push completes on the first poll, `p2_logged` never observes
`receive-pack`, and the polling loop then polls the finished future again.
Fourteen concurrency tests seed a repo through `f4_state_with_repo` or
`p2_parked_release_state` and then push under an unrelated DID, which the
owner-push gate now rejects before the machinery each test asserts on is ever
reached. They are transport and lease tests, not authorization tests, so each
now signs as the owner of the repo it targets.
Two needed more than a swapped constant:
- `receive_pack_burst_scans_serialized_and_both_pushes_succeed` pushes to two
repos with different owners, so the identity is derived per push from the
owner already threaded through the closure.
- `u1_parked_push_does_not_shed_another_pusher_behind_the_same_ip` used three
distinct pusher DIDs, which owner-only push makes impossible. The distinction
was narrative: the per-source key is the resolved peer IP and never the DID,
so three concurrent pushes from one edge IP contend exactly as before. The
comment now says so rather than leaving the reader to infer it.
The gate keeps its own coverage in `enforced_rejects_non_owner_with_forbidden`
and `enforced_rejects_missing_did_with_forbidden`, so nothing here removes the
assertion that a non-owner push is refused.
These tests are `#[cfg(unix)]`, so a Windows checkout cannot run them; they were
missed in the previous round because the Windows suite runs 717 of the 825 tests
Linux runs.
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.
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
The dependency allowlist caught this: sharing the predicate pulled url, and
behind it idna and the icu crates, into the closure of the one crate that is
supposed to stay embeddable. Both clients that need the predicate already parse
URLs, so they opt in and nothing else pays for it.
The test dependency is not optional, so the origin matrix still runs under a
bare cargo test -p gitlawb-core. Gating the module on the feature alone would
have left those tests silently unbuilt, which is the failure the module is
there to prevent.
The previous commit stopped the CLI carrying request signatures to a host the
node redirected it to, and left the other signing client alone. That one is the
git remote helper, which is what actually runs on clone, fetch and push, and it
had no redirect policy at all. A push signs from its first request, so a node
answering 302 handed out a working credential, and on a 307 or 308 the pack
body went with it. The rule now lives in core and both clients use it, so the
two cannot drift apart again.
Three guards were not holding what they claimed. Bounding the retry wait by the
remaining deadline could be deleted with every test still green, which the
timing shows was four real seconds of overshoot past the bound the docs
advertise; the assertion was loose enough to pass either way and is now tight
enough to fail. The refusing-gateway warning was asserted through a test-only
mirror rather than the write the user sees, so deleting the write kept the test
green. And both no-hint exceptions, the definitive not-found and the wrapped
scan, were only ever reached without a token in hand, which is the one state
where they do nothing.
A response whose body failed partway through the read still replayed the held
token for every remaining attempt, spending the whole ladder on one position and
then reporting that the object might lie further on. A body cut by the read cap
was already terminal for that reason; a body cut by a broken connection is the
same unreadable state and is terminal now too.
A successful fetch no longer buffers the whole object before writing a byte of
it, so a node cannot make the client hold an arbitrary amount of memory inside a
time limit that was never a memory limit. The remaining silent skips in the
recovery loops now say what went wrong, including the manifest fetch and the
body reads, so a failure there is reported rather than looking like an absent
blob.
Also: the redirect chain bound is off by one against reqwest's own, has a test,
and its comment no longer claims to prevent something the request timeout
already prevents.
The node client followed redirects with no policy, and reqwest strips only the
classic credential headers when a redirect crosses origins. RFC 9421 signature
headers are not on that list, so a node answering 302 could hand a caller's
signature to another host, which then served whatever bytes it liked for a
content address that never hashed to them. Following continuations made it
worse by turning one signed request per invocation into as many as nine. The
signature covers the method, the path and the body digest but not the
authority, and the node accepts any signature inside a five minute skew with no
nonce ledger, so one harvested header reads a caller's private paths at any
node in the fleet.
Redirects are now scoped to the origin that issued them. Same-origin ones still
follow, so an http to https upgrade or a trailing-slash normalization keeps
working, and a cross-origin one stops rather than errors, so the caller sees
the 3xx through the status path it already has. A downgrade back to http on the
same host is refused too: the origin matches, but the signature would leave in
cleartext, which is the same leak by a slower route. A custom policy replaces
reqwest's built-in chain limit, so the limit is restated rather than lost.
The recovery path in clone had the same silence the CLI just lost: it skipped a
blob on any non-success without saying why, so a truncated scan, a permission
denial and a dead gateway were one indistinguishable outcome. It now names the
status or the transport error, capped and stripped of control characters since
the text comes from a caller-chosen gateway, and continues as before.
Reading a response body no longer conflates three endings. The cap cutting the
body, the body finishing, and the read failing partway were all one silent
break, so a failed read surfaced as an error message that stopped after the
colon. Each is now reported, and a body that could not be read says so.
Coverage for three paths that were previously argued rather than run: a
transport failure mid-ladder still hands back the resume token, a scan that
ends before any ceiling still answers a definitive not-found, and a response
whose body stalls after its headers is still ended by the client's own
deadline.
Review of the scan-continuation work found the ladder surfacing its token on
three terminals and dropping it on three others: a mid-ladder 5xx that is not
an overload, a truncation whose offered continuation fails validation while a
good token is still held, and a transport error. In each case the user is told
to start over, and a bare re-run restarts at the first row and re-spends the
node's per-IP budget, which is the failure the token exists to prevent. It is
surfaced on those paths now, still never on a definitive 404 or on the node's
deliberate scan-wrapped signal, and a rejected token is still never echoed
back.
A body cut at the read cap used to change the answer. Truncated mid-JSON it
failed to parse, lost its error code, and fell to the transient-overload arm,
which retried the old position for every remaining attempt and then reported
that the object might lie beyond the rows scanned when nothing past the first
page had been. The cap now reports that it cut, and a cut body is terminal.
Three guards were doing nothing. Mutation testing showed the Retry-After clamp
could be deleted with every test still green, so a node advertising a day-long
wait would have parked the client inside a single sleep that neither the
attempt cap nor the deadline can interrupt. The rejection of a malformed
caller-supplied token was equally untested, though the same property is proven
on the node-offered side. The withheld-blob ladder asserted only that each rung
was not a success, which any early exit satisfies without the rule ever being
evaluated; it now witnesses that the scan reached the end of the table.
The inter-attempt wait is bounded by the time left on the deadline, so a run
can no longer overshoot the bound it advertises. That drops the wait out of the
worst case entirely rather than shrinking it: the last attempt starts before
the deadline and the client's total timeout covers that request through the end
of its body, so a run is over by the deadline plus that timeout. The docs said
about 95 seconds and now say about 90.
They also said a large download already streaming is never cut off part way.
That was wrong. The scan deadline does not cover the download, but the client's
timeout is a total request deadline running until the body finishes, so a slow
transfer is cut off. Both prose sites now say what the code does.
Also: stdout is flushed before the command returns, so a closed pipe fails
loudly instead of yielding a truncated object with a success status.
Document the behaviour rather than the mechanism: that an object pinned before
the node recorded its repo is found by scanning the inventory, that the scan
stops at the node's ceilings, and that the command follows the resume token the
node hands back instead of reporting a false not-found.
State the bounds in values, not constant names, and state honestly what each
one covers. The deadline bounds the search: every attempt gets the time left on
it to produce response headers, and it deliberately does not cover downloading
an object once found, so a large blob already streaming is never cut off. The
error-body read runs outside it under the client's own timeout, which is why
the longest a single run can take is about 95 seconds rather than the flat
deadline, and the in-code comment now derives that instead of asserting it.
Say what is not promised, too. A rate limit ends the ladder because its window
is an hour and its Retry-After cannot be honored inside one invocation, and the
node's per-IP fanout brake can end a ladder well short of the cap, so automatic
resumption is not a guarantee that the object will be reached.
Where a bound stops the ladder, the printed invocation is the only thing that
makes progress: a bare re-run restarts at the first row, reproduces the same
truncation and spends the per-IP budget again.
The bounded terminals printed "gl ipfs get <cid> --scan <token>", but the
command took no --scan argument, so following that advice failed on an
unexpected argument and the resume input existed only behind a private seam.
The flag now exists and forwards to the same validation a node-supplied token
gets, which is what makes surfacing the token worth anything: without it a
stopped ladder restarts from the front on every rerun.
Three test fixes ride along, all found by running the mutation matrix rather
than by reading.
The deadline scenario asserted its message "must name the deadline" while its
fixture CID was bafkreideadline, so any message echoing the CID satisfied it.
Under a mutant that removed the deadline the ladder ran to the cap and that
assertion still passed. The CID is renamed so the assertion can only pass when
the message genuinely names the deadline, and the mutant now reddens on it
instead of on the call count.
The default classification arm was certified only for the token-less case: no
fixture reached it with a token already held, so degrading it to retry was an
equivalent mutation against the whole suite. A scenario now drives an
unrecognized code mid-ladder with a token in hand and asserts it is terminal.
The wired path from the argument to the resume input had no coverage; only the
seam was tested. A scenario now goes through the function clap dispatches to,
so dropping the argument turns it red.
A truncated legacy CID scan answers 503 with a sealed continuation token, and
the command turned that into a hard error, so content past a scan ceiling was
unreachable from the first-party client. It now follows the continuation
automatically.
Three bounds, each protecting a different resource. The attempt cap bounds the
work asked of the node. A wall-clock deadline bounds the caller's own time and
is passed into each request as that request's timeout, so a slow-but-alive node
cannot outlast it. The per-wait clamp bounds a single Retry-After, so a hostile
value cannot stall the client. There is deliberately no non-advancing-token
check: tokens are nonce-randomized ciphertext, so two seals of one position are
never byte-equal and a client cannot detect a wedged ladder by comparing them.
Classification happens at one site with no wildcard to retry. Status is checked
before the body is read. A rate limit is terminal on status alone, since its
window is an hour and its Retry-After cannot be honored inside one invocation.
An overload mid-ladder is retried on the still-held token, because that shed
releases its permit when the request ends and the node advertises a retry. Any
other status or code is terminal through the default arm, including an overload
on the first request, where no token is held and there is nothing to resume.
Every terminal message is capped and sanitized before it reaches the terminal,
and a rejected token is never echoed back; the bound is named instead. Tokens
are accepted only as non-empty base64url within a generous ceiling, so a future
change to the sealed layout cannot silently disable resume.
Where a bound stops the ladder, the still-held token is printed with the
invocation that resumes from it, rather than wording that implies a bare re-run
makes progress.
gl suite 332 green, fmt and clippy clean.
The node answers a truncated legacy CID scan with a 503 carrying a sealed
continuation token, and gl ipfs get turns every non-success into a hard error,
so content reachable only past a scan ceiling cannot be fetched with the
first-party client at all. The old test asserted that terminal 503 as the
desired behaviour, and its fixture body predates the token, so it is replaced
rather than adjusted.
Thirteen scenarios pin the contract: a resume that completes, the attempt cap,
a wedged server that keeps minting fresh tokens for the same position, a
missing or malformed token, a first-request overload, the signed resume, the
wall-clock deadline, a mid-ladder rate limit that is terminal, a mid-ladder
overload that is retried, the token length boundary pair, the token surfaced at
every bounded terminal, and a caller-supplied continuation that starts from the
token rather than the front.
Hostile-input scenarios assert the surfaced message is capped and stripped of
control and bidi characters and that a rejected token is never echoed back.
Ladder fixtures answer Retry-After: 0 so the clamped waits are zero and the
whole gl suite still runs in under a second in real time.
The seam signatures land here so the tests compile and fail on their
assertions: cmd_get delegates to cmd_get_inner, which threads a deadline, an
attempt cap, and an optional starting continuation. Its body is unchanged and
still bails on any non-success, so all fourteen new tests fail on the property
each one names.
GITLAWB_IPFS_MAX_LEGACY_SCAN_ROWS accepts any value from one upward, but the
pager always asked the database for a full page and the ceiling was only
checked afterwards, in the scan loop. With the ceiling below the page size an
anonymous request still selected and rule-loaded a whole page while holding
walk admission, and any ceiling that is not a page multiple overshot by up to
a page. Compute the remaining budget first and ask for min(page, remaining),
so the page size stays a batching detail rather than an implicit floor under an
operator ceiling.
The exhaustion inference moves with it. A short page used to prove the table
was exhausted; once the limit can be shortened by the budget it proves nothing,
and marking the pager exhausted breaks at the top-of-loop arm that sits ahead
of every ceiling arm, taints nothing, and mints no continuation. That path ends
in a definitive 404 for content that exists, so the predicate now compares the
returned rows against the limit actually sent.
The budget computation sits above the timeout opener so the deadline-wrapping
guard's lookback window is unaffected, and the limit is read from the AppState
ceiling the row-ceiling arm reads, not the config field, so the arm and the
query cannot disagree.
Full suite 1059 green, fmt, clippy, and --locked clean.
The row ceiling is enforced in the scan loop, after a full page has already
been fetched and rule-loaded, so a cap below the page size is ignored for the
first fetch and any cap that is not a page multiple overshoots by up to a page.
The existing coverage could not catch that. Both row-ceiling tests asserted
rows <= ceiling + one page, which passes whether or not the query is bounded,
and rows-returned cannot distinguish a bounded query from a full fetch that is
trimmed afterwards. Add a scan_limit seam recording the limit actually sent to
the database, tighten both tolerant assertions to exact counts, and add two
tests that pin the limit: a ceiling below the page size and a ceiling that is
not a page multiple.
Also add the fail-open fixture. A repo evaluated with a partial rule set is
served as if it had no rules, so the boundary repo is seeded public at root
with a path-scoped rule and a real blob on disk, pinned with NULL provenance so
the scan path's rule map is the one consulted, and asserted never served across
its own ladder. Its positive control serves the same blob with the rule absent,
so a failure there is attributable to the rules rather than to a probe or a
budget skip.
Both new tests fail against current code on the scan_limit assertion.
Both write-lock tests hand-copied advisory_lock_key's derivation. The copy
still used DefaultHasher, so once the key moved to SHA-256 (#210) the probe
checked a key nobody held. The disconnect test then failed asserting the
lock was held, and the success test passed asserting a release it never
actually observed.
Import the production function, as the acquire-deadline test already does,
so the two cannot diverge again.
beardthelion