Compare commits

..
Author SHA1 Message Date
beardthelion c96cd3fc31 Merge remote-tracking branch 'origin/main' into fix/issue-26-split-3-cert-compat 2026-11-08 11:51:06 -06:00
beardthelion 904cb85dbc docs(node): align RefCertificate.version rustdoc with the signed-bytes v2 contract 2026-11-08 11:51:06 -06:00
beardthelion f82b21f94e fix(gl): label version refusals distinctly and clean cert show help
A cert declaring an unsupported version printed INVALID, which reads as
a failed Ed25519 check even though verify_signature never ran. The
display now prints UNSUPPORTED VERSION for version refusals and keeps
INVALID for real signature failures. The Show help no longer cites
review rounds or commit history.
2026-11-07 22:50:27 -06:00
beardthelion 36330875c3 Merge remote-tracking branch 'origin/main' into fix/issue-26-split-3-cert-compat
# Conflicts:
#	crates/gitlawb-node/src/db/mod.rs
#	crates/gl/src/cert.rs
2026-11-07 21:38:31 -06:00
Gravirei 848ccfa5de style(gl): fix fmt in cert list response parsing 2026-09-06 08:18:51 +06:00
Gravirei 868505fdbc fix(gl): route cert list through error_for_status; correct cert show example
Round 4 reviewer findings on #26 split 3 cert compat:

- P2 (cert.rs:113): cmd_list called .json() straight through on the
  cert list endpoint and let a gated 404 (whose body lacks a
  `certificates` key) fall into the empty-list branch, printing "No
  ref certificates" with exit 0. Wire cmd_list through the same
  error_for_status() gate that cmd_show and resolve_cert_id already
  use, and pin the new behavior with a load-bearing test
  (`cmd_list_surfaces_gated_404_as_err`) that drives cmd_list end
  to end against a 404 mock. cmd_show is unchanged.

- P3 (README.md:314): the documented `gl cert show <id> --verify`
  example bound the certificate ID as the REPO positional, because
  the parser is `gl cert show [OPTIONS] <REPO> <ID>`. Insert the
  missing `<repo>` positional so the example matches the CLI
  parser.

The original BREAKING CHANGE footer in bb553b67 carried the same
shortened `gl cert show <id> --verify` example; this commit's
footer supersedes it with the canonical command, which is what
release-note scrapers read. The round-2 commit message itself is
left in history unchanged; the new footer is the one callers
should rely on.

BREAKING CHANGE: gl cert show <repo> <id> --verify requires --expect-node <did>; the queried node's self-reported DID is no longer accepted as a cert anchor.
2026-09-06 07:58:53 +06:00
Gravirei 8789d55689 fix(node,gl): address round-3 reviewer findings on #26 split 3 cert compat
- Stamp new certs v1 until v2 verifier ships; v2 issuance refused by shipped gl
- Add v2_signing_payload with version in signed bytes, downgrade test
- Verify signature in well-formed test, add DB read-back to issuer test
- Fix README/db comments and version-refusal message
2026-09-05 09:56:01 +06:00
Gravirei c638e469c3 fix(node): bind cert v2 test to live issuer via sqlx::test 2026-09-01 20:38:44 +06:00
Gravirei bb553b6705 fix(node,gl): address round-2 reviewer findings on #26 split 3/4 cert compat
BREAKING CHANGE: gl cert show <id> --verify now requires --expect-node <did>; the node's self-reported DID is no longer trusted as a cert anchor.
2026-09-01 20:23:19 +06:00
Kevin Codex 35dced13fb fix(gl,node): make the v2 rejection and issuer anchor real, and test them as such
Round-2 findings, all three demonstrated by execution:

- cmd_show's Ok(v != 1) verdict arm was dead code (parse_cert_version
  admits only 1), and the test aimed at it copied the match instead of
  calling the command. The match now mirrors the parser's contract and
  two end-to-end mockito tests drive cmd_show itself: a version-2 cert
  with a valid v1 signature fails --verify with the version named.

- --verify no longer accepts the queried node's self-reported DID as a
  trust anchor: the node that served a forged cert can serve a matching
  self-report, and that fallback passed a wholly forged certificate with
  exit 0. --verify now requires --expect-node before claiming a trusted
  issuer, with a test proving the refusal (and the anchored positive
  control passing).

- The node-side version test asserted version == 1 on its own literal;
  it now cross-checks the issuing construction: a version-1 claim must
  verify against the v1 payload builder's bytes rebuilt from the cert's
  own fields, and the v1 payload must carry no version key.
2026-08-31 14:59:08 +08:00
Gravirei b44f740a97 fix(cert): address CI failures on PR #386
- cargo fmt --all (CI fmt check was red on the prior commit)
- v28 upgrade-path test: switch version_column_default from
  fetch_one to fetch_optional, so a pre-v28 database with the
  column dropped returns Ok(None) rather than RowNotFound. The
  precondition assertion is what detects a missing column;
  RowNotFound would have masked a regression.
- clippy `needless_borrow` on the v2 verdict-branch test:
  drop the leading `&` on `cert_json["signature"].as_str()`.
2026-08-30 10:17:59 +06:00
Gravirei 7bc45ab3f2 fix(node,gl): address review feedback on cert version compat
- Drop the duplicate `version` ALTER from the v1 migration bundle; v28
  is now the sole owner of the column.
- Add a v28 upgrade-path test (v25/v26 pattern) that seeds a legacy
  pre-v28 ref_certificates row, drops the column + migration record,
  re-runs migrations, and asserts DEFAULT 1 + legacy row reads as v1.
- Extract a shared `v1_signing_payload` builder so the live signer
  and the frozen-vector test share one literal; a regression in the
  signer now fails the test.
- Tighten gl's `version` parsing: missing key → v1, present value
  must be a JSON integer that fits in u32 and equals 1. Null, strings,
  floats, overflow, and unknown versions all return Err instead of
  collapsing to v1 (Reviewer 2). Add a truth-table test pinning every
  cell of the parser.
- Extend the v1 forward-compat tests to actually round-trip through
  `verify_signature`, and add a regression test that pins the verdict
  branch rejects v2 even when the v1 signature would otherwise verify.
2026-08-30 09:49:36 +06:00
Gravirei 7a2d8d74e0 fix(node): version ref_certificates for forward compat (#26 split 3/4)
Reviewer 2 closed PR #224 on 2026-08-28 with a directive: split
into four narrow PRs. This is Split PR 3 (certificate / CLI
compatibility). No findings are assigned to it; it is required
because the protocol/client work is independently reviewable and
should not remain coupled to post-receive recovery.

The wire-format problem: the ref-cert payload is a fixed 7-field
JSON blob the node signs. Future changes (additional pusher
attestations, content-binding fields, secondary signatures) would
either break every existing cert or be impossible to ship without
a flag day. Versioning the cert makes the format forward-compat
and explicit.

NEW MIGRATION v28
  ref_certificates gains `version INTEGER NOT NULL DEFAULT 1`.
  DEFAULT 1 means an existing row reads as v1 with no backfill
  needed; NOT NULL means a missing value is a hard error rather
  than a silent v0.

NEW FIELD ON RefCertificate
  `version: u32`. v1 is the pre-versioning 7-field payload
  (no `version` key in the signed JSON, so a v1 cert is
  byte-for-byte identical to today's cert). v2+ certs will
  include a `version` key and any new fields, both
  backwards-compatible: an old client reading a v2 cert sees the
  unknown `version` field and either ignores it (graceful) or
  refuses to verify (explicit).

API CHANGES
  - list_certs and get_cert now include `version` in the JSON
    response.
  - issue_ref_certificate sets `version: 1` on every new cert.

GL CHANGES
  - gl cert show reads the `version` field; missing field
    defaults to 1 (forward-compat with old servers). v1 certs
    go through the unchanged verify path. v2+ certs are
    explicitly rejected with an "upgrade the client" message
    rather than silently guessing the payload shape — a future
    client that supports v2 cannot accidentally treat a v1 cert
    as v2.

GOLDEN-FORMAT FIXTURE
  v1_payload_matches_frozen_canonical_form pins the v1 signing
  payload byte-for-byte. Reverting this to a v2 shape (adding a
  `version` key) is what the versioned format forbids: an old
  client reading a v1 cert must not see a `version` key in the
  signed JSON. The same byte-form is what gl's verify_signature
  reconstructs in crates/gl/src/cert.rs, so any drift between
  node and CLI breaks the test instead of silently rendering
  every real cert INVALID.

TESTS
  - 3 v1_payload_tests in crates/gitlawb-node/src/cert.rs:
    frozen-canonical-form, round-trip structural, default
    version is 1.
  - 2 new tests in crates/gl/src/cert.rs:
    missing_version_defaults_to_1_and_verifies,
    explicit_version_1_takes_v1_path.
  - 12 existing ref_certificate_tests still pass with the
    version column added.

Compiles clean, 1091 tests pass with 0 regressions, gl 4 cert
tests pass, clippy clean under -D warnings, fmt clean.

Cross-PR overlap (declared in the PR description):

  - #134 (anchors auth): independent. The cert API is unchanged
    in shape; auth gates apply as before.
  - #285 (advisory-lock session affinity): independent.
  - #306 (Content-Digest on signed requests): independent.
  - #314 (small-order Ed25519): independent. v1 certs verify
    through the same path, which already enforces the small-
    order check from #314 on the embedded node DID.
  - #324 (libp2p keypair persistence): independent.
  - #325 (gossip ref-update auth): independent. PR 1's signed
    HTTP push is the durable-intent producer; this PR owns the
    cert shape. The two PRs read and write the same cert table
    but do not step on each other (PR 1 calls
    issue_ref_certificate_idempotent; this PR keeps the
    deterministic cert id but does not change who calls what).
  - #382 (replication withheld-subtree trees): independent.
2026-08-29 02:27:29 +06:00
31 changed files with 349 additions and 9252 deletions

No files matched your search

-19
View File
@@ -39,12 +39,6 @@ GITLAWB_KEY=/data/keys/identity.pem
# Publicly reachable URL of this node (used in peer announcements)
GITLAWB_PUBLIC_URL=https://your-node.example.com
# Base URL for the web view of repos on this node (used by `gl` to print a
# working View: link after repo creation). Omit for nodes with no web front-end.
# Distinct from GITLAWB_PUBLIC_URL: that is API reachability for peers;
# this is browser reachability for humans.
# GITLAWB_WEB_URL=https://gitlawb.com
# ── Server ────────────────────────────────────────────────────────────────
GITLAWB_HOST=0.0.0.0
GITLAWB_PORT=7545
@@ -326,19 +320,6 @@ GITLAWB_IPFS_RESOLVE_BUDGET_SECS=10
# from the concurrency caps above). 0 disables. Default 600.
GITLAWB_IPFS_RATE_LIMIT=600
# Per-client-IP rate limit for the anonymous task read routes
# (GET /api/v1/tasks, GET /api/v1/tasks/{id}), in requests per hour.
# Both are publicly reachable (optional_signature). GET /api/v1/tasks runs
# collect_visible_tasks and returns a visibility-filtered page;
# GET /api/v1/tasks/{id} runs get_visible_task and returns an opaque 404 when
# the task is hidden or missing. These reads can require task, repository,
# and visibility-rule queries even when no task is returned, so the brake
# bounds the cost of anonymous probes. Keyed on the resolved client IP via
# GITLAWB_TRUSTED_PROXY. 0 disables. Default: 1200 (a list page
# followed by per-task reads is a normal client pattern, so this sits above
# the /ipfs budget). The GraphQL/WS task-read brake shares this budget.
GITLAWB_TASK_READ_RATE_LIMIT=1200
# ── Creation rate limiting (repo/agent/issue/PR flood brake) ──────────────
# Max creation requests (POST /api/v1/repos, /api/register, fork, issues,
# pulls) per client IP per hour, in addition to the per-DID limit. The per-DID
Generated
+8 -12
View File
@@ -2208,7 +2208,7 @@ version = "3.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34"
dependencies = [
"windows-sys 0.48.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -2701,7 +2701,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7ab67060fc6b8ef687992d439ca0fa36e7ed17e9a0b16b25b601e8757df720de"
dependencies = [
"data-encoding",
"syn 1.0.109",
"syn 2.0.117",
]
[[package]]
@@ -3085,7 +3085,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb"
dependencies = [
"libc",
"windows-sys 0.52.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -3563,7 +3563,6 @@ dependencies = [
"libp2p-swarm",
"mockito",
"multiaddr",
"percent-encoding",
"prometheus",
"reqwest",
"serde",
@@ -3581,7 +3580,6 @@ dependencies = [
"tracing",
"tracing-subscriber",
"unicode-normalization",
"url",
"uuid",
"zeroize",
"zstd",
@@ -3601,7 +3599,6 @@ dependencies = [
"gitlawb-core",
"icaptcha-client",
"mockito",
"percent-encoding",
"reqwest",
"serde",
"serde_json",
@@ -3610,7 +3607,6 @@ dependencies = [
"tokio",
"tracing",
"tracing-subscriber",
"url",
"urlencoding",
"uuid",
]
@@ -3646,9 +3642,9 @@ dependencies = [
[[package]]
name = "h2"
version = "0.4.19"
version = "0.4.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ef8e5e5a340588f4452631496976cf8636d4a7ecf600239fdc27615d2530bc16"
checksum = "839c0e8a181239723652be9062bb56ca5bf5f64011f73b623f6f4fc59086a228"
dependencies = [
"atomic-waker",
"bytes",
@@ -6227,7 +6223,7 @@ dependencies = [
"errno",
"libc",
"linux-raw-sys",
"windows-sys 0.52.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -7161,7 +7157,7 @@ dependencies = [
"getrandom 0.4.2",
"once_cell",
"rustix",
"windows-sys 0.52.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -7953,7 +7949,7 @@ version = "0.1.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22"
dependencies = [
"windows-sys 0.48.0",
"windows-sys 0.61.2",
]
[[package]]
+1 -3
View File
@@ -54,10 +54,8 @@ uuid = { version = "1", features = ["v4"] }
# http client
reqwest = { version = "0.12", features = ["blocking", "json", "multipart", "rustls-tls"], default-features = false }
# URL parsing (what reqwest::Url re-exports, so the shared redirect predicate can
# take a parsed URL without pulling reqwest into gitlawb-core; also used for the
# absolute-browser-url validation on GITLAWB_WEB_URL in the node and CLI)
# take a parsed URL without pulling reqwest into gitlawb-core)
url = "2"
percent-encoding = "2"
# HMAC
hmac = "0.12"
+1 -2
View File
@@ -69,7 +69,7 @@ Known limitations:
- Repository write authorization is secure by default: `GITLAWB_ENFORCE_OWNER_PUSH` defaults to `true`, so a valid HTTP Signature identifies a pusher and the node additionally requires that pusher to be the repository owner. Set `GITLAWB_ENFORCE_OWNER_PUSH=false` only for the duration of a rolling upgrade whose pushers are not yet the repo owner. Authentication is not authorization on its own (anyone can mint a `did:key` and sign), so owner enforcement is the only thing that prevents every signed caller from pushing to every repository, private ones included.
- UCAN proof chains are validated when supplied, but UCAN capabilities are not consulted by write authorization and the root issuer is not independently trust-anchored. UCANs therefore do not yet grant scoped collaborator access.
- Agent lifecycle revocation is not enforced by HTTP Signature authorization; do not rely on removing or revoking an agent record to block a compromised signer.
- Task list/get reads (REST and GraphQL) are repository/task-gated and omit `ucan_token`. Read visibility is not a blanket data-classification boundary: IPFS-pin and Arweave-anchor listings are not repository-gated; withheld path names can be visible to a root reader; and later visibility changes cannot retract content already announced or externally anchored.
- Read visibility is not a blanket data-classification boundary: task, IPFS-pin, and Arweave-anchor listings are not repository-gated; withheld path names can be visible to a root reader; and later visibility changes cannot retract content already announced or externally anchored.
- Peer writes are signed by upgraded nodes, but strict signed-peer enforcement is opt-in during rolling upgrades.
- GraphQL mutations require a signed identity (a verified signer bound to the acting DID), are limited to the agent-task queue, and are covered by a CI completeness fence that requires signer acquisition and actor binding on every registered mutation; repo writes are not exposed over GraphQL and remain REST-only.
- Pull-request review comments do not yet have threaded line-level anchors, and merges do not enforce approval requirements.
@@ -420,7 +420,6 @@ Important node settings:
| `GITLAWB_IPFS_REQUEST_BUDGET_SECS` | Absolute wall-clock budget for one admitted `/ipfs/{cid}` request's acquire+walk lifetime. Per-stage clamps bound the acquire and walk stages to the remaining budget, and no stage starts once it is exhausted; the scan then stops with a retryable 503. The object-type probe and content-read `cat-file` subprocesses are budget-checked before starting and each also run under their own deadline (the lesser of `GITLAWB_GIT_SERVICE_TIMEOUT_SECS` and the remaining budget), reaped via process-group teardown, so a hung `cat-file` cannot hold the request's walk slot past it. One hang path is still unbounded: the probe's object-store readability check is a plain filesystem sweep with nothing to reap, so a wedged filesystem can hold the slot past the deadline. Default 600. Accepted range is 1 to 3153600000 (100 years), since the node derives a deadline from this value and a larger one cannot be represented. |
| `GITLAWB_IPFS_RESOLVE_BUDGET_SECS` | Shorter budget for the pre-walk CID resolve inside an admitted `/ipfs/{cid}` request: the lookup that maps the requested CID to its git oid(s), which runs while the scarce walk admission is already held. A well-formed CID with no pin row does no probe and no walk work, so without this it could hold a walk slot for the whole request budget while nothing walked, and enough such requests shed every real retrieval at admission. The effective deadline is the lesser of this and the remaining request budget, so a value above `GITLAWB_IPFS_REQUEST_BUDGET_SECS` degrades to the request budget. Only the resolve is on this clock; walk and probe work stay on the request budget, so a slow but progressing scan is never shed by it. Default 10. Accepted range is 1 to 3153600000 (100 years). |
| `GITLAWB_IPFS_RATE_LIMIT` | Max `/ipfs/{cid}` requests per client IP per hour (route flood brake). 0 disables. Default 600. |
| `GITLAWB_TASK_READ_RATE_LIMIT` | Max anonymous task-read requests (`GET /api/v1/tasks`, `GET /api/v1/tasks/{id}`) per client IP per hour, sharing the budget with the GraphQL/WS task-read brake. Uses `GITLAWB_TRUSTED_PROXY` to resolve the client IP. 0 disables. Default 1200 (above the `/ipfs` budget to allow list pages followed by per-task reads). |
| `GITLAWB_TIGRIS_BUCKET` | Optional S3/Tigris shared repo storage bucket. |
| `GITLAWB_PINATA_JWT` | Optional Pinata/IPFS warm-storage pinning. |
| `GITLAWB_IRYS_URL` | Optional Irys/Arweave permanent anchoring. |
+9 -9
View File
@@ -26,7 +26,7 @@ We will acknowledge receipt within 48 hours and aim to release a fix within 14 d
**Content addressing**
- Every git object is content-addressed via CIDv1 (SHA-256)
- Tamper-evident by construction: a modified object changes its CID
- Tamper-evident by construction — a modified object changes its CID
**UCAN token validation**
- Bootstrap UCAN tokens are issued at registration.
@@ -35,8 +35,8 @@ We will acknowledge receipt within 48 hours and aim to release a fix within 14 d
- Capability grants are not yet consulted by repository write authorization; see the limitations below.
**Smart contracts (Base Sepolia testnet)**
- `GitlawbDIDRegistry`: on-chain DID → document registry
- `GitlawbNameRegistry`: human name → DID registry
- `GitlawbDIDRegistry` — on-chain DID → document registry
- `GitlawbNameRegistry` — human name → DID registry
- Both auditable on-chain, no admin keys
---
@@ -55,7 +55,7 @@ These are documented limitations of the current live release. They should be pri
### Repository write authorization defaults
- `git-receive-pack` verifies HTTP Signatures and, with the default configuration, also requires the pusher DID to be the repository owner: `GITLAWB_ENFORCE_OWNER_PUSH` defaults to `true`.
- **Impact:** With the default setting, a valid signature authenticates the pusher, and the pusher must be the repository owner. Authentication is not authorization on its own, since anyone can mint a `did:key` and sign, so owner enforcement is the only thing that prevents every signed caller from pushing to every repository, including private ones.
- **Impact:** With the default setting, a valid signature authenticates the pusher *and* the pusher must be the repository owner. Authentication is not authorization on its own — anyone can mint a `did:key` and sign — so owner enforcement is the only thing that prevents every signed caller from pushing to every repository, including private ones.
- **Mitigation:** `GITLAWB_ENFORCE_OWNER_PUSH=false` is the explicit rolling-upgrade compatibility override. Disable it only for the duration of an upgrade whose pushers are not yet the repo owner, then turn it back on. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so they cannot push while this is on.
### UCAN delegation and revocation
@@ -73,7 +73,7 @@ These are documented limitations of the current live release. They should be pri
### Private repository reads
- Repository and path-scoped visibility checks are enforced for repository API and Git content reads. A denied whole-repository or root read returns the same 404 shape as a missing repository, so the denial does not reveal private-repository existence.
- Sparse-clone support exposes withheld path globs to callers who may read the repository root. Do not put sensitive information in withheld path names.
- Task list/get reads (`GET /api/v1/tasks`, `GET /api/v1/tasks/{id}`, and GraphQL `tasks` / `task` over HTTP and WebSocket) are repository/task-gated through shared visibility collectors and omit `ucan_token`. All of those transports share the same per-IP `GITLAWB_TASK_READ_RATE_LIMIT` budget. `/api/v1/ipfs/pins` and `/api/v1/arweave/anchors` are not repository-gated; pin and anchor listings expose object and ref metadata.
- `GET /api/v1/tasks`, `/api/v1/ipfs/pins`, and `/api/v1/arweave/anchors` are not repository-gated. Task records include a UCAN token; pin and anchor listings expose object and ref metadata.
- Changing a repository's visibility controls future serving, but cannot retract ref metadata or configured external pins and anchors already announced while the repository was public. Do not push secrets to an announceable repository.
- **Impact:** Visibility policies protect the repository and Git content routes they gate, not every metadata endpoint or previously published content.
- **Remaining boundary:** This read control does not address the independent write-authorization and UCAN-delegation limitations described above.
@@ -119,10 +119,10 @@ These are documented limitations of the current live release. They should be pri
## Threat Model
Twigpine is designed to be secure against:
- **Unauthenticated writes:** HTTP Signature auth on protected write endpoints; see Known Limitations for owner and capability authorization gaps
- **Tampered git objects:** CIDv1 content addressing detects modification
- **Identity spoofing:** DIDs derived from public keys, unforgeable without the private key
- **Centralized takedown:** no single point of control; data on IPFS + Arweave
- **Unauthenticated writes** — HTTP Signature auth on protected write endpoints; see Known Limitations for owner and capability authorization gaps
- **Tampered git objects** — CIDv1 content addressing detects modification
- **Identity spoofing** — DIDs derived from public keys, unforgeable without the private key
- **Centralized takedown** — no single point of control; data on IPFS + Arweave
Twigpine is **not yet** designed to defend against:
- A compromised node operator (node operators are trusted for their own node)
@@ -137,11 +137,6 @@ enum ShimMode {
struct Shim {
base_url: String,
posts: Arc<AtomicUsize>,
/// Advertisement (GET /info/refs) count. One `git fetch` performs exactly
/// one advertisement, so tests assert this is EXACTLY one: any fetch-level
/// retry that comes back re-advertises and goes red on the spot, which the
/// lower-bound POST assertions alone cannot see (#275 round 6).
gets: Arc<AtomicUsize>,
stop: Arc<AtomicBool>,
handle: Option<std::thread::JoinHandle<()>>,
}
@@ -171,17 +166,15 @@ fn start_shim(repo: PathBuf, mode: ShimMode) -> Shim {
let addr = listener.local_addr().unwrap();
let base_url = format!("http://{addr}");
let posts = Arc::new(AtomicUsize::new(0));
let gets = Arc::new(AtomicUsize::new(0));
let stop = Arc::new(AtomicBool::new(false));
let posts_t = posts.clone();
let gets_t = gets.clone();
let stop_t = stop.clone();
let handle = std::thread::spawn(move || {
while !stop_t.load(Ordering::SeqCst) {
match listener.accept() {
Ok((stream, _)) => {
handle_conn(stream, &repo, mode, &posts_t, &gets_t);
handle_conn(stream, &repo, mode, &posts_t);
}
Err(ref e) if e.kind() == std::io::ErrorKind::WouldBlock => {
std::thread::sleep(Duration::from_millis(5));
@@ -194,7 +187,6 @@ fn start_shim(repo: PathBuf, mode: ShimMode) -> Shim {
Shim {
base_url,
posts,
gets,
stop,
handle: Some(handle),
}
@@ -220,13 +212,7 @@ fn normalize_accepted_stream(stream: &TcpStream) {
stream.set_read_timeout(Some(Duration::from_secs(30))).ok();
}
fn handle_conn(
stream: TcpStream,
repo: &Path,
mode: ShimMode,
posts: &AtomicUsize,
gets: &AtomicUsize,
) {
fn handle_conn(stream: TcpStream, repo: &Path, mode: ShimMode, posts: &AtomicUsize) {
normalize_accepted_stream(&stream);
let mut reader = BufReader::new(stream);
@@ -277,7 +263,6 @@ fn handle_conn(
receive_pack(repo, false, &body),
)
} else if method == "GET" && target.contains("/info/refs") {
gets.fetch_add(1, Ordering::SeqCst);
// v0 advertisement, wrapped exactly as the node's info_refs does.
let adv = upload_pack(repo, true, b"");
let mut wrapped = pkt(b"# service=git-upload-pack\n");
@@ -1022,7 +1007,6 @@ fn shim_answers_a_connection_that_arrives_non_blocking() {
let server = std::thread::spawn(move || {
let posts = AtomicUsize::new(0);
let gets = AtomicUsize::new(0);
let stream = loop {
match listener.accept() {
Ok((s, _)) => break s,
@@ -1037,7 +1021,7 @@ fn shim_answers_a_connection_that_arrives_non_blocking() {
// the assertions below run on every platform.
stream.set_nonblocking(true).unwrap();
accepted_tx.send(()).unwrap();
handle_conn(stream, &repo, ShimMode::Normal, &posts, &gets);
handle_conn(stream, &repo, ShimMode::Normal, &posts);
});
let mut client = TcpStream::connect(addr).unwrap();
@@ -1101,17 +1085,6 @@ fn real_git_multi_round_fetch_completes() {
posts >= 2,
"fixture did not force multi-round negotiation (observed {posts} POST(s)); the bridging path was not exercised"
);
// Exactly one advertisement: the POST bound above is a floor, so a
// reintroduced fetch-level retry could satisfy it by running a second full
// fetch instead of multi-round negotiation. Re-advertising is what a
// second fetch cannot avoid, so this is the committed guard on the
// retry's absence.
let gets = shim.gets.load(Ordering::SeqCst);
assert_eq!(
gets, 1,
"expected exactly one advertisement GET per fetch; {gets} means a \
fetch-level retry is back"
);
// The fetched tip is present and the clone's object graph is intact.
let server_head = String::from_utf8(git(&server, &["rev-parse", "HEAD"])).unwrap();
@@ -1204,17 +1177,6 @@ fn real_git_withheld_shaped_first_post() {
"helper hung on a withheld-shaped response (should forward-and-terminate, not deadlock). stderr:\n{stderr}"
);
// Exactly one advertisement, pass or fail: the POST assertions below are
// floors, so a reintroduced fetch-level retry could mask the withheld
// shape behind a second full fetch. A retry cannot avoid re-advertising,
// so this is the committed guard on its absence.
let gets = shim.gets.load(Ordering::SeqCst);
assert_eq!(
gets, 1,
"expected exactly one advertisement GET per fetch; {gets} means a \
fetch-level retry is back"
);
if out.status.success() {
// Real git accepted the mid-negotiation pack: the withheld multi-round
// path works end to end with no extra handling.
-2
View File
@@ -23,8 +23,6 @@ tracing = { workspace = true }
tracing-subscriber = { workspace = true }
chrono = { workspace = true }
uuid = { workspace = true }
url = { workspace = true }
percent-encoding = { workspace = true }
axum = { version = "0.8", features = ["http1", "http2", "json", "ws"] }
async-graphql = { version = "7", features = ["chrono", "uuid", "tracing"] }
async-graphql-axum = "7"
-1
View File
@@ -22,7 +22,6 @@ pub mod replicas;
pub mod repos;
pub mod resolve;
pub mod stars;
pub mod task_cursor;
pub mod tasks;
pub mod visibility;
pub mod webhooks;
-782
View File
@@ -1,782 +0,0 @@
//! Opaque, integrity-protected continuation tokens for the task read surfaces.
//!
//! The task list pages by keyset over `(created_at, id)`, but the rows a
//! caller may *see* are a filtered subset of the rows the query has to
//! *examine*. Handing the caller a raw `(created_at, id)` cursor therefore
//! forced a choice between two broken options (#327 review): anchor the cursor
//! on the last visible row, and a window of denied tasks longer than the scan
//! budget stalls paging forever; or anchor it on the last examined row, and a
//! denied read leaks the id and timestamp of a task `GET /tasks/{id}` other-
//! wise 404s.
//!
//! A server-issued token removes the choice. The position it carries is the
//! last *examined* candidate, so paging always advances, and the payload is
//! opaque and MAC'd, so the caller learns nothing from it and cannot forge one
//! naming a row of their choosing.
//!
//! The position must be *confidential*, not merely unforgeable: a token that
//! merely signed a base64 payload would still let its holder read the id and
//! timestamp of the denied row it names, which is the disclosure the token
//! exists to prevent. The payload is therefore encrypted under a
//! synthetic-IV construction (SIV): the tag is an HMAC over the filter and the
//! plaintext, and it doubles as the IV seeding the keystream the plaintext is
//! XORed with. That needs no randomness source and no dependency beyond the
//! `hmac`/`sha2` pair already used for webhook signatures and blob recipient
//! tags, and it is decrypt-last: nothing is parsed until the tag verifies.
//!
//! Making the token the *only* accepted cursor also fixes the ordering-domain
//! bug the same review found. `agent_tasks.created_at` is TEXT and compared as
//! TEXT, so `...Z` and `...+00:00` denote one instant but sort differently. A
//! caller-typed timestamp could silently skip or repeat same-time rows. The
//! token instead carries the stored string verbatim, so the value compared is
//! always a value the server wrote.
//!
//! A token is bound to the *caller* as well as the filter. The position it
//! names is the last examined candidate, not the last visible one, so it
//! encodes how far a scan got under one caller's visibility. Resuming it as a
//! different caller would start the scan past rows that caller is entitled to
//! read, silently dropping them from the answer (#327 review). The presenting
//! caller's normalized DID (empty and flagged absent when anonymous) is
//! therefore part of the MAC input, so a token presented by anyone else fails
//! to verify rather than under-reporting.
//!
//! Wire form: `v1.<iv>.<body>`, both parts base64url unpadded: the 16-byte
//! synthetic IV (the truncated MAC, which also seeds the keystream) and the
//! padded, encrypted JSON payload.
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
use base64::Engine;
use hmac::{Hmac, Mac};
use serde::{Deserialize, Serialize};
use sha2::Sha256;
use crate::error::AppError;
type HmacSha256 = Hmac<Sha256>;
const CURSOR_PREFIX: &str = "v1";
const KEY_DERIVATION_LABEL: &[u8] = b"gitlawb/tasks-cursor-key/v1";
const TAG_DOMAIN: &[u8] = b"gitlawb/tasks-cursor-tag/v1";
const STREAM_DOMAIN: &[u8] = b"gitlawb/tasks-cursor-stream/v1";
/// Truncated MAC length, and the synthetic IV width. 128 bits is far beyond
/// forgery reach for a token that carries no authority of its own (every page
/// re-runs the visibility gate against the presenting caller), and keeps the
/// token short enough to sit in a query string.
const TAG_LEN: usize = 16;
/// Padding bucket for the serialized cursor payload, in bytes. The keystream
/// XORs the plaintext byte for byte, so token length would otherwise track
/// payload length and leak the named row's field widths to a holder who
/// cannot decrypt anything. 128 comfortably covers the widest realistic
/// payload: a 35-char RFC3339 timestamp (`to_rfc3339()` renders 29 chars at
/// millisecond precision and 32 at microsecond, 35 only with all nine
/// fractional digits), a 36-char server-minted UUID, and the fixed JSON
/// envelope plus a 10-digit expiry, which together total 101 bytes. Should a future payload ever exceed the bucket, the length
/// rounds up to the next multiple instead of failing, so `encode` never
/// breaks — it just mints a longer token.
const PAYLOAD_BUCKET: usize = 128;
/// How long an issued cursor stays acceptable. Keyset positions never go stale
/// on their own — `created_at`/`id` are immutable — so this is not a
/// correctness bound. It bounds how long a token stays valid across a node
/// restart-and-rotate and keeps an abandoned page from being resumed
/// indefinitely.
const CURSOR_TTL_SECS: i64 = 24 * 60 * 60;
/// Node-keyed MAC key for continuation tokens, derived from the node keypair
/// seed so it needs no configuration and survives restarts of the same node.
/// Derived rather than used directly so a token forgery oracle could not
/// bear on the signing key itself.
#[derive(Clone)]
pub struct TaskCursorKey([u8; 32]);
impl std::fmt::Debug for TaskCursorKey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("TaskCursorKey(<redacted>)")
}
}
impl TaskCursorKey {
pub fn derive(node_seed: &[u8; 32]) -> Self {
let mut mac = HmacSha256::new_from_slice(node_seed).expect("HMAC accepts any key length");
mac.update(KEY_DERIVATION_LABEL);
let mut key = [0u8; 32];
key.copy_from_slice(&mac.finalize().into_bytes());
Self(key)
}
}
/// The keyset position a token carries: the last candidate row the previous
/// request examined, whether or not the caller was allowed to see it.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TaskPosition {
pub created_at: String,
pub id: String,
}
impl TaskPosition {
pub fn new(created_at: impl Into<String>, id: impl Into<String>) -> Self {
Self {
created_at: created_at.into(),
id: id.into(),
}
}
pub fn as_pair(&self) -> (&str, &str) {
(self.created_at.as_str(), self.id.as_str())
}
}
#[derive(Serialize, Deserialize)]
struct CursorPayload<'a> {
/// `created_at` of the last examined candidate, verbatim as stored.
t: &'a str,
/// `id` of the last examined candidate.
i: &'a str,
/// Unix expiry.
e: i64,
}
/// The filter a page was issued under. A cursor is only meaningful against the
/// same `status`/`assignee_did` filter that produced it: resuming a filtered
/// scan from an unfiltered position (or the reverse) silently skips rows.
/// Bound into the MAC rather than stored in the payload, so it costs no token
/// length and cannot be edited without invalidating the tag.
#[derive(Debug, Clone, Copy)]
pub struct TaskFilter<'a> {
pub status: Option<&'a str>,
pub assignee_did: Option<&'a str>,
}
/// The visibility identity a token was issued to. Normalized through
/// `normalize_owner_key` so the two spellings of one `did:key` identity
/// (`did:key:X` and bare `X`) bind identically, matching `did_matches` on the
/// read path: a caller who authenticates with the other spelling of their own
/// DID must be able to resume their own page.
fn caller_binding(caller: Option<&str>) -> &str {
caller.map(crate::db::normalize_owner_key).unwrap_or("")
}
/// The assignee filter a token was issued under. Normalized through
/// `normalize_owner_key` so the two spellings of one `did:key` identity
/// (`did:key:X` and bare `X`) bind identically, matching `list_tasks_keyset`
/// in SQL: minting under `did:key:X` and resuming with bare `X` (or the reverse)
/// hits the same rows and must verify under the MAC.
fn assignee_binding(assignee: Option<&str>) -> &str {
assignee.map(crate::db::normalize_owner_key).unwrap_or("")
}
fn cursor_mac(
key: &TaskCursorKey,
filter: TaskFilter<'_>,
caller: Option<&str>,
plaintext: &[u8],
) -> HmacSha256 {
let mut mac = HmacSha256::new_from_slice(&key.0).expect("HMAC accepts any key length");
mac.update(TAG_DOMAIN);
// Length-prefix every field so no two distinct filter/caller/plaintext
// tuples can produce the same MAC input.
for field in [
filter.status.unwrap_or("").as_bytes(),
assignee_binding(filter.assignee_did).as_bytes(),
caller_binding(caller).as_bytes(),
plaintext,
] {
mac.update(&(field.len() as u64).to_be_bytes());
mac.update(field);
}
// Presence is distinct from emptiness for all three optional fields, so an
// anonymous token cannot collide with one issued to a caller whose
// normalized DID is the empty string.
mac.update(&[
u8::from(filter.status.is_some()),
u8::from(filter.assignee_did.is_some()),
u8::from(caller.is_some()),
]);
mac
}
/// Synthetic IV: the authentication tag over the filter and plaintext, which
/// also seeds the keystream. Deterministic by construction, so no randomness
/// source is needed and two tokens for the same page are byte-identical.
fn siv(
key: &TaskCursorKey,
filter: TaskFilter<'_>,
caller: Option<&str>,
plaintext: &[u8],
) -> [u8; TAG_LEN] {
let mut out = [0u8; TAG_LEN];
out.copy_from_slice(
&cursor_mac(key, filter, caller, plaintext)
.finalize()
.into_bytes()[..TAG_LEN],
);
out
}
/// XOR `buf` with the keystream for `iv`. Its own inverse, so encrypt and
/// decrypt are the same call.
fn apply_keystream(key: &TaskCursorKey, iv: &[u8; TAG_LEN], buf: &mut [u8]) {
for (block_index, chunk) in buf.chunks_mut(32).enumerate() {
let mut mac = HmacSha256::new_from_slice(&key.0).expect("HMAC accepts any key length");
mac.update(STREAM_DOMAIN);
mac.update(iv);
mac.update(&(block_index as u64).to_be_bytes());
let block = mac.finalize().into_bytes();
for (byte, k) in chunk.iter_mut().zip(block.iter()) {
*byte ^= k;
}
}
}
/// Mint a token resuming at `position` for `filter`, usable only by `caller`.
///
/// The serialized payload is padded with trailing whitespace to a fixed
/// bucket before the SIV tag is computed: the keystream preserves plaintext
/// length, so an unpadded payload would let the token's length leak the
/// named row's field widths (a 25-char `to_rfc3339()` rendering mints a
/// shorter token than a 35-char one). `decode` tolerates the padding because
/// `serde_json` ignores trailing whitespace, and the tag covers the padded
/// bytes, so a token minted before padding landed still verifies.
pub fn encode(
key: &TaskCursorKey,
filter: TaskFilter<'_>,
caller: Option<&str>,
position: &TaskPosition,
) -> String {
let mut payload = serde_json::to_vec(&CursorPayload {
t: &position.created_at,
i: &position.id,
e: chrono::Utc::now().timestamp() + CURSOR_TTL_SECS,
})
.expect("cursor payload is plain strings and an integer");
let padded_len = payload.len().next_multiple_of(PAYLOAD_BUCKET);
payload.resize(padded_len, b' ');
let iv = siv(key, filter, caller, &payload);
apply_keystream(key, &iv, &mut payload);
format!(
"{CURSOR_PREFIX}.{}.{}",
URL_SAFE_NO_PAD.encode(iv),
URL_SAFE_NO_PAD.encode(&payload)
)
}
/// One rejection message for every way a token can fail to verify. A caller
/// who mangled a token, replayed an expired one, or tried to move one to a
/// different filter or a different presenting identity learns only that the
/// cursor is not usable — never which of those it was, and never anything
/// about the row it named.
const INVALID_CURSOR: &str = "invalid or expired cursor";
fn reject() -> AppError {
AppError::BadRequest(INVALID_CURSOR.into())
}
/// Verify a token against `filter` and the presenting `caller`, and return the
/// position it carries.
pub fn decode(
key: &TaskCursorKey,
filter: TaskFilter<'_>,
caller: Option<&str>,
token: &str,
) -> crate::error::Result<TaskPosition> {
let mut parts = token.split('.');
let (Some(version), Some(iv_b64), Some(body_b64), None) =
(parts.next(), parts.next(), parts.next(), parts.next())
else {
return Err(reject());
};
if version != CURSOR_PREFIX {
return Err(reject());
}
let iv_bytes = URL_SAFE_NO_PAD.decode(iv_b64).map_err(|_| reject())?;
let mut plaintext = URL_SAFE_NO_PAD.decode(body_b64).map_err(|_| reject())?;
let iv: [u8; TAG_LEN] = iv_bytes.try_into().map_err(|_| reject())?;
apply_keystream(key, &iv, &mut plaintext);
// Authenticate before parsing: until the tag matches, `plaintext` is just
// attacker-chosen bytes run through a keystream. `verify_truncated_left`
// is the constant-time compare, so a forgery attempt cannot be steered by
// timing the first differing byte.
cursor_mac(key, filter, caller, &plaintext)
.verify_truncated_left(&iv)
.map_err(|_| reject())?;
let decoded: CursorPayload<'_> = serde_json::from_slice(&plaintext).map_err(|_| reject())?;
if decoded.e < chrono::Utc::now().timestamp() {
return Err(reject());
}
Ok(TaskPosition::new(decoded.t, decoded.i))
}
#[cfg(test)]
mod tests {
use super::*;
fn key() -> TaskCursorKey {
TaskCursorKey::derive(&[7u8; 32])
}
fn unfiltered() -> TaskFilter<'static> {
TaskFilter {
status: None,
assignee_did: None,
}
}
#[test]
fn round_trips_position_verbatim() {
let k = key();
// A stored timestamp the server wrote, fractional digits and all.
let pos = TaskPosition::new("2026-01-03T00:00:00.123456789+00:00", "task-a");
let token = encode(&k, unfiltered(), None, &pos);
assert_eq!(decode(&k, unfiltered(), None, &token).unwrap(), pos);
// A sub-bucket payload pads to exactly one bucket: the padding
// claim is an exact width, and every other width assertion here is
// bucket-relative, so only an absolute check catches a pad that
// skips "next multiple" or a drifted bucket constant.
let body = token.split('.').nth(2).expect("token has a body part");
let raw = URL_SAFE_NO_PAD.decode(body).unwrap();
assert_eq!(
raw.len(),
PAYLOAD_BUCKET,
"normal token must pad to exactly one bucket"
);
}
/// The whole point of the token is that the caller may hold it without
/// learning the denied row it names. A signed-but-plaintext payload would
/// pass every other test here and still fail this one.
#[test]
fn token_does_not_expose_the_row_it_names() {
let k = key();
let token = encode(
&k,
unfiltered(),
None,
&TaskPosition::new("2026-01-03T00:00:00+00:00", "denied-task-id"),
);
let body = token.split('.').nth(2).expect("token has a body part");
let raw = URL_SAFE_NO_PAD.decode(body).unwrap();
let as_text = String::from_utf8_lossy(&raw);
for secret in ["denied-task-id", "2026-01-03", "\"t\"", "\"i\""] {
assert!(
!as_text.contains(secret),
"token body must not carry {secret:?} in the clear: {as_text:?}"
);
}
// Also assert it is not merely reordered or whitespace-mangled JSON.
assert!(
serde_json::from_slice::<serde_json::Value>(&raw).is_err(),
"token body must not be parseable as JSON"
);
}
/// The keystream preserves plaintext length, so without the fixed-width
/// padding a token naming a 35-char timestamp would be measurably longer
/// than one naming a 32-char one: the holder would learn the withheld
/// row's field widths without decrypting anything. Mint the same shape
/// both ways and require identical token length.
#[test]
fn token_length_does_not_vary_with_position_width() {
let k = key();
// Ids are server-minted fixed-length UUIDs; the timestamp is the
// width that varies in production (`to_rfc3339()` renders 35 chars
// with nanoseconds, 25 without fractional seconds).
let id = "b3f7a1c2-4d5e-4f6a-8b9c-0d1e2f3a4b5c";
let narrow = encode(
&k,
unfiltered(),
None,
&TaskPosition::new("2026-01-03T00:00:00+00:00", id),
);
let wide = encode(
&k,
unfiltered(),
None,
&TaskPosition::new("2026-01-03T00:00:00.123456789+00:00", id),
);
assert_eq!(
narrow.len(),
wide.len(),
"tokens must not leak the named row's field widths: {narrow} vs {wide}"
);
// The padded token still decodes to the exact stored string.
assert_eq!(
decode(&k, unfiltered(), None, &wide).unwrap(),
TaskPosition::new("2026-01-03T00:00:00.123456789+00:00", id)
);
}
/// A token minted before the fixed-width padding landed (unpadded payload,
/// tag over the raw bytes) must still verify: `decode` MACs whatever
/// plaintext the keystream recovers and `serde_json` tolerates the
/// missing trailing whitespace. This pins the back-compat claim in the
/// `encode` docstring with a frozen v1 artifact, so a change to the v1
/// recipe pins (domains, MAC input, keystream, segment layout, field
/// names) breaks this test instead of silently re-deriving under the
/// new internals. It does not pin the padding: `decode` never consults
/// `PAYLOAD_BUCKET` and the fixture is unpadded, so the padding-width
/// claim belongs to `oversized_payload_rounds_up_to_next_bucket`.
///
/// Fixture expiry is fixed at 2100-01-01 (`4102444800`): it must stay in
/// the future whenever the test runs.
#[test]
fn unpadded_legacy_token_still_verifies() {
let k = key();
let filter = unfiltered();
let position = TaskPosition::new("2026-01-03T00:00:00+00:00", "legacy-task");
// Pre-padding recipe minted once and frozen: serialize with no
// padding, tag, and encrypt. Deterministic by construction (SIV needs
// no randomness), so this literal is stable across runs.
let token = "v1.ILJyy7JZScm_h-HM1-CggA.l4kJUoHTbqSGTVKL0DP4hORnbOS1_gHYV2fwJYXd2QtM-6TBMCKh-wvluIiaEYC9rUSUPVSteAKkp7jBCmK_bR2D";
assert_eq!(
decode(&k, filter, None, token).unwrap(),
position,
"pre-padding token must verify under the current decode"
);
// The fixture is a genuine pre-padding artifact, so its decoded
// body must stay below one bucket: a literal re-minted under the
// padded recipe would retire this back-compat pin silently.
let body = token.split('.').nth(2).expect("token has a body part");
let raw = URL_SAFE_NO_PAD.decode(body).unwrap();
assert!(
raw.len() < PAYLOAD_BUCKET,
"legacy fixture must be the unpadded shape: {} bytes",
raw.len()
);
}
/// The `PAYLOAD_BUCKET` docstring promises an oversized payload rounds up
/// to the next multiple instead of failing. Mint a position whose id
/// pushes the serialized payload past one bucket and require the token
/// to round-trip (at two buckets).
#[test]
fn oversized_payload_rounds_up_to_next_bucket() {
let k = key();
// ~140-char id: payload crosses the 128-byte bucket.
let long_id = "a".repeat(140);
let position = TaskPosition::new("2026-01-03T00:00:00+00:00", long_id.clone());
let probe = serde_json::to_vec(&CursorPayload {
t: &position.created_at,
i: &position.id,
e: 1_700_000_000,
})
.expect("cursor payload is plain strings and an integer");
assert!(
probe.len() > PAYLOAD_BUCKET,
"fixture payload must exceed one bucket: {} bytes",
probe.len()
);
assert!(
probe.len() <= 2 * PAYLOAD_BUCKET,
"fixture payload must fit in two buckets: {} bytes",
probe.len()
);
let token = encode(&k, unfiltered(), None, &position);
assert_eq!(
decode(&k, unfiltered(), None, &token).unwrap(),
position,
"oversized payload must round-trip at two buckets"
);
// The padded body must be exactly two buckets, not merely longer
// than one: the padding claim is an exact width, not a direction.
let body = token.split('.').nth(2).expect("token has a body part");
let raw = URL_SAFE_NO_PAD.decode(body).unwrap();
assert_eq!(
raw.len(),
2 * PAYLOAD_BUCKET,
"oversized payload must pad to exactly two buckets"
);
}
/// Callers paste the token straight into a query string, so it must carry
/// no character that needs percent-encoding.
#[test]
fn token_is_url_safe_verbatim() {
let token = encode(
&key(),
TaskFilter {
status: Some("pending"),
assignee_did: Some("did:key:z6MkAssignee"),
},
None,
&TaskPosition::new("2026-01-03T00:00:00.123456789+00:00", "task-a"),
);
assert!(
token
.chars()
.all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.')),
"token must be query-safe without encoding: {token}"
);
}
/// Flipping any byte of a token must fail the tag, not decode to a
/// different position: the keystream is malleable on its own, the tag is
/// what stops a chosen-position forgery.
#[test]
fn rejects_any_tampered_byte() {
let k = key();
let token = encode(
&k,
unfiltered(),
None,
&TaskPosition::new("2026-01-03T00:00:00+00:00", "task-a"),
);
let parts: Vec<&str> = token.split('.').collect();
for part in [1usize, 2] {
for position in [0usize, 3] {
let mut bytes = parts[part].as_bytes().to_vec();
bytes[position] = if bytes[position] == b'A' { b'B' } else { b'A' };
let mut mangled: Vec<String> = parts.iter().map(|p| p.to_string()).collect();
mangled[part] = String::from_utf8(bytes).unwrap();
assert!(
decode(&k, unfiltered(), None, &mangled.join(".")).is_err(),
"byte {position} of part {part} must not be malleable"
);
}
}
// Sanity: the untouched token still decodes, so the loop above is not
// passing because every token is rejected.
assert!(decode(&k, unfiltered(), None, &token).is_ok());
}
#[test]
fn rejects_token_from_another_node() {
let pos = TaskPosition::new("2026-01-03T00:00:00+00:00", "task-a");
let token = encode(&TaskCursorKey::derive(&[1u8; 32]), unfiltered(), None, &pos);
assert!(decode(
&TaskCursorKey::derive(&[2u8; 32]),
unfiltered(),
None,
&token
)
.is_err());
}
#[test]
fn rejects_cursor_moved_to_a_different_filter() {
let k = key();
let pos = TaskPosition::new("2026-01-03T00:00:00+00:00", "task-a");
let token = encode(
&k,
TaskFilter {
status: Some("pending"),
assignee_did: None,
},
None,
&pos,
);
assert!(decode(&k, unfiltered(), None, &token).is_err());
assert!(decode(
&k,
TaskFilter {
status: Some("claimed"),
assignee_did: None
},
None,
&token
)
.is_err());
assert!(decode(
&k,
TaskFilter {
status: Some("pending"),
assignee_did: None
},
None,
&token
)
.is_ok());
}
/// A filter of `Some("")` must not verify a token minted with `None`.
#[test]
fn distinguishes_absent_filter_from_empty_filter() {
let k = key();
let pos = TaskPosition::new("2026-01-03T00:00:00+00:00", "task-a");
let token = encode(&k, unfiltered(), None, &pos);
assert!(decode(
&k,
TaskFilter {
status: Some(""),
assignee_did: None
},
None,
&token
)
.is_err());
}
/// A cursor names how far a scan got under one caller's visibility. Moved
/// to a different presenting identity it would start that caller's scan
/// past rows they are entitled to read, so it must fail to verify rather
/// than silently under-report (#327 review).
#[test]
fn rejects_cursor_moved_to_a_different_caller() {
let k = key();
let pos = TaskPosition::new("2026-01-03T00:00:00+00:00", "task-a");
let alice = Some("did:key:z6MkAlice");
let bob = Some("did:key:z6MkBob");
let anon_token = encode(&k, unfiltered(), None, &pos);
assert!(
decode(&k, unfiltered(), alice, &anon_token).is_err(),
"an anonymously minted cursor must not resume an authenticated scan"
);
assert!(decode(&k, unfiltered(), None, &anon_token).is_ok());
let alice_token = encode(&k, unfiltered(), alice, &pos);
assert!(
decode(&k, unfiltered(), bob, &alice_token).is_err(),
"one caller's cursor must not resume another caller's scan"
);
assert!(
decode(&k, unfiltered(), None, &alice_token).is_err(),
"an authenticated cursor must not resume an anonymous scan"
);
assert!(decode(&k, unfiltered(), alice, &alice_token).is_ok());
}
/// The binding is on identity, not spelling: `did:key:X` and bare `X` are
/// one caller everywhere else on the read path (`did_matches`), so a caller
/// who presents the other form of their own DID must keep their own page.
#[test]
fn caller_binding_is_normalized_across_did_key_spellings() {
let k = key();
let pos = TaskPosition::new("2026-01-03T00:00:00+00:00", "task-a");
let token = encode(&k, unfiltered(), Some("did:key:z6MkAlice"), &pos);
assert_eq!(
decode(&k, unfiltered(), Some("z6MkAlice"), &token).unwrap(),
pos
);
// A different method sharing the base58 tail is a different principal.
assert!(decode(&k, unfiltered(), Some("did:web:z6MkAlice"), &token).is_err());
}
/// The filter binding is on identity, not spelling: `assignee_did` matches
/// normalized keys in SQL (`normalize_owner_key`), so minting under
/// `did:key:X` and resuming under bare `X` (or vice versa) must succeed,
/// while `did:web:X` remains distinct.
#[test]
fn assignee_filter_binding_is_normalized_across_did_key_spellings() {
let k = key();
let pos = TaskPosition::new("2026-01-03T00:00:00+00:00", "task-a");
let filter_did_key = TaskFilter {
status: Some("pending"),
assignee_did: Some("did:key:z6MkAssignee"),
};
let filter_bare = TaskFilter {
status: Some("pending"),
assignee_did: Some("z6MkAssignee"),
};
let filter_web = TaskFilter {
status: Some("pending"),
assignee_did: Some("did:web:z6MkAssignee"),
};
let token_from_did_key = encode(&k, filter_did_key, None, &pos);
assert_eq!(
decode(&k, filter_bare, None, &token_from_did_key).unwrap(),
pos
);
assert!(decode(&k, filter_web, None, &token_from_did_key).is_err());
let token_from_bare = encode(&k, filter_bare, None, &pos);
assert_eq!(
decode(&k, filter_did_key, None, &token_from_bare).unwrap(),
pos
);
assert!(decode(&k, filter_web, None, &token_from_bare).is_err());
}
/// Anonymous is a distinct binding from a caller whose normalized DID is
/// the empty string, the same way `Some("")` is distinct from `None` for
/// the filter fields.
#[test]
fn distinguishes_anonymous_from_empty_caller() {
let k = key();
let pos = TaskPosition::new("2026-01-03T00:00:00+00:00", "task-a");
let token = encode(&k, unfiltered(), None, &pos);
assert!(decode(&k, unfiltered(), Some(""), &token).is_err());
}
/// Length-prefixing must stop a status/assignee pair from being re-split.
#[test]
fn rejects_field_boundary_shift() {
let k = key();
let pos = TaskPosition::new("2026-01-03T00:00:00+00:00", "task-a");
let token = encode(
&k,
TaskFilter {
status: Some("pend"),
assignee_did: Some("ing"),
},
None,
&pos,
);
assert!(decode(
&k,
TaskFilter {
status: Some("pending"),
assignee_did: Some("")
},
None,
&token
)
.is_err());
}
#[test]
fn rejects_expired_token() {
let k = key();
// Minted the same way `encode` does, but already past its expiry.
let mut payload = serde_json::to_vec(&CursorPayload {
t: "2026-01-03T00:00:00+00:00",
i: "task-a",
e: chrono::Utc::now().timestamp() - 1,
})
.unwrap();
let iv = siv(&k, unfiltered(), None, &payload);
apply_keystream(&k, &iv, &mut payload);
let expired = format!(
"v1.{}.{}",
URL_SAFE_NO_PAD.encode(iv),
URL_SAFE_NO_PAD.encode(&payload)
);
let err = decode(&k, unfiltered(), None, &expired).unwrap_err();
assert!(err.to_string().contains(INVALID_CURSOR));
}
#[test]
fn rejects_malformed_shapes() {
let k = key();
for bad in [
"",
"v1",
"v1.",
"v1.abc",
"v2.abc.def",
"v1.abc.def.ghi",
"v1.!!!.def",
] {
assert!(
decode(&k, unfiltered(), None, bad).is_err(),
"must reject {bad:?}"
);
}
}
/// Every rejection path renders the same text, so a caller cannot tell a
/// forged token from an expired one from a filter mismatch.
#[test]
fn every_rejection_is_indistinguishable() {
let k = key();
for bad in ["v1.abc.def", "not-a-cursor", "v1..", "v9.a.b"] {
assert_eq!(
decode(&k, unfiltered(), None, bad).unwrap_err().to_string(),
format!("invalid request: {INVALID_CURSOR}")
);
}
}
}
File diff suppressed because it is too large. Load diff
-4
View File
@@ -634,12 +634,10 @@ mod tests {
.connect_lazy("postgres://localhost/gitlawb_test_placeholder")
.expect("lazy pool creation should not fail");
let db = Arc::new(crate::db::Db::for_testing(pool.clone()));
let task_cursor_key = crate::api::task_cursor::TaskCursorKey::derive(&keypair.to_seed());
let schema = Arc::new(graphql::build_schema(
db.clone(),
ref_tx.clone(),
task_tx.clone(),
task_cursor_key.clone(),
));
crate::state::AppState {
config: Arc::new(Config::parse_from(["gitlawb-node"])),
@@ -651,7 +649,6 @@ mod tests {
ref_update_tx: ref_tx,
task_event_tx: task_tx,
graphql_schema: schema,
task_cursor_key,
machine_id: None,
repo_store: crate::git::repo_store::RepoStore::for_testing(PathBuf::from("/tmp"), pool),
rate_limiter: RateLimiter::new(100, Duration::from_secs(60)),
@@ -687,7 +684,6 @@ mod tests {
git_ipfs_walk_semaphore: Arc::new(tokio::sync::Semaphore::new(64)),
git_ipfs_walk_per_caller:
crate::rate_limit::PerCallerConcurrency::with_default_max_keys(16),
task_read_rate_limiter: RateLimiter::new(1200, Duration::from_secs(3600)),
git_bin: "git".to_string(),
}
}
+7 -265
View File
@@ -66,12 +66,6 @@ pub struct Config {
#[arg(long, env = "GITLAWB_PUBLIC_URL")]
pub public_url: Option<String>,
/// Base URL for the web view of repos on this node (e.g. https://gitlawb.com).
/// When set, `GET /` advertises it as `web_url` so the CLI can print a
/// working `View:` link. Omit for nodes that have no web front-end.
#[arg(long, env = "GITLAWB_WEB_URL")]
pub web_url: Option<String>,
/// Comma-separated list of bootstrap peer URLs to announce to on startup
#[arg(long, env = "GITLAWB_BOOTSTRAP_PEERS", value_delimiter = ',')]
pub bootstrap_peers: Vec<String>,
@@ -762,21 +756,6 @@ pub struct Config {
value_parser = clap::builder::RangedU64ValueParser::<u64>::new().range(0..=86_400)
)]
pub pin_repair_sweep_delay_secs: u64,
/// Per-client-IP rate limit for the anonymous task read routes
/// (`GET /api/v1/tasks`, `GET /api/v1/tasks/{id}`), in requests per hour.
/// Both are publicly reachable (`optional_signature`). `GET /api/v1/tasks`
/// runs `collect_visible_tasks` and returns a visibility-filtered page;
/// `GET /api/v1/tasks/{id}` runs `get_visible_task` and returns an opaque 404
/// when the task is hidden or missing. These reads can require task,
/// repository, and visibility-rule queries even when no task is returned,
/// so the brake bounds the cost of anonymous probes. Keyed on the resolved
/// client IP via `GITLAWB_TRUSTED_PROXY`. `0` disables. Default: 1200 (a list page
/// followed by per-task reads is a normal client pattern, so this sits above
/// the `/ipfs` budget). GraphQL `tasks` / `task` queries, including WebSocket
/// operations, share this per-IP budget with REST task reads.
#[arg(long, env = "GITLAWB_TASK_READ_RATE_LIMIT", default_value_t = 1200)]
pub task_read_rate_limit: usize,
}
impl Config {
@@ -824,13 +803,7 @@ impl Config {
/// Cross-field boot validation. Single-field ranges are enforced by clap; this
/// catches combinations that ship a denial-of-service under otherwise-valid
/// values. Call once at startup and fail fast on `Err`.
///
/// `&mut self` is required so the validator can normalize `web_url` to
/// its canonical form after validation: the configured value is
/// advertised on public `GET /`, and any consumer reading
/// `config.web_url` later must see the same string that was actually
/// validated.
pub fn validate(&mut self) -> Result<(), String> {
pub fn validate(&self) -> Result<(), String> {
// Concurrent git writes pin the dedicated lock pool for their whole
// duration (connection-affine advisory lock in acquire_write). The main
// pool no longer carries that occupancy.
@@ -947,101 +920,10 @@ impl Config {
.map_err(|e| e.to_string())?;
}
// GITLAWB_WEB_URL is advertised in GET / so the CLI can print a working
// View: link. Clap maps "" to Some("") which would produce a broken URL;
// reject early instead of serving a malformed link. Non-blank values must
// additionally parse as an absolute http(s) URL — the CLI appends
// `/{owner}/{repo}` to it, so scheme-less hosts ("gitlawb.com") and other
// garbage would render links no browser can follow.
if let Some(raw) = &self.web_url {
match validate_web_url(raw) {
Ok(()) => {}
Err(reason) => {
return Err(format!(
"GITLAWB_WEB_URL {reason} — \
set it to an absolute URL like https://gitlawb.com or leave it unset."
));
}
}
// Normalize to one canonical form so every consumer (the regular
// and degraded `GET /` handlers, anyone who reads
// `config.web_url` directly) sees the same string that was
// actually validated. The CLI's own defensive trim would paper
// over the divergence on the `View:` line, but that hides the
// bug from any other consumer that parses the value verbatim.
let canonical = raw
.trim()
.parse::<url::Url>()
.map(|u| u.to_string())
.map_err(|_| "is not a valid absolute URL".to_string())?;
if canonical != *raw {
self.web_url = Some(canonical);
}
}
Ok(())
}
}
/// Validate a `web_url` value for use as a browser-reachable base URL.
/// Blank values are rejected (clap maps `--web-url ""` to `Some("")`), and
/// non-blank values must parse as absolute `http`/`https` URLs with no
/// query, fragment, or userinfo — the CLI treats this as a string prefix
/// to append paths to, so anything else produces links no browser can
/// follow (`?a=1/owner/repo` puts the repo path inside the query string);
/// and userinfo is rejected outright because the configured value is
/// published on public `GET /` and reprinted on the CLI's `View:` line,
/// so an accidentally configured password would land in public metadata,
/// terminal scrollback, and CI logs.
pub(crate) fn validate_web_url(raw: &str) -> Result<(), String> {
if raw.trim().is_empty() {
return Err("must not be empty or whitespace-only".into());
}
let parsed: url::Url = raw
.trim()
.parse()
.map_err(|_| "is not a valid absolute URL".to_string())?;
match parsed.scheme() {
"http" | "https" => {}
other => return Err(format!("must use http or https, got '{other}:'")),
}
// url::Url cannot represent a URL without a host for http/https schemes, so
// reaching here guarantees an absolute browser-usable base.
if parsed.query().is_some() {
return Err(
"must not contain a query string — it is used as a path prefix for View links".into(),
);
}
if parsed.fragment().is_some() {
return Err(
"must not contain a fragment — it is used as a path prefix for View links".into(),
);
}
if !parsed.username().is_empty() || parsed.password().is_some() {
return Err(
"must not contain a username or password — it is advertised publicly and printed on the View: line"
.into(),
);
}
// Bidi/control bytes can hide behind percent-encoding: a literal scan of
// the wire form misses `.../%E2%80%AE` even though a browser decodes it
// into a reordering character on the rendered View: link. Check the
// decoded host and path before the value is advertised on GET / (INV-6).
let host = parsed.host_str().unwrap_or("");
let decoded_path = percent_encoding::percent_decode_str(parsed.path()).decode_utf8_lossy();
if host
.chars()
.chain(decoded_path.chars())
.any(|c| c.is_control() || gitlawb_core::sanitize::is_bidi_format(c))
{
return Err(
"must not contain control or bidi characters, even percent-encoded — the View: line is terminal output"
.into(),
);
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
@@ -1684,7 +1566,7 @@ mod tests {
.expect("default config must validate");
// An under-sized lock pool relative to the push cap is rejected.
let mut under_lock = Config::parse_from([
let under_lock = Config::parse_from([
"gitlawb-node",
"--db-lock-pool-max-connections",
"16",
@@ -1697,7 +1579,7 @@ mod tests {
);
// Exactly at the push cap without non-push headroom is rejected.
let mut push_only = Config::parse_from([
let push_only = Config::parse_from([
"gitlawb-node",
"--db-lock-pool-max-connections",
"32",
@@ -1710,7 +1592,7 @@ mod tests {
);
// Main pool can be smaller than pushes + headroom when the lock pool carries writes.
let mut split = Config::parse_from([
let split = Config::parse_from([
"gitlawb-node",
"--db-max-connections",
"16",
@@ -1725,153 +1607,13 @@ mod tests {
);
// Main pool still needs the app headroom floor.
let mut under_main = Config::parse_from(["gitlawb-node", "--db-max-connections", "4"]);
let under_main = Config::parse_from(["gitlawb-node", "--db-max-connections", "4"]);
assert!(
under_main.validate().is_err(),
"db_max_connections below DB_POOL_APP_HEADROOM must be rejected"
);
}
#[test]
fn web_url_rejects_empty_and_whitespace() {
// Unset is fine.
Config::parse_from(["gitlawb-node"])
.validate()
.expect("no web_url must validate");
// A real URL validates.
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some("https://gitlawb.com".into());
assert!(cfg.validate().is_ok());
// Empty string is rejected.
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some("".into());
assert!(cfg.validate().is_err(), "empty web_url must be rejected");
// Whitespace-only is rejected.
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some(" ".into());
assert!(
cfg.validate().is_err(),
"whitespace-only web_url must be rejected"
);
}
#[test]
fn web_url_rejects_non_absolute_or_non_browser_urls() {
// Scheme-less host: parses nowhere, renders a broken View: link.
for bad in [
"gitlawb.com",
"not a url",
"ftp://files.example.com",
"javascript:alert(1)",
"https://", // no host
] {
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some(bad.into());
assert!(cfg.validate().is_err(), "web_url {bad:?} must be rejected");
}
// Query strings and fragments corrupt the appended repo path, since
// web_url is used as a raw string prefix (`?a=1` would swallow
// `/{owner}/{repo}` into the query).
for bad in [
"https://gitlawb.com?a=1",
"https://gitlawb.com/?utm_source=docs",
"https://gitlawb.com#section",
"https://gitlawb.com/#faq",
] {
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some(bad.into());
assert!(
cfg.validate().is_err(),
"web_url {bad:?} (query or fragment) must be rejected"
);
}
// Surrounding whitespace around a valid URL is tolerated.
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some(" https://gitlawb.com ".into());
assert!(
cfg.validate().is_ok(),
"padded-but-valid web_url must validate"
);
// Percent-encoded bidi must not boot: the wire form carries no Cf
// code point, but a browser decodes %E2%80%AE into a reordering
// character on the rendered View: link (INV-6). Literal bidi and
// control bytes are rejected the same way.
for bad in [
"https://example.com/%E2%80%AE",
"https://example.com/\u{202e}spoof",
"https://example.com/%1b[31m",
] {
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some(bad.into());
assert!(
cfg.validate().is_err(),
"web_url {bad:?} (control or bidi, even encoded) must be rejected"
);
}
// Non-default ports and paths are fine.
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some("http://localhost:8080/ui".into());
assert!(cfg.validate().is_ok(), "port+path web_url must validate");
// Userinfo must be rejected: `web_url` is published on public
// `GET /` and reprinted on the CLI's `View:` line, so an
// accidentally configured password would land in public metadata,
// terminal scrollback, and CI logs. Reject username-only and
// user+password forms.
for bad in [
"https://[email protected]",
"https://user:[email protected]",
"https://:[email protected]",
] {
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some(bad.into());
assert!(
cfg.validate().is_err(),
"web_url {bad:?} (userinfo) must be rejected"
);
}
}
/// `validate()` accepts whitespace-padded input but the stored value
/// must match the canonical form actually validated, so every consumer
/// (the regular and degraded `GET /` handlers, anyone who reads
/// `config.web_url` directly) sees the same string. Without this, an
/// operator that sets `GITLAWB_WEB_URL=" https://git.example "` boots
/// a node that advertises `" https://git.example "` on public
/// metadata — different from what was validated, different from what
/// any other consumer would parse.
#[test]
fn web_url_is_normalized_to_canonical_form_after_validation() {
// Padded input is rewritten to the canonical form. `url::Url`'s
// own `to_string` is what we use for the canonicalization, and
// it always appends a path-separator slash to a bare-host URL,
// so the stored value is `https://git.example/` (which is also
// what every browser would normalize the operator-typed form to).
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some(" https://git.example ".into());
cfg.validate().unwrap();
assert_eq!(
cfg.web_url.as_deref(),
Some("https://git.example/"),
"stored web_url must be the canonical form, not the operator-typed one"
);
// Already-canonical input is left alone after a no-op
// re-normalization — same form comes back out, so any consumer
// reading the stored value sees a stable string across calls.
let mut cfg = Config::parse_from(["gitlawb-node"]);
cfg.web_url = Some("https://git.example/".into());
cfg.validate().unwrap();
assert_eq!(cfg.web_url.as_deref(), Some("https://git.example/"));
}
/// The DECLARED default, read off the parser rather than out of a parse.
///
/// `Config::parse_from` consults the process environment, so on a host that
@@ -2136,7 +1878,7 @@ mod tests {
std::os::unix::fs::symlink(&target, &link).unwrap();
let key_path = link.join("p2p.key");
let mut config = Config::parse_from([
let config = Config::parse_from([
"gitlawb-node",
"--p2p-port",
"0",
@@ -2231,7 +1973,7 @@ mod tests {
let key_path = build(base.path());
let before = snapshot(base.path());
let mut config = Config::parse_from([
let config = Config::parse_from([
"gitlawb-node",
"--p2p-port",
"7546",
File diff suppressed because it is too large. Load diff
-14
View File
@@ -12,9 +12,6 @@ pub enum AppError {
#[error("repo already exists: {0}")]
RepoExists(String),
#[error("conflict: {0}")]
Conflict(String),
#[error("not found: {0}")]
NotFound(String),
@@ -192,7 +189,6 @@ impl IntoResponse for AppError {
"repo_exists",
format!("repository '{r}' already exists"),
),
AppError::Conflict(msg) => (StatusCode::CONFLICT, "conflict", msg.clone()),
AppError::NotFound(msg) => (StatusCode::NOT_FOUND, "not_found", msg.clone()),
AppError::Unauthorized(msg) => (StatusCode::UNAUTHORIZED, "not_an_agent", msg.clone()),
AppError::Forbidden(msg) => (StatusCode::FORBIDDEN, "forbidden", msg.clone()),
@@ -325,16 +321,6 @@ pub type Result<T> = std::result::Result<T, AppError>;
mod tests {
use super::*;
#[test]
fn conflict_maps_to_409() {
assert_eq!(
AppError::Conflict("task not claimable".into())
.into_response()
.status(),
StatusCode::CONFLICT
);
}
#[test]
fn timeout_maps_to_504_distinct_from_git_500() {
assert_eq!(
+2 -73
View File
@@ -60,7 +60,6 @@ pub(crate) fn graphql_app_err(e: crate::error::AppError) -> async_graphql::Error
// Curated client-safe variants — `Display` is intentional API text.
safe @ (crate::error::AppError::RepoNotFound(_)
| crate::error::AppError::RepoExists(_)
| crate::error::AppError::Conflict(_)
| crate::error::AppError::NotFound(_)
| crate::error::AppError::Unauthorized(_)
| crate::error::AppError::Forbidden(_)
@@ -77,81 +76,15 @@ pub(crate) fn graphql_app_err(e: crate::error::AppError) -> async_graphql::Error
}
}
/// Classify a failed `claim_task` for GraphQL exactly as REST's claim handler
/// classifies it: a lost race is a client-safe conflict, a real sqlx fault
/// stays opaque. Lives here rather than at the three call sites so the
/// classification cannot drift between transports (#327 review), and so
/// `every_graphql_map_err_uses_opaque_helpers` can keep whitelisting by name.
pub(crate) fn graphql_claim_conflict(e: anyhow::Error) -> async_graphql::Error {
graphql_app_err(crate::api::tasks::task_write_conflict(
e,
"task not claimable: not found or already claimed",
))
}
/// The `finish_task` half of [`graphql_claim_conflict`], covering both
/// `completeTask` and `failTask`.
pub(crate) fn graphql_finish_conflict(e: anyhow::Error) -> async_graphql::Error {
graphql_app_err(crate::api::tasks::task_write_conflict(
e,
"task not found or not in claimed state",
))
}
pub struct TaskReadBrakeExtension;
impl async_graphql::extensions::ExtensionFactory for TaskReadBrakeExtension {
fn create(&self) -> Arc<dyn async_graphql::extensions::Extension> {
Arc::new(TaskReadBrakeExtensionImpl)
}
}
use async_graphql::async_trait::async_trait;
struct TaskReadBrakeExtensionImpl;
#[async_trait]
impl async_graphql::extensions::Extension for TaskReadBrakeExtensionImpl {
async fn prepare_request(
&self,
ctx: &async_graphql::extensions::ExtensionContext<'_>,
mut request: async_graphql::Request,
next: async_graphql::extensions::NextPrepareRequest<'_>,
) -> async_graphql::ServerResult<async_graphql::Request> {
if let Some(session_brake) = ctx
.session_data
.get(&std::any::TypeId::of::<crate::rate_limit::TaskReadBrake>())
.and_then(|d| d.downcast_ref::<crate::rate_limit::TaskReadBrake>())
{
request = request.data(crate::rate_limit::TaskReadBrake {
limiter: session_brake.limiter.clone(),
key: session_brake.key.clone(),
request_count: Arc::new(std::sync::atomic::AtomicUsize::new(0)),
});
}
if let Some(did) = ctx
.session_data
.get(&std::any::TypeId::of::<crate::auth::AuthenticatedDid>())
.and_then(|d| d.downcast_ref::<crate::auth::AuthenticatedDid>())
{
request = request.data(did.clone());
}
next.run(ctx, request).await
}
}
pub fn build_schema(
db: Arc<Db>,
ref_update_tx: tokio::sync::broadcast::Sender<RefUpdateBroadcast>,
task_event_tx: tokio::sync::broadcast::Sender<TaskEventBroadcast>,
task_cursor_key: crate::api::task_cursor::TaskCursorKey,
) -> GitlawbSchema {
Schema::build(QueryRoot, MutationRoot, SubscriptionRoot)
.data(db)
.data(ref_update_tx)
.data(task_event_tx)
.data(task_cursor_key)
.extension(TaskReadBrakeExtension)
.finish()
}
@@ -228,10 +161,8 @@ mod tests {
}
/// Every `.map_err(` in the GraphQL query/mutation resolvers must route
/// through one of the curated helpers in this module, or discard the error
/// (`|_|`). Same source-scrape pattern as `api::authz_guard` (#255
/// review). The list is deliberately a whitelist of names rather than a
/// prefix match, so adding a new mapper is a decision a reviewer sees.
/// through the opaque helpers, or discard the error (`|_|`). Same source-
/// scrape pattern as `api::authz_guard` (#255 review).
#[test]
fn every_graphql_map_err_uses_opaque_helpers() {
for (file, src) in [
@@ -246,8 +177,6 @@ mod tests {
let after = code[idx + ".map_err(".len()..].trim_start();
let ok = after.starts_with("crate::graphql::graphql_db_err")
|| after.starts_with("crate::graphql::graphql_app_err")
|| after.starts_with("crate::graphql::graphql_claim_conflict")
|| after.starts_with("crate::graphql::graphql_finish_conflict")
|| after.starts_with("|_|")
|| after.starts_with("|_ ");
assert!(
+46 -517
View File
@@ -79,14 +79,10 @@ impl MutationRoot {
updated_at: now,
deadline: input.deadline,
};
// Return the stored row, not the pre-insert input: create_task
// normalizes the assignee (whitespace-only becomes NULL/open), and a
// response that shows the raw input lies about the reservation.
let stored = db
.create_task(&task)
db.create_task(&task)
.await
.map_err(crate::graphql::graphql_db_err)?;
Ok(AgentTaskType::from(stored))
Ok(AgentTaskType::from(task))
}
async fn claim_task(
@@ -104,39 +100,20 @@ impl MutationRoot {
let assignee_did = caller.to_string();
let db = ctx.data_unchecked::<Arc<Db>>();
let tx = ctx.data_unchecked::<tokio::sync::broadcast::Sender<TaskEventBroadcast>>();
crate::api::tasks::get_claimable_task(db, &id, caller)
.await
.map_err(crate::graphql::graphql_app_err)?
.ok_or_else(|| async_graphql::Error::new("task not found"))?;
// Same classification REST's claim handler applies: a claim race is a
// client-safe conflict, a genuine sqlx failure stays opaque. Routing
// both transports through `task_write_conflict` is what stops a normal
// race from reaching a GraphQL caller as a generic database error
// (#327 review).
let task = db
.claim_task(&id, &assignee_did)
.await
.map_err(crate::graphql::graphql_claim_conflict)?;
let by_did = task
.assignee_did
.as_deref()
.map(crate::db::trim_assignee_did)
.unwrap_or(assignee_did.as_str())
.to_string();
crate::api::tasks::announce_task_event(
db,
tx,
TaskEventBroadcast {
task_id: id,
old_status: "pending".to_string(),
new_status: "claimed".to_string(),
by_did,
at: Utc::now().to_rfc3339(),
},
)
.await;
.map_err(crate::graphql::graphql_db_err)?;
let _ = tx.send(TaskEventBroadcast {
task_id: id,
old_status: "pending".to_string(),
new_status: "claimed".to_string(),
by_did: assignee_did,
at: Utc::now().to_rfc3339(),
});
Ok(AgentTaskType::from(task))
}
async fn complete_task(
&self,
ctx: &Context<'_>,
@@ -153,43 +130,29 @@ impl MutationRoot {
let by_did = caller.to_string();
let db = ctx.data_unchecked::<Arc<Db>>();
let tx = ctx.data_unchecked::<tokio::sync::broadcast::Sender<TaskEventBroadcast>>();
// Authorize the actor: the task must be visible to the caller (returning
// not found for invisible tasks so existence is not leaked), and only
// the task's assignee may finish it.
let existing = crate::api::tasks::get_visible_task(db, &id, Some(caller))
// Authorize the actor: binding by_did to the signer is necessary but not
// sufficient — only the task's assignee may finish it.
let existing = db
.get_task(&id)
.await
.map_err(crate::graphql::graphql_app_err)?
.map_err(crate::graphql::graphql_db_err)?
.ok_or_else(|| async_graphql::Error::new("task not found"))?;
if !crate::api::did_matches(
caller,
crate::db::trim_assignee_did(existing.assignee_did.as_deref().unwrap_or_default()),
) {
if !crate::api::did_matches(caller, existing.assignee_did.as_deref().unwrap_or_default()) {
return Err(async_graphql::Error::new(
"only the task assignee can complete it",
));
}
let task = db
.finish_task(&id, "completed", input.result.as_deref(), caller)
.finish_task(&id, "completed", input.result.as_deref())
.await
.map_err(crate::graphql::graphql_finish_conflict)?;
let by_did = task
.assignee_did
.as_deref()
.map(crate::db::trim_assignee_did)
.unwrap_or(by_did.as_str())
.to_string();
crate::api::tasks::announce_task_event(
db,
tx,
TaskEventBroadcast {
task_id: id,
old_status: "claimed".to_string(),
new_status: "completed".to_string(),
by_did,
at: Utc::now().to_rfc3339(),
},
)
.await;
.map_err(crate::graphql::graphql_db_err)?;
let _ = tx.send(TaskEventBroadcast {
task_id: id,
old_status: "claimed".to_string(),
new_status: "completed".to_string(),
by_did,
at: Utc::now().to_rfc3339(),
});
Ok(AgentTaskType::from(task))
}
@@ -209,44 +172,29 @@ impl MutationRoot {
let by_did = caller.to_string();
let db = ctx.data_unchecked::<Arc<Db>>();
let tx = ctx.data_unchecked::<tokio::sync::broadcast::Sender<TaskEventBroadcast>>();
// Authorize the actor: the task must be visible to the caller (returning
// not found for invisible tasks so existence is not leaked), and only
// the task's assignee may fail it.
let existing = crate::api::tasks::get_visible_task(db, &id, Some(caller))
// Authorize the actor: only the task's assignee may fail it.
let existing = db
.get_task(&id)
.await
.map_err(crate::graphql::graphql_app_err)?
.map_err(crate::graphql::graphql_db_err)?
.ok_or_else(|| async_graphql::Error::new("task not found"))?;
if !crate::api::did_matches(
caller,
crate::db::trim_assignee_did(existing.assignee_did.as_deref().unwrap_or_default()),
) {
if !crate::api::did_matches(caller, existing.assignee_did.as_deref().unwrap_or_default()) {
return Err(async_graphql::Error::new(
"only the task assignee can fail it",
));
}
let reason = input.reason.unwrap_or_default();
let task = db
.finish_task(&id, "failed", Some(&reason), caller)
.finish_task(&id, "failed", Some(&reason))
.await
.map_err(crate::graphql::graphql_finish_conflict)?;
let by_did = task
.assignee_did
.as_deref()
.map(crate::db::trim_assignee_did)
.unwrap_or(by_did.as_str())
.to_string();
crate::api::tasks::announce_task_event(
db,
tx,
TaskEventBroadcast {
task_id: id,
old_status: "claimed".to_string(),
new_status: "failed".to_string(),
by_did,
at: Utc::now().to_rfc3339(),
},
)
.await;
.map_err(crate::graphql::graphql_db_err)?;
let _ = tx.send(TaskEventBroadcast {
task_id: id,
old_status: "claimed".to_string(),
new_status: "failed".to_string(),
by_did,
at: Utc::now().to_rfc3339(),
});
Ok(AgentTaskType::from(task))
}
}
@@ -344,7 +292,7 @@ mod tests {
);
if *name == "claimTask" {
assert!(
errs.contains("task not found"),
errs.contains("task not claimable"),
"claim race / missing task must keep its business message: {errs}"
);
assert!(
@@ -388,210 +336,6 @@ mod tests {
);
}
/// #327 review: `claimTask`/`completeTask`/`failTask` called
/// `graphql_db_err` directly on db-layer business failures, so an ordinary
/// claim race or stale finish reached GraphQL clients as a generic
/// database error while REST clients got an actionable conflict. All three
/// now route through the shared `task_write_conflict` classifier.
///
/// Deleting either `map_err` from a mutation turns this red: the message
/// becomes the raw anyhow text instead of the fixed conflict form.
#[sqlx::test]
async fn task_write_races_surface_as_conflicts_not_db_errors(pool: PgPool) {
let state = crate::test_support::test_state(pool).await;
let schema = state.graphql_schema.as_ref();
let assignee = "did:key:zGQLRACEASSIGNEEAAAAAAAAAAAAAAAAAAAAAAA";
let rival = "did:key:zGQLRACERIVALBBBBBBBBBBBBBBBBBBBBBBBBBB";
let delegator = "did:key:zGQLRACEDELEGATORCCCCCCCCCCCCCCCCCCCCC";
let now = chrono::Utc::now().to_rfc3339();
state
.db
.create_repo(&crate::db::RepoRecord {
id: "race-repo".into(),
name: "race".into(),
owner_did: delegator.into(),
description: None,
is_public: true,
default_branch: "main".into(),
created_at: chrono::Utc::now(),
updated_at: chrono::Utc::now(),
disk_path: "/tmp/race-repo".into(),
forked_from: None,
machine_id: None,
})
.await
.unwrap();
state
.db
.create_task(&crate::db::AgentTask {
id: "race-task".into(),
repo_id: Some("race-repo".into()),
kind: "build".into(),
status: "pending".into(),
delegator_did: delegator.into(),
assignee_did: None,
capability: "repo:write".into(),
ucan_token: None,
payload: None,
result: None,
created_at: now.clone(),
updated_at: now,
deadline: None,
})
.await
.unwrap();
// The assignee wins the claim.
let claim = |did: &str| {
format!(r#"mutation {{ claimTask(id: "race-task", assigneeDid: "{did}") {{ id }} }}"#)
};
let resp = schema
.execute(Request::new(claim(assignee)).data(AuthenticatedDid(assignee.into())))
.await;
assert!(resp.errors.is_empty(), "first claim: {}", errors(&resp));
// The rival loses the race: ineligible caller receives opaque not-found.
let resp = schema
.execute(Request::new(claim(rival)).data(AuthenticatedDid(rival.into())))
.await;
let errs = errors(&resp);
assert!(
errs.contains("task not found"),
"ineligible caller after task is claimed must receive opaque not-found: {errs}"
);
assert!(
!errs.contains(crate::graphql::GRAPHQL_DB_ERROR_MESSAGE),
"ineligible claim is not a database failure: {errs}"
);
// A second claim that reaches the conditional SQL write (e.g. assignee claiming
// again on an already-claimed task) exercises the conflict mapper:
let resp = schema
.execute(Request::new(claim(assignee)).data(AuthenticatedDid(assignee.into())))
.await;
let errs = errors(&resp);
assert!(
errs.contains("task not claimable: not found or already claimed"),
"a lost claim race must render the fixed conflict message: {errs}"
);
assert!(
!errs.contains(crate::graphql::GRAPHQL_DB_ERROR_MESSAGE),
"a claim race is not a database failure: {errs}"
);
// A stale finish on an already-completed task is the same class.
let complete = format!(
r#"mutation {{ completeTask(id: "race-task", byDid: "{assignee}", input: {{ result: "ok" }}) {{ id }} }}"#
);
let resp = schema
.execute(Request::new(&complete).data(AuthenticatedDid(assignee.into())))
.await;
assert!(resp.errors.is_empty(), "first complete: {}", errors(&resp));
let resp = schema
.execute(Request::new(&complete).data(AuthenticatedDid(assignee.into())))
.await;
let errs = errors(&resp);
assert!(
errs.contains("task not found or not in claimed state"),
"a stale complete must render the fixed conflict message: {errs}"
);
assert!(
!errs.contains(crate::graphql::GRAPHQL_DB_ERROR_MESSAGE),
"a stale complete is not a database failure: {errs}"
);
let fail = format!(
r#"mutation {{ failTask(id: "race-task", byDid: "{assignee}", input: {{ reason: "nope" }}) {{ id }} }}"#
);
let resp = schema
.execute(Request::new(&fail).data(AuthenticatedDid(assignee.into())))
.await;
let errs = errors(&resp);
assert!(
errs.contains("task not found or not in claimed state"),
"a stale fail must render the fixed conflict message: {errs}"
);
assert!(
!errs.contains(crate::graphql::GRAPHQL_DB_ERROR_MESSAGE),
"a stale fail is not a database failure: {errs}"
);
}
/// The other half of the same classification: a genuine SQL fault on the
/// write itself must stay opaque. Without this, routing conflicts through
/// `task_write_conflict` could be "fixed" by making every db failure a
/// client-visible conflict message.
#[sqlx::test]
async fn task_write_sql_faults_stay_opaque(pool: PgPool) {
let state = crate::test_support::test_state(pool.clone()).await;
let assignee = "did:key:zGQLOPAQUEASSIGNEEAAAAAAAAAAAAAAAAAAAAA";
let delegator = "did:key:zGQLOPAQUEDELEGATORBBBBBBBBBBBBBBBBBBB";
let now = chrono::Utc::now().to_rfc3339();
state
.db
.create_task(&crate::db::AgentTask {
id: "opaque-task".into(),
repo_id: None,
kind: "build".into(),
status: "pending".into(),
delegator_did: delegator.into(),
assignee_did: Some(assignee.into()),
capability: "repo:write".into(),
ucan_token: None,
payload: None,
result: None,
created_at: now.clone(),
updated_at: now,
deadline: None,
})
.await
.unwrap();
// Fault the write itself, not the schema. Dropping a column the
// visibility pre-check also SELECTs would fail in `get_visible_task`
// and never reach `graphql_claim_conflict`, so this test would pass
// against a claim mapper that leaks (#327 review). A BEFORE UPDATE
// trigger keeps every read valid and faults only inside
// `Db::claim_task`.
sqlx::raw_sql(
"CREATE FUNCTION gl_test_fault_task_update() RETURNS trigger
LANGUAGE plpgsql AS $fn$
BEGIN RAISE EXCEPTION 'gl_test_forced_update_fault'; END;
$fn$;
CREATE TRIGGER gl_test_fault_task_update
BEFORE UPDATE ON agent_tasks
FOR EACH ROW EXECUTE FUNCTION gl_test_fault_task_update();",
)
.execute(&pool)
.await
.unwrap();
let resp = state
.graphql_schema
.execute(
Request::new(format!(
r#"mutation {{ claimTask(id: "opaque-task", assigneeDid: "{assignee}") {{ id }} }}"#
))
.data(AuthenticatedDid(assignee.into())),
)
.await;
let errs = errors(&resp);
assert!(
errs.contains(crate::graphql::GRAPHQL_DB_ERROR_MESSAGE),
"a real sqlx fault on the write must stay opaque: {errs}"
);
assert!(
!errs.contains("task not claimable"),
"a write-time sqlx fault must not be reclassified as a claim race: {errs}"
);
assert!(
!errs.contains("gl_test_forced_update_fault") && !errs.contains("trigger"),
"database text leaked through the conflict mapper: {errs}"
);
}
/// Adversarial-review GATE-1 (GraphQL): completing a task requires being its
/// assignee, not merely signing as the by_did you pass. A signer who is not
/// the assignee is rejected even though the by_did binding passes; the
@@ -614,7 +358,7 @@ mod tests {
payload: None,
result: None,
created_at: now.clone(),
updated_at: now.clone(),
updated_at: now,
deadline: None,
};
state.db.create_task(&task).await.expect("seed task");
@@ -631,69 +375,14 @@ mod tests {
)
};
// Stranger signs as themselves on a repo-less task they cannot see:
// invisible task returns "task not found" so existence is not leaked.
// Stranger signs as themselves and passes byDid=self (so the signer
// binding passes), but is not the assignee → rejected by authorization.
let resp = schema
.execute(Request::new(q(stranger)).data(AuthenticatedDid(stranger.into())))
.await;
assert!(
errors(&resp).contains("task not found"),
"an invisible task must return not found, got: {}",
errors(&resp)
);
// Seed a task on a public repo that stranger CAN see, but is not assignee of:
let pub_repo = crate::db::RepoRecord {
id: "pub-r".into(),
name: "pub-r".into(),
owner_did: "did:key:zGQLDELEGATORCCCCCCCCCCCCCCCCCCCCCCCCCC".into(),
description: None,
is_public: true,
default_branch: "main".into(),
created_at: chrono::Utc::now(),
updated_at: chrono::Utc::now(),
disk_path: "/tmp/pub-r".into(),
forked_from: None,
machine_id: None,
};
state.db.create_repo(&pub_repo).await.expect("create repo");
let pub_task = crate::db::AgentTask {
id: "task-pub".into(),
repo_id: Some("pub-r".into()),
kind: "build".into(),
status: "pending".into(),
delegator_did: "did:key:zGQLDELEGATORCCCCCCCCCCCCCCCCCCCCCCCCCC".into(),
assignee_did: None,
capability: "repo:write".into(),
ucan_token: None,
payload: None,
result: None,
created_at: now.clone(),
updated_at: now,
deadline: None,
};
state
.db
.create_task(&pub_task)
.await
.expect("seed pub task");
state
.db
.claim_task("task-pub", assignee)
.await
.expect("claim pub task");
let q_pub = |actor: &str| {
format!(
r#"mutation {{ completeTask(id: "task-pub", byDid: "{actor}", input: {{}}) {{ id status }} }}"#
)
};
let resp = schema
.execute(Request::new(q_pub(stranger)).data(AuthenticatedDid(stranger.into())))
.await;
assert!(
errors(&resp).contains("assignee"),
"a non-assignee signer on a visible task must be rejected: {}",
"a non-assignee signer must be rejected: {}",
errors(&resp)
);
@@ -759,7 +448,7 @@ mod tests {
// repo id afterwards.
let stored = state
.db
.list_tasks_keyset(None, None, 50, None)
.list_tasks(None, None, 50)
.await
.expect("list tasks");
assert!(
@@ -884,164 +573,4 @@ mod tests {
"schema text leaked through the gate: {errs}"
);
}
/// Pre-assigned GraphQL claimTask must reject a stranger (no UCAN leak)
/// and admit the reserved assignee.
#[sqlx::test]
async fn claim_task_honors_preassigned_assignee(pool: PgPool) {
let state = crate::test_support::test_state(pool).await;
let reserved = "did:key:zGQLRESERVEDAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
let thief = "did:key:zGQLTHIEFBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
let now = chrono::Utc::now().to_rfc3339();
let task = crate::db::AgentTask {
id: "task-gql-reserved".into(),
repo_id: None,
kind: "build".into(),
status: "pending".into(),
delegator_did: "did:key:zGQLDELEGATORCCCCCCCCCCCCCCCCCCCCCCCCCC".into(),
assignee_did: Some(reserved.into()),
capability: "repo:write".into(),
ucan_token: Some("gql-ucan-secret".into()),
payload: None,
result: None,
created_at: now.clone(),
updated_at: now,
deadline: None,
};
state.db.create_task(&task).await.expect("seed");
let schema = state.graphql_schema.as_ref();
let q = |actor: &str| {
format!(
r#"mutation {{ claimTask(id: "task-gql-reserved", assigneeDid: "{actor}") {{ id status ucanToken }} }}"#
)
};
let resp = schema
.execute(Request::new(q(thief)).data(AuthenticatedDid(thief.into())))
.await;
let errs = errors(&resp);
// The claim gate hides existence: an ineligible claimer gets the same
// not-found as a missing task, not a reserved deny that would leak it.
assert!(
errs.contains("task not found"),
"stranger claim must read as not-found, got: {errs}"
);
assert!(
!errs.contains("gql-ucan-secret"),
"UCAN must not leak in GraphQL errors: {errs}"
);
if let Ok(data) = resp.data.into_json() {
let s = data.to_string();
assert!(
!s.contains("gql-ucan-secret"),
"UCAN must not leak in GraphQL data on failed claim: {s}"
);
}
// Parity with REST: failed steal must leave the row untouched.
let still = state
.db
.get_task("task-gql-reserved")
.await
.expect("get")
.expect("task exists");
assert_eq!(
still.status, "pending",
"status must stay pending after steal"
);
assert_eq!(
still.assignee_did.as_deref(),
Some(reserved),
"assignee_did must stay reserved after steal"
);
let resp = schema
.execute(Request::new(q(reserved)).data(AuthenticatedDid(reserved.into())))
.await;
assert!(
errors(&resp).is_empty(),
"reserved assignee must claim: {}",
errors(&resp)
);
let data = resp.data.into_json().expect("json data");
assert_eq!(data["claimTask"]["status"], "claimed");
assert_eq!(data["claimTask"]["ucanToken"], "gql-ucan-secret");
}
/// create_task normalizes a whitespace-only assignee to NULL (open task),
/// so the create response must show the stored row, not echo the input: a
/// client trusting the echoed "reservation" would believe a claimable task
/// is reserved. Before the fix the response was built from the pre-insert
/// struct and returned the padded input verbatim.
#[sqlx::test]
async fn create_task_reports_stored_assignee_not_input(pool: PgPool) {
let state = crate::test_support::test_state(pool).await;
let caller = "did:key:zGQLCREATORAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
let schema = state.graphql_schema.as_ref();
let q = format!(
r#"mutation {{ createTask(delegatorDid: "{caller}", input: {{ kind: "build", capability: "repo:write", assigneeDid: " " }}) {{ id assigneeDid }} }}"#
);
let resp = schema
.execute(Request::new(q).data(AuthenticatedDid(caller.into())))
.await;
let data = resp.data.into_json().expect("json data");
assert!(
data["createTask"]["assigneeDid"].is_null(),
"whitespace-only assigneeDid must come back null (stored shape): {data}"
);
}
/// A task seeded with a whitespace-padded assignee must still be
/// completable by that assignee over GraphQL: REST and Db::finish_task
/// trim the stored value before comparing, and the GraphQL gate must do
/// the same or padded reservations finish only through REST.
#[sqlx::test]
async fn graphql_finish_accepts_padded_stored_assignee(pool: PgPool) {
let state = crate::test_support::test_state(pool).await;
let assignee = "did:key:zGQLPADDEDAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
let now = chrono::Utc::now().to_rfc3339();
let task = crate::db::AgentTask {
id: "task-gql-padded".into(),
repo_id: None,
kind: "build".into(),
status: "pending".into(),
delegator_did: "did:key:zGQLDELEGATORDDDDDDDDDDDDDDDDDDDDDDDDDD".into(),
assignee_did: Some(format!("\t{assignee}\n")),
capability: "repo:write".into(),
ucan_token: None,
payload: None,
result: None,
created_at: now.clone(),
updated_at: now,
deadline: None,
};
state.db.create_task(&task).await.expect("seed");
let schema = state.graphql_schema.as_ref();
let claim = format!(
r#"mutation {{ claimTask(id: "task-gql-padded", assigneeDid: "{assignee}") {{ id status }} }}"#
);
let resp = schema
.execute(Request::new(claim).data(AuthenticatedDid(assignee.into())))
.await;
assert!(
errors(&resp).is_empty(),
"reserved assignee must claim a padded reservation: {}",
errors(&resp)
);
let finish = format!(
r#"mutation {{ completeTask(id: "task-gql-padded", byDid: "{assignee}", input: {{}}) {{ id status }} }}"#
);
let resp = schema
.execute(Request::new(finish).data(AuthenticatedDid(assignee.into())))
.await;
let errs = errors(&resp);
assert!(
errs.is_empty(),
"assignee must complete a padded reservation: {errs}"
);
let data = resp.data.into_json().expect("json data");
assert_eq!(data["completeTask"]["status"], "completed");
}
}
+23 -479
View File
@@ -3,27 +3,10 @@ use std::sync::Arc;
use crate::db::Db;
use super::types::{AgentTaskReadType, RefUpdateType, RepoType, TaskPageType};
use super::types::{AgentTaskType, RefUpdateType, RepoType};
pub struct QueryRoot;
/// Debit the per-IP task-read brake for one task field, mirroring the
/// `rate_limit_by_ip` layer on `task_read_routes`. Both surfaces run the same
/// #268 visibility gate and pay the same queries before answering, so an
/// anonymous prober must not get an unbraked lane by asking over GraphQL
/// (#327 review).
///
/// A schema built without the brake (unit tests, subscriptions) is not braked,
/// exactly as `rate_limit_by_ip` is a no-op without its extension.
async fn task_read_brake(ctx: &Context<'_>, field: &str) -> Result<()> {
match ctx.data::<crate::rate_limit::TaskReadBrake>() {
Ok(brake) if !brake.check(field).await => Err(async_graphql::Error::new(
crate::rate_limit::RATE_LIMIT_MESSAGE,
)),
_ => Ok(()),
}
}
#[Object]
impl QueryRoot {
async fn repos(&self, ctx: &Context<'_>) -> Result<Vec<RepoType>> {
@@ -129,73 +112,29 @@ impl QueryRoot {
assignee_did: Option<String>,
#[graphql(
default = 50,
desc = "Max 200; larger requests are clamped to 200 (no error). Negative values clamp to 0. Use `nextCursor` to read past the clamp."
desc = "Max 200; larger requests are clamped to 200 (no error). Negative values clamp to 0."
)]
limit: i64,
#[graphql(
desc = "Opaque continuation token from a previous page's `nextCursor`. Must be presented with the same `status`/`assigneeDid` filter that issued it."
)]
cursor: Option<String>,
) -> Result<TaskPageType> {
use crate::api::task_cursor::{self, TaskCursorKey, TaskFilter};
task_read_brake(ctx, "tasks").await?;
) -> Result<Vec<AgentTaskType>> {
let db = ctx.data_unchecked::<Arc<Db>>();
// #268: gate rows via the same collector the REST list route uses (like
// `ref_updates` shares `collect_visible_ref_updates` with its REST feed),
// so the two surfaces cannot drift. The collector clamps `limit` itself,
// including the negative-LIMIT case #250 called out for this resolver.
let caller = ctx
.data::<crate::auth::AuthenticatedDid>()
.ok()
.map(|d| d.0.as_str());
let key = ctx.data_unchecked::<TaskCursorKey>();
let filter = TaskFilter {
status: status.as_deref(),
assignee_did: assignee_did.as_deref(),
};
let resume = cursor
.as_deref()
.map(|token| task_cursor::decode(key, filter, caller, token))
.transpose()
.map_err(crate::graphql::graphql_app_err)?;
let result = crate::api::tasks::collect_visible_tasks(
db,
status.as_deref(),
assignee_did.as_deref(),
limit,
resume.as_ref(),
caller,
)
.await
.map_err(crate::graphql::graphql_app_err)?;
Ok(TaskPageType {
next_cursor: result
.next_position
.as_ref()
.map(|pos| task_cursor::encode(key, filter, caller, pos)),
items: result
.tasks
.into_iter()
.map(AgentTaskReadType::from)
.collect(),
has_more: result.has_more,
incomplete: result.incomplete,
})
// Clamp before SQL: a negative LIMIT is a client fault that Postgres
// rejects with 2201W, which would otherwise trip the opaque DB path
// and write an error-level log on every probe (#250 review).
let limit = limit.clamp(0, 200);
let tasks = db
.list_tasks(status.as_deref(), assignee_did.as_deref(), limit)
.await
.map_err(crate::graphql::graphql_db_err)?;
Ok(tasks.into_iter().map(AgentTaskType::from).collect())
}
async fn task(&self, ctx: &Context<'_>, id: String) -> Result<Option<AgentTaskReadType>> {
task_read_brake(ctx, "task").await?;
async fn task(&self, ctx: &Context<'_>, id: String) -> Result<Option<AgentTaskType>> {
let db = ctx.data_unchecked::<Arc<Db>>();
// #268: same gate as the REST get route, via the shared helper.
let caller = ctx
.data::<crate::auth::AuthenticatedDid>()
.ok()
.map(|d| d.0.as_str());
let t = crate::api::tasks::get_visible_task(db, &id, caller)
let t = db
.get_task(&id)
.await
.map_err(crate::graphql::graphql_app_err)?;
Ok(t.map(AgentTaskReadType::from))
.map_err(crate::graphql::graphql_db_err)?;
Ok(t.map(AgentTaskType::from))
}
}
@@ -214,14 +153,10 @@ mod tests {
Arc::new(db)
}
fn cursor_key() -> crate::api::task_cursor::TaskCursorKey {
crate::api::task_cursor::TaskCursorKey::derive(&[42u8; 32])
}
fn schema(db: Arc<Db>) -> super::super::GitlawbSchema {
let (ref_tx, _) = tokio::sync::broadcast::channel(16);
let (task_tx, _) = tokio::sync::broadcast::channel(16);
super::super::build_schema(db, ref_tx, task_tx, cursor_key())
super::super::build_schema(db, ref_tx, task_tx)
}
fn repo(id: &str, owner_did: &str, name: &str, is_public: bool) -> RepoRecord {
@@ -531,7 +466,7 @@ mod tests {
async fn tasks_negative_limit_clamped(pool: PgPool) {
let db = db(pool).await;
let schema = schema(db);
let resp = anon(&schema, "{ tasks(limit: -1) { items { id } } }").await;
let resp = anon(&schema, "{ tasks(limit: -1) { id } }").await;
assert!(
resp.errors.is_empty(),
"negative limit must clamp, not fail: {:?}",
@@ -565,409 +500,18 @@ mod tests {
.unwrap();
}
let schema = schema(db);
// Queried as the delegator, not anonymously: since #268 the task read
// surface is visibility-gated, and an anonymous caller sees none of
// these repo-less tasks at all. The clamp is what this test pins, so it
// needs a caller who can legitimately see all 201 rows.
let resp = authed(&schema, "{ tasks(limit: 5000) { items { id } } }", OWNER).await;
let resp = anon(&schema, "{ tasks(limit: 5000) { id } }").await;
assert_eq!(count_tasks(&resp), 200, "limit above 200 must clamp to 200");
}
/// Seed one repo-less task carrying a `ucan_token`, so a leak on any read
/// surface is visible in the response body.
async fn seed_task(db: &Db, id: &str, delegator: &str) {
let now = Utc::now().to_rfc3339();
db.create_task(&crate::db::AgentTask {
id: id.into(),
repo_id: None,
kind: "build".into(),
status: "pending".into(),
delegator_did: delegator.into(),
assignee_did: None,
capability: "repo:write".into(),
ucan_token: Some("SECRET-UCAN-TOKEN".into()),
payload: None,
result: None,
created_at: now.clone(),
updated_at: now,
deadline: None,
})
.await
.unwrap();
}
/// #268: the `tasks` resolver must delegate to the gated collector, not
/// query the DB directly. A repo-less task belonging to someone else is
/// invisible to an anonymous caller. `tasks_negative_limit_clamped` cannot
/// catch a resolver that stops calling `collect_visible_tasks` because it
/// seeds no rows — this seeds one, so the gate is load-bearing here.
#[sqlx::test]
async fn tasks_repo_less_task_hidden_from_anon(pool: PgPool) {
let db = db(pool).await;
seed_task(&db, "t1", OWNER).await;
let schema = schema(db);
let resp = anon(&schema, "{ tasks { items { id } } }").await;
assert_eq!(
count_tasks(&resp),
0,
"anon must not enumerate another party's repo-less task"
);
assert!(
!format!("{:?}", resp.data).contains("SECRET-UCAN-TOKEN"),
"no ucan token may reach an anonymous caller"
);
}
/// #268 sibling for the single-task resolver: an invisible task reads as
/// `null`, indistinguishable from one that does not exist.
#[sqlx::test]
async fn task_by_id_is_null_for_anon(pool: PgPool) {
let db = db(pool).await;
seed_task(&db, "t1", OWNER).await;
let schema = schema(db);
let resp = anon(&schema, r#"{ task(id: "t1") { id } }"#).await;
assert!(resp.errors.is_empty(), "graphql errors: {:?}", resp.errors);
let async_graphql::Value::Object(obj) = &resp.data else {
panic!("data not an object: {:?}", resp.data);
};
assert_eq!(
obj.get("task"),
Some(&async_graphql::Value::Null),
"an invisible task must read as null, got {:?}",
obj.get("task")
);
}
/// #268: `ucanToken` is absent from the read type's schema entirely, so the
/// delegator cannot request it either. Asking for it is a validation error,
/// which pins the redaction at the schema level rather than per-resolver.
#[sqlx::test]
async fn task_read_schema_has_no_ucan_token_field(pool: PgPool) {
let db = db(pool).await;
seed_task(&db, "t1", OWNER).await;
let schema = schema(db);
let resp = authed(&schema, r#"{ task(id: "t1") { id ucanToken } }"#, OWNER).await;
assert!(
!resp.errors.is_empty(),
"ucanToken must not exist on the task read type"
);
}
#[sqlx::test]
async fn tasks_find_older_visible_row_behind_denied_window(pool: PgPool) {
let db = db(pool).await;
db.create_repo(&repo("public-repo", OWNER, "public", true))
.await
.unwrap();
let visible = crate::db::AgentTask {
id: "visible".into(),
repo_id: Some("public-repo".into()),
kind: "build".into(),
status: "pending".into(),
delegator_did: OWNER.into(),
assignee_did: None,
capability: "repo:write".into(),
ucan_token: None,
payload: None,
result: None,
created_at: "2026-01-01T00:00:00Z".into(),
updated_at: "2026-01-01T00:00:00Z".into(),
deadline: None,
};
db.create_task(&visible).await.unwrap();
for i in 0..200 {
let mut hidden = visible.clone();
hidden.id = format!("hidden-{i:03}");
hidden.repo_id = None;
hidden.created_at = "2026-01-02T00:00:00Z".into();
hidden.updated_at = hidden.created_at.clone();
db.create_task(&hidden).await.unwrap();
}
let schema = schema(db);
let resp = anon(&schema, "{ tasks(limit: 1) { items { id } incomplete } }").await;
assert_eq!(count_tasks(&resp), 1);
assert!(!task_incomplete(&resp));
assert!(format!("{:?}", resp.data).contains("visible"));
}
/// The GraphQL surface must page past a denied window on server-issued
/// cursors alone, exactly as REST does (#327 review). Before this, the
/// only cursor a caller could hold named the last row they saw, so the
/// query stalled at `incomplete: true` forever and never reached
/// `past-ceiling`.
#[sqlx::test]
async fn tasks_page_past_candidate_ceiling_on_server_cursors(pool: PgPool) {
let db = db(pool).await;
db.create_repo(&repo("public-repo", OWNER, "public", true))
.await
.unwrap();
let visible_newer = crate::db::AgentTask {
id: "newer-visible".into(),
repo_id: Some("public-repo".into()),
kind: "build".into(),
status: "pending".into(),
delegator_did: OWNER.into(),
assignee_did: None,
capability: "repo:write".into(),
ucan_token: None,
payload: None,
result: None,
created_at: "2026-01-03T00:00:00Z".into(),
updated_at: "2026-01-03T00:00:00Z".into(),
deadline: None,
};
db.create_task(&visible_newer).await.unwrap();
for i in 0..1500 {
let mut hidden = visible_newer.clone();
hidden.id = format!("hidden-{i:04}");
hidden.repo_id = None;
hidden.created_at = "2026-01-02T00:00:00Z".into();
hidden.updated_at = hidden.created_at.clone();
db.create_task(&hidden).await.unwrap();
}
let mut visible_older = visible_newer.clone();
visible_older.id = "past-ceiling".into();
visible_older.created_at = "2026-01-01T00:00:00Z".into();
visible_older.updated_at = visible_older.created_at.clone();
db.create_task(&visible_older).await.unwrap();
let schema = schema(db);
let mut seen: Vec<String> = Vec::new();
let mut cursor: Option<String> = None;
for request in 0..10 {
let query = match &cursor {
Some(c) => format!(
r#"{{ tasks(limit: 1, cursor: "{c}") {{ items {{ id }} hasMore incomplete nextCursor }} }}"#
),
None => {
"{ tasks(limit: 1) { items { id } hasMore incomplete nextCursor } }".to_string()
}
};
let resp = anon(&schema, &query).await;
let rendered = format!("{:?}", resp.data);
assert!(
!rendered.contains("hidden-"),
"request {request} disclosed a denied row: {rendered}"
);
for item in task_items(&resp) {
let async_graphql::Value::Object(row) = item else {
panic!("task item not an object");
};
seen.push(row.get("id").unwrap().to_string().replace('"', ""));
}
// Short page with more behind it is the scan wall, and says so.
if task_page_bool(&resp, "hasMore") && task_items(&resp).is_empty() {
assert!(
task_incomplete(&resp),
"request {request}: an empty page with rows behind it is a paused scan"
);
}
match task_page_cursor(&resp) {
Some(c) => cursor = Some(c),
None => {
assert!(!task_page_bool(&resp, "hasMore"));
assert!(!task_incomplete(&resp));
break;
}
}
}
assert_eq!(
seen,
vec!["newer-visible".to_string(), "past-ceiling".to_string()],
"both visible rows must be reachable using only server-issued cursors"
);
}
/// REST and GraphQL mint the same tokens from the same node key, so a
/// cursor must not be usable against a filter it was not issued for, and
/// must render the same single rejection every other bad cursor renders.
#[sqlx::test]
async fn tasks_rejects_unusable_cursor(pool: PgPool) {
use crate::api::task_cursor::{self, TaskFilter, TaskPosition};
let db = db(pool).await;
let schema = schema(db);
let wrong_filter = task_cursor::encode(
&cursor_key(),
TaskFilter {
status: Some("pending"),
assignee_did: None,
},
None,
&TaskPosition::new("2026-01-01T00:00:00Z", "t1"),
);
let foreign = task_cursor::encode(
&crate::api::task_cursor::TaskCursorKey::derive(&[9u8; 32]),
TaskFilter {
status: None,
assignee_did: None,
},
None,
&TaskPosition::new("2026-01-01T00:00:00Z", "t1"),
);
for (label, query) in [
(
"garbage",
r#"{ tasks(cursor: "not-a-cursor") { items { id } } }"#.to_string(),
),
(
"another node's key",
format!(r#"{{ tasks(cursor: "{foreign}") {{ items {{ id }} }} }}"#),
),
(
"different filter",
format!(r#"{{ tasks(cursor: "{wrong_filter}") {{ items {{ id }} }} }}"#),
),
] {
let resp = anon(&schema, &query).await;
assert_eq!(
resp.errors.len(),
1,
"{label}: must be rejected, not treated as page one"
);
assert!(
resp.errors[0].message.contains("invalid or expired cursor"),
"{label}: unexpected message {:?}",
resp.errors[0].message
);
}
let resp = anon(
&schema,
&format!(
r#"{{ tasks(status: "pending", cursor: "{wrong_filter}") {{ items {{ id }} }} }}"#
),
)
.await;
assert!(resp.errors.is_empty(), "graphql errors: {:?}", resp.errors);
}
#[sqlx::test]
async fn tasks_anonymous_denial_hides_repoless_task_and_leaks_no_token(pool: PgPool) {
let db = db(pool).await;
let task = crate::db::AgentTask {
id: "t1".into(),
repo_id: None,
kind: "code-review".into(),
status: "pending".into(),
delegator_did: "did:key:z6MkDelegator".into(),
assignee_did: None,
capability: "agent:task".into(),
ucan_token: Some("secret-ucan-token".into()),
payload: Some("payload".into()),
result: None,
created_at: "2026-01-01T00:00:00Z".into(),
updated_at: "2026-01-01T00:00:00Z".into(),
deadline: None,
};
db.create_task(&task).await.unwrap();
let schema = schema(db);
let query = "{ tasks { items { id } } }";
let resp = anon(&schema, query).await;
assert!(resp.errors.is_empty(), "graphql errors: {:?}", resp.errors);
assert_eq!(count_tasks(&resp), 0);
let rendered = format!("{:?}", resp.data);
assert!(!rendered.contains("secret-ucan-token"));
}
#[sqlx::test]
async fn task_anonymous_denial_returns_null_and_leaks_no_token(pool: PgPool) {
let db = db(pool).await;
let task = crate::db::AgentTask {
id: "t1".into(),
repo_id: None,
kind: "code-review".into(),
status: "pending".into(),
delegator_did: "did:key:z6MkDelegator".into(),
assignee_did: None,
capability: "agent:task".into(),
ucan_token: Some("secret-ucan-token".into()),
payload: Some("payload".into()),
result: None,
created_at: "2026-01-01T00:00:00Z".into(),
updated_at: "2026-01-01T00:00:00Z".into(),
deadline: None,
};
db.create_task(&task).await.unwrap();
let schema = schema(db);
let query = r#"{ task(id: "t1") { id } }"#;
let resp = anon(&schema, query).await;
assert!(resp.errors.is_empty(), "graphql errors: {:?}", resp.errors);
let async_graphql::Value::Object(obj) = &resp.data else {
panic!("data not an object: {:?}", resp.data);
};
assert_eq!(obj.get("task"), Some(&async_graphql::Value::Null));
let rendered = format!("{:?}", resp.data);
assert!(!rendered.contains("secret-ucan-token"));
}
fn task_page_bool(resp: &async_graphql::Response, field: &str) -> bool {
assert!(resp.errors.is_empty(), "graphql errors: {:?}", resp.errors);
let async_graphql::Value::Object(obj) = &resp.data else {
panic!("data not an object: {:?}", resp.data);
};
let async_graphql::Value::Object(page) = obj.get("tasks").expect("tasks key") else {
panic!("tasks not an object");
};
let async_graphql::Value::Boolean(value) = page.get(field).expect("field present") else {
panic!("{field} not a bool");
};
*value
}
fn task_page_cursor(resp: &async_graphql::Response) -> Option<String> {
assert!(resp.errors.is_empty(), "graphql errors: {:?}", resp.errors);
let async_graphql::Value::Object(obj) = &resp.data else {
panic!("data not an object: {:?}", resp.data);
};
let async_graphql::Value::Object(page) = obj.get("tasks").expect("tasks key") else {
panic!("tasks not an object");
};
match page.get("nextCursor").expect("nextCursor key") {
async_graphql::Value::Null => None,
async_graphql::Value::String(c) => Some(c.clone()),
other => panic!("nextCursor not a string: {other:?}"),
}
}
fn task_items(resp: &async_graphql::Response) -> &Vec<async_graphql::Value> {
assert!(resp.errors.is_empty(), "graphql errors: {:?}", resp.errors);
let async_graphql::Value::Object(obj) = &resp.data else {
panic!("data not an object: {:?}", resp.data);
};
let async_graphql::Value::Object(page) = obj.get("tasks").expect("tasks key") else {
panic!("tasks not an object");
};
let async_graphql::Value::List(rows) = page.get("items").expect("items key") else {
panic!("items not a list");
};
rows
}
fn count_tasks(resp: &async_graphql::Response) -> usize {
task_items(resp).len()
}
fn task_incomplete(resp: &async_graphql::Response) -> bool {
assert!(resp.errors.is_empty(), "graphql errors: {:?}", resp.errors);
let async_graphql::Value::Object(obj) = &resp.data else {
panic!("data not an object: {:?}", resp.data);
};
let async_graphql::Value::Object(page) = obj.get("tasks").expect("tasks key") else {
panic!("tasks not an object");
let async_graphql::Value::List(rows) = obj.get("tasks").expect("tasks key") else {
panic!("tasks not a list");
};
let async_graphql::Value::Boolean(incomplete) =
page.get("incomplete").expect("incomplete key")
else {
panic!("incomplete not a bool");
};
*incomplete
rows.len()
}
}
@@ -11,9 +11,9 @@ pub struct SubscriptionRoot;
#[Subscription]
impl SubscriptionRoot {
/// Live ref-update stream. `/graphql/ws` is mounted under the
/// `optional_signature` layer and carries optional caller identity, but this
/// resolver does not inspect it — it relays whatever enters the broadcast
/// Live ref-update stream. `/graphql/ws` is mounted outside the
/// `optional_signature` layer, so this resolver has NO caller identity and
/// cannot gate per-subscriber — it relays whatever enters the broadcast
/// channel to any anonymous client. Its visibility safety therefore rests
/// entirely on the WRITE side: the push handler only sends a
/// `RefUpdateBroadcast` for repos the anonymous public may read (inside its
-59
View File
@@ -48,65 +48,6 @@ impl From<AgentTask> for AgentTaskType {
}
}
/// Read-only projection of `AgentTask` for the `tasks`/`task` queries, as
/// opposed to `AgentTaskType`, which the task mutations (`createTask`,
/// `claimTask`, `completeTask`, `failTask` — all `require_signer`-gated)
/// return. Identical except for the missing `ucan_token` (#268): a read
/// surface never needs to echo it back, since the assignee already received it
/// at delegation/claim time via the mutation response.
#[derive(SimpleObject, Clone)]
pub struct AgentTaskReadType {
pub id: String,
pub repo_id: Option<String>,
pub kind: String,
pub status: String,
pub delegator_did: String,
pub assignee_did: Option<String>,
pub capability: String,
pub payload: Option<String>,
pub result: Option<String>,
pub created_at: String,
pub updated_at: String,
pub deadline: Option<String>,
}
/// Wraps a `tasks` page with the same completion signals the REST list route
/// exposes, so the two surfaces answer pagination identically.
///
/// `hasMore` and `incomplete` are separate because they are separate facts
/// (#327 review): `hasMore` says more candidates remain, `incomplete` says
/// this page is short *only* because the authorization scan hit its safety
/// wall. `nextCursor` is present exactly when `hasMore` is true and is an
/// opaque MAC'd token — it can name the last examined candidate, denied or
/// not, without disclosing it, which is what lets a caller page past a denied
/// window instead of stalling on it.
#[derive(SimpleObject, Clone)]
pub struct TaskPageType {
pub items: Vec<AgentTaskReadType>,
pub has_more: bool,
pub incomplete: bool,
pub next_cursor: Option<String>,
}
impl From<AgentTask> for AgentTaskReadType {
fn from(t: AgentTask) -> Self {
Self {
id: t.id,
repo_id: t.repo_id,
kind: t.kind,
status: t.status,
delegator_did: t.delegator_did,
assignee_did: t.assignee_did,
capability: t.capability,
payload: t.payload,
result: t.result,
created_at: t.created_at,
updated_at: t.updated_at,
deadline: t.deadline,
}
}
}
#[derive(SimpleObject, Clone)]
pub struct RefUpdateType {
pub repo: String,
+2 -31
View File
@@ -51,7 +51,6 @@ use state::AppState;
struct DegradedState {
node_did: String,
db_startup: Arc<DbStartupStatus>,
web_url: Option<String>,
}
/// Two independent counters with no cross-field invariant — atomics, not a
@@ -160,7 +159,6 @@ async fn main() -> Result<()> {
degraded_listener,
node_did.to_string(),
Arc::clone(&db_startup),
config.web_url.clone(),
db_ready_rx,
shutdown_tx.subscribe(),
));
@@ -306,15 +304,10 @@ async fn main() -> Result<()> {
let (ref_update_tx, _) = tokio::sync::broadcast::channel::<state::RefUpdateBroadcast>(256);
let (task_event_tx, _) = tokio::sync::broadcast::channel::<state::TaskEventBroadcast>(256);
// Node-keyed, so continuation tokens this node mints are only accepted by
// this node and survive its restarts without any configured secret.
let task_cursor_key = api::task_cursor::TaskCursorKey::derive(&keypair.to_seed());
let graphql_schema = Arc::new(graphql::build_schema(
Arc::clone(&db),
ref_update_tx.clone(),
task_event_tx.clone(),
task_cursor_key.clone(),
));
let machine_id = std::env::var("FLY_MACHINE_ID").ok();
@@ -455,7 +448,6 @@ async fn main() -> Result<()> {
db,
node_did: node_did.clone(),
node_keypair: keypair,
task_cursor_key,
p2p: p2p_handle,
http_client,
ref_update_tx,
@@ -554,19 +546,11 @@ async fn main() -> Result<()> {
std::time::Duration::from_secs(3600),
200_000,
),
task_read_rate_limiter: rate_limit::RateLimiter::new_bounded(
config.task_read_rate_limit,
std::time::Duration::from_secs(3600),
200_000,
),
git_bin: "git".to_string(),
};
if config.ipfs_rate_limit == 0 {
tracing::warn!("GITLAWB_IPFS_RATE_LIMIT=0 — per-IP /ipfs rate limiting disabled");
}
if config.task_read_rate_limit == 0 {
tracing::warn!("GITLAWB_TASK_READ_RATE_LIMIT=0 — per-IP task read rate limiting disabled");
}
// Periodic peer-count poll for the metrics gauge. If p2p is disabled
// we still set the gauge to 0 so dashboards don't show "no data".
@@ -888,12 +872,11 @@ async fn run_degraded_server(
listener: TcpListener,
node_did: String,
db_startup: Arc<DbStartupStatus>,
web_url: Option<String>,
mut db_ready_rx: watch::Receiver<bool>,
mut shutdown_rx: watch::Receiver<bool>,
) -> Result<()> {
let addr = listener.local_addr().ok();
let router = build_degraded_router(node_did, db_startup, web_url);
let router = build_degraded_router(node_did, db_startup);
info!(?addr, "degraded HTTP server ready");
axum::serve(listener, router)
@@ -910,15 +893,10 @@ async fn run_degraded_server(
Ok(())
}
fn build_degraded_router(
node_did: String,
db_startup: Arc<DbStartupStatus>,
web_url: Option<String>,
) -> Router {
fn build_degraded_router(node_did: String, db_startup: Arc<DbStartupStatus>) -> Router {
let state = DegradedState {
node_did,
db_startup,
web_url,
};
// Everything answers 503 with the same body — including /health and
// /ready, so peer readiness probes and uptime monitors correctly see a
@@ -950,9 +928,6 @@ async fn degraded_node_info(State(state): State<DegradedState>) -> impl IntoResp
obj.insert("name".into(), "gitlawb-node".into());
obj.insert("version".into(), env!("CARGO_PKG_VERSION").into());
obj.insert("did".into(), state.node_did.clone().into());
if let Some(web_url) = &state.web_url {
obj.insert("web_url".into(), web_url.clone().into());
}
}
(StatusCode::SERVICE_UNAVAILABLE, Json(body))
}
@@ -1201,8 +1176,6 @@ mod rate_limiter_sweep_tests {
state.peer_write_rate_limiter = RateLimiter::new(10, window);
state.federated_rate_limiter = RateLimiter::new(10, window);
state.ipfs_rate_limiter = RateLimiter::new(10, window);
state.ipfs_work_rate_limiter = RateLimiter::new(10, window);
state.task_read_rate_limiter = RateLimiter::new(10, window);
let limiters = |s: &crate::state::AppState| {
[
@@ -1214,8 +1187,6 @@ mod rate_limiter_sweep_tests {
s.peer_write_rate_limiter.clone(),
s.federated_rate_limiter.clone(),
s.ipfs_rate_limiter.clone(),
s.ipfs_work_rate_limiter.clone(),
s.task_read_rate_limiter.clone(),
]
};
for l in limiters(&state) {
+2 -56
View File
@@ -296,7 +296,7 @@ pub async fn rate_limit_by_did(request: Request, next: Next) -> Response {
return (
StatusCode::TOO_MANY_REQUESTS,
[("retry-after", "60")],
RATE_LIMIT_MESSAGE,
"rate limit exceeded — try again later",
)
.into_response();
}
@@ -401,11 +401,6 @@ impl<S: Send + Sync> axum::extract::FromRequestParts<S> for PeerAddr {
}
}
/// The message every per-IP brake answers with, shared so the GraphQL surface
/// (which cannot return a 429 status inside a 200 GraphQL envelope) says the
/// same thing as the REST routes.
pub const RATE_LIMIT_MESSAGE: &str = "rate limit exceeded — try again later";
/// The shared 429 response for the per-IP flood brakes. Route-agnostic: this
/// middleware now serves the push path AND the peer-sync routes, so the message
/// stays generic (the offending path is recorded in the warn log below).
@@ -413,60 +408,11 @@ pub fn too_many_requests() -> Response {
(
StatusCode::TOO_MANY_REQUESTS,
[("retry-after", "60")],
RATE_LIMIT_MESSAGE,
"rate limit exceeded — try again later",
)
.into_response()
}
/// The per-IP task-read brake carried as GraphQL request data: the limiter plus
/// the client key the transport already resolved.
///
/// `/graphql` is a single POST endpoint, so this brake cannot sit in middleware
/// the way `task_read_routes` mounts `rate_limit_by_ip`: that would charge every
/// unrelated query and every mutation against the task-read bucket. The two task
/// resolvers debit it instead, which also prices an aliased query honestly — ten
/// aliased `tasks` fields run the visibility gate ten times and pay ten slots,
/// where one request-scoped debit would pay one (#327 review).
/// Default cap on the number of task read fields (across all aliases) permitted
/// in a single GraphQL request before rejecting further field executions (#327 review).
pub const MAX_GRAPHQL_TASK_READS_PER_REQUEST: usize = 5;
#[derive(Clone)]
pub struct TaskReadBrake {
pub limiter: RateLimiter,
/// `None` when no key could be resolved at all (no trusted header and no
/// `ConnectInfo`, e.g. a synthetic test request). Never braked, matching
/// [`rate_limit_by_ip`].
pub key: Option<String>,
pub request_count: Arc<std::sync::atomic::AtomicUsize>,
}
impl TaskReadBrake {
/// Debit one slot for one task-read field. `true` = proceed.
pub async fn check(&self, field: &str) -> bool {
let Some(key) = self.key.as_deref() else {
return true;
};
let count = self
.request_count
.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
if count >= MAX_GRAPHQL_TASK_READS_PER_REQUEST {
tracing::warn!(
key = %key,
field = %field,
count = count + 1,
"per-request GraphQL task field limit exceeded"
);
return false;
}
if self.limiter.check(key).await {
return true;
}
tracing::warn!(key = %key, field = %field, "per-IP rate limit exceeded");
false
}
}
/// Throttle the git push path by resolved client IP. The socket peer address is
/// read from `ConnectInfo` (see `into_make_service_with_connect_info` in
/// `main`). Only skips the limiter when no key can be resolved at all.
+14 -415
View File
@@ -1,6 +1,5 @@
use async_graphql::http::ALL_WEBSOCKET_PROTOCOLS;
use async_graphql_axum::{GraphQLProtocol, GraphQLRequest, GraphQLResponse, GraphQLWebSocket};
use axum::extract::{DefaultBodyLimit, WebSocketUpgrade};
use async_graphql_axum::{GraphQLRequest, GraphQLResponse, GraphQLSubscription};
use axum::extract::DefaultBodyLimit;
use axum::{
extract::State,
middleware,
@@ -24,8 +23,6 @@ use crate::state::AppState;
async fn graphql_handler(
State(state): State<AppState>,
auth: Option<axum::Extension<crate::auth::AuthenticatedDid>>,
headers: axum::http::HeaderMap,
rate_limit::PeerAddr(peer): rate_limit::PeerAddr,
req: GraphQLRequest,
) -> GraphQLResponse {
// `optional_signature` attaches the verified DID when a signature is present.
@@ -35,49 +32,9 @@ async fn graphql_handler(
if let Some(axum::Extension(did)) = auth {
inner = inner.data(did);
}
// The anonymous `tasks`/`task` resolvers run the same #268 visibility gate
// as the REST read routes and cost the node the same queries, so they carry
// the same per-IP brake. It rides as request data rather than a router layer
// because /graphql is one endpoint for every operation — see `TaskReadBrake`
// (#327 review). It debits before the gate runs, but unlike the REST layer
// it sits inside `optional_signature`, so it brakes the gate's query cost
// and not signature verification.
inner = inner.data(rate_limit::TaskReadBrake {
limiter: state.task_read_rate_limiter.clone(),
key: rate_limit::client_key(&headers, peer, state.push_limiter_trust),
request_count: std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)),
});
state.graphql_schema.execute(inner).await.into()
}
async fn graphql_ws_handler(
State(state): State<AppState>,
headers: axum::http::HeaderMap,
rate_limit::PeerAddr(peer): rate_limit::PeerAddr,
auth: Option<axum::Extension<auth::AuthenticatedDid>>,
protocol: GraphQLProtocol,
upgrade: WebSocketUpgrade,
) -> axum::response::Response {
let mut data = async_graphql::Data::default();
if let Some(axum::Extension(did)) = auth {
data.insert(did);
}
data.insert(rate_limit::TaskReadBrake {
limiter: state.task_read_rate_limiter.clone(),
key: rate_limit::client_key(&headers, peer, state.push_limiter_trust),
request_count: std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)),
});
let schema = state.graphql_schema.as_ref().clone();
upgrade
.protocols(ALL_WEBSOCKET_PROTOCOLS)
.on_upgrade(move |stream| {
GraphQLWebSocket::new(stream, schema, protocol)
.with_data(data)
.serve()
})
.into_response()
}
async fn graphql_playground() -> impl IntoResponse {
axum::response::Html(async_graphql::http::playground_source(
async_graphql::http::GraphQLPlaygroundConfig::new("/graphql")
@@ -120,14 +77,17 @@ fn add_auth_layers(
pub fn build_router(state: AppState) -> Router {
// ── GraphQL routes ─────────────────────────────────────────────────────
let schema = state.graphql_schema.as_ref().clone();
let graphql_routes = Router::new()
.route("/graphql", get(graphql_playground).post(graphql_handler))
.route("/graphql/ws", get(graphql_ws_handler))
// Attach the verified DID to /graphql and /graphql/ws when a signature is present.
// Attach the verified DID to /graphql when a signature is present. The
// layer covers only routes added before it, so /graphql/ws (added after,
// read-only subscriptions) stays open.
.layer(middleware::from_fn_with_state(
auth::SignatureBodyLimit(SIGNED_BODY_LIMIT),
auth::optional_signature,
));
))
.route_service("/graphql/ws", GraphQLSubscription::new(schema));
// ── Task routes (write — require HTTP Signature) ───────────────────────
let task_write_routes = add_auth_layers(
@@ -140,31 +100,10 @@ pub fn build_router(state: AppState) -> Router {
SIGNED_BODY_LIMIT,
);
// ── Task routes (read — open, but scoped) ──────────────────────────────
// `optional_signature` attaches the verified DID when a signature is present
// so the handlers can identify the caller; the routes stay anonymous-reachable,
// but each task/row is gated to its delegator, its assignee, or (for a
// repo-scoped task) whoever can read that repo (#268 — these routes previously
// carried no gate and no identity at all).
// Both routes also carry a per-IP flood brake, mirroring `/ipfs/{cid}`: they are
// anon-reachable and the gate above costs a task lookup plus deduped-repo and
// visibility-rule queries *before* the opaque 404, so a prober pays nothing and
// the node pays per request. The limiter is the outermost layer so a flood is
// rejected before signature verification and the visibility queries run. The
// extension MUST be attached or `rate_limit_by_ip` is a silent no-op.
let task_read_limiter = rate_limit::IpRateLimiter {
limiter: state.task_read_rate_limiter.clone(),
trust: state.push_limiter_trust,
};
// ── Task routes (read — open) ──────────────────────────────────────────
let task_read_routes = Router::new()
.route("/api/v1/tasks", get(tasks::list_tasks))
.route("/api/v1/tasks/{id}", get(tasks::get_task))
.layer(middleware::from_fn_with_state(
auth::SignatureBodyLimit(SIGNED_BODY_LIMIT),
auth::optional_signature,
))
.layer(middleware::from_fn(rate_limit::rate_limit_by_ip))
.layer(axum::Extension(task_read_limiter));
.route("/api/v1/tasks/{id}", get(tasks::get_task));
// ── Rate-limited creation routes — require HTTP Signature, plus a per-DID
// throttle AND a per-IP flood brake. The per-DID limiter (inner) caps a
@@ -680,7 +619,7 @@ async fn ready(State(state): State<AppState>) -> axum::response::Response {
async fn node_info(State(state): State<AppState>) -> Json<serde_json::Value> {
let p2p_peer_id = state.p2p.as_ref().map(|h| h.local_peer_id.to_string());
let mut body = json!({
Json(json!({
"name": "gitlawb-node",
"version": env!("CARGO_PKG_VERSION"),
"did": state.node_did.to_string(),
@@ -689,11 +628,7 @@ async fn node_info(State(state): State<AppState>) -> Json<serde_json::Value> {
"auth": "http-signature-rfc9421",
"identity": "ed25519",
"p2p_peer_id": p2p_peer_id,
});
if let Some(web_url) = &state.config.web_url {
body["web_url"] = json!(web_url);
}
Json(body)
}))
}
pub(crate) async fn stats(State(state): State<AppState>) -> Json<serde_json::Value> {
@@ -776,349 +711,13 @@ async fn p2p_info(State(state): State<AppState>) -> Json<serde_json::Value> {
#[cfg(test)]
mod tests {
use super::*;
use crate::test_support::test_state;
use sqlx::PgPool;
use tokio::io::{AsyncReadExt, AsyncWriteExt};
async fn ws_send_text(stream: &mut tokio::net::TcpStream, text: &str) {
let payload = text.as_bytes();
let len = payload.len();
let mut frame = Vec::new();
frame.push(0x81);
let mask = [0x12, 0x34, 0x56, 0x78];
if len <= 125 {
frame.push(0x80 | (len as u8));
} else if len <= 65535 {
frame.push(0x80 | 126);
frame.extend_from_slice(&(len as u16).to_be_bytes());
} else {
frame.push(0x80 | 127);
frame.extend_from_slice(&(len as u64).to_be_bytes());
}
frame.extend_from_slice(&mask);
for (i, b) in payload.iter().enumerate() {
frame.push(b ^ mask[i % 4]);
}
stream.write_all(&frame).await.unwrap();
stream.flush().await.unwrap();
}
async fn ws_recv_text(stream: &mut tokio::net::TcpStream) -> String {
let mut header = [0u8; 2];
stream.read_exact(&mut header).await.unwrap();
let b1 = header[1];
let masked = (b1 & 0x80) != 0;
let mut len = (b1 & 0x7f) as usize;
if len == 126 {
let mut ext = [0u8; 2];
stream.read_exact(&mut ext).await.unwrap();
len = u16::from_be_bytes(ext) as usize;
} else if len == 127 {
let mut ext = [0u8; 8];
stream.read_exact(&mut ext).await.unwrap();
len = u64::from_be_bytes(ext) as usize;
}
let mask = if masked {
let mut m = [0u8; 4];
stream.read_exact(&mut m).await.unwrap();
Some(m)
} else {
None
};
let mut payload = vec![0u8; len];
stream.read_exact(&mut payload).await.unwrap();
if let Some(m) = mask {
for (i, b) in payload.iter_mut().enumerate() {
*b ^= m[i % 4];
}
}
String::from_utf8(payload).unwrap()
}
async fn ws_recv_op_frames(stream: &mut tokio::net::TcpStream, op_id: &str) -> Vec<String> {
let mut frames = Vec::new();
loop {
let text = ws_recv_text(stream).await;
frames.push(text.clone());
if let Ok(v) = serde_json::from_str::<serde_json::Value>(&text) {
if v.get("id").and_then(|id| id.as_str()) == Some(op_id) {
let msg_type = v.get("type").and_then(|t| t.as_str()).unwrap_or("");
if msg_type == "complete" || msg_type == "error" {
break;
}
}
}
}
frames
}
async fn connect_ws(addr: std::net::SocketAddr) -> tokio::net::TcpStream {
let mut stream = tokio::net::TcpStream::connect(addr).await.unwrap();
let req = format!(
"GET /graphql/ws HTTP/1.1\r\n\
Host: {}\r\n\
Upgrade: websocket\r\n\
Connection: Upgrade\r\n\
Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\
Sec-WebSocket-Version: 13\r\n\
Sec-WebSocket-Protocol: graphql-transport-ws\r\n\r\n",
addr
);
stream.write_all(req.as_bytes()).await.unwrap();
stream.flush().await.unwrap();
let mut buf = [0u8; 1024];
let n = stream.read(&mut buf).await.unwrap();
let resp = String::from_utf8_lossy(&buf[..n]);
assert!(resp.starts_with("HTTP/1.1 101 Switching Protocols"));
// Init connection
ws_send_text(&mut stream, r#"{"type":"connection_init"}"#).await;
let ack = ws_recv_text(&mut stream).await;
assert!(ack.contains("connection_ack"));
stream
}
#[sqlx::test]
async fn graphql_ws_task_query_enforces_per_request_field_limit(pool: PgPool) {
let state = test_state(pool).await;
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let app = build_router(state);
tokio::spawn(async move {
axum::serve(
listener,
app.into_make_service_with_connect_info::<std::net::SocketAddr>(),
)
.await
.unwrap();
});
let mut stream = connect_ws(addr).await;
// Query with 6 aliased task fields (exceeding MAX_GRAPHQL_TASK_READS_PER_REQUEST = 5)
let query = r#"{"id":"1","type":"subscribe","payload":{"query":"query { f1: tasks { items { id } } f2: tasks { items { id } } f3: tasks { items { id } } f4: tasks { items { id } } f5: tasks { items { id } } f6: tasks { items { id } } }"}}"#;
ws_send_text(&mut stream, query).await;
let frames = ws_recv_op_frames(&mut stream, "1").await;
let resp = frames.join("\n");
assert!(
resp.contains("rate limit exceeded"),
"6th task field over WS must be braked: {resp}"
);
}
#[sqlx::test]
async fn graphql_ws_task_query_enforces_per_ip_rate_limit(pool: PgPool) {
let mut state = test_state(pool).await;
// Restrict task read rate limiter to 1 request per 60s
state.task_read_rate_limiter =
crate::rate_limit::RateLimiter::new(1, std::time::Duration::from_secs(60));
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let app = build_router(state);
tokio::spawn(async move {
axum::serve(
listener,
app.into_make_service_with_connect_info::<std::net::SocketAddr>(),
)
.await
.unwrap();
});
let mut stream = connect_ws(addr).await;
// First task query succeeds (or returns valid data)
let query1 = r#"{"id":"1","type":"subscribe","payload":{"query":"query { tasks { items { id } } }"}}"#;
ws_send_text(&mut stream, query1).await;
let frames1 = ws_recv_op_frames(&mut stream, "1").await;
let resp1 = frames1.join("\n");
assert!(!resp1.contains("rate limit exceeded"));
// Second task query on the same connection hits per-IP limiter
let query2 = r#"{"id":"2","type":"subscribe","payload":{"query":"query { tasks { items { id } } }"}}"#;
ws_send_text(&mut stream, query2).await;
let frames2 = ws_recv_op_frames(&mut stream, "2").await;
let resp2 = frames2.join("\n");
assert!(
resp2.contains("rate limit exceeded"),
"exceeded per-IP limiter over WS must return rate limit message: {resp2}"
);
}
#[sqlx::test]
async fn graphql_ws_task_query_resets_field_budget_per_operation(pool: PgPool) {
let state = test_state(pool).await;
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let app = build_router(state);
tokio::spawn(async move {
axum::serve(
listener,
app.into_make_service_with_connect_info::<std::net::SocketAddr>(),
)
.await
.unwrap();
});
let mut stream = connect_ws(addr).await;
// Execute 6 sequential 1-field queries on the same connection.
// Each operation must receive its own 5-field budget, so none are braked by the per-request limit.
for i in 1..=6 {
let id = i.to_string();
let query = format!(
r#"{{"id":"{id}","type":"subscribe","payload":{{"query":"query {{ tasks {{ items {{ id }} }} }}"}}}}"#
);
ws_send_text(&mut stream, &query).await;
let frames = ws_recv_op_frames(&mut stream, &id).await;
let resp = frames.join("\n");
assert!(
!resp.contains("rate limit exceeded"),
"operation {id} on the same WS connection must have a fresh field budget: {resp}"
);
}
}
async fn connect_ws_signed(
addr: std::net::SocketAddr,
keypair: &gitlawb_core::identity::Keypair,
) -> tokio::net::TcpStream {
let mut stream = tokio::net::TcpStream::connect(addr).await.unwrap();
let signed = gitlawb_core::http_sig::sign_request(keypair, "GET", "/graphql/ws", b"");
let req = format!(
"GET /graphql/ws HTTP/1.1\r\n\
Host: {}\r\n\
Upgrade: websocket\r\n\
Connection: Upgrade\r\n\
Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\
Sec-WebSocket-Version: 13\r\n\
Sec-WebSocket-Protocol: graphql-transport-ws\r\n\
content-digest: {}\r\n\
signature-input: {}\r\n\
signature: {}\r\n\r\n",
addr, signed.content_digest, signed.signature_input, signed.signature
);
stream.write_all(req.as_bytes()).await.unwrap();
stream.flush().await.unwrap();
let mut buf = [0u8; 1024];
let n = stream.read(&mut buf).await.unwrap();
let resp = String::from_utf8_lossy(&buf[..n]);
assert!(resp.starts_with("HTTP/1.1 101 Switching Protocols"));
// Init connection
ws_send_text(&mut stream, r#"{"type":"connection_init"}"#).await;
let ack = ws_recv_text(&mut stream).await;
assert!(ack.contains("connection_ack"));
stream
}
#[sqlx::test]
async fn graphql_ws_authenticated_query_accesses_private_task(pool: PgPool) {
let state = test_state(pool).await;
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
let app = build_router(state.clone());
tokio::spawn(async move {
axum::serve(
listener,
app.into_make_service_with_connect_info::<std::net::SocketAddr>(),
)
.await
.unwrap();
});
let keypair = gitlawb_core::identity::Keypair::generate();
let delegator_did = keypair.did().to_string();
let task = crate::db::AgentTask {
id: "ws-auth-task-01".into(),
repo_id: None,
delegator_did: delegator_did.clone(),
kind: "test".into(),
capability: "read".into(),
status: "pending".into(),
assignee_did: Some(delegator_did.clone()),
ucan_token: None,
payload: None,
result: None,
created_at: "2026-01-01T00:00:00Z".into(),
updated_at: "2026-01-01T00:00:00Z".into(),
deadline: None,
};
state.db.create_task(&task).await.unwrap();
// 1. Unauthenticated WS connection cannot see the private task
let mut unauth_stream = connect_ws(addr).await;
let query = r#"{"id":"1","type":"subscribe","payload":{"query":"query { tasks { items { id } } }"}}"#;
ws_send_text(&mut unauth_stream, query).await;
let unauth_frames = ws_recv_op_frames(&mut unauth_stream, "1").await;
let unauth_resp = unauth_frames.join("\n");
assert!(
!unauth_resp.contains("ws-auth-task-01"),
"unauthenticated WS must not disclose private task: {unauth_resp}"
);
// 2. Forged signature claiming the delegator DID must be rejected at handshake
let attacker_keypair = gitlawb_core::identity::Keypair::generate();
let attacker_signed =
gitlawb_core::http_sig::sign_request(&attacker_keypair, "GET", "/graphql/ws", b"");
let forged_sig_input = attacker_signed
.signature_input
.replace(&attacker_keypair.did().to_string(), &delegator_did);
let mut forged_stream = tokio::net::TcpStream::connect(addr).await.unwrap();
let forged_req = format!(
"GET /graphql/ws HTTP/1.1\r\n\
Host: {}\r\n\
Upgrade: websocket\r\n\
Connection: Upgrade\r\n\
Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\
Sec-WebSocket-Version: 13\r\n\
Sec-WebSocket-Protocol: graphql-transport-ws\r\n\
content-digest: {}\r\n\
signature-input: {}\r\n\
signature: {}\r\n\r\n",
addr, attacker_signed.content_digest, forged_sig_input, attacker_signed.signature
);
forged_stream
.write_all(forged_req.as_bytes())
.await
.unwrap();
forged_stream.flush().await.unwrap();
let mut forged_bytes = Vec::new();
let mut chunk = [0u8; 1024];
while !forged_bytes.windows(4).any(|w| w == b"\r\n\r\n") {
let n = forged_stream.read(&mut chunk).await.unwrap();
if n == 0 {
break;
}
forged_bytes.extend_from_slice(&chunk[..n]);
}
let forged_resp = String::from_utf8_lossy(&forged_bytes);
assert!(
forged_resp.starts_with("HTTP/1.1 401 Unauthorized"),
"forged WS signature must be rejected with 401: {forged_resp}"
);
// 3. Signed WS connection authenticates caller and retrieves the private task
let mut auth_stream = connect_ws_signed(addr, &keypair).await;
ws_send_text(&mut auth_stream, query).await;
let auth_frames = ws_recv_op_frames(&mut auth_stream, "1").await;
let auth_resp = auth_frames.join("\n");
assert!(
auth_resp.contains("ws-auth-task-01"),
"signed WS must authenticate delegator and return private task: {auth_resp}"
);
}
use axum::body::Body;
use axum::http::{Request, StatusCode};
use sqlx::PgPool;
use tower::ServiceExt;
use crate::db::RepoRecord;
use crate::test_support::test_state;
/// Regression: anonymous callers must not see the pin/anchor index (#121, #134).
#[sqlx::test]
-15
View File
@@ -59,10 +59,6 @@ pub struct AppState {
pub task_event_tx: tokio::sync::broadcast::Sender<TaskEventBroadcast>,
/// GraphQL schema (queries + mutations + subscriptions)
pub graphql_schema: Arc<crate::graphql::GitlawbSchema>,
/// MAC key for task-list continuation tokens, derived from the node
/// keypair seed. Held here (and handed to the GraphQL schema) so REST and
/// GraphQL mint and accept the same tokens.
pub task_cursor_key: crate::api::task_cursor::TaskCursorKey,
/// Fly.io machine ID — used for fly-replay routing in multi-machine deployments
pub machine_id: Option<String>,
/// Centralized repo storage: local disk cache + optional Tigris backend
@@ -330,16 +326,6 @@ pub struct AppState {
/// (`with_default_max_keys`, reject-before-insert) so a source-key farm cannot grow
/// it (INV-15).
pub git_ipfs_walk_per_caller: crate::rate_limit::PerCallerConcurrency,
/// Per-client-IP rate limiter for the task read routes. `GET /api/v1/tasks`
/// and `GET /api/v1/tasks/{id}` are anonymously reachable and run the #268
/// visibility gate (a task lookup plus deduped-repo and visibility-rule
/// queries) before returning the opaque 404, so an unauthenticated prober
/// costs the node real work per request whether or not anything is visible.
/// Keyed on the resolved client IP via `push_limiter_trust`. Layered on
/// `task_read_routes` via `rate_limit_by_ip`.
/// The same per-IP hourly budget applies to GraphQL `tasks` / `task`
/// queries and WebSocket task queries via `TaskReadBrake`.
pub task_read_rate_limiter: RateLimiter,
/// The `git` executable the served-git withheld-blob walk spawns. Production is
/// `"git"` (resolved via PATH); injectable so a fake `git` can drive the walk's
/// process-group teardown in handler tests without mutating the process-global
@@ -368,7 +354,6 @@ impl AppState {
self.ipfs_work_rate_limiter.cleanup().await;
self.sync_trigger_rate_limiter.cleanup().await;
self.peer_write_rate_limiter.cleanup().await;
self.task_read_rate_limiter.cleanup().await;
self.federated_rate_limiter.cleanup().await;
}
+8 -478
View File
@@ -83,12 +83,10 @@ fn build_state(db: Arc<crate::db::Db>, pool: PgPool) -> AppState {
let node_did = keypair.did();
let (ref_tx, _) = tokio::sync::broadcast::channel(1);
let (task_tx, _) = tokio::sync::broadcast::channel(1);
let task_cursor_key = crate::api::task_cursor::TaskCursorKey::derive(&keypair.to_seed());
let schema = Arc::new(graphql::build_schema(
db.clone(),
ref_tx.clone(),
task_tx.clone(),
task_cursor_key.clone(),
));
AppState {
config: Arc::new(Config::parse_from(["gitlawb-node"])),
@@ -100,7 +98,6 @@ fn build_state(db: Arc<crate::db::Db>, pool: PgPool) -> AppState {
ref_update_tx: ref_tx,
task_event_tx: task_tx,
graphql_schema: schema,
task_cursor_key,
machine_id: None,
repo_store: crate::git::repo_store::RepoStore::for_testing(PathBuf::from("/tmp"), pool),
rate_limiter: RateLimiter::new(100, Duration::from_secs(60)),
@@ -139,7 +136,6 @@ fn build_state(db: Arc<crate::db::Db>, pool: PgPool) -> AppState {
git_ipfs_walk_per_caller: crate::rate_limit::PerCallerConcurrency::with_default_max_keys(
16,
),
task_read_rate_limiter: RateLimiter::new(1200, Duration::from_secs(3600)),
git_bin: "git".to_string(),
}
}
@@ -1112,7 +1108,7 @@ mod tests {
// repo id afterwards — a 403 that still writes is the injection.
let stored = state
.db
.list_tasks_keyset(None, None, 50, None)
.list_tasks(None, None, 50)
.await
.expect("list tasks");
assert!(
@@ -1471,11 +1467,11 @@ mod tests {
let assignee = "did:key:zTASKASSIGNEEBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
let stranger = "did:key:zTASKSTRANGERCCCCCCCCCCCCCCCCCCCCCCCCCCC";
let state = test_state(pool).await;
let repo = seed_repo(delegator, "task-pub-repo");
state.db.create_repo(&repo).await.expect("seed repo");
let mut t1 = seed_task("task-1", delegator);
t1.repo_id = Some(repo.id);
state.db.create_task(&t1).await.expect("seed task");
state
.db
.create_task(&seed_task("task-1", delegator))
.await
.expect("seed task");
// Assignee claims it: pending -> claimed, assignee_did = assignee.
state
.db
@@ -1494,25 +1490,8 @@ mod tests {
let uri = "/api/v1/tasks/task-1/complete";
let body = || Body::from("{}");
// Stranger on an invisible (repo-less) task receives opaque 404 (no existence leak).
let inv_task = seed_task("task-inv", delegator);
state.db.create_task(&inv_task).await.unwrap();
let inv_resp = router()
.oneshot(signed_request_as(
stranger,
Method::POST,
"/api/v1/tasks/task-inv/complete",
body(),
))
.await
.unwrap();
assert_eq!(
inv_resp.status(),
StatusCode::NOT_FOUND,
"an invisible task must 404 so existence is not leaked"
);
// Stranger on a visible task is rejected by the authorization gate with exact 403.
// Stranger (not the assignee) is rejected by the authorization gate, even
// with the empty body that previously bypassed the binding. Exact 403.
let resp = router()
.oneshot(signed_request_as(stranger, Method::POST, uri, body()))
.await
@@ -1547,455 +1526,6 @@ mod tests {
);
}
/// Pre-assigned tasks must not be stealable: a stranger claiming a pending
/// task reserved for another agent must fail, and must not receive the
/// UCAN. The reserved assignee still claims successfully.
#[sqlx::test]
async fn claim_task_honors_preassigned_assignee(pool: PgPool) {
let delegator = "did:key:zCLAIMDELEGATORAAAAAAAAAAAAAAAAAAAAAAAAA";
let reserved = "did:key:zCLAIMRESERVEDBBBBBBBBBBBBBBBBBBBBBBBBBBB";
let thief = "did:key:zCLAIMTHIEFCCCCCCCCCCCCCCCCCCCCCCCCCCCCC";
let state = test_state(pool).await;
let mut task = seed_task("task-reserved", delegator);
task.assignee_did = Some(reserved.to_string());
task.ucan_token = Some("ucan-secret-for-reserved-only".into());
state.db.create_task(&task).await.expect("seed");
let router = || {
Router::new()
.route(
"/api/v1/tasks/{id}/claim",
axum::routing::post(crate::api::tasks::claim_task),
)
.with_state(state.clone())
};
let uri = "/api/v1/tasks/task-reserved/claim";
// Thief signs as themselves and asks to claim — must fail, UCAN stays put.
let resp = router()
.oneshot(signed_request_as(
thief,
Method::POST,
uri,
Body::from(format!(r#"{{"assignee_did":"{thief}"}}"#)),
))
.await
.unwrap();
// The claim gate hides existence: an ineligible claimer gets the same
// 404 as a missing task, not a 403 that would reveal the reservation.
assert_eq!(
resp.status(),
StatusCode::NOT_FOUND,
"stranger must not steal a pre-assigned task, got {}",
resp.status()
);
let body = axum::body::to_bytes(resp.into_body(), usize::MAX)
.await
.unwrap();
let text = String::from_utf8_lossy(&body);
assert!(
!text.contains("ucan-secret-for-reserved-only"),
"UCAN must not leak on a failed steal: {text}"
);
let still = state.db.get_task("task-reserved").await.unwrap().unwrap();
assert_eq!(still.status, "pending");
assert_eq!(still.assignee_did.as_deref(), Some(reserved));
// Reserved assignee claims successfully and receives the UCAN.
let resp = router()
.oneshot(signed_request_as(
reserved,
Method::POST,
uri,
Body::from(format!(r#"{{"assignee_did":"{reserved}"}}"#)),
))
.await
.unwrap();
assert!(
resp.status().is_success(),
"reserved assignee must claim, got {}",
resp.status()
);
let body = axum::body::to_bytes(resp.into_body(), usize::MAX)
.await
.unwrap();
let text = String::from_utf8_lossy(&body);
assert!(
text.contains("ucan-secret-for-reserved-only"),
"reserved assignee must receive UCAN: {text}"
);
assert!(text.contains("\"status\":\"claimed\"") || text.contains("claimed"));
}
/// Open (unassigned) pending tasks remain first-claimer-wins after the
/// pre-assignment gate.
#[sqlx::test]
async fn claim_task_open_still_first_claimer_wins(pool: PgPool) {
let delegator = "did:key:zOPENDELEGATORAAAAAAAAAAAAAAAAAAAAAAAAAA";
let claimer = "did:key:zOPENCLAIMERBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
let state = test_state(pool).await;
state
.db
.create_task(&seed_task("task-open", delegator))
.await
.expect("seed");
let router = Router::new()
.route(
"/api/v1/tasks/{id}/claim",
axum::routing::post(crate::api::tasks::claim_task),
)
.with_state(state.clone());
let resp = router
.oneshot(signed_request_as(
claimer,
Method::POST,
"/api/v1/tasks/task-open/claim",
Body::from(format!(r#"{{"assignee_did":"{claimer}"}}"#)),
))
.await
.unwrap();
assert!(
resp.status().is_success(),
"open task must still be claimable, got {}",
resp.status()
);
let got = state.db.get_task("task-open").await.unwrap().unwrap();
assert_eq!(got.status, "claimed");
assert_eq!(got.assignee_did.as_deref(), Some(claimer));
}
/// ASCII-blank assignees (`ASSIGNEE_BLANK`: space, tab, LF, CR) must be open
/// at the claim SQL blank branch. Seeds via raw INSERT so create_task
/// normalization is not in the path.
#[sqlx::test]
async fn claim_task_ascii_blank_reservations_are_open(pool: PgPool) {
let delegator = "did:key:zBLANKDELEGATORAAAAAAAAAAAAAAAAAAAAAAAAA";
let claimer = "did:key:zBLANKCLAIMERBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
let state = test_state(pool.clone()).await;
let now = chrono::Utc::now().to_rfc3339();
for (id, blank) in [
("task-blank-empty", ""),
("task-blank-tab", "\t"),
("task-blank-space", " "),
("task-blank-lf", "\n"),
("task-blank-cr", "\r"),
] {
sqlx::query(
"INSERT INTO agent_tasks (id, repo_id, kind, status, delegator_did, assignee_did, capability, ucan_token, payload, result, created_at, updated_at, deadline)
VALUES ($1,NULL,'build','pending',$2,$3,'repo:write',NULL,NULL,NULL,$4,$4,NULL)",
)
.bind(id)
.bind(delegator)
.bind(blank)
.bind(&now)
.execute(&pool)
.await
.unwrap_or_else(|e| panic!("seed {id}: {e}"));
let claimed = state
.db
.claim_task(id, claimer)
.await
.unwrap_or_else(|e| panic!("{id} must be open: {e}"));
assert_eq!(claimed.status, "claimed", "{id}");
assert_eq!(claimed.assignee_did.as_deref(), Some(claimer), "{id}");
}
}
/// create_task must null whitespace-only assignees and return that shape
/// (fail-on-remove for the blank filter).
#[sqlx::test]
async fn create_task_normalizes_whitespace_assignee(pool: PgPool) {
let delegator = "did:key:zCREATENORMDELEGAAAAAAAAAAAAAAAAAAAAAAAA";
let state = test_state(pool).await;
let mut task = seed_task("task-create-norm", delegator);
task.assignee_did = Some(" \t\n ".into());
let stored = state.db.create_task(&task).await.expect("create");
assert!(
stored.assignee_did.is_none(),
"create_task must return normalized NULL assignee, got {:?}",
stored.assignee_did
);
let got = state
.db
.get_task("task-create-norm")
.await
.unwrap()
.unwrap();
assert!(got.assignee_did.is_none(), "row must store NULL assignee");
}
/// Claiming a reserved task must keep the stored DID form so exact-match
/// list filters still find it. Also covers bare→full claim + list by bare
/// (claimer full DID must not be required for list equality).
#[sqlx::test]
async fn claim_task_keeps_stored_assignee_did_form(pool: PgPool) {
let delegator = "did:key:zFORMDELEGATORAAAAAAAAAAAAAAAAAAAAAAAAAA";
let bare = "zFORMRESERVEDBBBBBBBBBBBBBBBBBBBBBBBBBBB";
let full = format!("did:key:{bare}");
let state = test_state(pool).await;
let mut task = seed_task("task-form", delegator);
task.assignee_did = Some(bare.to_string());
state.db.create_task(&task).await.expect("seed");
let claimed = state
.db
.claim_task("task-form", &full)
.await
.expect("claim with full DID");
assert_eq!(
claimed.assignee_did.as_deref(),
Some(bare),
"reserved claim must keep the stored bare-key form"
);
let listed_bare = state
.db
.list_tasks_keyset(Some("claimed"), Some(bare), 10, None)
.await
.unwrap();
assert!(
listed_bare.iter().any(|t| t.id == "task-form"),
"delegator filtering by bare key must still find the task"
);
let listed_full = state
.db
.list_tasks_keyset(Some("claimed"), Some(&full), 10, None)
.await
.unwrap();
// The keyset filter normalizes to the did:key short form, so the
// claimer's full DID now matches the bare stored form too.
assert!(
listed_full.iter().any(|t| t.id == "task-form"),
"normalized list by claimer's full DID must match the bare stored form"
);
}
/// Padded non-blank reservation must survive claim unchanged (exact form).
#[sqlx::test]
async fn claim_task_keeps_padded_reservation_exact(pool: PgPool) {
let delegator = "did:key:zPADDELEGATORAAAAAAAAAAAAAAAAAAAAAAAAAAA";
let key = "zPADRESERVEDBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
let padded = format!(" {key}\t");
let full = format!("did:key:{key}");
let state = test_state(pool.clone()).await;
let now = chrono::Utc::now().to_rfc3339();
sqlx::query(
"INSERT INTO agent_tasks (id, repo_id, kind, status, delegator_did, assignee_did, capability, ucan_token, payload, result, created_at, updated_at, deadline)
VALUES ($1,NULL,'build','pending',$2,$3,'repo:write',NULL,NULL,NULL,$4,$4,NULL)",
)
.bind("task-padded")
.bind(delegator)
.bind(&padded)
.bind(&now)
.execute(&pool)
.await
.expect("seed padded");
let claimed = state
.db
.claim_task("task-padded", &full)
.await
.expect("claim padded reservation");
assert_eq!(
claimed.assignee_did.as_deref(),
Some(padded.as_str()),
"claim must not BTRIM a non-blank reservation"
);
let listed = state
.db
.list_tasks_keyset(Some("claimed"), Some(&padded), 10, None)
.await
.unwrap();
assert!(
listed.iter().any(|t| t.id == "task-padded"),
"exact list by original padded form must still find the task"
);
let finished = state
.db
.finish_task("task-padded", "completed", None, &full)
.await
.expect("padded assignee must finish after trim match");
assert_eq!(finished.status, "completed");
}
/// Direct coverage for the DB-layer finish_task assignee Rust gate (handlers
/// 403 before calling in, so HTTP tests alone leave this unproven).
#[sqlx::test]
async fn finish_task_rejects_non_assignee_at_db(pool: PgPool) {
let delegator = "did:key:zFINISHDELEGATORAAAAAAAAAAAAAAAAAAAAAAAA";
let assignee = "did:key:zFINISHASSIGNEEBBBBBBBBBBBBBBBBBBBBBBBBB";
let stranger = "did:key:zFINISHSTRANGERCCCCCCCCCCCCCCCCCCCCCCCCC";
let state = test_state(pool).await;
state
.db
.create_task(&seed_task("task-finish-gate", delegator))
.await
.expect("seed");
state
.db
.claim_task("task-finish-gate", assignee)
.await
.expect("claim");
let err = state
.db
.finish_task("task-finish-gate", "completed", None, stranger)
.await
.expect_err("stranger must not finish");
assert!(
err.to_string().contains("assignee") || err.to_string().contains("finishable"),
"expected assignee denial, got {err}"
);
let still = state
.db
.get_task("task-finish-gate")
.await
.unwrap()
.unwrap();
assert_eq!(
still.status, "claimed",
"row must stay claimed after denial"
);
let done = state
.db
.finish_task("task-finish-gate", "completed", None, assignee)
.await
.expect("assignee finishes");
assert_eq!(done.status, "completed");
}
/// Fail-on-remove for `finish_task`'s `AND assignee_did=$5`: a concurrent writer
/// that changes the stored assignee between the production read and UPDATE must
/// leave the task claimed (production `Db::finish_task` path).
#[sqlx::test]
async fn finish_task_sql_assignee_predicate_is_load_bearing(pool: PgPool) {
let delegator = "did:key:zFINISHSQLDELEGAAAAAAAAAAAAAAAAAAAAAAAAA";
let assignee = "did:key:zFINISHSQLASSIGNBBBBBBBBBBBBBBBBBBBBBBBB";
let other = "did:key:zFINISHSQLOTHERCCCCCCCCCCCCCCCCCCCCCCCCC";
let state = test_state(pool.clone()).await;
let id = "task-finish-sql";
state
.db
.create_task(&seed_task(id, delegator))
.await
.expect("seed");
state.db.claim_task(id, assignee).await.expect("claim");
let pool_bg = pool.clone();
let id_bg = id.to_string();
let other_bg = other.to_string();
let (locked_tx, locked_rx) = tokio::sync::oneshot::channel();
let hog = tokio::spawn(async move {
let mut tx = pool_bg.begin().await.expect("begin");
sqlx::query("SELECT assignee_did FROM agent_tasks WHERE id = $1 FOR UPDATE")
.bind(&id_bg)
.fetch_one(&mut *tx)
.await
.expect("lock row");
locked_tx.send(()).ok();
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
sqlx::query(
"UPDATE agent_tasks SET assignee_did = $2 WHERE id = $1 AND status = 'claimed'",
)
.bind(&id_bg)
.bind(&other_bg)
.execute(&mut *tx)
.await
.expect("race assignee");
tx.commit().await.expect("commit");
});
locked_rx.await.expect("row lock held before finish");
let err = state
.db
.finish_task(id, "completed", None, assignee)
.await
.expect_err("assignee slot race must fail production finish_task");
hog.await.expect("hog task");
assert!(
err.to_string().contains("not in claimed state"),
"expected finish denial, got {err}"
);
let still = state.db.get_task(id).await.unwrap().unwrap();
assert_eq!(still.status, "claimed");
assert_eq!(still.assignee_did.as_deref(), Some(other));
}
/// Fail-on-remove for the claim UPDATE assignee-slot re-check: a concurrent
/// writer that changes `assignee_did` after the pre-read must block claim.
#[sqlx::test]
async fn claim_task_update_assignee_slot_recheck_is_load_bearing(pool: PgPool) {
let delegator = "did:key:zCLAIMSLOTDELEGAAAAAAAAAAAAAAAAAAAAAAAAA";
let reserved = "did:key:zCLAIMSLOTASSIGNEEBBBBBBBBBBBBBBBBBBBBBBBBB";
let other = "did:key:zCLAIMSLOTOTHERCCCCCCCCCCCCCCCCCCCCCCCCC";
let state = test_state(pool.clone()).await;
let id = "task-claim-slot";
let now = chrono::Utc::now().to_rfc3339();
sqlx::query(
"INSERT INTO agent_tasks
(id, repo_id, kind, status, delegator_did, assignee_did, capability,
ucan_token, payload, result, created_at, updated_at, deadline)
VALUES ($1, NULL, 'build', 'pending', $2, $3, 'repo:write',
NULL, NULL, NULL, $4, $4, NULL)",
)
.bind(id)
.bind(delegator)
.bind(reserved)
.bind(&now)
.execute(&pool)
.await
.expect("seed reserved");
let pool_bg = pool.clone();
let id_bg = id.to_string();
let other_bg = other.to_string();
let (locked_tx, locked_rx) = tokio::sync::oneshot::channel();
let hog = tokio::spawn(async move {
let mut tx = pool_bg.begin().await.expect("begin");
sqlx::query("SELECT assignee_did FROM agent_tasks WHERE id = $1 FOR UPDATE")
.bind(&id_bg)
.fetch_one(&mut *tx)
.await
.expect("lock row");
locked_tx.send(()).ok();
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
sqlx::query(
"UPDATE agent_tasks SET assignee_did = $2 WHERE id = $1 AND status = 'pending'",
)
.bind(&id_bg)
.bind(&other_bg)
.execute(&mut *tx)
.await
.expect("race assignee");
tx.commit().await.expect("commit");
});
locked_rx.await.expect("row lock held before claim");
let err = state
.db
.claim_task(id, reserved)
.await
.expect_err("assignee slot race must fail production claim_task");
hog.await.expect("hog task");
assert!(
err.to_string().contains("not found or already claimed"),
"expected SQL re-check denial, got {err}"
);
assert!(
err.downcast_ref::<crate::db::TaskReservedForOtherAssignee>()
.is_none(),
"must prove the UPDATE predicate, not the Rust pre-check: {err}"
);
let still = state.db.get_task(id).await.unwrap().unwrap();
assert_eq!(still.status, "pending");
assert_eq!(still.assignee_did.as_deref(), Some(other));
}
/// Adversarial-review GATE-2 (create_pr): opening a PR requires read access.
/// A non-reader is denied on a private repo before any PR is created; the
/// owner is allowed.
-2
View File
@@ -26,8 +26,6 @@ clap = { version = "4", features = ["derive", "env"] }
dirs = "5"
reqwest = { workspace = true }
uuid = { workspace = true }
url = { workspace = true }
percent-encoding = { workspace = true }
urlencoding = "2"
alloy = { version = "1", default-features = false, features = [
"contract",
-21
View File
@@ -99,27 +99,6 @@ pub fn load_keypair_from_dir(dir: Option<&std::path::Path>) -> Result<Keypair> {
Keypair::from_pem(&pem).context("failed to load keypair from PEM")
}
/// Load keypair from an optional directory override, distinguishing missing default
/// identity (clean anonymous) from an explicit directory error or corrupt PEM.
pub fn load_optional_keypair(dir: Option<&std::path::Path>) -> Result<Option<Keypair>> {
if let Some(d) = dir {
return load_keypair_from_dir(Some(d)).map(Some);
}
let base = match dirs::home_dir() {
Some(h) => h.join(".gitlawb"),
None => return Ok(None),
};
let path = key_path(&base);
match fs::read_to_string(&path) {
Ok(pem) => Keypair::from_pem(&pem)
.context("failed to load keypair from PEM")
.map(Some),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
Err(e) => Err(anyhow::Error::from(e)
.context(format!("failed to read identity from {}", path.display()))),
}
}
async fn cmd_new(dir: Option<PathBuf>, force: bool) -> Result<()> {
cmd_new_with_reader(dir, force, &mut std::io::stdin().lock()).await
}
+13 -384
View File
@@ -43,6 +43,7 @@ use std::io::{BufRead, Write};
use std::path::PathBuf;
use crate::http::NodeClient;
use crate::identity::load_keypair_from_dir;
#[derive(Args)]
pub struct McpArgs {
@@ -515,8 +516,7 @@ fn tool_definitions() -> Value {
"properties": {
"status": { "type": "string", "description": "Filter by status: pending, claimed, completed, failed" },
"assignee_did": { "type": "string", "description": "Filter by assignee DID" },
"limit": { "type": "integer", "minimum": 1, "description": "Total results to return (default: 50). Must be positive; a non-positive limit is rejected rather than answered with an empty list. Values above the node's 200-row page cap are gathered by following continuation tokens.", "default": 50 },
"cursor": { "type": "string", "description": "Resume from a previous call's next_cursor. Must be paired with the same status/assignee_did filter that produced it." }
"limit": { "type": "integer", "description": "Max results (default: 50)", "default": 50 }
}
}
},
@@ -640,14 +640,7 @@ async fn call_tool(
node: &str,
dir: Option<&std::path::Path>,
) -> Result<String> {
// An explicit dir with no identity file is a clean anonymous caller
// (main's unsigned `webhook_list` path lets the node answer the denial);
// a present but unloadable identity is an error, never a silent
// downgrade to anonymous (this PR's `load_optional_keypair` contract).
let keypair = match dir {
Some(d) if !d.join("identity.pem").exists() => None,
_ => crate::identity::load_optional_keypair(dir)?,
};
let keypair = load_keypair_from_dir(dir).ok();
let client = NodeClient::new(node, keypair.clone());
match name {
@@ -1121,31 +1114,16 @@ async fn call_tool(
// ── Task tools ────────────────────────────────────────────────────
"task_list" => {
// Follows the node's opaque `next_cursor` so a limit above the
// 200-row server page cap returns what was asked for instead of a
// silently truncated page, and reports an explicit incomplete
// result when a guard or the node's scan ceiling stops it (#327).
let limit = match args.get("limit") {
None | Some(Value::Null) => 50,
Some(val) => val.as_i64().context("invalid limit: expected an integer")?,
};
let cursor = match args.get("cursor") {
None | Some(Value::Null) => None,
Some(val) => Some(val.as_str().context("invalid cursor: expected a string")?),
};
let result = crate::task::fetch_tasks(
&client,
args.get("status").and_then(|v| v.as_str()),
args.get("assignee_did").and_then(|v| v.as_str()),
limit,
cursor,
)
.await?;
let mut out = result.to_json();
if let Some(warning) = result.truncation_warning() {
out["warning"] = Value::String(warning);
let limit = args["limit"].as_i64().unwrap_or(50);
let mut path = format!("/api/v1/tasks?limit={limit}");
if let Some(s) = args.get("status").and_then(|v| v.as_str()) {
path.push_str(&format!("&status={}", urlencoding::encode(s)));
}
Ok(serde_json::to_string_pretty(&out)?)
if let Some(a) = args.get("assignee_did").and_then(|v| v.as_str()) {
path.push_str(&format!("&assignee_did={}", urlencoding::encode(a)));
}
let resp = crate::http::read_json(client.get(&path).await?, "list tasks").await?;
Ok(serde_json::to_string_pretty(&resp)?)
}
"task_create" => {
@@ -1748,7 +1726,7 @@ mod tests {
)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"tasks":[{"id":"t1","kind":"test","status":"pending"}],"has_more":false,"incomplete":false,"next_cursor":null}"#)
.with_body(r#"{"tasks":[{"id":"t1","kind":"test","status":"pending"}]}"#)
.create_async()
.await;
@@ -1762,275 +1740,6 @@ mod tests {
.unwrap();
let parsed: Value = serde_json::from_str(&result).unwrap();
assert_eq!(parsed["tasks"][0]["id"], "t1");
assert_eq!(parsed["complete"], json!(true));
}
#[tokio::test]
async fn test_task_list_via_mcp_uses_loaded_identity() {
let mut server = mockito::Server::new_async().await;
let dir = tempfile::TempDir::new().unwrap();
let kp = gitlawb_core::identity::Keypair::generate();
std::fs::write(
dir.path().join("identity.pem"),
kp.to_pem().unwrap().as_bytes(),
)
.unwrap();
let _m = server
.mock(
"GET",
mockito::Matcher::Regex(r"/api/v1/tasks\?".to_string()),
)
.match_header("signature", mockito::Matcher::Any)
.match_header("signature-input", mockito::Matcher::Any)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(
r#"{"tasks":[{"id":"t1"}],"has_more":false,"incomplete":false,"next_cursor":null}"#,
)
.create_async()
.await;
let result = call_tool("task_list", json!({}), &server.url(), Some(dir.path()))
.await
.unwrap();
let parsed: Value = serde_json::from_str(&result).unwrap();
assert_eq!(parsed["tasks"][0]["id"], "t1");
assert_eq!(parsed["complete"], json!(true));
}
/// #327 review: MCP `task_list` issued one request and exposed no
/// continuation, so a limit above the node's 200-row page cap returned a
/// silently truncated result the model had no way to detect. It now
/// follows cursors and, when a guard stops it, says so in the payload.
#[tokio::test]
async fn test_task_list_via_mcp_follows_cursors() {
let mut server = mockito::Server::new_async().await;
let first = server
.mock(
"GET",
mockito::Matcher::Regex(r"^/api/v1/tasks\?limit=200$".to_string()),
)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(
r#"{"tasks":[{"id":"t1"}],"has_more":true,"incomplete":false,"next_cursor":"c1"}"#,
)
.create_async()
.await;
let second = server
.mock("GET", mockito::Matcher::Regex(r"cursor=c1".to_string()))
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"tasks":[{"id":"t2"}],"has_more":false,"next_cursor":null}"#)
.create_async()
.await;
let result = call_tool("task_list", json!({"limit": 500}), &server.url(), None)
.await
.unwrap();
first.assert_async().await;
second.assert_async().await;
let parsed: Value = serde_json::from_str(&result).unwrap();
assert_eq!(parsed["count"], 2);
assert_eq!(parsed["tasks"][1]["id"], "t2");
assert_eq!(parsed["complete"], json!(true));
assert!(parsed.get("warning").is_none());
}
/// A truncated MCP result must carry an explicit warning and a resume
/// cursor, so the model cannot read it as the whole answer.
#[tokio::test]
async fn test_task_list_via_mcp_flags_incomplete_results() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", mockito::Matcher::Any)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(
r#"{"tasks":[{"id":"t1"}],"has_more":true,"incomplete":true,"next_cursor":"resume-me"}"#,
)
.create_async()
.await;
let result = call_tool("task_list", json!({"limit": 1}), &server.url(), None)
.await
.unwrap();
let parsed: Value = serde_json::from_str(&result).unwrap();
assert_eq!(parsed["complete"], json!(false));
assert_eq!(parsed["incomplete"], json!(true));
assert_eq!(parsed["next_cursor"], "resume-me");
assert!(
parsed["warning"]
.as_str()
.unwrap()
.contains("result incomplete"),
"{parsed}"
);
}
/// A legacy response without pagination metadata is reported as incomplete via MCP.
#[tokio::test]
async fn test_task_list_via_mcp_legacy_response_is_incomplete() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", mockito::Matcher::Any)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"tasks":[{"id":"t1"}],"count":1}"#)
.create_async()
.await;
let result = call_tool("task_list", json!({"limit": 50}), &server.url(), None)
.await
.unwrap();
let parsed: Value = serde_json::from_str(&result).unwrap();
assert_eq!(parsed["complete"], json!(false));
assert_eq!(parsed["incomplete"], json!(true));
assert!(parsed["next_cursor"].is_null());
assert!(
parsed["warning"]
.as_str()
.unwrap()
.contains("node does not support pagination metadata"),
"{parsed}"
);
}
/// A legacy response with limit > 200 stops after 1 page and does not claim completeness via MCP.
#[tokio::test]
async fn test_task_list_via_mcp_legacy_limit_above_page_cap() {
let mut server = mockito::Server::new_async().await;
let ids: Vec<String> = (0..200).map(|i| format!("t{i}")).collect();
let tasks_json: Vec<Value> = ids.iter().map(|id| json!({ "id": id })).collect();
let body = json!({ "tasks": tasks_json, "count": 200 }).to_string();
let m = server
.mock(
"GET",
mockito::Matcher::Regex(r"^/api/v1/tasks\?limit=200$".to_string()),
)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(body)
.expect(1)
.create_async()
.await;
let result = call_tool("task_list", json!({"limit": 500}), &server.url(), None)
.await
.unwrap();
m.assert_async().await;
let parsed: Value = serde_json::from_str(&result).unwrap();
assert_eq!(parsed["count"], 200);
assert_eq!(parsed["complete"], json!(false));
assert_eq!(parsed["incomplete"], json!(true));
assert!(parsed["next_cursor"].is_null());
assert!(
parsed["warning"]
.as_str()
.unwrap()
.contains("node does not support pagination metadata"),
"{parsed}"
);
}
/// A cursor the model passes back must reach the node.
#[tokio::test]
async fn test_task_list_via_mcp_forwards_cursor_argument() {
let mut server = mockito::Server::new_async().await;
let m = server
.mock(
"GET",
mockito::Matcher::Regex(r"cursor=given-cursor".to_string()),
)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"tasks":[],"has_more":false,"incomplete":false,"next_cursor":null}"#)
.expect(1)
.create_async()
.await;
call_tool(
"task_list",
json!({"cursor": "given-cursor"}),
&server.url(),
None,
)
.await
.unwrap();
m.assert_async().await;
}
#[tokio::test]
async fn test_task_list_via_mcp_returns_http_errors() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock(
"GET",
mockito::Matcher::Regex(r"/api/v1/tasks\?".to_string()),
)
.with_status(500)
.with_header("content-type", "application/json")
.with_body(r#"{"error":"failed"}"#)
.create_async()
.await;
let err = call_tool("task_list", json!({}), &server.url(), None)
.await
.unwrap_err();
assert!(err.to_string().contains("500"));
}
/// Malformed server responses fail visibly in MCP.
#[tokio::test]
async fn test_task_list_via_mcp_malformed_response_errors() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock(
"GET",
mockito::Matcher::Regex(r"/api/v1/tasks\?".to_string()),
)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"tasks":"not-an-array"}"#)
.create_async()
.await;
let err = call_tool("task_list", json!({}), &server.url(), None)
.await
.unwrap_err();
assert!(
err.to_string().contains("malformed") || err.to_string().contains("invalid JSON"),
"{err}"
);
}
/// #327 review: a model that sends `limit: 0` used to get an empty list
/// marked complete, which reads as "this node has no tasks". The shared
/// helper rejects it, so the model sees an invalid argument instead.
#[tokio::test]
async fn test_task_list_via_mcp_rejects_non_positive_limit() {
let mut server = mockito::Server::new_async().await;
let m = server
.mock(
"GET",
mockito::Matcher::Regex(r"/api/v1/tasks\?".to_string()),
)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"tasks":[],"has_more":false,"incomplete":false,"next_cursor":null}"#)
.expect(0)
.create_async()
.await;
let err = call_tool("task_list", json!({"limit": 0}), &server.url(), None)
.await
.unwrap_err();
assert!(
err.to_string().contains("limit must be a positive"),
"{err}"
);
m.assert_async().await;
}
#[tokio::test]
@@ -2499,86 +2208,6 @@ mod tests {
assert!(err.to_string().contains("no identity found"), "got: {err}");
}
#[tokio::test]
async fn test_task_list_invalid_limit_rejected() {
let server = mockito::Server::new_async().await;
let dir = tempfile::TempDir::new().unwrap();
let kp = gitlawb_core::identity::Keypair::generate();
std::fs::write(
dir.path().join("identity.pem"),
kp.to_pem().unwrap().as_bytes(),
)
.unwrap();
let err = call_tool(
"task_list",
json!({"limit": "50"}),
&server.url(),
Some(dir.path()),
)
.await
.expect_err("string limit must be rejected");
assert!(
err.to_string()
.contains("invalid limit: expected an integer"),
"got: {err}"
);
let err_float = call_tool(
"task_list",
json!({"limit": 50.5}),
&server.url(),
Some(dir.path()),
)
.await
.expect_err("float limit must be rejected");
assert!(
err_float
.to_string()
.contains("invalid limit: expected an integer"),
"got: {err_float}"
);
}
#[tokio::test]
async fn test_task_list_invalid_cursor_rejected() {
let server = mockito::Server::new_async().await;
for invalid in [json!(123), json!({}), json!([]), json!(true)] {
let err = call_tool("task_list", json!({"cursor": invalid}), &server.url(), None)
.await
.expect_err("non-string cursor must be rejected");
assert!(
err.to_string()
.contains("invalid cursor: expected a string"),
"for {invalid}: got {err}"
);
}
}
#[tokio::test]
async fn test_task_list_null_or_absent_cursor_accepted() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock(
"GET",
mockito::Matcher::Regex(r"/api/v1/tasks\?".to_string()),
)
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"tasks":[],"has_more":false,"incomplete":false,"next_cursor":null}"#)
.expect(2)
.create_async()
.await;
call_tool("task_list", json!({}), &server.url(), None)
.await
.unwrap();
call_tool("task_list", json!({"cursor": null}), &server.url(), None)
.await
.unwrap();
}
#[test]
fn test_tool_count_is_42() {
let tools = tool_definitions();
+1 -5
View File
@@ -142,11 +142,7 @@ pub async fn run(args: MirrorArgs) -> Result<()> {
println!();
println!("✓ Mirror complete: {name}");
println!(" Clone: git clone {gitlawb_url}");
// Only print View: when the node advertises a web_url — self-hosted nodes
// without a web front-end would otherwise produce a 404 link (#370).
if let Some(web_url) = crate::repo::fetch_node_web_url(&args.node).await {
println!(" View: {web_url}/{owner_short}/{name}");
}
println!(" View: https://twigpine.com/{owner_short}/{name}");
Ok(())
}
+2 -405
View File
@@ -5,7 +5,7 @@ use clap::{Args, Subcommand};
use serde_json::{json, Value};
use std::path::PathBuf;
use crate::http::{sanitize_node_msg, NodeClient};
use crate::http::NodeClient;
use crate::identity::load_keypair_from_dir;
#[derive(Args)]
@@ -274,126 +274,6 @@ async fn resolve_owner_did(_node: &str, dir: Option<&std::path::Path>) -> Result
Ok(did.split(':').next_back().unwrap_or(&did).to_string())
}
/// Fetch `GET /` from the node and return the `web_url` if the node advertises one.
///
/// The View: link is a nice-to-have, so every failure mode degrades to `None`
/// rather than failing the enclosing command — but failures are surfaced as
/// stderr warnings (request error, non-success HTTP status with the status,
/// malformed advertised value), since they all mean the node itself is
/// misbehaving or misconfigured, which the user would otherwise never learn.
/// A successful response that lacks a usable `web_url` (field missing or
/// blank) omits the link silently: self-hosted nodes without a web front-end
/// are expected to not advertise one (#370).
///
/// The body is treated like every other caller-chosen node reply (INV-6): the
/// read is capped and an accepted `web_url` must be free of control/bidi bytes
/// — it reaches the terminal verbatim through the View: line.
pub(crate) async fn fetch_node_web_url(node: &str) -> Option<String> {
let info_client = NodeClient::new(node, None);
let info_resp = match info_client.get("/").await {
Ok(resp) => resp,
Err(err) => {
eprintln!("warning: node info request failed ({err}); skipping View link");
return None;
}
};
let status = info_resp.status();
if !status.is_success() {
eprintln!("warning: node info request returned {status}; skipping View link");
return None;
}
// An info reply is a DID, a few URLs and counts — 8 KiB is well past what
// the shape needs; anything longer is hostile or broken (peer.rs precedent).
let raw_body = crate::http::read_body_capped(info_resp, 8 * 1024).await;
match web_url_from_info_body(&raw_body) {
Ok(url) => url,
Err(reason) => {
eprintln!("warning: {reason}; skipping View link");
None
}
}
}
/// Classify a capped `GET /` body for the `View:` advertisement. Split out of
/// `fetch_node_web_url` so tests can pin the classification contract directly:
/// a truncated or half-read body is indistinguishable from a node that
/// advertised nothing, so it must warn rather than silently omit the link.
fn web_url_from_info_body(
raw_body: &crate::http::CappedBody,
) -> std::result::Result<Option<String>, String> {
if raw_body.truncated || raw_body.read_failed {
return Err("node info body was oversized or could not be read in full".to_string());
}
let info: Value = match serde_json::from_str(&raw_body.text) {
Ok(json) => json,
Err(_) => return Err("node info body was not valid JSON".to_string()),
};
// Missing field / non-string degrade to None without warning — same contract
// as before; only transport- and advertisement-level failures warn there.
let raw = match info["web_url"].as_str() {
Some(v) => v,
None => return Ok(None),
};
let trimmed = raw.trim().trim_end_matches('/');
if trimmed.is_empty() {
return Ok(None);
}
// A present-but-malformed value means the node is misconfigured; say so
// instead of quietly dropping the link. Mirrors the node-side boot
// validation: must be an absolute http(s) URL with no query or fragment —
// the link is built by string-appending `/{owner}/{repo}` to it. Control
// and bidi-format bytes are rejected outright (not stripped): they have no
// legitimate place in a base URL and would reach the terminal through the
// View: line — including when they arrive percent-encoded (INV-6).
let malformed_reason: Option<String> = if trimmed
.chars()
.any(|c| c.is_control() || gitlawb_core::sanitize::is_bidi_format(c))
{
Some("contains control or bidi characters".to_string())
} else {
match trimmed.parse::<url::Url>() {
Err(_) => Some("not an absolute URL".to_string()),
Ok(parsed) if !matches!(parsed.scheme(), "http" | "https") => {
Some("not http(s)".to_string())
}
Ok(parsed) if parsed.query().is_some() || parsed.fragment().is_some() => {
Some("contains a query or fragment".to_string())
}
// userinfo is rejected because the `View:` line prints the
// value verbatim; a remote advertisement carrying
// `user:pass@host` would land that credential in terminal
// scrollback and CI logs.
Ok(parsed) if !parsed.username().is_empty() || parsed.password().is_some() => {
Some("contains a username or password".to_string())
}
Ok(parsed)
if parsed
.host_str()
.unwrap_or("")
.chars()
.chain(
percent_encoding::percent_decode_str(parsed.path())
.decode_utf8_lossy()
.chars(),
)
.any(|c| c.is_control() || gitlawb_core::sanitize::is_bidi_format(c)) =>
{
Some("contains control or bidi characters, even percent-encoded".to_string())
}
Ok(_) => None,
}
};
match malformed_reason {
// The reason string is ours; the advertised value is not — defang it
// exactly as it would have been defanged had it been accepted.
Some(reason) => Err(format!(
"node advertised a malformed web_url ({:?}, {reason})",
sanitize_node_msg(trimmed)
)),
None => Ok(Some(trimmed.to_string())),
}
}
async fn cmd_create(
name: String,
description: Option<String>,
@@ -431,11 +311,7 @@ async fn cmd_create(
println!("✓ Created repository: {name}");
println!(" Clone: git clone {gitlawb_url}");
println!(" HTTP: {clone_url}");
// Only print View: when the node advertises a web_url — self-hosted nodes
// without a web front-end would otherwise produce a 404 link (#370).
if let Some(web_url) = fetch_node_web_url(&node).await {
println!(" View: {web_url}/{owner_short}/{name}");
}
println!(" View: https://twigpine.com/{owner_short}/{name}");
if let Some(desc) = payload["description"].as_str().filter(|s| !s.is_empty()) {
println!(" Desc: {desc}");
}
@@ -972,285 +848,6 @@ mod tests {
.unwrap();
}
#[tokio::test]
async fn test_fetch_node_web_url_with_web_url() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"web_url":"https://example.com","did":"did:key:z6Mk"}"#)
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(web_url.as_deref(), Some("https://example.com"));
}
#[tokio::test]
async fn test_fetch_node_web_url_trims_trailing_slash() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"web_url":"https://example.com/"}"#)
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(web_url.as_deref(), Some("https://example.com"));
}
#[tokio::test]
async fn test_fetch_node_web_url_without_web_url() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"did":"did:key:z6Mk"}"#)
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(web_url, None);
}
#[tokio::test]
async fn test_fetch_node_web_url_empty_string() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"web_url":""}"#)
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(web_url, None);
}
#[tokio::test]
async fn test_fetch_node_web_url_whitespace_only() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"web_url":" "}"#)
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(web_url, None);
}
#[tokio::test]
async fn test_fetch_node_web_url_server_error() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(500)
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(web_url, None);
}
/// A node advertising a present-but-malformed web_url (not an absolute
/// http(s) URL) must not yield a View link — and must warn, since a
/// malformed advertisement means the node is misconfigured (#370).
#[tokio::test]
async fn test_fetch_node_web_url_malformed_value_is_rejected() {
for bad in [
"gitlawb.com",
"not a url",
"ftp://files.example.com",
// Query/fragment corrupt the appended /{owner}/{repo} path.
"https://example.com?a=1",
"https://example.com/#faq",
] {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(format!(r#"{{"web_url":"{bad}"}}"#))
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(
web_url, None,
"malformed web_url {bad:?} must not render a View link"
);
}
}
/// A transport-level failure (connection refused) degrades to None but is
/// no longer silent: the request error is surfaced on stderr so an
/// unreachable node isn't indistinguishable from one without a web_url.
#[tokio::test]
async fn test_fetch_node_web_url_connection_refused_warns() {
// Port 1 on localhost is reserved (tcpmux) and refuses connections.
let web_url = fetch_node_web_url("http://127.0.0.1:1").await;
assert_eq!(web_url, None);
}
/// A hostile node can smuggle ANSI/bell/bidi controls inside a web_url that
/// still passes http(s) URL parsing — those bytes would reach the terminal
/// verbatim through the View: line. The advertised value must be rejected
/// outright: no control byte may survive into the returned string.
#[tokio::test]
async fn test_fetch_node_web_url_rejects_control_bytes() {
for bad in [
"https://example.com/\x1b[31mred", // ANSI CSI escape in path
"https://example.com\x07", // bell
"https://ex\u{202e}ample.com", // bidi override (RLO)
"https://example.com/\u{200f}", // RLM format char
"\x1b]0;title\x07https://evil.example", // OSC title-set prefix
] {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
// serde_json escapes the control bytes so the body is VALID
// JSON — a raw literal would make parse fail and the test
// would pass without reaching the control check at all.
.with_body(serde_json::json!({"web_url": bad}).to_string())
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(
web_url, None,
"web_url with control bytes {bad:?} must be rejected"
);
}
}
/// An oversized or half-read `GET /` body must warn, not silently
/// classify as "no web front-end": a cut body fails JSON parse, which is
/// indistinguishable from a node that advertised nothing without the
/// truncated flag. Removing the flag check makes this test pass
/// vacuously through the parse-fail arm.
#[test]
fn web_url_from_info_body_warns_on_truncated_reads() {
for (body, label) in [
(
crate::http::CappedBody {
text: r#"{"web_url":"https://x"#.repeat(2000),
truncated: true,
read_failed: false,
},
"truncated",
),
(
crate::http::CappedBody {
text: r#"{"web_url":"https://x"#.to_string(),
truncated: false,
read_failed: true,
},
"read_failed",
),
] {
let err = web_url_from_info_body(&body)
.expect_err(&format!("{label} body must warn, not silently classify"));
assert!(
err.contains("oversized or could not be read"),
"{label}: got {err}"
);
}
// A clean body missing web_url still classifies as a silent None.
let clean = crate::http::CappedBody {
text: r#"{"did":"did:key:x"}"#.to_string(),
truncated: false,
read_failed: false,
};
assert_eq!(web_url_from_info_body(&clean).unwrap(), None);
}
/// Percent-encoded bidi must not slip past the literal scan: a node
/// advertising `https://example.com/%E2%80%AE` serves wire bytes with no
/// Cf code point, but a browser decodes it into a reordering character
/// on the rendered View: link (INV-6).
#[test]
fn web_url_from_info_body_rejects_percent_encoded_bidi() {
let body = crate::http::CappedBody {
text: r#"{"web_url":"https://example.com/%E2%80%AE"}"#.to_string(),
truncated: false,
read_failed: false,
};
let err = web_url_from_info_body(&body).expect_err("percent-encoded bidi must be rejected");
assert!(err.contains("bidi"), "got {err}");
}
#[tokio::test]
async fn test_fetch_node_web_url_validates_after_trimming() {
// Trailing slash is trimmed before validation; result stays usable.
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"web_url":"http://localhost:8080/ui/"}"#)
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(web_url.as_deref(), Some("http://localhost:8080/ui"));
}
/// A remote node that advertises `web_url` with userinfo (`user@`,
/// `user:pass@`) must be dropped: the value is reprinted on the `View:`
/// line, and a credential embedded in the URL would land in terminal
/// scrollback and CI logs. Mirrors the node-side boot validation.
#[tokio::test]
async fn test_fetch_node_web_url_rejects_userinfo() {
for bad in [
"https://[email protected]",
"https://user:[email protected]",
] {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(format!(r#"{{"web_url":"{}"}}"#, bad))
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(
web_url, None,
"web_url with userinfo {bad:?} must be rejected"
);
}
}
/// A non-success status must still return None (View link is cosmetic) but
/// the status surfaces as a user-visible stderr warning instead of being
/// swallowed silently (#370 review).
#[tokio::test]
async fn test_fetch_node_web_url_non_success_warns_with_status() {
let mut server = mockito::Server::new_async().await;
let _m = server
.mock("GET", "/")
.with_status(503)
.create_async()
.await;
let web_url = fetch_node_web_url(&server.url()).await;
assert_eq!(web_url, None);
// The warning itself goes to stderr; mockito can't capture it here, so
// this asserts only the None contract. Manual check: run against a
// 503-ing node and observe "node info request returned 503" on stderr.
}
#[tokio::test]
async fn test_cmd_create_server_error() {
let dir = TempDir::new().unwrap();
+34 -1364
View File
File diff suppressed because it is too large. Load diff
-138
View File
@@ -1,138 +0,0 @@
//! End-to-end test for the `View:` line printed by `gl repo create` (#370).
//!
//! `cmd_create` is the load-bearing #370 path: it is the one place the CLI
//! prints a `View:` line, and the only thing keeping it from being a
//! hardcoded `https://gitlawb.com/...` 404 is `fetch_node_web_url`. The
//! helper-level tests in `repo.rs` cover the helper, not the command path —
//! if `cmd_create` went back to a hardcoded constant, every helper test
//! would still pass.
//!
//! This integration test drives the real `gl` binary against a mockito
//! server that answers both `POST /api/v1/repos` and `GET /`. Asserting on
//! the subprocess's stdout is straightforward: bytes are bytes, with no
//! libtest / gag / in-process capture race.
//!
//! CARGO_BIN_EXE_gl is set by Cargo for integration tests of a `[[bin]]`
//! in the same package, so the test always picks up the in-tree build.
use std::process::Stdio;
use tempfile::TempDir;
use tokio::io::AsyncWriteExt;
use tokio::process::Command;
fn gl_bin() -> String {
std::env::var("CARGO_BIN_EXE_gl").expect("CARGO_BIN_EXE_gl is unset for this test")
}
async fn write_identity(dir: &TempDir) {
let kp = gitlawb_core::identity::Keypair::generate();
let pem = kp.to_pem().unwrap();
let path = dir.path().join("identity.pem");
let mut f = tokio::fs::File::create(&path).await.unwrap();
f.write_all(pem.as_bytes()).await.unwrap();
}
/// The trailing key segment of the freshly-generated identity. Mirrors the
/// in-source `resolve_owner_did` test helper but lives in this integration
/// file because that helper is `pub(crate)`.
async fn owner_short(dir: &TempDir) -> String {
let pem = tokio::fs::read_to_string(dir.path().join("identity.pem"))
.await
.unwrap();
let kp = gitlawb_core::identity::Keypair::from_pem(&pem).unwrap();
let did = kp.did().to_string();
did.split(':').next_back().unwrap_or(&did).to_string()
}
async fn run_gl_create(node_url: &str, dir: &TempDir) -> String {
let output = Command::new(gl_bin())
.arg("repo")
.arg("create")
.arg("myrepo")
.arg("--private")
.arg("--branch")
.arg("main")
.arg("--node")
.arg(node_url)
.arg("--dir")
.arg(dir.path())
.env("NO_COLOR", "1")
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.output()
.await
.expect("failed to spawn `gl repo create`");
assert!(
output.status.success(),
"`gl repo create` failed: stderr:\n{}\nstdout:\n{}",
String::from_utf8_lossy(&output.stderr),
String::from_utf8_lossy(&output.stdout),
);
String::from_utf8(output.stdout).expect("`gl` wrote non-UTF-8 to stdout")
}
#[tokio::test]
async fn cmd_create_view_url_tracks_node_advertisement() {
let dir = TempDir::new().unwrap();
write_identity(&dir).await;
let owner = owner_short(&dir).await;
// Case 1: node advertises a web_url. The `View:` line must use the
// advertised origin and the resolved owner/short name.
{
let mut server = mockito::Server::new_async().await;
let _m_create = server
.mock("POST", "/api/v1/repos")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"name":"myrepo","clone_url":"gitlawb://did:key:z6Mk/myrepo"}"#)
.create_async()
.await;
let _m_info = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"web_url":"https://git.example","did":"did:key:z6Mk"}"#)
.create_async()
.await;
let server_url = server.url();
let out = run_gl_create(&server_url, &dir).await;
let view_line = out
.lines()
.find(|l| l.trim_start().starts_with("View:"))
.unwrap_or_else(|| panic!("View: line missing from stdout:\n{out}"));
assert!(
view_line.contains(&format!("https://git.example/{owner}/myrepo")),
"View: line must use the advertised origin, got: {view_line:?}"
);
}
// Case 2: node does NOT advertise a web_url. The `View:` line must be
// absent, so a self-hosted node without a web front-end never produces
// a 404 link (#370).
{
let mut server = mockito::Server::new_async().await;
let _m_create = server
.mock("POST", "/api/v1/repos")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"name":"myrepo","clone_url":"gitlawb://did:key:z6Mk/myrepo"}"#)
.create_async()
.await;
let _m_info = server
.mock("GET", "/")
.with_status(200)
.with_header("content-type", "application/json")
.with_body(r#"{"did":"did:key:z6Mk"}"#)
.create_async()
.await;
let server_url = server.url();
let out = run_gl_create(&server_url, &dir).await;
assert!(
!out.lines().any(|l| l.trim_start().starts_with("View:")),
"View: line must be absent when node omits web_url; got:\n{out}"
);
}
}