diff --git a/docs/architecture/backend.md b/docs/architecture/backend.md index c539b75..b7cc5ca 100644 --- a/docs/architecture/backend.md +++ b/docs/architecture/backend.md @@ -127,7 +127,10 @@ Two independent seams, two default-on features: Crate-root re-exports (the alktty pattern): backend traits + types (including `RepoRecord`, `AccessAction`, the `authorize` policy -function, and the `git/repo/*` op spec+handler pairs), `GitAdapter`/ +function, the authorized-repo marker type (the door constructs it only +after the ADR-007 check passes — session tuples carry it, transport.md +substrate layer), and the `git/repo/*` op spec+handler pairs), +`GitAdapter`/ `register_openable` (producer), `GitSession` (consumer), substrate types, `Limits`, protocol error enums; feature types (`GixBackend` family under `gix`, the file registry under `registry-file`) exported diff --git a/docs/architecture/decisions/007-acl-before-advertisement.md b/docs/architecture/decisions/007-acl-before-advertisement.md index a612a0e..aa458ad 100644 --- a/docs/architecture/decisions/007-acl-before-advertisement.md +++ b/docs/architecture/decisions/007-acl-before-advertisement.md @@ -36,6 +36,13 @@ Every session/request performs, in order, before any protocol output: the "advertisement is the only anonymous surface" line in the same doc's principle 1 is read as the *minimum* boundary, and this decision sets the operative rule). + *(Mechanism amended by ADR-011 §3 (review 001 D-2): the static ACL + engine fails closed on `identity: None`, so it cannot express + anonymous-public fetch. The per-repo check is alkgit-core's + `authorize` policy function evaluated on the registry record; the + alkcall registry gate still applies where the op has scopes. The + step *order* — resolve, then authorize, before any protocol output — + is unchanged.)* 4. Only then hand the session to transport. Adapters embed this sequence; transport asserts it (the session entry diff --git a/docs/architecture/decisions/014-v2-negotiation-ack-loop.md b/docs/architecture/decisions/014-v2-negotiation-ack-loop.md index f1290bc..51b0cea 100644 --- a/docs/architecture/decisions/014-v2-negotiation-ack-loop.md +++ b/docs/architecture/decisions/014-v2-negotiation-ack-loop.md @@ -53,6 +53,21 @@ Captures and the source-derived grammar are recorded in source-confirmed). An empty resulting pack (client already has everything) is a valid zero-object packfile response. + *(Boundary set amended per review 001 A-4: the raw request's haves + include never-verified client claims; the boundary set is the + *recognized* subset — request haves filtered through + `common_haves` (§5) — applied on the done round exactly as on the + ack rounds. The ack rule and the subtraction rule are the same + honest-boundary rule at different points: we do not honor haves we + could not verify. Existence is the operative predicate for + subtraction (a non-commit have can legitimately bound traversal; + §1's is-commit refinement exists for ACK-line correctness, not for + subtraction — `gix_traverse::commit::Simple::filtered` takes the + boundary predicate directly, so verify-then-subtract is one + predicate, not a custom walk). Cost: one existence check per have + on the done round — the wire layer already accepts paying it on + every ack round; no new budget kind.)* + 3. **Want-less rounds are answered empty.** A fetch round with no want lines (captured: an up-to-date client sends an empty round before exiting) gets an empty acknowledgments section — `acknowledgments` + diff --git a/docs/architecture/transport.md b/docs/architecture/transport.md index e591a56..fbf63fa 100644 --- a/docs/architecture/transport.md +++ b/docs/architecture/transport.md @@ -22,12 +22,16 @@ full decision (what each substrate owns and why); the surface is: - **Duplex session** (the `alk/git` ALPN producer, channels-opened git sessions, embedder stream doors) — input: (peer identity, resolved repo - id, duplex stream, `Limits`). Encapsulates the split/compat/packetline + id, authorized-repo marker, duplex stream, `Limits`). The marker is a + type the door/adapter constructs only after the ADR-007 check passes + (skipping the check is a type error — ADR-007; D-3). Encapsulates the + split/compat/packetline bridge, the request reader (delim-aware parsing, `reset()` discipline, break-on-error), and the sideband writer. - **Stateless session** (smart-http doors, e.g. alkhttp's future `git` - feature) — input: (peer identity, resolved repo id, request-reader, - response-writer, `Limits`) per http POST; adds the http-framing rules + feature) — input: (peer identity, resolved repo id, authorized-repo + marker, request-reader, response-writer, `Limits`) per http POST; adds + the http-framing rules (capability-dump skip, flush-only responses, probe handling). IO-abstract: the door supplies reader/writer; see [doors.md](doors.md) for the mounting. @@ -66,7 +70,10 @@ substrate property (per-request state), not a protocol fork. get an `acknowledgments` section (`ACK ` per recognized have via the backend's `common_haves`, `NAK` when none, flush; never `ready`), the `done` round generates closure(wants) − closure(haves) via - `GitPackGen`. No cross-round state on either substrate (the client + `GitPackGen`, with the boundary set the *recognized* subset (request + haves filtered through `common_haves` — the same honest-boundary rule + as the ack rounds; never honor an unverified have; ADR-014 §2). No + cross-round state on either substrate (the client re-sends wants + commons each round — negotiation-captures.md). Advertisement text is unchanged: `fetch=wait-for-done`. - Pack generation via `GitPackGen` (ADR-004), streamed over sideband on @@ -118,12 +125,21 @@ grammar-inferred. - io errors are terminal (session ends); protocol errors map to pkt-line error bands (duplex) or http status + body (stateless). Substrate-level `thiserror` enum; no panics in library code (convention 2). +- At the wire-mapping point (review 001 N-2), `RegistryError::NotFound` + and an authorization failure collapse to the same wire error — the + unknown-repo ≡ unauthorized indistinguishability rule (ADR-007/ADR-008) + holds at the variant→wire mapping, so no variant leaks an existence + oracle to the client. ## Limits Every session carries `Limits` (ADR-009): negotiation rounds, haves per round, receive-pack max size, wall clock, sideband chunk size (fixed 65000), advertisement ref cap. Missing `Limits` is a type-level error. +The advertisement ref cap is fail-closed like every other budget +(review 001 N-1): breach is a session error, never a silent truncation — +a partial ref list is the worst failure mode an advertisement can have +(clones appear to succeed). ## Public API surface (v1) diff --git a/docs/reviews/001-architecture-pre-decomposition-review.md b/docs/reviews/001-architecture-pre-decomposition-review.md index 9f224fb..4273adc 100644 --- a/docs/reviews/001-architecture-pre-decomposition-review.md +++ b/docs/reviews/001-architecture-pre-decomposition-review.md @@ -1,6 +1,6 @@ --- status: open -last_updated: 2026-09-25 +last_updated: 2026-09-29 reviewed_artifacts: - docs/architecture/README.md - docs/architecture/overview.md @@ -761,14 +761,14 @@ criticals are ADR-writing work, not code): | A-1 | op-gate OR not expressible in `AccessControl` | new ADR (or ADR-012 §3 amendment): handler-side two-tier check, create keeps static scope gate | small | none | **resolved (ADR-015)** — option (a) shape with the OR-term generalized to the `manage` grant | | A-2 | `async fn` traits not dyn-compatible | ADR-012 §1 + backend.md amendment: `#[async_trait]`; add `async-trait = "0.1"` to manifest | small | none | **resolved** — all five traits `#[async_trait]`, desugared boxed form pinned in the freeze inventory (OQ-03), dep in manifest | | A-3 | native preamble / service dimension unpinned | new ADR: open-op params `{repo, service}`, session tuple + stateless entry gain the service selector, `GitAdapter` preamble pinned | moderate | wire-format (freeze inventory) | open | -| A-4 | done-round boundary set unverified | ADR-014 §2 + transport.md clause: boundary = `common_haves`-filtered haves | trivial | none | open | +| A-4 | done-round boundary set unverified | ADR-014 §2 + transport.md clause: boundary = `common_haves`-filtered haves | trivial | none | **resolved** — boundary set is the recognized subset (`common_haves`-filtered), amendment clause in ADR-014 §2 + transport.md §fetch | | A-5 | consumer half unspecified | user scope decision, then amendment or small ADR (recommended: thin wrapper, deps carried with purpose) | small | scope | open | | A-6 | trait execution model unspecified | backend.md paragraph + transport.md rephrase: async traits, wire-layer permit, impl-internal spawn_blocking | small | none | **resolved** — backend.md concurrency model: wire layer enforces the ADR-009 permit around gen/ingest trait calls; impls own internal `spawn_blocking` (ADR-009/ADR-013 aligned) | | D-1 | stale Internal-ops + OQ-list text | supersession notes (vision, alk-stack, AGENTS) | trivial | none | open | -| D-2 | ADR-007 step-3 mechanism superseded | amendment note on ADR-007 | trivial | none | open | -| D-3 | authorized-repo marker missing from tuples | add to transport.md tuples + backend.md API list | trivial | none | open | -| N-1 | ref-cap breach behavior | one fail-closed clause in transport.md | trivial | none | open | -| N-2 | unknown ≡ unauthorized at wire mapping | one sentence in transport.md error taxonomy | trivial | none | open | +| D-2 | ADR-007 step-3 mechanism superseded | amendment note on ADR-007 | trivial | none | **resolved** — amendment note on ADR-007 step 3 (mechanism → ADR-011 `authorize`; step order unchanged) | +| D-3 | authorized-repo marker missing from tuples | add to transport.md tuples + backend.md API list | trivial | none | **resolved** — marker added to both substrate input tuples (transport.md) and backend.md public-API list | +| N-1 | ref-cap breach behavior | one fail-closed clause in transport.md | trivial | none | **resolved** — ref cap fail-closed clause in transport.md §Limits (breach is an error, never truncation) | +| N-2 | unknown ≡ unauthorized at wire mapping | one sentence in transport.md error taxonomy | trivial | none | **resolved** — collapse rule stated at the variant→wire mapping in transport.md §error taxonomy | | N-3 | schemas unpinned | backend.md types/schemas section | small | freeze inventory | open | | N-4 | push-options seam | pin additive parameter shape | trivial | none | open | | N-5 | `ls-refs=unborn` unverified | implementation-phase test rider (record in transport.md or a task) | trivial | none | open |