Skip to content

fix(session): validate broker runtime boundaries - #903

Merged
benvinegar merged 3 commits into
mainfrom
security/session-broker-runtime-validation
Aug 30, 2026
Merged

fix(session): validate broker runtime boundaries#903
benvinegar merged 3 commits into
mainfrom
security/session-broker-runtime-validation

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • add one fixed-contract parser registry for registration, snapshots, exact command input/result pairs, and daemon/producer envelopes
  • validate external JSON as unknown before bridge dispatch or state mutation, with stable redacted protocol errors
  • parse app payloads exactly once and derive daemon parsing from the controller-owned registry
  • strictly validate Hunk command payloads, responses, health, capabilities, and legacy registration limits
  • add malformed corpora, parser-throw, atomicity, compatibility, and client-response coverage

Validation

  • full tests: 1,778 passed, 8 skipped
  • PTY integration: 135 passed, 1 skipped
  • focused broker/Hunk tests: 169 passed
  • bun run lint
  • bun run typecheck
  • bun run deps:check
  • git diff --check

Stack

This PR description was generated by Pi using gpt-5.6-sol

@vercel

vercel Bot commented Aug 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hunk-web Ignored Ignored Preview Aug 30, 2026 1:06am

Request Review

@greptile-apps

greptile-apps Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR centralizes strict runtime parsing for broker registration, snapshots, commands, daemon envelopes, capabilities, health data, and Hunk client responses. It also adds malformed-input, parser-failure, atomicity, and compatibility coverage.

  • Introduces a controller-owned parser registry shared by daemon and producer boundaries.
  • Validates command inputs and results before dispatch, state mutation, or request resolution.
  • Adds strict Hunk schemas for reload inputs, responses, capabilities, registrations, and snapshots.
  • Preserves redacted protocol failures across authenticated HTTP and WebSocket boundaries.

Confidence Score: 4/5

The PR should not merge until accepted dispatch deadlines and idempotency keys are either enforced end-to-end or rejected instead of being silently ignored.

The new public parser accepts execution-control fields, but the daemon omits them when dispatching, so callers can observe commands executing despite expired deadlines or duplicate idempotency keys.

Files Needing Attention: packages/session-broker/src/protocolParsers.ts, packages/session-broker/src/daemon.ts

Important Files Changed

Filename Overview
packages/session-broker-core/src/validation.ts Adds bounded primitives and stable redacted protocol errors for cross-process input validation.
packages/session-broker-core/src/brokerState.ts Validates command inputs/results and preserves existing session state when replacement parsing fails.
packages/session-broker/src/protocolParsers.ts Centralizes exact broker envelope parsing, but accepts deadline and idempotency controls that daemon dispatch ignores.
packages/session-broker/src/daemon.ts Routes validated HTTP and WebSocket envelopes through the parser registry while dropping two newly accepted dispatch controls.
packages/session-broker/src/connection.ts Parses daemon commands before bridge dispatch and validates application results before returning them.
src/session/broker/protocolParsers.ts Defines the fixed Hunk command parser registry and strict command-specific input/result contracts.
src/session/protocolSchemas.ts Adds strict request, response, capability, and reload-input schemas aligned with current Hunk protocol types.
src/session/agent/cliClient.ts Validates successful daemon responses instead of trusting asserted response types.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Daemon
    participant Registry as Protocol parser registry
    participant Broker
    participant App
    Caller->>Daemon: Authenticated JSON dispatch
    Daemon->>Registry: Parse structural request
    Registry-->>Daemon: Selector, command, input, controls
    Daemon->>Broker: Dispatch parsed command
    Broker->>Registry: Parse command input
    Broker->>App: Validated WebSocket command
    App->>Registry: Parse command result
    App-->>Broker: Validated result
    Broker-->>Daemon: Resolve pending request
    Daemon-->>Caller: Authenticated response
Loading
Prompt To Fix All With AI
### Issue 1
packages/session-broker/src/protocolParsers.ts:325-330
**Dispatch controls are discarded**

When an authenticated caller supplies an expired `deadline` or an `idempotencyKey`, this parser accepts both fields but the daemon omits them from `dispatchCommand`, causing expired requests to execute and duplicate requests to run independently without indicating that their controls were ignored.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(session): validate broker runtime bo..." | Re-trigger Greptile

Comment thread packages/session-broker/src/protocolParsers.ts Outdated
@benvinegar
benvinegar changed the base branch from security/session-broker-authentication to main August 29, 2026 22:07
@benvinegar
benvinegar force-pushed the security/session-broker-runtime-validation branch from bba569a to 3740446 Compare August 29, 2026 22:08
@benvinegar
benvinegar merged commit 52aa362 into main Aug 30, 2026
17 of 18 checks passed
@benvinegar
benvinegar deleted the security/session-broker-runtime-validation branch August 30, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant