protocol: FrameWriter::new defaults to this build's own PROTOCOL_MINOR #176

New issue

Closed

opened 2026-09-07 14:47:30 +09:00 by natsukium · 0 comments

natsukium commented

2026-09-07 14:47:30 +09:00

Owner

What

FrameWriter::new (crates/felis-protocol/src/framing.rs) initializes effective_minor: PROTOCOL_MINOR —
this build's own maximum minor — rather than an unnegotiated/baseline value. Only an explicit .at_minor(effective)
call narrows it. So the plain public constructor plus send() will happily emit every addition this build knows
about even though no peer has agreed to any of them, which contradicts #50's acceptance criterion that a caller
"cannot emit an unauthorized addition through the ordinary API".

Where

crates/felis-protocol/src/framing.rs — FrameWriter::new, FrameWriter::at_minor, FrameWriter::send

Why it was deferred from #50

The gap was raised in an earlier review round on #50 and deliberately deferred there: all four production call
sites do set .at_minor correctly, so the shipped risk is mitigated, and closing the hole in the public API
means rewriting roughly forty mostly self-talking test-harness call sites — outside #50's size budget.

Suggested correction

Make the effective minor mandatory at construction (take it as a new parameter), or make an unnegotiated
writer refuse every non-baseline arm, instead of defaulting to the build's own ceiling. Test-harness call sites
can take a baseline-or-max helper constructor so the default stops being the permissive one.

Split from #50.

## What `FrameWriter::new` (`crates/felis-protocol/src/framing.rs`) initializes `effective_minor: PROTOCOL_MINOR` — this build's own maximum minor — rather than an unnegotiated/baseline value. Only an explicit `.at_minor(effective)` call narrows it. So the plain public constructor plus `send()` will happily emit every addition this build knows about even though no peer has agreed to any of them, which contradicts #50's acceptance criterion that a caller "cannot emit an unauthorized addition through the ordinary API". ## Where - `crates/felis-protocol/src/framing.rs` — `FrameWriter::new`, `FrameWriter::at_minor`, `FrameWriter::send` ## Why it was deferred from #50 The gap was raised in an earlier review round on #50 and deliberately deferred there: all four production call sites do set `.at_minor` correctly, so the shipped risk is mitigated, and closing the hole in the public API means rewriting roughly forty mostly self-talking test-harness call sites — outside #50's size budget. ## Suggested correction Make the effective minor mandatory at construction (take it as a `new` parameter), or make an unnegotiated writer refuse every non-baseline arm, instead of defaulting to the build's own ceiling. Test-harness call sites can take a baseline-or-max helper constructor so the default stops being the permissive one. Split from #50.

natsukium referenced this issue

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

transport: gate every send on the connection's effective minor #177

natsukium referenced this issue from a commit

2026-09-07 15:18:25 +09:00

transport: gate every send on the connection's effective minor (#177)

natsukium added the

priority/P2

label

2026-09-07 16:01:15 +09:00

natsukium referenced this issue

2026-09-07 16:16:40 +09:00