feat(session-broker): authenticate local peers by benvinegar · Pull Request #932 · modem-dev/hunk

benvinegar · 2026-08-30T21:51:31Z

Stack

1 of 3 → #933 → #934. Merge bottom-to-top.

Problem

The reusable session broker accepted local producer and HTTP control claims without cryptographic connection authority, and several transport/resource edge cases could outlive revocation or shutdown.

Approach

add Ed25519 challenge/proof authentication for producers and callers;
bind grants, requests, responses, caller sessions, sequences, daemon generations, and exact targets in signed transcripts;
derive producer ownership from the authenticated connection and retire displaced/revoked authority before further work or disclosure;
coalesce caller negotiation and preserve unique replay sequences across concurrent requests;
enforce strict exact-object, UTF-8, body, response, queue, and source-plus-copy limits;
make Node and Bun adapter shutdown terminal and add real Node adapter CI coverage;
keep broker packages runtime-neutral and add only the minimal Hunk compatibility adapter required by the expanded controller contract.

This layer does not add Hunk credentials, daemon migration policy, browser-review authority, or install/upgrade acceptance coverage; those follow in #933 and #934.

Verification

bun run typecheck
package-focused Bun tests — 179 passed
bun run test:session-broker-node — 3 passed
bun run deps:check
bun run changeset:status
git diff --check

Known limitations

Native Windows owner/DACL and reparse-point validation, plus durable reconnect-key rotation, remain publication gates for the eventual public package.

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

vercel · 2026-08-30T21:51:32Z

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
hunk-web	Ignored	Preview	Aug 30, 2026 11:33pm

greptile-apps · 2026-08-30T21:55:17Z

Greptile Summary

This PR adds signed producer and caller authentication to the reusable session broker, including challenge/proof negotiation, replay-protected caller requests, reconnect ownership, and Node/Bun transport limits. It also adds adapter conformance coverage and CI execution for the real Node adapter.

Adds Ed25519 challenge, acknowledgement, request, and response transcript handling
Adds authenticated producer ownership replacement and retained reconnect state
Adds a caller client with bounded response verification and one-time renegotiation
Extends Node and Bun adapters with authentication admission timeouts
Updates broker limits, documentation, tests, fixtures, and CI

Confidence Score: 4/5

The reconnect-authority loss should be fixed before merging because it can permanently reject a valid reconnect-only producer after stale-session reconciliation.

Reconciliation removes the current owner before its close callback can retain reconnect identity, so the next authenticated connection is incorrectly authorized as a new registration; the audit-operation issue is non-blocking but should also be corrected.

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

Important Files Changed

Filename	Overview
packages/session-broker/src/daemon.ts	Adds producer and caller authentication orchestration; reconnect reconciliation drops bounded reconnect authority after stale retirement, and authentication-failure audits use the wrong operation.
packages/session-broker/src/clientAuthentication.ts	Adds strict caller negotiation, signed request sequencing, bounded response parsing, and response-signature verification without an accepted correctness finding.
packages/session-broker/src/authentication.ts	Strengthens producer authority rechecks and binds signed responses to caller sessions and sequences.
packages/session-broker/src/connection.ts	Adds producer-side hello negotiation, authentication-gated registration, and reconnect preparation.
packages/session-broker-core/src/brokerState.ts	Adds terminal shutdown state and atomic authenticated owner replacement with reservation transfer.
packages/session-broker-node/src/serve.ts	Adds handshake admission timeouts and tighter WebSocket message handling for Node.
packages/session-broker-bun/src/serve.ts	Adds handshake admission timeouts and native per-message payload bounds for Bun.

Sequence Diagram

sequenceDiagram
  participant Producer
  participant Adapter
  participant Daemon
  participant Authenticator
  participant Broker
  Producer->>Adapter: hello-init
  Adapter->>Daemon: hello-init
  Daemon->>Authenticator: issueChallenge
  Authenticator-->>Daemon: signed challenge
  Daemon-->>Producer: hello-challenge
  Producer->>Daemon: hello-proof
  Daemon->>Authenticator: verify proof
  Authenticator-->>Daemon: signed ack + retained authority
  Daemon-->>Producer: hello-ack
  Producer->>Daemon: register or reconnect
  Daemon->>Broker: registerSession
  Broker-->>Daemon: registered
  Daemon->>Adapter: markAuthenticated

Prompt To Fix All With AI

### Issue 1
packages/session-broker/src/daemon.ts:713-715
**Reconciliation Drops Reconnect Authority**

If a stale session is pruned while its authenticated producer socket remains open, reconciliation removes the owner before closing the socket, so the close callback cannot retain reconnect authority. A producer with only `reconnect` scope is then treated as a fresh registration and rejected, leaving the session unable to reconnect until credentials are reissued.

### Issue 2
packages/session-broker/src/daemon.ts:824
**Audit Records Mislabel Operations**

`handleAuthenticatedControl` always passes `list` to `authenticateRequest`, so authentication failures for dispatches and administrative controls are recorded as failed list operations. This makes security and operational audit records misleading.

---

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

_{Reviews (1): Last reviewed commit: "feat(session-broker): authenticate local..." | Re-trigger Greptile}

greptile-apps · 2026-08-30T21:55:20Z

+      this.producerOwners.delete(sessionId);
+      this.broker.unregisterConnection(owner.brokerPeer);
+      owner.connection.close?.(1000, "Session producer authority retired.");


Reconciliation Drops Reconnect Authority

If a stale session is pruned while its authenticated producer socket remains open, reconciliation removes the owner before closing the socket, so the close callback cannot retain reconnect authority. A producer with only reconnect scope is then treated as a fresh registration and rejected, leaving the session unable to reconnect until credentials are reissued.

Knowledge Base Used:

Broker service and packages

Session management

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/session-broker/src/daemon.ts Line: 713-715 Comment: **Reconciliation Drops Reconnect Authority** If a stale session is pruned while its authenticated producer socket remains open, reconciliation removes the owner before closing the socket, so the close callback cannot retain reconnect authority. A producer with only `reconnect` scope is then treated as a fresh registration and rejected, leaving the session unable to reconnect until credentials are reissued. **Knowledge Base Used:** - [Broker service and packages](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/broker-service-and-packages.md) - [Session management](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/session-management.md) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Valid. Reconciliation now revalidates the producer grant, retains the active binding before closing a stale owner, and clears the retired transport authentication before close. Revoked grants retain no reconnect authority. Added regression coverage for same-binding reconnect, a different binding, missing reconnect scope, and revocation.

_{Responded by Pi using gpt-5.6-sol.}

This comment was generated by Pi using gpt-5.6-sol

greptile-apps · 2026-08-30T21:55:20Z

+    },
+  ): Promise<Response> {
+    return this.handleBoundedControl(request, async (body) => {
+      const authenticated = await this.authenticateRequest(request, body, "list");


Audit Records Mislabel Operations

handleAuthenticatedControl always passes list to authenticateRequest, so authentication failures for dispatches and administrative controls are recorded as failed list operations. This makes security and operational audit records misleading.

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/session-broker/src/daemon.ts Line: 824 Comment: **Audit Records Mislabel Operations** `handleAuthenticatedControl` always passes `list` to `authenticateRequest`, so authentication failures for dispatches and administrative controls are recorded as failed list operations. This makes security and operational audit records misleading. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Valid. Authentication intentionally precedes parsing attacker-controlled payloads, so body-selected controls now audit pre-authentication failures as unknown instead of incorrectly claiming list. Routes with known semantics can supply an explicit label; capabilities uses diagnostics. Added coverage for generic, capabilities, and custom authenticated controls.

_{Responded by Pi using gpt-5.6-sol.}

This comment was generated by Pi using gpt-5.6-sol

benvinegar changed the title ~~feat/session broker authentication~~ feat(session-broker): authenticate local peers Aug 30, 2026

This was referenced Aug 30, 2026

feat(session): migrate incompatible daemons safely #933

Merged

test(install-vm): cover authenticated daemon upgrades #934

Merged

feat(session): secure daemon upgrades #929

Closed

greptile-apps Bot reviewed Aug 30, 2026

View reviewed changes

benvinegar force-pushed the feat/session-broker-authentication branch from 249bb0f to 9a26e93 Compare August 30, 2026 22:40

feat(session-broker): authenticate local peers

e4b283f

benvinegar force-pushed the feat/session-broker-authentication branch from 9a26e93 to e4b283f Compare August 30, 2026 23:33

benvinegar merged commit 62216eb into main Aug 30, 2026
13 checks passed

Navigation Menu

Uh oh!

feat(session-broker): authenticate local peers - #932

feat(session-broker): authenticate local peers#932
benvinegar merged 1 commit into
mainfrom
feat/session-broker-authentication

benvinegar commented Aug 30, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Aug 30, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Aug 30, 2026

Uh oh!

greptile-apps Bot Aug 30, 2026

Uh oh!

benvinegar Aug 30, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Aug 30, 2026

Uh oh!

benvinegar Aug 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

benvinegar commented Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stack

Problem

Approach

Verification

Known limitations

Uh oh!

vercel Bot commented Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Aug 30, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot Aug 30, 2026

Choose a reason for hiding this comment

Uh oh!

benvinegar Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Aug 30, 2026

Choose a reason for hiding this comment

Uh oh!

benvinegar Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

benvinegar commented Aug 30, 2026 •

edited

Loading

vercel Bot commented Aug 30, 2026 •

edited

Loading

benvinegar Aug 30, 2026 •

edited

Loading

benvinegar Aug 30, 2026 •

edited

Loading