Skip to content

feat(session): add signed broker authorization - #901

Merged
benvinegar merged 1 commit into
mainfrom
security/session-broker-authentication
Aug 29, 2026
Merged

benvinegar merged 1 commit into
mainfrom
security/session-broker-authentication

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • add fixed revision-1 Ed25519 challenge/proof authentication with canonical, domain-separated transcripts
  • add immutable app-scoped grants, exact command-version scopes, revocation/expiry checks, and default-deny app authorization
  • add replay-protected short-lived caller sessions and signed authenticated response envelopes
  • require explicit authentication and authorization before exposing raw generic broker control
  • keep Hunk credential discovery and activation for the final PR in this stack

Validation

  • 78 generic broker package tests
  • 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 29, 2026 9:29pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds revision-1 Ed25519 authentication primitives, immutable app-scoped grants, replay-protected caller sessions, signed response envelopes, and default-deny authorization for the generic broker HTTP API.

  • Adds canonical JSON and domain-separated signing transcripts.
  • Adds challenge/proof authentication, grant validation, revocation and expiry checks, and caller replay windows.
  • Requires explicit authentication and authorization configuration before exposing generic broker control.
  • Adds command-version scoping and updates Bun, Node, core, and daemon tests.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking indentation inconsistency across the newly added TypeScript implementation.

Authentication, replay protection, authorization, and signed-response paths are covered by focused tests, while the sole accepted concern is consistency with the repository's documented indentation convention.

Files Needing Attention: packages/session-broker/src/authentication.ts and other changed TypeScript implementation files

Important Files Changed

Filename Overview
packages/session-broker/src/authentication.ts Adds credential validation, signed challenge/proof handshakes, caller-session replay protection, and authenticated response signing; formatting conflicts with the applicable repository guide.
packages/session-broker/src/daemon.ts Gates the generic HTTP API behind authentication and app authorization, validates exact body bytes, and returns signed envelopes.
packages/session-broker-core/src/auth.ts Defines grants, principals, authorization checks, canonical transcripts, and the uint64 replay window.
packages/session-broker-core/src/canonicalJson.ts Adds bounded RFC 8785-style canonical JSON serialization for signature inputs.
packages/session-broker-core/src/brokerState.ts Adds validated command-version propagation to websocket command messages.
packages/session-broker/src/crypto.ts Adds canonical base64url helpers and WebCrypto-backed Ed25519 and SHA-256 operations.

Sequence Diagram

sequenceDiagram
  participant Caller
  participant Auth as Broker Authenticator
  participant Daemon
  participant App as App Authorizer
  participant Session
  Caller->>Auth: Signed hello challenge/proof
  Auth-->>Caller: Short-lived caller session
  Caller->>Daemon: Signed request and sequence
  Daemon->>Auth: Verify signature, grant, expiry, revocation, replay
  Auth-->>Daemon: Authenticated principal
  Daemon->>App: Authorize operation and command version
  App-->>Daemon: Allow or deny
  alt Allowed dispatch
    Daemon->>Session: Versioned command
    Session-->>Daemon: Command result
  end
  Daemon-->>Caller: Signed response envelope
Loading
Prompt To Fix All With AI
### Issue 1
packages/session-broker/src/authentication.ts:768-776
**Inconsistent TypeScript indentation**

The new authentication implementation uses two-space indentation throughout, conflicting with the repository's documented four-space TypeScript convention and establishing an inconsistent pattern across several substantial new modules.

---

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): add signed broker authori..." | Re-trigger Greptile

Comment on lines +768 to +776
private assertCallerSessionActive(callerSessionId: string, session: CallerSessionRecord): void {
if (this.callerSessions.get(callerSessionId) !== session) {
authenticationError("caller-session-expired");
}
this.requireActiveGrant(session.grant);
if (this.currentTime() >= session.expiresAt) {
this.callerSessions.delete(callerSessionId);
authenticationError("caller-session-expired");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Inconsistent TypeScript indentation

The new authentication implementation uses two-space indentation throughout, conflicting with the repository's documented four-space TypeScript convention and establishing an inconsistent pattern across several substantial new modules.

Context Used: guidelines.mdc Cursor rule (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/session-broker/src/authentication.ts
Line: 768-776

Comment:
**Inconsistent TypeScript indentation**

The new authentication implementation uses two-space indentation throughout, conflicting with the repository's documented four-space TypeScript convention and establishing an inconsistent pattern across several substantial new modules.

**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))

---

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@benvinegar
benvinegar force-pushed the security/session-broker-authentication branch from 0cb1a10 to d2a2336 Compare August 29, 2026 21:29
@benvinegar
benvinegar merged commit 5e46e7e into main Aug 29, 2026
12 checks passed
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