transport: decode drained uniform bodies before dropping them #317

Merged

natsukium merged 7 commits from drain-validation into main 2026-09-21 15:58:12 +09:00

Conversation 0 Commits 7 Files changed 5 +206 -38

natsukium commented 2026-09-21 13:21:00 +09:00

Owner

Copy link

The uniform uncorrelated families (Grid, Image, Push) skipped the body decode on the drain path, so a malformed
body ended the connection or not depending on which role received it. The drain path now runs a validation-only decode
before discarding the delivery, so an undecodable body, an oneof arm this build does not know, or a receive-side limit
breach is corruption on every role, exactly as it is on the consuming path.

The fold of the arm table into the kind columns is kept: it is exact where every arm declares the same row, so only the
decode comes back, and the consuming hot path is untouched.

The boundary is stated rather than implied. felis-transport depends on felis-protocol alone, so packed_cells
contents, coordinates against a shadow's dimensions, and image transfer sequencing stay with the consuming shadow. Both
docs/reference/ipc.md "Corruption" and the uniform-family decision in docs/explanation/architecture/ipc.md name the
two tiers.

Tests (crates/felis-transport/src/driver/tests.rs): a body that is not protobuf, an arm outside the schema, and an
over-cap ImageMsg::Header are each corruption on the drain path; a RowDelta carrying garbage packed_cells is
admitted there, which pins the boundary rather than leaving it implied.

Gates: cargo nextest run --workspace --all-features (3336 passed), cargo clippy --workspace --all-targets (clean),
just prose-check, just docs-links.

Closes #305

The uniform uncorrelated families (`Grid`, `Image`, `Push`) skipped the body decode on the drain path, so a malformed body ended the connection or not depending on which role received it. The drain path now runs a validation-only decode before discarding the delivery, so an undecodable body, an oneof arm this build does not know, or a receive-side limit breach is corruption on every role, exactly as it is on the consuming path. The fold of the arm table into the kind columns is kept: it is exact where every arm declares the same row, so only the decode comes back, and the consuming hot path is untouched. The boundary is stated rather than implied. `felis-transport` depends on `felis-protocol` alone, so `packed_cells` contents, coordinates against a shadow's dimensions, and image transfer sequencing stay with the consuming shadow. Both `docs/reference/ipc.md` "Corruption" and the uniform-family decision in `docs/explanation/architecture/ipc.md` name the two tiers. Tests (`crates/felis-transport/src/driver/tests.rs`): a body that is not protobuf, an arm outside the schema, and an over-cap `ImageMsg::Header` are each corruption on the drain path; a `RowDelta` carrying garbage `packed_cells` is admitted there, which pins the boundary rather than leaving it implied. Gates: `cargo nextest run --workspace --all-features` (3336 passed), `cargo clippy --workspace --all-targets` (clean), `just prose-check`, `just docs-links`. Closes #305

natsukium added 1 commit 2026-09-21 13:21:00 +09:00

transport: decode drained uniform bodies before dropping them ... 677e04996b

The uniform uncorrelated families (Grid, Image, Push) skipped the body
decode on the drain path, so whether a malformed body ended the
connection depended on the reader: a window client failed on it, a role
that drains the family discarded it as if it were well-formed. That
makes corruption a property of the receiver rather than of the frame,
which is what a cross-language client would report as a felis bug, and
the ambiguity was baked into the source as a comment calling a frame
untrustworthy while returning Ok.

The fold of the arm table into the kind columns is kept, because it is
exact where every arm declares the same row; only the decode comes back.
The cost argument the shortcut rested on does not survive the
comparison: the frames it saves work on are the ones being dropped, not
the ones being rendered, so the deserialization lands where there is
headroom for it.

The boundary is stated rather than implied. felis-transport depends on
felis-protocol alone, so packed_cells contents, coordinates against a
shadow's dimensions, and image transfer sequencing cannot be judged
here; those stay with the consuming shadow, and a test pins that a
garbage opaque row is admitted at the wire tier.

Closes natsukium/felis issue 305.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-21 13:31:22 +09:00

docs: bound the wire validation tier to what one body settles ... 22e2e3a554

The two-tier statement claimed the driver performs every receive-side
limit the "Semantic limits" table carries. That table also holds limits
no decode can reach: a session's image total accrues across frames, a
row's index and width are read against the shadow's current dimensions,
a registry id against the table the client has received, and an
OpsMsg::Tag delta over the caps is answered with `denied` rather than a
teardown. Read literally, the old wording made those the connection's
death and contradicted the table's own Outcome column.

The wire tier is therefore stated as exactly what codec::decode settles
on one body: structure, the arm, and the stateless limits the conversion
reads off that body. Everything stateful or handler-specific is named on
the consumer side, together with the fact that a breach there is not
automatically fatal.

The uniform uncorrelated families are Input, Grid and Image; Push arms
disagree on their mode rows, so Push takes the full decode path and
never took the shortcut this describes.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-21 13:38:16 +09:00

docs: let the limits table decide which validation tier a cap sits in ... e423168269

The two-tier statement derived a limit's tier from the kind of value it
bounds, which the table it sits above contradicts in three places:
SpawnArgs.env_base is read in the spawn path and fails one spawn,
MAX_REGION_REPLY_BYTES is the sender's alone, and an overlong tag entry
is body-local yet answered with `denied` instead of a teardown. The
claim that every length limit is checked by codec::decode had the same
defect one section earlier.

Both now defer to the table's Checked and Outcome columns, which are the
only place that knows. Semantic validation is defined by the two reasons
a check actually lands there, needing receiver state or having a
deliberately handler-specific outcome, so a body-local cap with a
non-fatal answer is no longer misfiled as the driver's. The two rows no
receiver decode touches at all are named rather than left as exceptions
a reader has to notice.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-21 13:44:39 +09:00

docs: name the two-sided limits by their Checked marking ... dab151c2ed

"Most of these limits" claims a majority the table does not have: half
its rows are one-sided. Naming the `sender and receiver` marking instead
of counting rows keeps the sentence true as rows are added, and leaves
the table the single place that says which half a given cap gets.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-21 14:05:29 +09:00

docs: name the bound each grid coordinate is actually judged against ... 5d0cb33f43

The semantic-tier example said a row's index and width are checked
against the shadow's current dimensions. They are not: they are checked
against the protocol-wide MAX_GRID_ROWS and MAX_GRID_COLS, precisely so
that a row outrunning a resize grows the mirror instead of being
refused, and the outcome is a drop or a truncation rather than a
teardown. It is the scroll region, cursor and viewport coordinates that
are read against the geometry the daemon announced.

Stating it the old way would have a reader deriving the wrong rule for
the grid admission checks, and it also filed the row bound under the
wrong reason: it is body-local with a deliberately non-fatal outcome,
not a check that needs receiver state.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-21 14:06:51 +09:00

docs: state the row bound without its outcome ... 16623147cc

Naming the row index and width bound in the list of deliberately
non-fatal checks makes this page a second statement of an outcome the
limits table already owns, and the two would then have to be kept in
step by hand. The bound stands on its own sentence, saying which number
it is read against and pointing at the table's row for what a breach
costs, so a change to that outcome edits one place.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-21 15:54:43 +09:00

Merge branch 'main' into drain-validation ... b30a237412

Assisted-by: Claude Code

natsukium merged commit fab83f860b into main 2026-09-21 15:58:12 +09:00

natsukium deleted branch drain-validation 2026-09-21 15:58:12 +09:00

natsukium referenced this pull request from a commit 2026-09-21 15:58:13 +09:00

transport: decode drained uniform bodies before dropping them (#317)

natsukium referenced this pull request 2026-09-21 16:12:12 +09:00

Encode keys where the keyboard modes are authoritative #323

natsukium referenced this pull request 2026-09-21 21:35:04 +09:00

[v0.1/Tracker] Close release blockers before v0.1.0 #12

Commenting is not possible because the repository is archived.

Reviewers

No reviewers

Labels

Clear labels   

priority/P0

Must be resolved before the v0.1.0 tag

  

priority/P1

Must be settled before the first public contract freezes

  

priority/P2

Explicitly deferred until after v0.1.0

  

release/v0.1.0

Work tracked for the first tagged release

  

status/blocked

Cannot start now: waits on an upstream release, another issue, or a recorded revisit trigger

  

status/planned

Triaged with a source-grounded plan and a passed pi review; ready to implement

  

type/bug

Shipped behavior that contradicts the docs or the code's own intent; needs a product code change

  

type/design

An open scope or trade-off question; needs a recorded decision in the owning explanation doc before code

  

type/test-gap

Documented or reviewed behavior that no test pins; usually split from a review finding, needs no product code change

  

type/tracker

Parent issue that tracks child work

No labels

priority/P0

priority/P1

priority/P2

release/v0.1.0

status/blocked

status/planned

type/bug

type/design

type/test-gap

type/tracker

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

natsukium/felis!317

No description provided.