Skip to content

harden local auth - #1012

Merged
RhysSullivan merged 4 commits into
mainfrom
claude/mystifying-pasteur-f2142b
Jun 14, 2026
Merged

harden local auth#1012
RhysSullivan merged 4 commits into
mainfrom
claude/mystifying-pasteur-f2142b

Conversation

@RhysSullivan

Copy link
Copy Markdown
Collaborator

No description provided.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 13, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
executor-marketing 49dac9f Commit Preview URL

Branch Preview URL
Jun 14 2026, 06:18 AM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 13, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
executor-cloud 49dac9f Jun 14 2026, 06:18 AM

…k rebase)

# Conflicts:
#	apps/local/src/mcp.ts
#	apps/local/src/serve.test.ts
#	apps/local/src/serve.ts
#	e2e/src/surfaces/browser.ts
#	packages/react/src/api/client.tsx
#	packages/react/src/routes/resume.$executionId.tsx
- apps/cli open: reference ChildProcess error handler structurally (bun-types'
  ChildProcess exposes unref() but not the inherited EventEmitter on()).
- Drop the approval_token query wiring from the MCP resume page + test: it was
  never minted or validated server-side. The /api/mcp-sessions/* endpoints are
  bearer-gated, which is the real guard for the resume POST.
@RhysSullivan
RhysSullivan force-pushed the claude/mystifying-pasteur-f2142b branch from 59a93c1 to 49dac9f Compare June 14, 2026 06:16
@RhysSullivan
RhysSullivan marked this pull request as ready for review June 14, 2026 06:16
@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Cloudflare preview

Torn down — the PR is closed.

@pkg-pr-new

pkg-pr-new Bot commented Jun 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

@executor-js/cli

npm i https://pkg.pr.new/@executor-js/cli@1012

@executor-js/config

npm i https://pkg.pr.new/@executor-js/config@1012

@executor-js/execution

npm i https://pkg.pr.new/@executor-js/execution@1012

@executor-js/sdk

npm i https://pkg.pr.new/@executor-js/sdk@1012

@executor-js/codemode-core

npm i https://pkg.pr.new/@executor-js/codemode-core@1012

@executor-js/runtime-quickjs

npm i https://pkg.pr.new/@executor-js/runtime-quickjs@1012

@executor-js/plugin-file-secrets

npm i https://pkg.pr.new/@executor-js/plugin-file-secrets@1012

@executor-js/plugin-graphql

npm i https://pkg.pr.new/@executor-js/plugin-graphql@1012

@executor-js/plugin-keychain

npm i https://pkg.pr.new/@executor-js/plugin-keychain@1012

@executor-js/plugin-mcp

npm i https://pkg.pr.new/@executor-js/plugin-mcp@1012

@executor-js/plugin-onepassword

npm i https://pkg.pr.new/@executor-js/plugin-onepassword@1012

@executor-js/plugin-openapi

npm i https://pkg.pr.new/@executor-js/plugin-openapi@1012

executor

npm i https://pkg.pr.new/executor@1012

commit: 49dac9f

@RhysSullivan
RhysSullivan merged commit 19c2657 into main Jun 14, 2026
14 checks passed
@greptile-apps

greptile-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown

Greptile Summary

Replaces the Basic-auth (username/password) model on the local single-user server with a persistent bearer token stored in ~/.executor/server-control/auth.json (mode 0600). Auth is now unconditional — loopback binds no longer bypass authentication — and the Host-header allowlist is removed in favour of a CORS-only origin filter, on the reasoning that a bearer token already defeats DNS-rebinding since the credential is never an ambient cookie.

  • New token lifecycle: auth.ts mints a 32-byte base64url token once and reuses it across restarts so MCP configs stay valid; rotateLocalAuthToken and executor server rotate-token let users cycle it. The CLI prints /?_token=<token> on startup; executor open re-opens the browser from the manifest's stored token.
  • Web SPA bootstrap: local-auth.tsx reads ?_token from the URL (strips and persists to localStorage) so standalone-web users are auto-authenticated on the first open; a LocalAuthGate form catches 401 regressions without crashing the app.
  • Electron scoping: installBearerAuthHeader now gates injection to the app's own mainWindow webContents, preventing OAuth popup windows from riding the ambient bearer as a CSRF vector.
  • Bug fixes included: resume.$executionId.tsx and oauth-popup.ts now attach the bearer to the previously unauthenticated /api/mcp-sessions/* and /api/oauth/await/* calls; a new /api/health endpoint serves as an unauthenticated liveness probe so the CLI never forwards a credential during reachability checks.

Confidence Score: 3/5

The core bearer model is sound and well-tested, but the post-daemon-startup credential handoff in the CLI has a silent no-auth fallback that would cause hard failures in edge cases.

The auth infrastructure itself (token minting, storage permissions, timing-safe comparison, Electron scoping, CORS hardening) is careful and correct, with good unit and e2e coverage. The one gap is in resolveExecutorServerConnection: after ensureDaemon returns, the code reads the manifest to recover the bearer, but the fallback when that read fails silently returns a connection with no credentials, causing 401 failures with no diagnostic.

apps/cli/src/main.ts — specifically the resolveExecutorServerConnection fallback after ensureDaemon.

Important Files Changed

Filename Overview
apps/local/src/auth.ts New file: mints/persists a 32-byte base64url bearer token in ~/.executor/server-control/auth.json (mode 0600); uses chmodSync after write to cover overwrites of pre-existing world-readable files.
apps/local/src/identity.ts Replaces the always-succeed identity layer with a real gate: validates Authorization: Bearer via timing-safe safeEqual; exempts /oauth/callback (state-gated).
apps/local/src/serve.ts Auth is now unconditional (no loopback bypass). Adds /api/health as an unauthenticated liveness probe; gates /api and /mcp on the bearer; drops Host-header allowlist in favour of CORS-only origin filtering.
apps/cli/src/main.ts Major CLI overhaul: removes Basic-auth env vars, adds executor open and server rotate-token commands, prints ?_token= URL, reads manifest after ensureDaemon to pick up the bearer. Silent fallback when manifest read fails returns a connection without credentials (P1 gap).
apps/desktop/src/main/index.ts Switches from Basic to Bearer header injection at the Electron session layer; adds fromOtherWebContents scoping so OAuth popups don't receive the ambient credential. A comment about the popup path is now inaccurate after the scoping change.
packages/react/src/api/local-auth.tsx New file: bootstraps the local bearer token from ?_token URL param or localStorage; provides LocalAuthGate component that gates the app behind a token-entry form on 401. No-ops on desktop.
packages/react/src/routes/resume.$executionId.tsx Fixes both the GET and POST calls to /api/mcp-sessions/* to include the bearer header, resolving the bug where standalone-web approval pages 401'd.
e2e/desktop/local-auth-mcp.test.ts New desktop e2e: scenario A drives the full bearer injection path through real Electron + sidecar; scenario B verifies the popup webContents scoping confirms its requests get 401.

Sequence Diagram

sequenceDiagram
    participant CLI as executor CLI
    participant auth.json
    participant Daemon as Local Server (Bun)
    participant Browser
    participant Electron as Electron Main

    CLI->>auth.json: loadOrMintLocalAuthToken()
    auth.json-->>CLI: token (existing or new, 0600)
    CLI->>Daemon: "startServer({ authToken: token })"
    Daemon-->>CLI: "ServerInstance { port, authToken }"
    CLI->>CLI: "write manifest (server.json, 0600, auth.bearer=token)"
    CLI->>CLI: "print Open URL with ?_token=TOKEN"

    Browser->>Daemon: "GET /?_token=TOKEN"
    Daemon-->>Browser: index.html (unauthenticated SPA shell)
    Browser->>Browser: bootstrapLocalAuthToken() strips URL, persists to localStorage

    Browser->>Daemon: GET /api/health (no auth)
    Daemon-->>Browser: 200 ok

    Browser->>Daemon: GET /api/scope Authorization: Bearer TOKEN
    Daemon->>Daemon: safeEqual(presented, bootToken)
    Daemon-->>Browser: 200 authenticated

    Browser->>Daemon: GET /api/scope (no auth)
    Daemon-->>Browser: 401 Unauthorized
    Browser->>Browser: notifyLocalAuthRequired() shows LocalAuthGate

    Note over Electron,Daemon: Desktop path bearer injected at session layer
    Electron->>auth.json: loadOrMintLocalAuthToken(dataDir)
    auth.json-->>Electron: token
    Electron->>Daemon: "spawn sidecar EXECUTOR_AUTH_TOKEN=token"
    Electron->>Electron: installBearerAuthHeader(origin, token)
    Note over Electron: onBeforeSendHeaders scoped to mainWindow.webContents.id
    Electron->>Daemon: GET /api/scope session injects Authorization: Bearer TOKEN
    Daemon-->>Electron: 200
Loading

Comments Outside Diff (1)

  1. apps/cli/src/main.ts, line 562-584 (link)

    P1 Silent no-auth fallback after daemon startup

    When readActiveLocalServerManifest() returns null (manifest unreadable, health check miss) or when startedOrigin !== daemonOrigin (e.g. normalization edge), the code falls through to a connection with no credentials. The inline comment explicitly names the consequence: "otherwise the next API call hits the now-gated server with no credential and 401s." Under the old model a local loopback connection worked credential-free; now auth is unconditional, so this fallback silently turns any such edge case into a hard CLI failure. No error is raised or logged when the fallback branch is taken, so the caller gets a 401 from the first API call with no signal about why.

Reviews (1): Last reviewed commit: "local: fix CLI open typecheck + drop unw..." | Re-trigger Greptile

Comment on lines 240 to 246
// No preload, no nodeIntegration — popup loads third-party
// OAuth provider pages, then a final navigation back to
// 127.0.0.1:<port>/oauth/callback which the session-level
// Basic auth header injection (installBasicAuthHeader)
// bearer header injection (installBearerAuthHeader)
// catches automatically. The popup never needs the
// executor IPC bridge.
contextIsolation: true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Misleading comment about popup bearer injection

The comment says the popup's final navigation back to /oauth/callback is caught by installBearerAuthHeader automatically. With the new fromOtherWebContents scoping check, the popup's webContentsId will NOT equal mainWindow.webContents.id, so the bearer is deliberately withheld from the popup. The callback works because the server exempts /api/oauth/callback from auth (state-gated), not because of header injection — the two mechanisms are now correctly separate. The comment should be updated to reflect that.

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!

Comment on lines +120 to +133
const resumedStructured = resumed.structuredContent as { status: string };
expect(
resumedStructured.status,
"the MCP resume completed once the browser approved (bearer reached the gated endpoint)",
).toBe("completed");

await step("The approval endpoint rejects a request with no bearer", async () => {
const unauthed = await fetch(
`${server.origin}/api/mcp-sessions/${encodeURIComponent(
paused.executionId,
)}/executions/${encodeURIComponent(paused.executionId)}/resume?approval_token=x`,
{
method: "POST",
headers: { "content-type": "application/json" },

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 executionId used as mcpSessionId in negative test URL

The constructed path is /api/mcp-sessions/:mcpSessionId/executions/:executionId/resume, but paused.executionId is substituted for both parameters. The MCP session ID is a distinct value. Because the 401 auth gate fires before the router parses the path parameters, this passes today, but it creates a misleading path that could let a future routing regression on the session-ID segment go undetected.

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