natsukium/felis

Archived

Watch

[v0.1/P1] Reject impossible Grid coordinates instead of normalizing them #315

Closed

opened 2026-09-21 12:22:46 +09:00 by natsukium · 1 comment

natsukium commented

2026-09-21 12:22:46 +09:00

Owner

Parent: #12. Related: #304 (out-of-range row), #305 (drained-body validation), #308 (registry references).

Problem

Several daemon-impossible Grid values are silently normalized by the client instead of following the connection-local corruption policy:

ShadowScreen::write_row truncates a decoded row wider than MAX_GRID_COLS after decode_row has accepted and allocated up to the row codec's u16 count.
ScreenBuffer::apply_scroll_directive clamps region_top / region_bottom, turns n_rows = 0 into 1, and caps an oversized count to the region height.
ScreenBuffer::set_cursor_state clamps coordinates outside the current grid.
sized-cell sideband coordinates outside the decoded row are ignored by the screen setter.

The IPC reference already says GridScrolled.n_rows >= 1, bounds admitted geometry at 2048 × 2048, and treats an undecodable body or impossible authoritative state as corruption. Clamping these values can leave the shadow different from the daemon while reporting a healthy connection.

This is distinct from a legitimate resize race. A RowDelta within the protocol-wide bounds may temporarily be wider or lower than the client's current shadow and can grow it as documented. A value outside the protocol-wide bounds, or a Scrolled / CursorState impossible under the ordered authoritative dimensions, has no honest race interpretation.

Proposed resolution

Validate Grid semantic invariants before mutating the shadow and return ShadowError for daemon-impossible values. The client should close only that attachment; reconnect and rehydrate recover it without affecting the daemon session or other subscribers.

At minimum:

reject a row width above MAX_GRID_COLS before allocating its cell vector;
reject sized-cell columns outside the decoded row;
require Scrolled.region_top <= region_bottom, both inside the current grid, and 1 <= n_rows <= region_height;
require a visible cursor coordinate to be inside the current authoritative grid, and define whether an invisible cursor has the same coordinate requirement;
decide and validate ViewportState.lines_from_bottom <= max rather than accepting an impossible mirror silently.

Keep the documented resize-race accommodation only for row indices and widths within MAX_GRID_ROWS / MAX_GRID_COLS.

Acceptance criteria

Every Grid coordinate and count has one documented admission rule at the wire-to-shadow boundary.
A row above MAX_GRID_COLS is rejected before Vec<Cell> reserves from the peer's count.
Invalid scroll regions/counts, cursor coordinates, sized-cell columns, and viewport pairs cannot be clamped, dropped, or applied silently.
Valid old-size/new-size RowDelta races still grow, pad, or trim within the published global bounds as explicitly documented.
Failure closes only the affected connection and normal reconnect/rehydrate can recover.
Tests cover exact bounds, one-past bounds, zero scroll count, reversed regions, oversized scroll counts, cursor edges, sized-cell edges, and both directions of a resize race.
docs/reference/ipc.md, the row-codec reference, Grid explanation, and ShadowError agree.

Principle check

Add only what earns its place — Pass. This settles existing decoder invariants and adds no user-facing capability.
Render everything, fast — Pass. It prevents silent fidelity loss; pre-allocation width validation also avoids needless work.
The daemon owns state, the client owns pixels — Pass. A client that cannot establish daemon state reconnects instead of inventing normalized state.
Explicit over heuristic — Pass. Validation uses typed coordinates and published bounds.

Parent: #12. Related: #304 (out-of-range row), #305 (drained-body validation), #308 (registry references). ## Problem Several daemon-impossible Grid values are silently normalized by the client instead of following the connection-local corruption policy: - `ShadowScreen::write_row` truncates a decoded row wider than `MAX_GRID_COLS` after `decode_row` has accepted and allocated up to the row codec's `u16` count. - `ScreenBuffer::apply_scroll_directive` clamps `region_top` / `region_bottom`, turns `n_rows = 0` into 1, and caps an oversized count to the region height. - `ScreenBuffer::set_cursor_state` clamps coordinates outside the current grid. - sized-cell sideband coordinates outside the decoded row are ignored by the screen setter. The IPC reference already says `GridScrolled.n_rows >= 1`, bounds admitted geometry at 2048 × 2048, and treats an undecodable body or impossible authoritative state as corruption. Clamping these values can leave the shadow different from the daemon while reporting a healthy connection. This is distinct from a legitimate resize race. A `RowDelta` within the protocol-wide bounds may temporarily be wider or lower than the client's current shadow and can grow it as documented. A value outside the protocol-wide bounds, or a `Scrolled` / `CursorState` impossible under the ordered authoritative dimensions, has no honest race interpretation. ## Proposed resolution Validate Grid semantic invariants before mutating the shadow and return `ShadowError` for daemon-impossible values. The client should close only that attachment; reconnect and rehydrate recover it without affecting the daemon session or other subscribers. At minimum: - reject a row width above `MAX_GRID_COLS` before allocating its cell vector; - reject sized-cell columns outside the decoded row; - require `Scrolled.region_top <= region_bottom`, both inside the current grid, and `1 <= n_rows <= region_height`; - require a visible cursor coordinate to be inside the current authoritative grid, and define whether an invisible cursor has the same coordinate requirement; - decide and validate `ViewportState.lines_from_bottom <= max` rather than accepting an impossible mirror silently. Keep the documented resize-race accommodation only for row indices and widths within `MAX_GRID_ROWS` / `MAX_GRID_COLS`. ## Acceptance criteria - [ ] Every Grid coordinate and count has one documented admission rule at the wire-to-shadow boundary. - [ ] A row above `MAX_GRID_COLS` is rejected before `Vec<Cell>` reserves from the peer's count. - [ ] Invalid scroll regions/counts, cursor coordinates, sized-cell columns, and viewport pairs cannot be clamped, dropped, or applied silently. - [ ] Valid old-size/new-size `RowDelta` races still grow, pad, or trim within the published global bounds as explicitly documented. - [ ] Failure closes only the affected connection and normal reconnect/rehydrate can recover. - [ ] Tests cover exact bounds, one-past bounds, zero scroll count, reversed regions, oversized scroll counts, cursor edges, sized-cell edges, and both directions of a resize race. - [ ] `docs/reference/ipc.md`, the row-codec reference, Grid explanation, and `ShadowError` agree. ## Principle check 1. **Add only what earns its place — Pass.** This settles existing decoder invariants and adds no user-facing capability. 2. **Render everything, fast — Pass.** It prevents silent fidelity loss; pre-allocation width validation also avoids needless work. 3. **The daemon owns state, the client owns pixels — Pass.** A client that cannot establish daemon state reconnects instead of inventing normalized state. 4. **Explicit over heuristic — Pass.** Validation uses typed coordinates and published bounds.

natsukium added the

priority/P1

release/v0.1.0

type/bug

labels

2026-09-21 12:22:57 +09:00

natsukium referenced this issue

2026-09-21 12:23:11 +09:00

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

natsukium referenced this issue

2026-09-21 13:02:36 +09:00

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

natsukium referenced this issue

2026-09-21 13:02:46 +09:00

[v0.1/P1] Do not silently drop out-of-range Grid RowDelta rows #304

natsukium commented

2026-09-21 13:02:46 +09:00

Author

Owner

Direction agreed 2026-09-21 after three sol design-review rounds (final verdict PASS). Full plan and batch order: see the comment on #12.

#304 / #315 / #308 — one policy: a daemon-impossible Grid value returns
ShadowError and closes only that attachment; reconnect/rehydrate recovers.
Resize-race tolerance is kept only for row indices/widths within
MAX_GRID_ROWS / MAX_GRID_COLS. Unresolved cluster/hyperlink handles in a
decoded row are corruption; degraded rendering is removed. Id 0, over-cap
ids and over-length text are rejected before allocation.

Direction agreed 2026-09-21 after three `sol` design-review rounds (final verdict PASS). Full plan and batch order: see the comment on #12. - **#304 / #315 / #308** — one policy: a daemon-impossible Grid value returns `ShadowError` and closes only that attachment; reconnect/rehydrate recovers. Resize-race tolerance is kept only for row indices/widths within `MAX_GRID_ROWS` / `MAX_GRID_COLS`. Unresolved cluster/hyperlink handles in a decoded row are corruption; degraded rendering is removed. Id 0, over-cap ids and over-length text are rejected before allocation.

natsukium referenced this issue

2026-09-21 13:02:46 +09:00

[v0.1/IPC Review] Decide unresolved grid registry handles before decoder freeze #308

natsukium referenced this issue from a pull request that will close it,

2026-09-21 13:23:53 +09:00

shadow: refuse daemon-impossible grid values instead of normalizing them #318

natsukium closed this issue

2026-09-21 18:35:32 +09:00

natsukium referenced this issue

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.