diff --git a/packages/junior-github/src/credential-support.ts b/packages/junior-github/src/credential-support.ts index 5333425bdd..a964d3a77c 100644 --- a/packages/junior-github/src/credential-support.ts +++ b/packages/junior-github/src/credential-support.ts @@ -114,6 +114,18 @@ export const GITHUB_APP_PRIVATE_KEY_ENV = "GITHUB_APP_PRIVATE_KEY"; export const GITHUB_INSTALLATION_ID_ENV = "GITHUB_INSTALLATION_ID"; export const GITHUB_AUTH_TOKEN_ENV = "GITHUB_TOKEN"; export const GITHUB_AUTH_TOKEN_PLACEHOLDER = "ghp_host_managed_credential"; +/** + * Git config that makes git send the token placeholder to github.com. + * + * Git sends no auth header by itself, and sandbox egress adds credentials only + * to requests that carry a placeholder. + */ +export const GITHUB_GIT_PLACEHOLDER_HEADER = { + key: "http.https://github.com/.extraHeader", + value: `Authorization: Basic ${Buffer.from( + `x-access-token:${GITHUB_AUTH_TOKEN_PLACEHOLDER}`, + ).toString("base64")}`, +}; const MAX_LEASE_MS = 60 * 60 * 1000; const REFRESH_BUFFER_MS = 5 * 60 * 1000; const USER_REFRESH_TIMEOUT_MS = 20_000; @@ -457,7 +469,9 @@ function createCredentialLease( type: "lease", lease: { ...(input.account ? { account: input.account } : undefined), - ...(input.authorization ? { authorization: input.authorization } : undefined), + ...(input.authorization + ? { authorization: input.authorization } + : undefined), expiresAt: new Date(input.expiresAtMs).toISOString(), headerTransforms: ( input.domains ?? diff --git a/packages/junior-github/src/plugin.ts b/packages/junior-github/src/plugin.ts index 42a1a817d8..ca22341b92 100644 --- a/packages/junior-github/src/plugin.ts +++ b/packages/junior-github/src/plugin.ts @@ -68,6 +68,7 @@ import { GITHUB_APP_PRIVATE_KEY_ENV, GITHUB_AUTH_TOKEN_ENV, GITHUB_AUTH_TOKEN_PLACEHOLDER, + GITHUB_GIT_PLACEHOLDER_HEADER, GITHUB_GRAPHQL_RESPONSE_BODY_LIMIT_BYTES, GITHUB_INSTALLATION_ID_ENV, USER_TOKEN_GRANTS, @@ -387,6 +388,11 @@ export function githubPlugin( await configureGit(ctx, "commit.gpgsign", "false"); await configureGit(ctx, "credential.helper", ""); await configureGit(ctx, "http.emptyAuth", "true"); + await configureGit( + ctx, + GITHUB_GIT_PLACEHOLDER_HEADER.key, + GITHUB_GIT_PLACEHOLDER_HEADER.value, + ); }, async beforeToolExecute(ctx) { if (ctx.tool.name !== "bash") { diff --git a/packages/junior-github/src/workspace-prepare.ts b/packages/junior-github/src/workspace-prepare.ts index bb26cc71d3..11ba24183f 100644 --- a/packages/junior-github/src/workspace-prepare.ts +++ b/packages/junior-github/src/workspace-prepare.ts @@ -2,6 +2,7 @@ import type { WorkspaceFinalize, WorkspacePrepareHookContext, } from "@sentry/junior-plugin-api"; +import { GITHUB_GIT_PLACEHOLDER_HEADER } from "./credential-support.js"; import { isReservedSandboxDirectory } from "./sandbox-paths.js"; /** Clone missing GitHub repositories or refresh existing Workspace checkouts. */ @@ -43,6 +44,12 @@ export async function prepareWorkspace( GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_NOSYSTEM: "1", }; + // Git sends no auth by itself. The placeholder header asks egress to add + // GitHub credentials for clone and fetch. + const gitAuthConfig = [ + "-c", + `${GITHUB_GIT_PLACEHOLDER_HEADER.key}=${GITHUB_GIT_PLACEHOLDER_HEADER.value}`, + ]; const finalize: Array<{ path: string; repo: string; target: string }> = []; for (const { owner, name, path, repo } of repos) { @@ -124,6 +131,7 @@ export async function prepareWorkspace( args: [ "-c", "credential.helper=", + ...gitAuthConfig, "-C", path, "fetch", @@ -163,7 +171,7 @@ export async function prepareWorkspace( // merge-base, branch switches, and later refreshes. const result = await ctx.sandbox.run({ cmd: "git", - args: ["clone", "--quiet", "--", cloneUrl, path], + args: [...gitAuthConfig, "clone", "--quiet", "--", cloneUrl, path], cwd: ctx.sandbox.root, env: gitEnv, }); diff --git a/packages/junior-github/tests/github-plugin.test.ts b/packages/junior-github/tests/github-plugin.test.ts index f5a8a74a70..5b062fcf9b 100644 --- a/packages/junior-github/tests/github-plugin.test.ts +++ b/packages/junior-github/tests/github-plugin.test.ts @@ -19,6 +19,11 @@ import { mswServer } from "./msw"; const ORIGINAL_ENV = { ...process.env }; const db = {}; +// Git sends the GitHub token placeholder so sandbox egress adds credentials. +const GIT_AUTH_CONFIG = [ + "-c", + "http.https://github.com/.extraHeader=Authorization: Basic eC1hY2Nlc3MtdG9rZW46Z2hwX2hvc3RfbWFuYWdlZF9jcmVkZW50aWFs", +]; class TestTokenStore { private refreshLock = Promise.resolve(); @@ -3334,6 +3339,7 @@ Conversation: \`local:test:old-conversation\` "commit.gpgsign", "credential.helper", "http.emptyAuth", + "http.https://github.com/.extraHeader", ]); }); @@ -3372,6 +3378,7 @@ Conversation: \`local:test:old-conversation\` ["-C", "repos/sentry", "rev-parse", "--is-inside-work-tree"], ["-rf", "--", "repos/sentry"], [ + ...GIT_AUTH_CONFIG, "clone", "--quiet", "--", @@ -3382,6 +3389,7 @@ Conversation: \`local:test:old-conversation\` ["-C", "repos/junior", "rev-parse", "--is-inside-work-tree"], ["-rf", "--", "repos/junior"], [ + ...GIT_AUTH_CONFIG, "clone", "--quiet", "--", @@ -3461,6 +3469,7 @@ Conversation: \`local:test:old-conversation\` [ "-c", "credential.helper=", + ...GIT_AUTH_CONFIG, "-C", "repos/junior", "fetch", @@ -3473,7 +3482,7 @@ Conversation: \`local:test:old-conversation\` ["-C", "repos/junior", "reset", "--hard", "refs/remotes/origin/stable"], ["-C", "repos/junior", "clean", "-fd"], ]); - expect(runs.some((args) => args[0] === "clone")).toBe(false); + expect(runs.some((args) => args.includes("clone"))).toBe(false); }); it("refreshes a valid checkout without an upstream and preserves setup outputs", async () => { @@ -3604,6 +3613,7 @@ Conversation: \`local:test:old-conversation\` ["-C", "repos/junior", "rev-parse", "--is-inside-work-tree"], ["-rf", "--", "repos/junior"], [ + ...GIT_AUTH_CONFIG, "clone", "--quiet", "--", @@ -3640,7 +3650,7 @@ Conversation: \`local:test:old-conversation\` if (input.args?.includes("--symbolic-full-name")) { return { exitCode: 1, stderr: "", stdout: "" }; } - if (input.args?.[0] === "clone") { + if (input.args?.includes("clone")) { cloneAttempts += 1; if (cloneAttempts === 1) { return { exitCode: 130, stderr: "interrupted", stdout: "" }; diff --git a/packages/junior-tts/skills/tts/scripts/speech.mjs b/packages/junior-tts/skills/tts/scripts/speech.mjs index f2be7ed4b0..84b8fab95d 100644 --- a/packages/junior-tts/skills/tts/scripts/speech.mjs +++ b/packages/junior-tts/skills/tts/scripts/speech.mjs @@ -58,12 +58,14 @@ export async function generateSpeech(text, output, delivery = "playful") { const file = await open(path, "wx", 0o600); let completed = false; try { - // The plugin's host proxy supplies Authorization. Never read a key here. + // The placeholder asks the host proxy to supply the real Authorization. + // Never read a key here. const response = await fetch(ENDPOINT, { method: "POST", redirect: "error", signal: AbortSignal.timeout(120_000), headers: { + Authorization: "Bearer host_managed_credential", "Content-Type": "application/json", "ai-gateway-protocol-version": "0.0.1", "ai-speech-model-specification-version": "4", diff --git a/packages/junior-tts/tests/speech.test.mjs b/packages/junior-tts/tests/speech.test.mjs index 51dcea5b8b..34c6cc71cc 100644 --- a/packages/junior-tts/tests/speech.test.mjs +++ b/packages/junior-tts/tests/speech.test.mjs @@ -38,7 +38,10 @@ test("Gateway request keeps style separate and writes WAV without a Sandbox key" const fetch = t.mock.method(globalThis, "fetch", async (url, init) => { assert.equal(url, "https://ai-gateway.vercel.sh/v4/ai/speech-model"); assert.equal(init.redirect, "error"); - assert.equal(new Headers(init.headers).has("authorization"), false); + assert.equal( + new Headers(init.headers).get("authorization"), + "Bearer host_managed_credential", + ); assert.equal(init.headers["ai-model-id"], "google/gemini-3.8-flash-tts"); assert.equal(init.headers["ai-speech-model-specification-version"], "4"); const body = JSON.parse(init.body); diff --git a/packages/junior/src/chat/egress/credentialed.ts b/packages/junior/src/chat/egress/credentialed.ts index 92558eb861..5f5b5bb277 100644 --- a/packages/junior/src/chat/egress/credentialed.ts +++ b/packages/junior/src/chat/egress/credentialed.ts @@ -1,6 +1,9 @@ import { logException, logInfo, logWarn } from "@/chat/logging"; import { onPluginEgressResponse } from "@/chat/plugins/credential-hooks"; -import { matchesSandboxEgressDomain } from "@/chat/sandbox/egress/policy"; +import { + egressRequestHeaders, + egressResponseHeaders, +} from "@/chat/egress/headers"; import { hasSandboxEgressLeaseTransformForHost, sandboxEgressCredentialLease, @@ -16,10 +19,7 @@ import { type SandboxEgressCredentialContext, type SandboxEgressCredentialLease, } from "@/chat/sandbox/egress/session"; -import { - shouldPropagateSandboxEgressTrace, - type SandboxEgressTracePropagationConfig, -} from "@/chat/sandbox/egress/tracing"; +import type { SandboxEgressTracePropagationConfig } from "@/chat/sandbox/egress/tracing"; import { EgressAuthRequired, EgressPolicyDenied, @@ -39,33 +39,6 @@ import { // persistence and auth/permission signal storage stay in their caller-specific // adapters until a second host/plugin caller needs a different storage surface. -const HOP_BY_HOP_HEADERS = new Set([ - "connection", - "host", - "keep-alive", - "proxy-authenticate", - "proxy-authorization", - "te", - "trailer", - "transfer-encoding", - "upgrade", -]); -const PROXY_ONLY_HEADERS = new Set([ - "vercel-sandbox-oidc-token", - "vercel-forwarded-host", - "vercel-forwarded-scheme", - "vercel-forwarded-port", - "vercel-forwarded-path", -]); -const TRACE_PROPAGATION_HEADERS = new Set([ - "baggage", - "sentry-trace", - "traceparent", -]); -const DECODED_RESPONSE_HEADERS = new Set([ - "content-encoding", - "content-length", -]); const UPSTREAM_TOKEN_REJECTION_STATUS = 401; const UPSTREAM_PERMISSION_REJECTION_STATUS = 403; const GRANT_SELECTION_BODY_TEXT_LIMIT_BYTES = 64 * 1024; @@ -170,15 +143,23 @@ function egressAttributes(input: { status?: number; }): Record { return { - ...(input.egressId ? { "app.sandbox.egress_id": input.egressId } : undefined), + ...(input.egressId + ? { "app.sandbox.egress_id": input.egressId } + : undefined), ...(input.provider ? { "app.provider.name": input.provider } : undefined), ...(input.grantName ? { "app.grant.name": input.grantName } : undefined), - ...(input.grantAccess ? { "app.grant.access": input.grantAccess } : undefined), - ...(input.grantReason ? { "app.grant.reason": input.grantReason } : undefined), + ...(input.grantAccess + ? { "app.grant.access": input.grantAccess } + : undefined), + ...(input.grantReason + ? { "app.grant.reason": input.grantReason } + : undefined), ...(input.host ? { "server.address": input.host } : undefined), ...(input.method ? { "http.request.method": input.method } : undefined), ...(input.path ? { "url.path": input.path } : undefined), - ...(input.status ? { "http.response.status_code": input.status } : undefined), + ...(input.status + ? { "http.response.status_code": input.status } + : undefined), }; } @@ -459,70 +440,6 @@ async function responseTextWithinLimit( return new TextDecoder().decode(combined); } -/** - * Build the upstream request headers. - * - * Sandbox/proxy-only headers are stripped so they cannot leak to providers, and - * trace headers are preserved only for hosts opted into trace propagation. - * Credential lease transforms win last because they are the host-issued - * authority for provider auth headers. - */ -function requestHeaders( - request: Request, - lease: SandboxEgressCredentialLease, - upstreamHost: string, - tracePropagation: SandboxEgressTracePropagationConfig, -): Headers { - const headers = new Headers(); - const mayPropagateTrace = shouldPropagateSandboxEgressTrace( - upstreamHost, - tracePropagation, - ); - request.headers.forEach((value, key) => { - const normalized = key.toLowerCase(); - if ( - HOP_BY_HOP_HEADERS.has(normalized) || - PROXY_ONLY_HEADERS.has(normalized) - ) { - return; - } - if (TRACE_PROPAGATION_HEADERS.has(normalized) && !mayPropagateTrace) { - return; - } - headers.append(key, value); - }); - - for (const transform of lease.headerTransforms) { - if (!matchesSandboxEgressDomain(upstreamHost, transform.domain)) { - continue; - } - for (const [key, value] of Object.entries(transform.headers)) { - if ( - TRACE_PROPAGATION_HEADERS.has(key.toLowerCase()) && - !mayPropagateTrace - ) { - continue; - } - headers.set(key, value); - } - } - return headers; -} - -function responseHeaders(upstream: Response): Headers { - const headers = new Headers(); - upstream.headers.forEach((value, key) => { - const normalized = key.toLowerCase(); - if ( - !HOP_BY_HOP_HEADERS.has(normalized) && - !DECODED_RESPONSE_HEADERS.has(normalized) - ) { - headers.append(key, value); - } - }); - return headers; -} - function leaseLogAttributes(input: { egressId: string; lease: SandboxEgressCredentialLease; @@ -563,7 +480,9 @@ async function recordSandboxAuthRequired(input: { provider: input.provider, grant: input.grant, kind: input.kind ?? "auth_required", - ...(input.authorization ? { authorization: input.authorization } : undefined), + ...(input.authorization + ? { authorization: input.authorization } + : undefined), message: input.message, }); } @@ -591,6 +510,86 @@ async function recordSandboxPermissionDenied(input: { }); } +/** Read the request body once, or return the Junior error for an unreadable body. */ +async function readEgressRequestBody(input: { + activeEgressId: string; + provider: string; + request: Request; + upstreamUrl: URL; +}): Promise { + const { activeEgressId, provider, request, upstreamUrl } = input; + try { + return await requestBodyBytes(request); + } catch (error) { + if (!(error instanceof TypeError)) { + throw error; + } + return unreadableBodyResponse( + logException(error, "sandbox.egress.request_body.unreadable", { + ...egressAttributes({ + egressId: activeEgressId, + host: upstreamUrl.hostname, + method: request.method, + path: upstreamUrl.pathname, + provider, + status: 500, + }), + ...routingAttributes(request, upstreamUrl), + }), + ); + } +} + +/** + * Forward one verified sandbox egress request without provider credentials. + * + * Use this for requests that do not carry a credential placeholder, such as + * browser traffic. The request keeps its own auth headers and cookies. No grant + * is selected and no credential lease is issued. + */ +export async function forwardEgressRequestWithoutCredentials(input: { + activeEgressId: string; + deps: Pick; + provider: string; + request: Request; + upstreamUrl: URL; +}): Promise { + const { activeEgressId, deps, provider, request, upstreamUrl } = input; + const body = await readEgressRequestBody(input); + if (body instanceof Response) { + return body; + } + const upstream = await (deps.fetch ?? fetch)(upstreamUrl, { + method: request.method, + headers: egressRequestHeaders( + request, + undefined, + upstreamUrl.hostname, + deps.tracePropagation ?? {}, + ), + ...(body !== undefined ? { body } : undefined), + redirect: "manual", + }); + if (shouldLogSandboxEgressInfo()) { + logInfo("sandbox.egress.upstream.passthrough", { + ...egressAttributes({ + egressId: activeEgressId, + host: upstreamUrl.hostname, + method: request.method, + path: upstreamUrl.pathname, + provider, + status: upstream.status, + }), + ...routingAttributes(request, upstreamUrl), + }); + } + return new Response(upstream.body, { + status: upstream.status, + statusText: upstream.statusText, + headers: egressResponseHeaders(upstream), + }); +} + /** * Forward one verified sandbox egress request with host-managed credentials. * @@ -618,26 +617,9 @@ export async function executeCredentialedEgressRequest(input: { request, upstreamUrl, } = input; - let body: ArrayBuffer | undefined; - try { - body = await requestBodyBytes(request); - } catch (error) { - if (!(error instanceof TypeError)) { - throw error; - } - return unreadableBodyResponse( - logException(error, "sandbox.egress.request_body.unreadable", { - ...egressAttributes({ - egressId: activeEgressId, - host: upstreamUrl.hostname, - method: request.method, - path: upstreamUrl.pathname, - provider, - status: 500, - }), - ...routingAttributes(request, upstreamUrl), - }), - ); + const body = await readEgressRequestBody(input); + if (body instanceof Response) { + return body; } const bodyForGrantSelection = githubBodyInspection({ provider, @@ -687,7 +669,9 @@ export async function executeCredentialedEgressRequest(input: { const recordPermissionDenied = deps.recordPermissionDenied ?? recordSandboxPermissionDenied; - async function resolveLease(): Promise { + async function resolveLease(): Promise< + SandboxEgressCredentialLease | Response + > { try { return await issueCredentialLease( provider, @@ -773,7 +757,7 @@ export async function executeCredentialedEgressRequest(input: { let retriedAfter403 = false; while (true) { - const headers = requestHeaders( + const headers = egressRequestHeaders( request, lease, upstreamUrl.hostname, @@ -792,8 +776,7 @@ export async function executeCredentialedEgressRequest(input: { return intercepted; } - const requestBody = - body instanceof ArrayBuffer ? body.slice(0) : body; + const requestBody = body instanceof ArrayBuffer ? body.slice(0) : body; const upstream = await fetchImpl(upstreamUrl, { method: request.method, headers, @@ -918,7 +901,7 @@ export async function executeCredentialedEgressRequest(input: { return new Response(upstream.body, { status: upstream.status, statusText: upstream.statusText, - headers: responseHeaders(upstream), + headers: egressResponseHeaders(upstream), }); } @@ -939,7 +922,7 @@ export async function executeCredentialedEgressRequest(input: { return new Response(upstream.body, { status: upstream.status, statusText: upstream.statusText, - headers: responseHeaders(upstream), + headers: egressResponseHeaders(upstream), }); } } diff --git a/packages/junior/src/chat/egress/headers.ts b/packages/junior/src/chat/egress/headers.ts new file mode 100644 index 0000000000..8642fe5996 --- /dev/null +++ b/packages/junior/src/chat/egress/headers.ts @@ -0,0 +1,102 @@ +import { matchesSandboxEgressDomain } from "@/chat/sandbox/egress/policy"; +import type { SandboxEgressCredentialLease } from "@/chat/sandbox/egress/session"; +import { + shouldPropagateSandboxEgressTrace, + type SandboxEgressTracePropagationConfig, +} from "@/chat/sandbox/egress/tracing"; + +// Module overview: build the headers for a forwarded provider request and its +// response. Callers decide whether a credential lease applies. + +const HOP_BY_HOP_HEADERS = new Set([ + "connection", + "host", + "keep-alive", + "proxy-authenticate", + "proxy-authorization", + "te", + "trailer", + "transfer-encoding", + "upgrade", +]); +const PROXY_ONLY_HEADERS = new Set([ + "vercel-sandbox-oidc-token", + "vercel-forwarded-host", + "vercel-forwarded-scheme", + "vercel-forwarded-port", + "vercel-forwarded-path", +]); +const TRACE_PROPAGATION_HEADERS = new Set([ + "baggage", + "sentry-trace", + "traceparent", +]); +const DECODED_RESPONSE_HEADERS = new Set([ + "content-encoding", + "content-length", +]); + +/** + * Build the upstream request headers. + * + * Sandbox/proxy-only headers are stripped so they cannot leak to providers, and + * trace headers are preserved only for hosts opted into trace propagation. + * Credential lease transforms win last because they are the host-issued + * authority for provider auth headers. + */ +export function egressRequestHeaders( + request: Request, + lease: SandboxEgressCredentialLease | undefined, + upstreamHost: string, + tracePropagation: SandboxEgressTracePropagationConfig, +): Headers { + const headers = new Headers(); + const mayPropagateTrace = shouldPropagateSandboxEgressTrace( + upstreamHost, + tracePropagation, + ); + request.headers.forEach((value, key) => { + const normalized = key.toLowerCase(); + if ( + HOP_BY_HOP_HEADERS.has(normalized) || + PROXY_ONLY_HEADERS.has(normalized) + ) { + return; + } + if (TRACE_PROPAGATION_HEADERS.has(normalized) && !mayPropagateTrace) { + return; + } + headers.append(key, value); + }); + + for (const transform of lease?.headerTransforms ?? []) { + if (!matchesSandboxEgressDomain(upstreamHost, transform.domain)) { + continue; + } + for (const [key, value] of Object.entries(transform.headers)) { + if ( + TRACE_PROPAGATION_HEADERS.has(key.toLowerCase()) && + !mayPropagateTrace + ) { + continue; + } + headers.set(key, value); + } + } + return headers; +} + +/** Copy upstream response headers that are safe to return to the sandbox. */ +export function egressResponseHeaders(upstream: Response): Headers { + const headers = new Headers(); + upstream.headers.forEach((value, key) => { + const normalized = key.toLowerCase(); + if ( + !HOP_BY_HOP_HEADERS.has(normalized) && + !DECODED_RESPONSE_HEADERS.has(normalized) + ) { + headers.append(key, value); + } + }); + return headers; +} diff --git a/packages/junior/src/chat/plugins/auth/auth-token-placeholder.ts b/packages/junior/src/chat/plugins/auth/auth-token-placeholder.ts index ef53bac242..5eba3e387a 100644 --- a/packages/junior/src/chat/plugins/auth/auth-token-placeholder.ts +++ b/packages/junior/src/chat/plugins/auth/auth-token-placeholder.ts @@ -1,7 +1,16 @@ import type { OAuthBearerCredentials } from "../types"; +/** + * Marks a sandbox credential placeholder. + * + * Every sandbox credential placeholder contains this text. Sandbox egress adds + * provider credentials only to requests that carry it, so other clients in the + * sandbox, such as a browser, keep their own login. + */ +export const HOST_MANAGED_CREDENTIAL_MARKER = "host_managed_credential"; + const DEFAULT_PLACEHOLDERS: Record = { - "oauth-bearer": "host_managed_credential", + "oauth-bearer": HOST_MANAGED_CREDENTIAL_MARKER, }; /** Resolve the non-secret sandbox token placeholder for token-backed credentials. */ @@ -13,3 +22,39 @@ export function resolveAuthTokenPlaceholder( DEFAULT_PLACEHOLDERS[credentials.type] ); } + +function decodedBasicCredentials(value: string): string | undefined { + const match = /^basic\s+(\S+)\s*$/i.exec(value); + if (!match) { + return undefined; + } + try { + return Buffer.from(match[1]!, "base64").toString("utf8"); + } catch { + return undefined; + } +} + +/** + * Return whether a sandbox request asks the host to add provider credentials. + * + * Clients ask by sending a credential placeholder in any auth header, in any + * scheme (`Bearer`, `token`, an API key header, or `Basic` user:password). + * Cookies are not checked: a browser login is never a placeholder. + */ +export function hasHostManagedCredentialPlaceholder(headers: Headers): boolean { + let found = false; + headers.forEach((value, key) => { + if (found || key.toLowerCase() === "cookie") { + return; + } + found = + value.includes(HOST_MANAGED_CREDENTIAL_MARKER) || + Boolean( + decodedBasicCredentials(value)?.includes( + HOST_MANAGED_CREDENTIAL_MARKER, + ), + ); + }); + return found; +} diff --git a/packages/junior/src/chat/sandbox/egress/proxy.ts b/packages/junior/src/chat/sandbox/egress/proxy.ts index 6f61d37782..35086833a9 100644 --- a/packages/junior/src/chat/sandbox/egress/proxy.ts +++ b/packages/junior/src/chat/sandbox/egress/proxy.ts @@ -1,8 +1,10 @@ import { logWarn, withSpan } from "@/chat/logging"; import { executeCredentialedEgressRequest, + forwardEgressRequestWithoutCredentials, type CredentialedEgressHttpInterceptor, } from "@/chat/egress/credentialed"; +import { hasHostManagedCredentialPlaceholder } from "@/chat/plugins/auth/auth-token-placeholder"; import { resolveSandboxEgressProviderForHost } from "@/chat/sandbox/egress/policy"; import { verifyVercelSandboxOidcToken } from "@/chat/sandbox/egress/oidc"; import { @@ -65,12 +67,16 @@ function egressAttributes(input: { status?: number; }): Record { return { - ...(input.egressId ? { "app.sandbox.egress_id": input.egressId } : undefined), + ...(input.egressId + ? { "app.sandbox.egress_id": input.egressId } + : undefined), ...(input.provider ? { "app.provider.name": input.provider } : undefined), ...(input.host ? { "server.address": input.host } : undefined), ...(input.method ? { "http.request.method": input.method } : undefined), ...(input.path ? { "url.path": input.path } : undefined), - ...(input.status ? { "http.response.status_code": input.status } : undefined), + ...(input.status + ? { "http.response.status_code": input.status } + : undefined), }; } @@ -362,13 +368,23 @@ export async function proxySandboxEgressRequest( upstreamUrl.hostname, deps.tracePropagation ?? {}, async () => - await executeCredentialedEgressRequest({ - activeEgressId, - credentialContext, - deps, - provider, - request, - upstreamUrl, - }), + // Only clients that send a credential placeholder get provider + // credentials. Other clients, such as a browser, keep their own login. + hasHostManagedCredentialPlaceholder(request.headers) + ? await executeCredentialedEgressRequest({ + activeEgressId, + credentialContext, + deps, + provider, + request, + upstreamUrl, + }) + : await forwardEgressRequestWithoutCredentials({ + activeEgressId, + deps, + provider, + request, + upstreamUrl, + }), ); } diff --git a/packages/junior/tests/component/handlers/sandbox-egress-proxy.test.ts b/packages/junior/tests/component/handlers/sandbox-egress-proxy.test.ts index 283e3f1f93..87896431e0 100644 --- a/packages/junior/tests/component/handlers/sandbox-egress-proxy.test.ts +++ b/packages/junior/tests/component/handlers/sandbox-egress-proxy.test.ts @@ -215,6 +215,8 @@ function egressRequest( ? { "vercel-forwarded-path": forwardedPath } : undefined), ...(input.port ? { "vercel-forwarded-port": input.port } : undefined), + // Sandbox CLIs send the token placeholder to ask for credentials. + authorization: "Bearer host_managed_credential", ...(input.headers ?? {}), }, ...(input.body === undefined ? undefined : { body: input.body }), diff --git a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts index de0f8eb9aa..17a7a6772f 100644 --- a/packages/junior/tests/integration/sandbox-egress-proxy.test.ts +++ b/packages/junior/tests/integration/sandbox-egress-proxy.test.ts @@ -121,6 +121,8 @@ function proxiedRequest(input: { ...(input.body !== undefined ? { "content-type": "application/json" } : undefined), + // Sandbox CLIs send the token placeholder to ask for credentials. + authorization: "Bearer host_managed_credential", ...(input.headers ?? {}), }, }); @@ -375,6 +377,67 @@ describe("sandbox egress proxy integration", () => { }); }); + it("adds credentials only to requests that carry a credential placeholder", async () => { + const credentialToken = modules.session.createSandboxEgressCredentialToken({ + credentials: { actor: { type: "user", userId: ACTOR_ID } }, + egressId: EGRESS_ID, + ttlMs: 60_000, + }); + const forwardURL = forwardUrlFor( + modules.policy.buildSandboxEgressNetworkPolicy({ credentialToken }), + PROVIDER_HOST, + ); + const upstreamFetch = vi.fn( + async (_url: URL | string, init?: RequestInit) => { + const headers = new Headers(init?.headers); + return Response.json({ + authorization: headers.get("authorization"), + cookie: headers.get("cookie"), + }); + }, + ); + const deps = { + fetch: upstreamFetch as typeof fetch, + verifyOidc: async () => ({ sandbox_id: EGRESS_ID }), + }; + + // A browser sends its own login and no placeholder. + const browserRequest = proxiedRequest({ + forwardURL, + headers: { cookie: "session=browser-login" }, + }); + browserRequest.headers.delete("authorization"); + const browser = await modules.proxy.proxySandboxEgressRequest( + browserRequest, + deps, + ); + expect(browser.status).toBe(200); + await expect(browser.json()).resolves.toEqual({ + authorization: null, + cookie: "session=browser-login", + }); + + // Git sends the placeholder as the Basic auth password. + const git = await modules.proxy.proxySandboxEgressRequest( + proxiedRequest({ + forwardURL, + headers: { + authorization: `Basic ${Buffer.from( + "x-access-token:ghp_host_managed_credential", + ).toString("base64")}`, + }, + }), + deps, + ); + expect(git.status).toBe(200); + await expect(git.json()).resolves.toMatchObject({ + authorization: "Bearer integration-egress-token", + }); + await expect( + modules.session.consumeSandboxEgressAuthRequiredSignal(EGRESS_ID), + ).resolves.toBeUndefined(); + }); + it("records OAuth authorization metadata for broker credential gaps", async () => { delete process.env.OAUTH_BROKER_ACCESS_TOKEN; await registerOAuthBrokerPlugin(); diff --git a/policies/security.md b/policies/security.md index e6b4907b9a..9d4978626f 100644 --- a/policies/security.md +++ b/policies/security.md @@ -26,6 +26,10 @@ credentials, plugins, skills, and operational telemetry. a security edge. - Tool-native auth environment variables may contain non-secret placeholders only. The proxy applies real authorization headers after verification. +- The proxy adds credentials only to a request that sends a placeholder in an + auth header. Every placeholder contains `host_managed_credential`. The proxy + forwards other requests without credentials, so a browser keeps its own + login. The placeholder is public, so this check is not a security edge. The implementation lives in `packages/junior/src/chat/sandbox/egress/` and