fix(pegboard-gateway): enforce tunnel message state

[NathanFlurry]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[NathanFlurry]

• fix(pegboard): isolate runner config dc lookup failures #4625 : 2 dependent PRs (#4626 , #4630 )
• docs(pegboard): note runner config upsert split write risk #4624
• fix(pegboard): unpack global runner config keys correctly #4623
• fix(pegboard-gateway): enforce tunnel message state #4622 👈 (View in Graphite)
• fix(pegboard-runner): clear terminal tunnel routes #4621
• fix(pegboard): skip protocol version keys in runner pool backfill #4620
• fix(pegboard): create missing normal runner configs #4619
• fix(epoxy): reuse cached optimistic value #4618
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR Review: fix(pegboard-gateway): enforce tunnel message state

This PR adds a state machine to the pegboard gateway to enforce valid message sequences for in-flight requests. Good defensive security improvement overall.

What the PR does

Introduces InFlightRequestState with four variants (AwaitingHttpResponseStart, AwaitingWebSocketOpen, ActiveWebSocket, Closed) and an accept_message method that enforces valid transitions. Messages arriving in the wrong state are dropped with a warn! log rather than forwarded, preventing potential protocol confusion or abuse from rogue runners.

Issues

Bug: ToServerResponseChunk is silently rejected in AwaitingHttpResponseStart

The protocol includes a ToServerResponseChunk variant (visible in versioned.rs v6↔v7 conversion), but the state machine has no arm for it. It falls through to _ => false and is dropped. The current gateway hardcodes stream: false in ToClientRequestStart, so chunks aren't expected — but any runner that sends them will generate a spurious warn! log and the message will be silently dropped.

This isn't a correctness bug today, but it's worth either:

1. Adding a comment explaining why chunks are intentionally rejected (non-streaming only), or
2. Accepting ToServerResponseChunk in AwaitingHttpResponseStart if streaming is ever planned

Potential log noise: message_kind=?msg.message_kind in the warning

tracing::warn!(
    ...
    message_kind=?msg.message_kind,
    "dropping invalid tunnel message for request state"
);

For a ToServerWebSocketMessage with a large payload, the Debug representation will include the full binary data in the log. Consider logging just the discriminant (e.g., a variant name) rather than the full message content.

Missing test coverage

The tests are a good start, but a few cases are uncovered:

• ToServerResponseStart → Closed transition: The HTTP test only exercises ToServerResponseAbort. There's no test confirming ToServerResponseStart also transitions to Closed and that subsequent messages are then rejected.
• Closed state rejects everything: No test asserts that once Closed, all message kinds return false.
• WebSocket close from AwaitingWebSocketOpen: The arm (AwaitingWebSocketOpen, ToServerWebSocketClose) → Closed exists but isn't tested.
• ActiveWebSocket initial state (hibernation reconnect path): No test for the after_hibernation=true case which starts a request directly in ActiveWebSocket and verifies it can accept messages.

Minor notes

• The two WebSocket close arms (AwaitingWebSocketOpen and ActiveWebSocket) could be collapsed with an | pattern guard since both transition to Closed and the bound state variable isn't used in the arm body:

  (
      state @ (InFlightRequestState::AwaitingWebSocketOpen | InFlightRequestState::ActiveWebSocket),
      ToServerTunnelMessageKind::ToServerWebSocketClose(_),
  ) => {
      *state = InFlightRequestState::Closed;
      true
  }

  Current code is correct — this is purely cosmetic.

• Commented-out code at the bottom of the file (// fn wrapping_lt) predates this PR and could be removed in a follow-up.

Summary

The core state machine logic is correct and the approach is sound. The main actionable items are: (1) clarify the ToServerResponseChunk omission with a comment, (2) guard the warning log against logging large payloads, and (3) expand test coverage to include the Closed state, the ToServerResponseStart transition, the WS-close-from-awaiting path, and the hibernation reconnect initial state.

[MasterPtato]

Merge activity

• Apr 13, 8:40 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Apr 13, 8:49 PM UTC: Graphite rebased this pull request as part of a merge.
• Apr 13, 8:50 PM UTC: @MasterPtato merged this pull request with Graphite.