natsukium/felis

Watch

[v0.1/Proto Review] FRLY carrier block の 1MiB cap と診断は 1.0 で凍結すべき #139

New issue

Closed

opened 2026-09-05 11:38:21 +09:00 by natsukium · 2 comments

natsukium commented

2026-09-05 11:38:21 +09:00

Owner

背景

FRLY carrier block は version preface の前で relay が daemon に自身の環境を渡すために 8 + 10 byte の固定 preface より先に書かれる唯一の可変長ブロック（crates/felis-transport/src/preface.rs）。凍結層であり、version negotiation 前なので minor による交渉ができない:

FRLY | payload len u32 | entries u32 | (name len u32 | name | value len u32 | value)*

MAX_CARRIER_ENTRIES = 4096 / MAX_CARRIER_PAYLOAD_BYTES = 1 MiB が protocol major 単位で凍結。relay が送信前、daemon が受信時に両方で検査し、payload 長を先に読んでから payload を確保する。connect_or_spawn_daemon の auto-spawn 経路でも relay 経由の env が SpawnArgs.env_base の第一候補になる（minor 4）。

問題:

1 MiB は大きいように見えるが、Nix の PATH や systemd --user の環境では 1 MiB を超える現実的なケースがある（nix develop の多段 PATH、LOCPATH、長い XDG_DATA_DIRS）。超えると relay は送信前に CarrierError::TooLarge で失敗し、daemon は受信時に CarrierError::TooLarge で接続を close する。ユーザには「daemon unreachable」に見えるだけで「env が大きすぎる」と診断されない。
4096 entries は十分だが、1 MiB との組み合わせで「多数の小さな変数」より「少数の巨大な変数」で先に引っかかる。どちらが現実的な DoS 境界か根拠が reference/ipc.md にない。
relay は frame aware でない byte pump なので、payload を streaming できず全量を一括で確保する。1 MiB の一括確保自体は小さいが、cap を上げるなら allocation 策も見直す必要がある。
cap は major 単位で凍結だが、値は「env の現実的な上限と DoS 耐性」のトレードオフで決めたはずなのに、1.0 で確定させる根拠が docs の表にない（#5「FRLY carrier block の凍結 limits」でも追記を提案済みだが値は未確定）。

1.0 で reference/ipc.md の表に「なぜ 4096 / 1 MiB か」を刻むと同時に、値を上げるなら今しか破壊的に変えられない（major bump が必要）。

問い

1.0 で cap をこのまま凍結するか、上げるか、あるいは交渉可能にするか:

案A: 凍結維持 + 診断改善 — 1 MiB / 4096 のまま、CarrierError を ConnectError の daemon_unreachable ではなく invalid_request 相当の typed error として CLI に「carrier block too large (N bytes > 1 MiB): trim env」と報告。reference/ipc.md に根拠（「典型的な env | wc -c は 10–50 KiB、1 MiB は 20×」）を追記。
案B: payload cap を 4 MiB に引き上げ — MAX_CARRIER_PAYLOAD_BYTES = 4 MiB に。受信側は payload_len を見てから Vec::with_capacity するので DoS 上昇は 4×。Nix ユーザの実測を根拠にする。entries は 4096 のまま。
案C: streaming / chunked carrier block — FRLY payload を entries u32 の一括ではなく chunked にし、daemon 側は逐次読む。frame aware にする必要があるため relay が frame boundary を知ることになり、現行の「relay は byte pump」原則を崩す。1.0 でやるなら設計が大きい。

提案

最低でも案Aの診断改善は 1.0 で必須（現行の daemon_unreachable は誤診断）。cap 値自体は実測で決める: env | wc -c を Nix / non-Nix の数環境で測り、P99 ×4 を cap にする。超えるなら案Bで 1.0 の破壊的変更として上げる。案Cは deferred として explanation/architecture/ipc.md「Carrier choices」に Revisit trigger（「1 MiB を超える正当な env が継続的に現れたら chunked を検討」）を残す。
reference/ipc.md「Version preface」「Semantic limits」節と explanation/architecture/ipc.md「Handshake bootstrap」節に cap の根拠と凍結宣言を追記。
crates/felis-transport/src/preface.rs の定数コメントに reference/ipc.md へのリンクと「major 単位で凍結」ラベルを付与。

判定基準

reference/ipc.md の FRLY 行を読んだ新 contributor が「なぜ 4096 / 1 MiB か」を再発明せずに理解できること。
FRLY payload が cap を超えた時に CLI が daemon_unreachable (exit 2, retry) ではなく invalid_request (exit 1, 環境を削れ) を返すこと。
1.0 で cap を変えるなら BREAKING.md に base: <sha> 行を追加し just proto-compat が green であること。
現行 Nix 環境で felis --host <remote-nix> が cap で落ちないこと（実測で確認）。

対象ファイル

crates/felis-protocol/src/preface.rs / crates/felis-transport/src/preface.rs (MAX_CARRIER_*)
crates/felis-transport/src/relay.rs (送信前 check)
crates/felis-client-core/src/connector.rs / crates/felis-daemon/src/serve.rs (受信時 handling)
docs/reference/ipc.md「Version preface」「Semantic limits」
docs/explanation/architecture/ipc.md「Handshake bootstrap」「Carrier choices」

Parent: #12 および #11 / #5, #52 (P1 proto review)

## 背景 `FRLY` carrier block は version preface の前で relay が daemon に自身の環境を渡すために 8 + 10 byte の固定 preface より先に書かれる唯一の可変長ブロック（`crates/felis-transport/src/preface.rs`）。凍結層であり、version negotiation 前なので minor による交渉ができない: ``` FRLY | payload len u32 | entries u32 | (name len u32 | name | value len u32 | value)* ``` `MAX_CARRIER_ENTRIES = 4096` / `MAX_CARRIER_PAYLOAD_BYTES = 1 MiB` が protocol major 単位で凍結。relay が送信前、daemon が受信時に両方で検査し、payload 長を先に読んでから payload を確保する。`connect_or_spawn_daemon` の auto-spawn 経路でも relay 経由の env が `SpawnArgs.env_base` の第一候補になる（minor 4）。問題: - 1 MiB は大きいように見えるが、`Nix` の `PATH` や `systemd --user` の環境では 1 MiB を超える現実的なケースがある（`nix develop` の多段 `PATH`、`LOCPATH`、長い `XDG_DATA_DIRS`）。超えると relay は送信前に `CarrierError::TooLarge` で失敗し、daemon は受信時に `CarrierError::TooLarge` で接続を close する。ユーザには「daemon unreachable」に見えるだけで「env が大きすぎる」と診断されない。 - 4096 entries は十分だが、1 MiB との組み合わせで「多数の小さな変数」より「少数の巨大な変数」で先に引っかかる。どちらが現実的な DoS 境界か根拠が `reference/ipc.md` にない。 - relay は frame aware でない byte pump なので、payload を streaming できず全量を一括で確保する。1 MiB の一括確保自体は小さいが、cap を上げるなら allocation 策も見直す必要がある。 - cap は major 単位で凍結だが、値は「env の現実的な上限と DoS 耐性」のトレードオフで決めたはずなのに、1.0 で確定させる根拠が docs の表にない（#5「FRLY carrier block の凍結 limits」でも追記を提案済みだが値は未確定）。 1.0 で `reference/ipc.md` の表に「なぜ 4096 / 1 MiB か」を刻むと同時に、値を上げるなら今しか破壊的に変えられない（major bump が必要）。 ## 問い 1.0 で cap をこのまま凍結するか、上げるか、あるいは交渉可能にするか: - **案A: 凍結維持 + 診断改善** — 1 MiB / 4096 のまま、`CarrierError` を `ConnectError` の `daemon_unreachable` ではなく `invalid_request` 相当の typed error として CLI に「carrier block too large (N bytes > 1 MiB): trim env」と報告。`reference/ipc.md` に根拠（「典型的な `env | wc -c` は 10–50 KiB、1 MiB は 20×」）を追記。 - **案B: payload cap を 4 MiB に引き上げ** — `MAX_CARRIER_PAYLOAD_BYTES = 4 MiB` に。受信側は `payload_len` を見てから `Vec::with_capacity` するので DoS 上昇は 4×。Nix ユーザの実測を根拠にする。entries は 4096 のまま。 - **案C: streaming / chunked carrier block** — `FRLY` payload を `entries u32` の一括ではなく chunked にし、daemon 側は逐次読む。frame aware にする必要があるため relay が frame boundary を知ることになり、現行の「relay は byte pump」原則を崩す。1.0 でやるなら設計が大きい。 ## 提案 - 最低でも案Aの診断改善は 1.0 で必須（現行の `daemon_unreachable` は誤診断）。cap 値自体は実測で決める: `env | wc -c` を Nix / non-Nix の数環境で測り、P99 ×4 を cap にする。超えるなら案Bで 1.0 の破壊的変更として上げる。案Cは deferred として `explanation/architecture/ipc.md`「Carrier choices」に Revisit trigger（「1 MiB を超える正当な env が継続的に現れたら chunked を検討」）を残す。 - `reference/ipc.md`「Version preface」「Semantic limits」節と `explanation/architecture/ipc.md`「Handshake bootstrap」節に cap の根拠と凍結宣言を追記。 - `crates/felis-transport/src/preface.rs` の定数コメントに `reference/ipc.md` へのリンクと「major 単位で凍結」ラベルを付与。 ## 判定基準 - `reference/ipc.md` の `FRLY` 行を読んだ新 contributor が「なぜ 4096 / 1 MiB か」を再発明せずに理解できること。 - `FRLY` payload が cap を超えた時に CLI が `daemon_unreachable` (exit 2, retry) ではなく `invalid_request` (exit 1, 環境を削れ) を返すこと。 - 1.0 で cap を変えるなら `BREAKING.md` に `base: <sha>` 行を追加し `just proto-compat` が green であること。 - 現行 Nix 環境で `felis --host <remote-nix>` が cap で落ちないこと（実測で確認）。 ## 対象ファイル - `crates/felis-protocol/src/preface.rs` / `crates/felis-transport/src/preface.rs` (`MAX_CARRIER_*`) - `crates/felis-transport/src/relay.rs` (送信前 check) - `crates/felis-client-core/src/connector.rs` / `crates/felis-daemon/src/serve.rs` (受信時 handling) - `docs/reference/ipc.md`「Version preface」「Semantic limits」 - `docs/explanation/architecture/ipc.md`「Handshake bootstrap」「Carrier choices」 Parent: #12 および #11 / #5, #52 (P1 proto review)

natsukium added the

priority/P1

release/v0.1.0

labels

2026-09-05 12:11:14 +09:00

natsukium commented

2026-09-05 12:11:18 +09:00

Author

Owner

Triage (2026-09-05)

Verdict: accept option A only, P1. Measured today: env | wc -c is 53 KiB in a login shell and 59 KiB inside this repo's nix develop shell, so the "Nix exceeds 1 MiB" premise has no evidence and option B (4 MiB) is not justified. Option C is deferred with a Revisit trigger ("a legitimate env over 1 MiB is observed") in explanation/architecture/ipc.md.

Do for v0.1.0: the rationale row for 4096 / 1 MiB in reference/ipc.md (with the measurement), the "frozen per major" label on the constants in preface.rs, and the typed "carrier block too large" diagnostic. The diagnostic moves the failure from exit 2 (retry) to exit 1 (fix your env), which is exit-class behavior, so it lands before the tag or not at all. Parent: #52.

## Triage (2026-09-05) **Verdict: accept option A only, P1.** Measured today: `env | wc -c` is 53 KiB in a login shell and 59 KiB inside this repo's `nix develop` shell, so the "Nix exceeds 1 MiB" premise has no evidence and option B (4 MiB) is not justified. Option C is deferred with a Revisit trigger ("a legitimate env over 1 MiB is observed") in `explanation/architecture/ipc.md`. Do for v0.1.0: the rationale row for 4096 / 1 MiB in `reference/ipc.md` (with the measurement), the "frozen per major" label on the constants in `preface.rs`, and the typed "carrier block too large" diagnostic. The diagnostic moves the failure from exit 2 (retry) to exit 1 (fix your env), which is exit-class behavior, so it lands before the tag or not at all. Parent: #52.

natsukium referenced this issue

2026-09-05 12:11:19 +09:00

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

natsukium referenced this issue

2026-09-05 12:11:19 +09:00

[v0.1/Proto Review] Freeze protobuf and IPC contracts (7 issues) #52

natsukium commented

2026-09-05 21:25:59 +09:00

Author

Owner

Triage plan (2026-09-05)

Source-grounded triage against main at 07a77ae4, reviewed through six rounds of an independent reviewer (pi luna then sol); every finding was verified against the source and folded in, and the design decisions below were settled with the maintainer on 2026-09-05. The order that supersedes the tracker's is posted on #12: this issue lands before #29 (schemas, goldens, and reference tables freeze there).

FRLY carrier caps: freeze 4096 / 1 MiB, document the degrade path

Claim check

The caps: crates/felis-protocol/src/preface.rs:449-458. Their doc
comments already carry the "frozen for the protocol major" label and
the allocation argument, so that part of the triage is done.
The failure premise is wrong. On the send side the relay does not
fail: crates/felis-daemon/src/relay.rs:57-73 prepend_environment
logs a warn! and continues without the block when
write_carrier_block fails (including CarrierError::OverCap), and the
daemon then takes the documented fallback (its own birth environment,
docs/reference/ipc.md:2119). The relay's stderr is the SSH session's
stderr, which connector.rs:334 leaves at the parent default, so the
warning reaches the caller's terminal. Nothing reports
daemon_unreachable. The daemon-side OverCap close
(felis-transport/src/preface.rs:63-75) can only be hit by a peer that
is not a felis relay, and connector.rs:166 correctly classifies that as
a verdict, not a transient.
The measurement in the verdict comment (53 KiB login shell, 59 KiB in
nix develop) stands; 1 MiB is ~17-20× that.

Verdict

accept, docs-only (no typed diagnostic, no cap change). The triage's
"exit 1 invalid_request" item is dropped: there is no failure to type.
Option C stays deferred with a Revisit trigger.

Approach

docs/reference/ipc.md "Relay carrier block": add a "Limits" paragraph
or two table rows: 4096 entries / 1 MiB payload, frozen per major,
with the rationale (a login environment measures tens of KiB; the payload
is read into one allocation after the length word, so the cap bounds
that allocation before any bytes are trusted; entries cap bounds the
per-entry length words the same way). State the degrade contract: an
environment over either cap is not sent; the relay warns on its stderr
and the daemon falls back to its own environment for that connection's
creates.
docs/explanation/architecture/ipc.md "Carrier choices": why degrade
rather than fail (losing agent freshness beats losing the session, the
reason already in the relay's comment), why not chunked (the relay is a
byte pump; framing would make it protocol-aware), Revisit if a
legitimate environment over 1 MiB is observed.
Test: if crates/felis-daemon/src/relay.rs has no test that an
over-cap environment degrades to "no block" (the tests at :195-230
cover the happy path and encode().is_ok()), add one that drives
prepend_environment with an over-cap CarrierBlock and asserts the
daemon-bound half received no bytes.
crates/felis-protocol/src/preface.rs:449-458: add the ipc.md
section name to the constant docs so the two stay linked; nothing else.

Dependencies / risk / labels

None. S. Keep labels. Parent #52.

## Triage plan (2026-09-05) Source-grounded triage against `main` at `07a77ae4`, reviewed through six rounds of an independent reviewer (`pi` luna then sol); every finding was verified against the source and folded in, and the design decisions below were settled with the maintainer on 2026-09-05. The order that supersedes the tracker's is posted on #12: this issue lands before #29 (schemas, goldens, and reference tables freeze there). **FRLY carrier caps: freeze 4096 / 1 MiB, document the degrade path** ### Claim check - The caps: `crates/felis-protocol/src/preface.rs:449-458`. Their doc comments already carry the "frozen for the protocol major" label and the allocation argument, so that part of the triage is done. - **The failure premise is wrong.** On the send side the relay does not fail: `crates/felis-daemon/src/relay.rs:57-73` `prepend_environment` logs a `warn!` and continues *without* the block when `write_carrier_block` fails (including `CarrierError::OverCap`), and the daemon then takes the documented fallback (its own birth environment, `docs/reference/ipc.md:2119`). The relay's stderr is the SSH session's stderr, which `connector.rs:334` leaves at the parent default, so the warning reaches the caller's terminal. Nothing reports `daemon_unreachable`. The daemon-side `OverCap` close (`felis-transport/src/preface.rs:63-75`) can only be hit by a peer that is not a felis relay, and `connector.rs:166` correctly classifies that as a verdict, not a transient. - The measurement in the verdict comment (53 KiB login shell, 59 KiB in `nix develop`) stands; 1 MiB is ~17-20× that. ### Verdict **accept, docs-only (no typed diagnostic, no cap change).** The triage's "exit 1 `invalid_request`" item is dropped: there is no failure to type. Option C stays deferred with a Revisit trigger. ### Approach - `docs/reference/ipc.md` "Relay carrier block": add a "Limits" paragraph or two table rows: `4096` entries / `1 MiB` payload, frozen per major, with the rationale (a login environment measures tens of KiB; the payload is read into one allocation after the length word, so the cap bounds that allocation before any bytes are trusted; entries cap bounds the per-entry length words the same way). State the degrade contract: an environment over either cap is not sent; the relay warns on its stderr and the daemon falls back to its own environment for that connection's creates. - `docs/explanation/architecture/ipc.md` "Carrier choices": why degrade rather than fail (losing agent freshness beats losing the session, the reason already in the relay's comment), why not chunked (the relay is a byte pump; framing would make it protocol-aware), Revisit if a legitimate environment over 1 MiB is observed. - Test: if `crates/felis-daemon/src/relay.rs` has no test that an over-cap environment degrades to "no block" (the tests at `:195-230` cover the happy path and `encode().is_ok()`), add one that drives `prepend_environment` with an over-cap `CarrierBlock` and asserts the daemon-bound half received no bytes. - `crates/felis-protocol/src/preface.rs:449-458`: add the `ipc.md` section name to the constant docs so the two stay linked; nothing else. ### Dependencies / risk / labels None. **S.** Keep labels. Parent #52.

natsukium added the

status/planned

label

2026-09-05 21:25:59 +09:00

natsukium referenced this issue

2026-09-05 21:26:13 +09:00

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

natsukium referenced this issue from a commit

2026-09-06 03:26:15 +09:00

docs: state why the FRLY carrier caps are 4096 / 1 MiB

natsukium referenced this issue from a commit

2026-09-06 03:26:15 +09:00