The hosts atgc reads from are not atgc's. A did:web in a pull URI names a
hostname the author chose; a did:plc document's serviceEndpoint is equally
theirs. So atgc pr view at://did:web:evil.example/… fetches that host's
.well-known/did.json and then that host's getRecord response, and for a
long time read whatever they sent.
Bounds are enforced against bytes actually received, never against a header the
sender controls or a size the record's author wrote: a hostile pull could
declare size: 100 and stream gigabytes. Measured against the flood, peak RSS
went from 143 MB to 33 MB.
One hole is left and is stated rather than papered over: a body read through
jacquard's transport is buffered whole before atgc sees it.
One constructor was not one client: http::client() built a fresh
reqwest::Client per call at twenty call sites, so nothing was ever
reused and each request paid its own connect, TLS handshake and
root-store parse — pr list --limit 5 opened 45 connections to one
appview host. client() now hands out clones of a OnceLock instance
and no_redirect() is the second one, for the three callers that read
a Location header instead of following it. Bounds are read when the
first client is built, which main's init call already precedes.
Measured, same 99 requests, debug build: 23-94s before, 12-16s after
The three bounds here are each a bound on a part of an exchange, and
nothing in the tree turned them into a bound on a whole one: 5s connect
plus a per-read 30s stall allowance plus up to 30s of MAX_RETRY_AFTER
backoff is not a total, and no number here says how long one request may
take end to end. That is the right shape for this module — a total
timeout is a hidden ceiling on payload size, which is why READ_TIMEOUT
is per-read — but it left the one caller that needs a total without one.
A token refresh holds the process-global config lock across the whole
exchange (plan/credential-store.md), so its length is what every other
atgc on the machine waits out. config::lock::HOLD_BUDGET supplies that
total, at the caller and not here: a tokio::time::timeout around the
refresh, sized under the waiters' own deadline. Recorded in both files
because the number is only correct as long as both halves are read
together
One HTTP client constructor (src/clients/http.rs) with a 5s connect timeout and
a 30s read timeout, behind every request atgc makes. reqwest's
defaults have no deadline at any layer, so a host that drops packets
rather than refusing them was bounded only by the local kernel's SYN
retry schedule. Read timeout rather than a total one on purpose: the
same client carries pr create's patch blob, and capping a transfer by
total duration is a hidden ceiling on payload size
Upload retries were classified by substring-matching the error's prose.
images.rs's transient read any message containing 404 as a
permanent client error and everything else as worth retrying, so a byte
count, a port or a CID fragment carrying those digits abandoned an
upload, while a real 403 whose message did not spell them was tried
three times — exit.rs's opening complaint, applied to a retry
decision. It now reads AgentError::client_error()'s typed status,
which covers all three shapes process_response produces: a status
other than 400 or 401 becomes ClientErrorKind::Http, a transport
failure becomes Transport with no status and is the one statusless
case worth retrying, and 400 and 401 are let through as the endpoint's
typed error with no ClientError attached, both permanent
Retry-After was read nowhere in the crate, so a 429 was retried on the
fixed doubling schedule whatever the server asked for. The retry policy
now lives in clients/http.rs for the whole crate, and reads both
spellings the spec allows — delta-seconds and an HTTP-date — clamped to
MAX_RETRY_AFTER, thirty seconds, so a server asking for an hour cannot
hang a CLI. The blob upload is the one caller that still cannot see the
header: it goes through jacquard's upload_blob, and neither
ClientError nor AgentError keeps the response headers, so the value
is gone before atgc sees the failure. It runs the same budget, schedule
and status table as everything else, minus the header
pds::blob_bounded's Content-Length check was the whole bound, despite
its own comment calling it that — chunked transfer-encoding and HTTP/2
can both omit that header, and a pull's declared size is only ever
the author's own claim, written to their own PDS with whatever tooling
they like, never checked against what the CID actually serves. A
hostile pull could declare size: 100 and stream gigabytes past pr diff, pr view and stack merge alike, since fetch_patch and
latest_round_patch both trust the record before ever calling this.
Chunks are now counted as they arrive too, and reading stops the
instant the running total passes max_bytes — a bound enforced
against bytes actually received rather than a header the sender
controls
Every JSON response atgc reads is unbounded. resp.text() or
resp.json() with no size cap, at ten sites across atproto/pds.rs,
atproto/did.rs, tangled/knot.rs, web/pulls.rs and the repo
family. clients/http.rs bounds time only — its own doc says
READ_TIMEOUT caps the gap between bytes of a response — so a server
that streams steadily never trips it, and pds.rs's claim of "the
bounded request through crate::clients::http" is true of latency and
false of size. The hosts are not atgc's: a did:web in a pull URI
names a hostname the author chose, and a did:plc document's
serviceEndpoint is equally theirs, so atgc pr view at://did:web:evil.example/… fetches .well-known/did.json from them
and reads whatever they send, then reads that host's getRecord
response the same way. Same reach through pr diff, pr list and
repo clone <owner>/<name> for any repo whose owner or pull author is
hostile. The blob path was hardened for exactly this and has
blob_bounded and a RunningTotal, with a doc noting a
Content-Length is only as trustworthy as its sender; the JSON path
never got it. Now it does: http::MAX_BODY is 8 MiB and fixed,
text_bounded/bytes_bounded share the one streaming loop that
blob_bounded now calls rather than owning, and eleven call sites go
through it. A refusal names the host and the limit, hands back none of
what it read, and exits 6 — the status this module's other two
bounds already carry. Measured against the flood: peak RSS 143 MB
before, 33 MB after. bobbin.rs:79 and web/backfill.rs:147 are the
two reads still unbounded; another branch owned both files at the
time, and converting them is a two-line change each
