harden local auth - #1012
Conversation
Deploying with
|
| 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 |
Deploying with
|
| 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.
59a93c1 to
49dac9f
Compare
Cloudflare previewTorn down — the PR is closed. |
@executor-js/cli
@executor-js/config
@executor-js/execution
@executor-js/sdk
@executor-js/codemode-core
@executor-js/runtime-quickjs
@executor-js/plugin-file-secrets
@executor-js/plugin-graphql
@executor-js/plugin-keychain
@executor-js/plugin-mcp
@executor-js/plugin-onepassword
@executor-js/plugin-openapi
executor
commit: |
Greptile SummaryReplaces the Basic-auth (username/password) model on the local single-user server with a persistent bearer token stored in
Confidence Score: 3/5The 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
Sequence DiagramsequenceDiagram
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
|
| // 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, |
There was a problem hiding this comment.
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!
| 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" }, |
There was a problem hiding this comment.
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.
No description provided.