feat(protocol): admit wire claims that order a receiver's allocation

natsukium commented

2026-09-04 13:38:32 +09:00

Owner

Fixes #49.

A scalar that tells a receiver how much memory to reserve was not bounded
by anything on the wire: a twenty-byte GridMsg::Size announcing
65535 × 65535, or an ImageMsg::Header claiming u64::MAX decoded bytes,
is a legal small frame that orders an allocation nothing on the wire pays
for. The frame-body cap bounds a length, never a claim.

This branch admits those claims where they are decoded, before anything
downstream allocates against them:

felis-protocol gains MAX_IMAGE_BYTES (64 MiB), MAX_SESSION_IMAGE_BYTES
(256 MiB) and MAX_IMAGE_FRAMES (4096), and admits announced geometry and
image headers in the TryFrom<v1::*> conversion; an image geometry is
priced without overflowing it, and a wire limit is reported in the unit it
counts.
felis-client-core's image mirror refuses a claim before it allocates,
bills a frame claim for the slots it fills, and grows the shadow only to a
geometry a peer may announce.
felis-grid caps one image at 4096 animation frames and prices a
replacement before the old image is dropped.
felis-daemon announces the image store's own evictions, and trims an
oversize RegionMsg::Reply to the youngest bytes a reader can resume at
rather than handing the writer a body it must refuse — a whole-scrollback
pipe is an in-spec request, so refusing the write would cost the subscriber
its connection.
SpawnArgs.env_base is left to REQ-912a's spawn refusal instead of a
decode-side check, which cost the whole connection for one over-cap
environment snapshot.
felis-cli's bridge refuses an over-limit sessions.send before it
resolves a session, and its line accounting saturates rather than wrapping.

Verification beyond the unit boundaries: felis-client-core's
tests/hostile_claims.rs decodes hand-encoded protobuf frames under 64 bytes
carrying maximal scalars under a dhat global allocator and asserts the run's
peak heap stays under 1 MiB — an allocation proportional to a claim is what no
Result inspection can catch.

Doc cascade: docs/reference/ipc.md (the "Semantic limits" table gains a
Checked column and the claim rows, plus the sender-only region row),
docs/reference/spec.md (REQ-105 / REQ-105a), docs/reference/testing.md
(a "Receiver-side claims" layer), docs/reference/protocols/kitty-graphics.md,
docs/explanation/architecture/ipc.md ("A claim is not a length"),
docs/explanation/security-model.md ("Client-side admission"),
docs/explanation/protocols/kitty-graphics.md, and CHANGELOG.md.

Rebased onto main after #16 merged; the parent's patches were dropped and
three commits superseded by #16's own review (the paste-disposition and
search-refusal refactors, and the pre-dial plan test) were skipped. This
branch replaces #67, which went unmergeable when #68 landed a conflicting
edit to docs/explanation/security-model.md; the two entries now sit side
by side.

It also carries one unrelated CI fix, without which no PR can go green:
.forgejo/actions/cargo-env parks CARGO_TARGET_DIR outside the workspace,
but the bench orchestrator still read the literal target/criterion, so the
regression gate failed every run with "nothing to check".

Verified: just check green (fmt, clippy, nextest — 3110 passed, deny),
pi review not run this session (the branch's own review ran before the rebase).

Fixes #49. A scalar that tells a receiver how much memory to reserve was not bounded by anything on the wire: a twenty-byte `GridMsg::Size` announcing 65535 × 65535, or an `ImageMsg::Header` claiming `u64::MAX` decoded bytes, is a legal small frame that orders an allocation nothing on the wire pays for. The frame-body cap bounds a *length*, never a *claim*. This branch admits those claims where they are decoded, before anything downstream allocates against them: - `felis-protocol` gains `MAX_IMAGE_BYTES` (64 MiB), `MAX_SESSION_IMAGE_BYTES` (256 MiB) and `MAX_IMAGE_FRAMES` (4096), and admits announced geometry and image headers in the `TryFrom<v1::*>` conversion; an image geometry is priced without overflowing it, and a wire limit is reported in the unit it counts. - `felis-client-core`'s image mirror refuses a claim before it allocates, bills a frame claim for the slots it fills, and grows the shadow only to a geometry a peer may announce. - `felis-grid` caps one image at 4096 animation frames and prices a replacement before the old image is dropped. - `felis-daemon` announces the image store's own evictions, and trims an oversize `RegionMsg::Reply` to the youngest bytes a reader can resume at rather than handing the writer a body it must refuse — a whole-scrollback pipe is an in-spec request, so refusing the write would cost the subscriber its connection. - `SpawnArgs.env_base` is left to REQ-912a's spawn refusal instead of a decode-side check, which cost the whole connection for one over-cap environment snapshot. - `felis-cli`'s bridge refuses an over-limit `sessions.send` before it resolves a session, and its line accounting saturates rather than wrapping. Verification beyond the unit boundaries: `felis-client-core`'s `tests/hostile_claims.rs` decodes hand-encoded protobuf frames under 64 bytes carrying maximal scalars under a `dhat` global allocator and asserts the run's peak heap stays under 1 MiB — an allocation proportional to a claim is what no `Result` inspection can catch. Doc cascade: `docs/reference/ipc.md` (the "Semantic limits" table gains a `Checked` column and the claim rows, plus the sender-only region row), `docs/reference/spec.md` (REQ-105 / REQ-105a), `docs/reference/testing.md` (a "Receiver-side claims" layer), `docs/reference/protocols/kitty-graphics.md`, `docs/explanation/architecture/ipc.md` ("A claim is not a length"), `docs/explanation/security-model.md` ("Client-side admission"), `docs/explanation/protocols/kitty-graphics.md`, and `CHANGELOG.md`. Rebased onto `main` after #16 merged; the parent's patches were dropped and three commits superseded by #16's own review (the paste-disposition and search-refusal refactors, and the pre-dial `plan` test) were skipped. This branch replaces #67, which went unmergeable when #68 landed a conflicting edit to `docs/explanation/security-model.md`; the two entries now sit side by side. It also carries one unrelated CI fix, without which no PR can go green: `.forgejo/actions/cargo-env` parks `CARGO_TARGET_DIR` outside the workspace, but the bench orchestrator still read the literal `target/criterion`, so the regression gate failed every run with "nothing to check". Verified: just check green (fmt, clippy, nextest — 3110 passed, deny), pi review not run this session (the branch's own review ran before the rebase).

natsukium added 23 commits

2026-09-04 13:38:32 +09:00

feat(protocol): admit wire claims that order a receiver's allocation c6523a0531

The per-operation limits bound what a frame carries. They do not bound
what a small frame can talk a receiver into reserving: a 20-byte
GridMsg::Size announcing 65535 x 65535 narrowed cleanly to u16 and
sized a 4.3-billion-cell shadow grid, and an ImageMsg::Header's
total_bytes and frame index reached vec![0; n] and a gap-filling push
loop as the u64 and u32 the wire sent. No frame-body ceiling reaches
any of them, because the bytes they describe never have to arrive.

So the decode admits the claim itself. Announced geometry re-runs the
REQ-605a bounds at wire width (with no rows/cols zero sentinel, since
only resolved geometry is ever announced), and an image header is
checked against MAX_IMAGE_BYTES, MAX_IMAGE_FRAMES, and the byte count
its own width x height implies — felis ships coalesced frames, so the
geometry is an independent witness for total_bytes and a disagreement
is a lie about the allocation.

The constants live in felis-protocol rather than in daemon-private
code because a client mirroring the store sizes its buffers from these
headers and has to hold to the same numbers. MAX_IMAGE_FRAMES is new:
the byte caps do not imply a frame count, since a 4-byte frame lets
64 MiB admit sixteen million of them.

Refs #49

Assisted-by: Claude Code

feat(grid): cap one image at 4096 animation frames 1e4aac07b7

The image store's byte caps bound pixels, not frame slots: a 1x1 RGBA
frame costs four pixel bytes, so 64 MiB of per-image budget admitted
something like sixteen million frames, each paying for a slot in the
frames vector and, on any client mirroring the store, a slot the
mirror fills up to whatever index a header names.

push_frame refuses past the cap instead of evicting, which is the
opposite of what the byte caps do. Their unit is an image, and losing
a whole picture is something a producer can notice and re-send;
dropping the oldest frames of a running animation would silently
rewrite what plays with nothing in the protocol to say so. The refusal
answers ENOTSUP because Kitty's status set is closed and has no
ENOSPC, and the remedy is the byte cap's anyway.

The daemon's two image caps become aliases of the protocol constants
so the store and a client's mirror cannot drift to two answers for how
large an image may be.

Refs #49

Assisted-by: Claude Code

feat(client-core): refuse an image claim before the mirror allocates 42580771a2

ImageShadow sized every buffer from the header's own numbers, with a
comment arguing that re-capping added nothing because a buggy daemon
would ship the bytes regardless. That is the wrong failure: the bytes
never have to arrive, the claim is what allocates, and the chunk
writer's bounds check runs after the buffer it bounds already exists.
The mirror also had no aggregate at all, so five hundred individually
legal 64 MiB headers were 32 GiB.

apply now returns a typed refusal and keeps a running retained total,
validating before it mutates so a refused header leaves no half-built
entry for a later chunk to fill. The aggregate is the mirror's own
bound rather than an echo of the daemon's: the daemon holds the same
total by evicting, and the mirror cannot evict without inventing a
retention policy the daemon did not run.

The window ends the connection on a refusal, the same answer a frame
that failed to decode already gets. Principle 3 has the client trust
the daemon for screen content; that is not a licence for a peer — a
remote one over the SSH relay included — to order memory.

Refs #49

Assisted-by: Claude Code

test(client-core): pin that a refused claim allocates nothing first b9f38f3303

The boundary tests beside each check assert that a claim is refused.
They cannot assert the property the checks exist for — that the
refusal happened before the allocation — because a Result says
nothing about what the heap did on the way to it, and an ordering bug
would leave every one of them green.

So the hand-built frames run under a dhat global allocator and the
run's peak heap is the assertion. Every frame is under 64 bytes and
every claim in them is orders of magnitude past 1 MiB, so the ceiling
needs no tuning: honoring any one claim before judging it moves the
peak by gigabytes. The frames are encoded as protobuf rather than
built from domain values, because a hostile frame is defined by what
it says on the wire and a domain value has already been admitted.

Refs #49

Assisted-by: Claude Code

docs: separate a claimed allocation from a carried length 0138a33a56

The IPC docs named one kind of bound — bytes a frame carries — and the
anti-corruption layer promised that schema-inexpressible invariants
are validated once on ingress. Neither covered the scalars that tell a
receiver how much memory to reserve for bytes that have not arrived,
which is why the client's ingress did not validate announced geometry
or an image header at all.

The reference now states the rule and the numbers: which limits a
receiver checks, what a claim is as against a length, and the three
image caps as protocol-visible constants. The explanation twin carries
the argument and the rejected alternative it replaces (trust the
daemon, on the grounds that it would ship the bytes anyway), the
security model gains the client-side admission bullet, and the
frame-count cap is recorded as a decision with its revisit trigger.

Refs #49

Assisted-by: Claude Code

fix(protocol): price an image geometry without overflowing it 9d7c83cea8

The header's geometry check multiplied `width × height × bpp` at u64
width, and nothing bounds the two axes on their own: the `total_bytes`
cap is checked first but says nothing about width or height. A
hand-built header with both axes at u32::MAX and a one-byte
`total_bytes` therefore overflowed the product, which panics under the
test profile's overflow checks and wraps silently in release, where a
wrapped product can even match a bogus declaration and admit it.

Saturating the multiplication keeps the verdict intact for every input:
a saturated product is far above `MAX_IMAGE_BYTES`, so it can only read
as the mismatch it is. A frame of a few dozen bytes carrying maximal
scalars is exactly what this admission exists to survive, so it answers
with a typed error like every other refusal here.

Refs #49

Assisted-by: Claude Code

fix(daemon): announce the image store's own evictions e38fc84244

`insert`, `push_frame` and `replace_frame` freed other refcount-0
entries to make room and told nobody: only the dispatcher's explicit
`evict_oldest_to_fit` emitted `Delete`. That was invisible until the
client gained a byte gauge of its own, and now it is a teardown: a
session near the 256 MiB cap that adds an animation frame leaves the
daemon at the same total while the client's mirror keeps counting the
evicted image, so the mirror refuses the next in-spec header and the
window drops the connection.

The three mutators now return what they dropped and the dispatcher
turns each id into the same `Delete` a deliberate removal emits.
Eviction is also priced before anything is dropped, so a mutation that
still ends `OverCapacity` leaves the store untouched rather than
shrinking it with no way to report what went.

Refs #49

Assisted-by: Claude Code

fix(daemon): trim an oversize region reply instead of failing the write 17ebb2d5d3

`RegionMsg::Reply` is one stitched serialization with no cap and no
splitting, so a `pipe source = scrollback --ansi` over a full 10 000-row
buffer of dense colored output can build a body past the 64 MiB framing
ceiling. Since the writer now refuses such a body before it reaches the
wire, that in-spec request would cost the subscriber its connection,
which is the outcome the outbound cap exists to prevent.

`MAX_REGION_REPLY_BYTES` (32 MiB) bounds the reply and the daemon trims
to its youngest whole lines, the end `Rows`'s `max_rows` already keeps
and the end a pager or a filter is opened for. The viewport position
goes with the dropped head, which no longer indexes the bytes that
remain. The bound is sender-side only: a receiver enforcing it would
drop the connection to an older daemon that never trimmed, over bytes
it can hold.

Refs #49

Assisted-by: Claude Code

docs: record what the receiver-side admission costs an older peer fb276a0e49

Three gaps the limits cascade left. The minor ledger had no row for the
decode tightening because it is not an addition: no field, variant or
value changed meaning, only the set of values a decoder accepts, which
is the one change the ledger's old-peer column cannot answer. That is
free only because major 1 has not shipped, so the reference page says
so and carries a revisit trigger against the first release, and the
explanation twin says why a narrowed decoder after the freeze is a
break in the sense a user feels.

The region reply's sender-side bound and the store's now-announced
evictions are stated where each belongs: the semantic-limits table, the
image family's `Delete`, the image-store's memory accounting, and the
changelog, since a trimmed pipe is user-visible.

The new prose also carried em dashes at forty times the rate of the
pages it was added to; they are commas, colons and parentheses now.

Refs #49

Assisted-by: Claude Code

fix(protocol): report a wire limit in the unit it counts ab6c23e984

`WireError::OverLimit` hardcoded "bytes" in its message, but the same
variant carries entry counts: a 4097-entry argv printed as "carried
4097 bytes, over the 4096 limit" on the `invalid_request` machine
surface, sending a user hunting for a 4 KiB payload that was never
sent. The unit now rides the error, so a byte cap, an entry cap and
the image frame-count cap each read in their own terms.

`check_count` names the entry-cap sites rather than leaving the unit
to a parameter every caller could get wrong by default; `check_claim`
takes it explicitly because it serves both `total_bytes` and the
frame count.

Refs #49

Assisted-by: Claude Code

fix(protocol): leave an over-cap env_base to the spawn refusal 28b0bf5ab9

Checking `SpawnArgs.env_base` inside `Validate` put it in the decode
path, where a breach becomes `CodecError::Wire` and the daemon drops
the connection — taking every other session on it down over one
oversized environment snapshot. REQ-912a and the field's own doc
comment promise the opposite: `child_env::sanitize_base` answers a
breach with `EnvError::OverCap`, which fails that one create and
leaves the connection up. Those arms were unreachable from the wire
while this check stood in front of them.

Nothing is lost by dropping it: the entries are already decoded by
the time `Validate` runs, so the check bounded no allocation, and the
frame-body cap still bounds the decode itself. The explicit `env`
field keeps its check — the daemon has no cap of its own for it.

Refs #49

Assisted-by: Claude Code

test(cli): read the bridge line buffer while the line arrives 26f5e2a8b6

The line-cap test asserted `lines.buf.is_empty()` after `next()`
returned, which `take_line` guarantees on every path — the assertion
passed with the refusal reverted to buffering the whole over-limit
line, leaving the one property `BridgeLines` exists for (a 128 MiB
line is never held) unpinned.

The reader is now a duplex the test feeds by hand: the over-limit
bytes go in without a newline, `next()` is cancelled while it parks
on the rest, and the buffer is read mid-line, which is the only
moment at which holding the line is observable.

Refs #49

Assisted-by: Claude Code

fix(grid): price a replacement before the old image is dropped 780b93103e

`insert` removed the entry under the same id and only then asked
`evict_to_fit` whether the replacement fit. On a refusal the caller got
`Err` and a store that had silently lost that image: no evicted id to
announce, no `ImageEvent::Delete`, and live placements still pointing at
it. That is the same silent-removal the round-2 pre-pricing set out to
close, so the pricing now happens while the old entry is still in place
and counts its bytes as room the replacement may take.

Not reachable through the daemon dispatcher today, which always makes
room first, but `felis-grid` is an extraction-boundary crate and its own
comment asserted the opposite.

Refs #49

Assisted-by: Claude Code

fix(daemon): cut a trimmed region reply where a reader can resume 1f8af0a001

The trim looked for a newline only inside the youngest `cap` bytes and,
finding none, cut at the byte count. A region whose tail carries no
newline — a binary blob, or one very long line under `--ansi` — then
reached the subscriber starting mid-scalar or mid-escape, so the pager
or filter on the other end printed a replacement char or a literal
`[1;2m`.

The boundary is now chosen by walking the buffer from its head: nothing
local to the byte count can say whether an `ESC` opened earlier is still
unterminated, so the walk is what makes the fallback safe. A line
boundary is still preferred where the window has one.

Refs #49

Assisted-by: Claude Code

test(client-core): drop a mirror assertion the loop never exercises 9d0fcb288b

The hostile image headers are only fed to `decode`, which refuses them,
so `ImageShadow::apply` is never called and the mirror is empty whatever
the admission checks do — the assertion held with every one of them
reverted. It read as coverage for "a refused claim mutates nothing"
while proving nothing; that criterion is covered by the `image_shadow`
unit tests, which apply a refused claim to a real mirror.

Refs #49

Assisted-by: Claude Code

docs: rewrap the narrowing paragraph to the tree's wrap width 652bb99a14

A merge of two sentences during an edit left one 141-column line in an
otherwise 70-column paragraph, which doc-prose §0 rules out and which
makes the diff of any later edit to that paragraph unreadable.

Refs #49

Assisted-by: Claude Code

fix(client-core): bill a frame claim for the slots it fills 9e89e70cc2

The mirror's session aggregate counted pixel bytes only, so the one
allocation a frame header really orders went unpriced: a header naming
frame 4095 of a 1x1 image reserves 4096 frame records to reach the
slot, and is charged four bytes for roughly 128 KiB. The frame cap
bounds that fill per image but nothing bounded it across images, and
the aggregate admitted the trade until the process ran out of memory —
the tiny-image flood the daemon's store already refuses, one level
down.

So the mirror charges what the store charges: the entry, every frame
record, and the pixels, the unit `ImageEntry::byte_len` uses. Charging
in the store's unit is also what keeps the direction safe — the mirror
stays no looser than the daemon, so a client can only ever refuse a
session the daemon would itself have evicted.

Refs #49

Assisted-by: Claude Code

fix(client-core): grow the shadow only to a geometry a peer may announce 21fafd10d7

`write_row`'s auto-grow exists for the cold-start race, where the
daemon's damage burst lands on a mirror still at the attach dims. Both
of its inputs are peer wire fields, though, and only the row index was
bounded — by 4096, which is twice what announced geometry now admits.
A single `RowDelta` naming row 2047 with a 65535-cell body fits in one
64 KiB frame and resized the mirror to a 134-million-cell grid: the
claim-shaped allocation the announced-geometry admission closes, taken
through the side door of a row that carries its own width.

Both halves are now bounded by `MAX_GRID_ROWS`/`MAX_GRID_COLS`, the
same REQ-605a geometry a receiver admits announced, so a delta the
daemon could not have produced at a size it is allowed to announce
cannot resize the shadow either. The width is truncated rather than
refused because the reconciliation for a mismatched width is already
there and a resize race is not corruption.

Refs #49

Assisted-by: Claude Code

fix(cli): refuse an over-limit bridge send before it resolves a session 0edb8ab953

`sessions.send` was the one bridge operation that validated its payload
inside `with_session`, so an over-limit text dialed the daemon, attached
deliberately and drained the rehydrate burst for a message that was
never going to be sent — and a bad session prefix answered `no_match`
before the payload's `invalid_request`, which is the wrong diagnosis
for a request that is over the limit whatever session it names.
`spawn` and `search` already judge their payloads first; this matches
them and the "refuses before it dials" rule in the IPC reference.

The bridge's line accounting saturates for the same class of reason:
the counter deliberately keeps running past the cap so the whole
over-limit line is measured, and on a 32-bit target a newline-free
stream past 4 GiB would otherwise wrap it back under the cap.

Refs #49

Assisted-by: Claude Code

docs: say where a trimmed region is actually cut ea3cd340ce

The region trim cuts at the first line boundary in the retained window
and, for a window holding no line boundary at all, at the first byte
that starts a scalar and no escape sequence — a full `--ansi` capture
of binary output has no newline in 32 MiB, and a cut by byte count
alone hands a pager half a character or a literal `[1;2m`. The
requirement, the limit table and the release note still promised
"youngest whole lines", which is the behavior in the common case but
not the contract the daemon keeps.

Refs #49

Assisted-by: Claude Code

test: pin the frame cap at the boundary a producer meets 1c091ce4a7

The 4096-frame cap had coverage on the protocol and mirror sides but
none where a producer reaches it: the store's own refusal and the
`a=f` answer were only reasoned about. Both are now pinned at the
boundary — the 4096th frame lands, the next is refused as
`TooManyFrames` / `ENOTSUP`, and the animation comes out of the
refusal unchanged with nothing announced for a frame that was never
stored. The store is filled directly rather than through 4095 parser
round trips, since what is under test is the answer at the cap.

Refs #49

Assisted-by: Claude Code

docs: align REQ-105's region summary with the trim boundary 37df53ff71

REQ-105a and reference/ipc.md now say the trim cuts at the youngest
boundary a reader can resume at, which is a line start only when the
retained window holds one. REQ-105's one-line summary still promised
"whole lines", which is false for a window holding no line boundary
at all — the case the fallback exists for.

Refs #49

Assisted-by: Claude Code

fix(ci): read Criterion results from the parked build directory

bench / Criterion full-suite snapshot (pull_request) Has been skipped

Details

fuzz / cargo fuzz nightly long-run (pull_request) Has been skipped

Details

darwin / build felis (aarch64-darwin) (pull_request) Successful in 1m57s

Details

windows / cargo nextest (Windows) (pull_request) Successful in 5m53s

Details

bench / Criterion regression gate (pull_request) Failing after 2m38s

Details

fuzz / cargo fuzz smoke (per target) (pull_request) Successful in 1m35s

Details

pr / nix flake check (pull_request) Successful in 22s

Details

windows / frontend smoke (Windows) (pull_request) Successful in 2m17s

Details

windows / package felis (x86_64-pc-windows-msvc) (pull_request) Has been skipped

Details

pr / cargo build / clippy / test / deny (pull_request) Successful in 2m11s

Details

pr / generated code is current (pull_request) Successful in 2s

Details

pr / wire schema is compatible with the base (pull_request) Successful in 9s

Details

pr / cargo check (MSRV) (pull_request) Successful in 22s

Details

pr / frontend smoke (x86_64-linux) (pull_request) Successful in 5m3s

Details

pr / publish felis (x86_64-linux) (pull_request) Has been skipped

Details

windows / cargo clippy (Windows cross) (pull_request) Successful in 26s

Details

cf1f4ddb08

`.forgejo/actions/cargo-env` points `CARGO_TARGET_DIR` at a persistent
directory outside the workspace, so the benches write their estimates
there — but the orchestrator still resolved its root as the literal
`target/criterion`. Every PR run therefore benched both revisions,
printed an empty summary table, and then failed the gate with
"target/criterion does not exist; nothing to check", turning the one
job that compares the two revisions into an unconditional red.

Resolving the root from `CARGO_TARGET_DIR` keeps the local default
(`target/criterion`) for a shell that sets nothing.

Assisted-by: Claude Code

natsukium referenced this pull request

2026-09-04 13:38:47 +09:00

feat(protocol): admit wire claims that order a receiver's allocation #67

natsukium merged commit ff5567ae28 into main

2026-09-04 14:03:48 +09:00

natsukium deleted branch feat/receiver-admission-49-v2

2026-09-04 14:03:48 +09:00

natsukium referenced this pull request from a commit

2026-09-04 17:56:28 +09:00

chore: prune narrating comments from the batch-3 merges

natsukium referenced this pull request

2026-09-04 17:56:50 +09:00