fix(cloud): MCP sync review — generic OAuth relay accepts only a loopback GET for its port (2-min TTL, never displaces a sign-in), first sync is an explicit consent before tokens leave this computer, no '.' on the image PATH

This commit is contained in:
Neil
2026-11-09 15:09:45 -08:00
parent 1c67472320
commit 53dced42c8
5 changed files with 111 additions and 8 deletions
@@ -170,8 +170,12 @@ else
fi
# The image's node/npm/npx live only on the build command's PATH; append them so agents (codex's
# `env node` shebang) and stdio MCP servers launched with npx resolve them in Orca's login shells.
system_node_bin="$(dirname "$(command -v npx || command -v node)")"
printf 'export PATH=%s/tools/bin:%s/bin:$PATH:%s\n' "$ORCA_HOME" "$ORCA_HOME" "$system_node_bin" |
system_node="$(command -v npx || command -v node || true)"
if [ -z "$system_node" ]; then
echo 'no node/npx on the build PATH' >&2
exit 1
fi
printf 'export PATH=%s/tools/bin:%s/bin:$PATH:%s\n' "$ORCA_HOME" "$ORCA_HOME" "$(dirname "$system_node")" |
sudo tee /etc/profile.d/orca.sh >/dev/null
# Every new shell (login or not: /etc/bashrc sources profile.d) re-reads the session env file.
sudo install -m 0644 "$ORCA_HOME/bin/orca-session-env.sh" /etc/profile.d/zz-orca-session-env.sh
@@ -23,7 +23,9 @@ const MAX_EXCLUDED = 512
export type { CloudMcpSyncPreferences }
const DEFAULT_PREFERENCES: CloudMcpSyncPreferences = { autoSync: true, excluded: [] }
// Off until the user syncs once by hand: MCP entries often carry tokens (env, headers), so nothing
// leaves this computer for a cloud sandbox without that first explicit Sync.
const DEFAULT_PREFERENCES: CloudMcpSyncPreferences = { autoSync: false, excluded: [] }
export type CloudMcpSyncDeps = {
userDataPath: string
@@ -93,7 +95,7 @@ export function createCloudMcpSync(deps: CloudMcpSyncDeps): CloudMcpSync {
const parsed: unknown = JSON.parse(readFileSync(preferencesPath, 'utf8'))
const excluded = isMcpRecord(parsed) ? parsed.excluded : undefined
preferences = {
autoSync: !isMcpRecord(parsed) || parsed.autoSync !== false,
autoSync: isMcpRecord(parsed) && parsed.autoSync === true,
excluded: Array.isArray(excluded)
? excluded
.filter((key): key is string => typeof key === 'string' && key.length <= 200)
@@ -245,6 +247,10 @@ export function createCloudMcpSync(deps: CloudMcpSyncDeps): CloudMcpSync {
if (trigger === 'connect' && !getPreferences().autoSync) {
return Promise.resolve(statuses.get(environmentId) ?? null)
}
if (trigger === 'manual' && !getPreferences().autoSync) {
// The first explicit Sync is the consent; later connects keep the sandbox current.
setPreferences({ autoSync: true })
}
// One sync per workspace at a time; a click during an auto-sync runs after it.
const next = (queues.get(environmentId) ?? Promise.resolve())
.catch(() => {})
@@ -5,6 +5,7 @@ import { BrowserNetworkTunnelSession } from '../../browser/network/tunnels/brows
import { BROWSER_NETWORK_TUNNEL_WINDOW_V2_FLOW_CONTROL } from '../../browser/network/tunnels/browser-network-tunnel-stream-state'
import { createAuthCallbackDialer } from '../../runtime/rpc/methods/accounts/auth-callback-tunnel'
import { AuthCallbackForward, type AuthCallbackTunnelOpener } from './auth-callback-forward'
import { isLoopbackCallbackRequest } from './loopback-callback-request'
// Past one flow-control window, so the reply only finishes if the forward returns credit.
const REPLY_BYTES = 600 * 1024
@@ -92,3 +93,36 @@ describe('AuthCallbackForward', () => {
expect(received).toBe(REPLY_BYTES)
})
})
describe('generic loopback relay request gate', () => {
const req = (lines: string[]) => Buffer.from(`${lines.join('\r\n')}\r\n\r\n`, 'latin1')
it('accepts a browser OAuth redirect to this loopback port', () => {
expect(
isLoopbackCallbackRequest(
req(['GET /callback?code=x&state=y HTTP/1.1', 'Host: localhost:8976']),
8976
)
).toBe(true)
expect(isLoopbackCallbackRequest(req(['GET /cb HTTP/1.1', 'host: 127.0.0.1:8976']), 8976)).toBe(
true
)
})
it('refuses DNS-rebinding hosts, other ports, non-GET and duplicate Host headers', () => {
expect(
isLoopbackCallbackRequest(req(['GET / HTTP/1.1', 'Host: evil.example:8976']), 8976)
).toBe(false)
expect(isLoopbackCallbackRequest(req(['GET / HTTP/1.1', 'Host: localhost:9999']), 8976)).toBe(
false
)
expect(isLoopbackCallbackRequest(req(['POST / HTTP/1.1', 'Host: localhost:8976']), 8976)).toBe(
false
)
expect(
isLoopbackCallbackRequest(
req(['GET / HTTP/1.1', 'Host: localhost:8976', 'Host: evil.example']),
8976
)
).toBe(false)
expect(isLoopbackCallbackRequest(Buffer.from('\x16\x03\x01binary'), 8976)).toBe(false)
})
})
@@ -1,3 +1,4 @@
import { GENERIC_RELAY_TTL_MS, isLoopbackCallbackRequest } from './loopback-callback-request'
import { createServer, type Server, type Socket } from 'node:net'
import { ipcMain } from 'electron'
import '../../../shared/remote-runtime/requests/remote-runtime-request-socket'
@@ -139,7 +140,12 @@ export class AuthCallbackForward {
readonly request: AuthCallbackForwardTarget,
private readonly onClosed: () => void
) {
this.ttl = setTimeout(() => this.close(), TTL_MS)
// A generic loopback relay (an MCP server's OAuth redirect, no sign-in flow) lives briefly:
// the browser hits it within seconds of the user approving.
this.ttl = setTimeout(
() => this.close(),
request.flowId === null ? GENERIC_RELAY_TTL_MS : TTL_MS
)
}
async start(openTunnel: AuthCallbackTunnelOpener): Promise<AuthCallbackForwardResult> {
@@ -196,7 +202,31 @@ export class AuthCallbackForward {
this.close()
}
}
if (this.request.flowId === null) {
// Generic relays carry only a browser's OAuth redirect: a GET whose Host is this loopback
// port. Anything else (DNS rebinding pages, other local processes probing the sandbox's
// services on that port) is refused before a byte reaches the sandbox.
socket.on('error', () => socket.destroy())
socket.once('data', (first: Buffer) => {
if (!isLoopbackCallbackRequest(first, this.request.port)) {
socket.end('HTTP/1.1 403 Forbidden\r\nContent-Length: 0\r\nConnection: close\r\n\r\n')
return
}
socket.pause()
socket.unshift(first)
this.pipeToHost(socket, count)
})
return
}
socket.pause()
this.pipeToHost(socket, count)
}
private pipeToHost(socket: Socket, count: (chunk: Buffer) => void): void {
if (!this.tunnel) {
socket.destroy()
return
}
this.tunnel.client.open({ host: '127.0.0.1', port: this.request.port }).then(
(remote) => {
if (socket.destroyed) {
@@ -256,11 +286,16 @@ export async function openAuthCallbackForward(
request: AuthCallbackForwardRequest | AuthCallbackForwardTarget
): Promise<AuthCallbackForwardResult> {
const target: AuthCallbackForwardTarget = { ...request, flowId: request.flowId ?? null }
// A retried sign-in replaces this computer's earlier listener on the same port.
// A retried sign-in replaces this computer's earlier listener on the same port, but a generic
// relay (a clicked MCP OAuth link) never displaces a sign-in in progress.
for (const existing of forwards.values()) {
if (existing.request.port === request.port) {
existing.close()
if (existing.request.port !== request.port) {
continue
}
if (target.flowId === null && existing.request.flowId !== null) {
return { ok: false, code: 'port_busy', message: 'A sign-in is using this port.' }
}
existing.close()
}
const key = keyOf(target)
const forward = new AuthCallbackForward(target, () => {
@@ -0,0 +1,24 @@
// The only traffic a generic loopback relay carries: a browser's OAuth redirect to this port.
/** A generic loopback relay (MCP OAuth redirect) lives briefly: the browser hits it within seconds. */
export const GENERIC_RELAY_TTL_MS = 2 * 60_000
const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]'])
/** First bytes of an HTTP GET whose Host header names this computer's loopback on `port`. */
export function isLoopbackCallbackRequest(first: Buffer, port: number): boolean {
const head = first.subarray(0, 8192).toString('latin1')
const end = head.indexOf('\r\n\r\n')
const lines = (end === -1 ? head : head.slice(0, end)).split('\r\n')
if (!/^GET \/\S* HTTP\/1\.[01]$/.test(lines[0] ?? '')) {
return false
}
const hosts = lines.filter((line) => /^host:/i.test(line))
if (hosts.length !== 1) {
return false
}
const value = hosts[0]!.slice(5).trim().toLowerCase()
const colon = value.lastIndexOf(':')
if (colon <= 0 || value.slice(colon + 1) !== String(port)) {
return false
}
return LOOPBACK_HOSTS.has(value.slice(0, colon))
}