From ac09bdabfef3cd8cc56227d976400d936217a620 Mon Sep 17 00:00:00 2001 From: Alex Lavaee Date: Wed, 4 Nov 2026 16:27:18 -0800 Subject: [PATCH] Pass the proxy and certificate variables to the Copilot CLI the Planner runs The copilot-sdk harness created its client with `env: { COPILOT_PLUGIN_DIR_ONLY: "true" }`. The SDK uses the `env` it is given as the CLI process's whole environment, so a server run behind an HTTP or HTTPS proxy kept using the proxy for its own requests while the CLI, which never received `HTTPS_PROXY`, `HTTP_PROXY` or `NO_PROXY`, went direct. Planner turns then failed at `session.create` with a network fetch error for `api.github.com/copilot_internal/user` (#759). Nothing else in the server creates a CopilotClient with an `env`; the runtime only receives the client the adapter builds. The adapter now builds the CLI's environment from a fixed list: the proxy variables in upper and lower case, plus the certificate variables a proxy that re-signs TLS needs (`NODE_EXTRA_CA_CERTS`, `SSL_CERT_FILE`, `SSL_CERT_DIR`, each checked against the bundled CLI's own option table). A variable the server has not set is not added; one set to an empty string is passed as it is. `COPILOT_PLUGIN_DIR_ONLY` stays "true", and tokens, other variables and `COPILOT_ENABLE_BUILTIN_GITHUB_MCP` still never reach the CLI. `defaultConnect` is exported so the test can drive it. The self-hosting and hosted-agent guides say which variables the CLI receives and that nothing else is passed. The new tests spawn a stand-in CLI through `COPILOT_CLI_PATH` under the SDK's own spawn and read the environment it is given: the variables appear when set and are absent when unset, and an unrelated token and secret are not passed. A fourth sends an HTTPS request from that stand-in to a local proxy and checks the proxy sees `CONNECT api.github.com:443`. The real CLI is not run through a proxy: it needs a Copilot credential and live GitHub, so that is not deterministic. Fixes #759 Assistant-model: Claude Sonnet 5.5 Assistant-workflow: goal (run f6b196ad-78d2-4d43-addc-1ea742a72646) Assistant-duration: 4m converged, estimated 30m Assistant-verification: red before fix passed: with defaultConnect only exported and the source change otherwise reverted, 3 of the 4 new tests failed (proxy and certificate variables missing from the CLI environment; no CONNECT reached the local proxy) and the secrets-absent test passed; with the fix all 4 pass Assistant-verification: bun test apps/server/src/harness passed: 384 pass, 0 fail across 23 files Assistant-verification: bun run fix passed: no changes beyond the edits, 72 existing lint warnings Assistant-verification: bun run types passed: whole workspace and e2e Assistant-verification: bun run ci passed: dprint, oxlint (72 warnings, 0 errors), token and design checks Assistant-verification: local CONNECT proxy test passed: a stand-in CLI spawned by the SDK sent its HTTPS request to a 127.0.0.1 proxy that saw CONNECT api.github.com:443; the real CLI through a proxy was not run Co-authored-by: Alex Lavaee --- .../server/src/harness/copilot-sdk/adapter.ts | 36 ++++- .../copilot-sdk/cli-environment.test.ts | 147 ++++++++++++++++++ docs/hosted-agent.md | 6 + docs/self-hosting.md | 14 ++ 4 files changed, 199 insertions(+), 4 deletions(-) create mode 100644 apps/server/src/harness/copilot-sdk/cli-environment.test.ts diff --git a/apps/server/src/harness/copilot-sdk/adapter.ts b/apps/server/src/harness/copilot-sdk/adapter.ts index b7aa81518..8ab6b0950 100644 --- a/apps/server/src/harness/copilot-sdk/adapter.ts +++ b/apps/server/src/harness/copilot-sdk/adapter.ts @@ -76,7 +76,37 @@ function bounded( }); } -function defaultConnect(): RuntimeSource { +/** + * The only variables the CLI inherits from the server besides its own + * hardening flag: a proxy and the certificate authorities a proxy setup + * needs. The SDK treats the `env` it is given as the CLI's whole + * environment, so anything not listed here never reaches it. + */ +const FORWARDED_ENV = [ + "HTTPS_PROXY", + "HTTP_PROXY", + "NO_PROXY", + "https_proxy", + "http_proxy", + "no_proxy", + "NODE_EXTRA_CA_CERTS", + "SSL_CERT_FILE", + "SSL_CERT_DIR", +]; + +function cliEnvironment(source: NodeJS.ProcessEnv): Record { + let env: Record = {}; + for (let name of FORWARDED_ENV) { + let value = source[name]; + if (value !== undefined) env[name] = value; + } + // Deliberately omits COPILOT_ENABLE_BUILTIN_GITHUB_MCP: GitHub tools + // reach the Planner as host tools instead. + env.COPILOT_PLUGIN_DIR_ONLY = "true"; + return env; +} + +export function defaultConnect(): RuntimeSource { let cli = locate(); if (!cli.ok) throw new Error(cli.reason); let home = mkdtempSync(join(tmpdir(), "chopin-copilot-planner-")); @@ -87,9 +117,7 @@ function defaultConnect(): RuntimeSource { baseDirectory: home, connection: RuntimeConnection.forStdio({ path: cli.path }), useLoggedInUser: false, - // Deliberately omits COPILOT_ENABLE_BUILTIN_GITHUB_MCP: GitHub tools - // reach the Planner as host tools instead. - env: { COPILOT_PLUGIN_DIR_ONLY: "true" }, + env: cliEnvironment(process.env), logLevel: "info", }); return { client, cleanup: () => rmSync(home, { recursive: true, force: true }) }; diff --git a/apps/server/src/harness/copilot-sdk/cli-environment.test.ts b/apps/server/src/harness/copilot-sdk/cli-environment.test.ts new file mode 100644 index 000000000..1af4a35a1 --- /dev/null +++ b/apps/server/src/harness/copilot-sdk/cli-environment.test.ts @@ -0,0 +1,147 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { defaultConnect } from "./adapter"; + +const FORWARDED = [ + "HTTPS_PROXY", + "HTTP_PROXY", + "NO_PROXY", + "https_proxy", + "http_proxy", + "no_proxy", + "NODE_EXTRA_CA_CERTS", + "SSL_CERT_FILE", + "SSL_CERT_DIR", +]; + +const touched = [ + ...FORWARDED, + "COPILOT_CLI_PATH", + "GITHUB_TOKEN", + "CHOPIN_TEST_SECRET", + "COPILOT_ENABLE_BUILTIN_GITHUB_MCP", +]; + +let saved = new Map(); +let scratch: string[] = []; + +function setEnv(values: Record): void { + for (let name of touched) { + if (!saved.has(name)) saved.set(name, process.env[name]); + delete process.env[name]; + } + for (let [name, value] of Object.entries(values)) process.env[name] = value; +} + +afterEach(() => { + for (let [name, value] of saved) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } + saved.clear(); + for (let dir of scratch) rmSync(dir, { recursive: true, force: true }); + scratch = []; +}); + +/** A CLI stand-in that runs `body` under Bun with exactly the environment the SDK gives it. */ +function fakeCli(body: string): string { + let dir = mkdtempSync(join(tmpdir(), "chopin-fake-cli-")); + scratch.push(dir); + writeFileSync(join(dir, "cli.ts"), body); + let path = join(dir, "cli"); + writeFileSync(path, `#!/bin/sh\nexec "${process.execPath}" "${join(dir, "cli.ts")}"\n`); + chmodSync(path, 0o755); + return path; +} + +async function spawnCli(): Promise { + let source = defaultConnect(); + try { + await source.client.start(); + } catch { + // The stand-in exits without speaking the protocol; its side effects are what is checked. + } finally { + await source.client.forceStop(); + source.cleanup(); + } +} + +async function cliEnvironment(): Promise> { + let out = join(mkdtempSync(join(tmpdir(), "chopin-cli-env-")), "env.json"); + scratch.push(join(out, "..")); + process.env.COPILOT_CLI_PATH = fakeCli( + `import { writeFileSync } from "node:fs";\n` + + `writeFileSync(${JSON.stringify(out)}, JSON.stringify(process.env));\n`, + ); + await spawnCli(); + return JSON.parse(readFileSync(out, "utf8")); +} + +describe("the Copilot CLI's environment", () => { + it("carries the proxy and certificate variables the server has set, verbatim", async () => { + let values: Record = {}; + for (let name of FORWARDED) values[name] = `${name} value`; + values.NO_PROXY = ""; + setEnv(values); + let env = await cliEnvironment(); + for (let name of FORWARDED) expect(env[name]).toBe(values[name]); + expect(env.COPILOT_PLUGIN_DIR_ONLY).toBe("true"); + }); + + it("adds no variable the server has not set", async () => { + setEnv({ HTTPS_PROXY: "http://proxy.test:3128" }); + let env = await cliEnvironment(); + expect(env.HTTPS_PROXY).toBe("http://proxy.test:3128"); + for (let name of FORWARDED.filter(name => name !== "HTTPS_PROXY")) { + expect(name in env).toBe(false); + } + }); + + it("passes nothing else from the server, secrets and the built-in GitHub MCP switch included", async () => { + setEnv({ + HTTPS_PROXY: "http://proxy.test:3128", + GITHUB_TOKEN: "ghp_not-for-the-cli", + CHOPIN_TEST_SECRET: "not-for-the-cli", + COPILOT_ENABLE_BUILTIN_GITHUB_MCP: "true", + }); + let env = await cliEnvironment(); + expect(env.GITHUB_TOKEN).toBeUndefined(); + expect(env.CHOPIN_TEST_SECRET).toBeUndefined(); + expect(env.COPILOT_ENABLE_BUILTIN_GITHUB_MCP).toBeUndefined(); + expect(env.COPILOT_PLUGIN_DIR_ONLY).toBe("true"); + expect(JSON.stringify(env)).not.toContain("not-for-the-cli"); + }); + + it("sends the CLI's own HTTPS requests through the proxy the server runs behind", async () => { + let connects = Promise.withResolvers(); + let proxy = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + socket: { + data(socket, data) { + let request = Buffer.from(data).toString("latin1"); + connects.resolve(request.split("\r\n")[0] ?? ""); + socket.end("HTTP/1.1 403 Forbidden\r\nContent-Length: 0\r\n\r\n"); + }, + }, + }); + try { + setEnv({ HTTPS_PROXY: `http://127.0.0.1:${proxy.port}` }); + process.env.COPILOT_CLI_PATH = fakeCli( + `await fetch("https://api.github.com/copilot_internal/user").catch(() => {});\n`, + ); + let timer: Timer | undefined; + let timeout = new Promise(resolve => { + timer = setTimeout(() => resolve("no request reached the proxy"), 10_000); + }); + let [, line] = await Promise.all([spawnCli(), Promise.race([connects.promise, timeout])]); + clearTimeout(timer); + expect(line).toBe("CONNECT api.github.com:443 HTTP/1.1"); + } finally { + proxy.stop(true); + } + }, 20_000); +}); diff --git a/docs/hosted-agent.md b/docs/hosted-agent.md index a7b3bb874..a32e8b4ff 100644 --- a/docs/hosted-agent.md +++ b/docs/hosted-agent.md @@ -174,6 +174,12 @@ disposable session receives its owner's token when created and has no client-level service token or logged-in-user fallback. That detail is specific to the Copilot SDK adapter, not a property every harness in `HARNESS` shares. +The CLI process does not inherit the server's environment. It receives the +proxy variables (`HTTPS_PROXY`, `HTTP_PROXY`, `NO_PROXY`, lowercase forms) and the +certificate variables (`NODE_EXTRA_CA_CERTS`, `SSL_CERT_FILE`, `SSL_CERT_DIR`) the +server has set, so a server behind a proxy reaches GitHub through it, and nothing +else; see [Self-hosting](self-hosting.md). + The two pull-request tool names and input schemas are available when a Planner session opens. GitHub MCP discovery runs on the first pull-request tool call, using the owner's current credential. An unavailable server or missing required diff --git a/docs/self-hosting.md b/docs/self-hosting.md index 7b6db4b16..61f092aa1 100644 --- a/docs/self-hosting.md +++ b/docs/self-hosting.md @@ -75,6 +75,20 @@ each owner's GitHub App user token and does not consume `HARNESS_AUTH`; setting `auto` on loopback passes the startup guard but does not enable host login in that adapter. +Under `HARNESS=copilot-sdk` the Copilot CLI runs with a minimal environment of +its own, not the server's. Behind an HTTP or HTTPS proxy, set these on the +server process and the CLI receives them unchanged: `HTTPS_PROXY`, +`HTTP_PROXY`, `NO_PROXY` and their lowercase forms, and the certificate +variables a proxy that re-signs TLS needs, `NODE_EXTRA_CA_CERTS`, +`SSL_CERT_FILE` and `SSL_CERT_DIR`. A variable the server has not set is not +passed, and one set to an empty string is passed as empty. The CLI also gets +`COPILOT_PLUGIN_DIR_ONLY=true` and the variables the Copilot SDK itself sets; +nothing else from the server's environment reaches it, including tokens and +`COPILOT_ENABLE_BUILTIN_GITHUB_MCP`. If the host reaches GitHub only through a +proxy that the server learns some other way, such as a system setting, set +these variables on the server process too: otherwise the CLI connects directly, +and a Planner turn fails with "The Planner stream failed." + `HARNESS=atomic` runs Atomic 0.9.71 in the Chopin server process through its headless SDK (`createAgentSession()`). It does not spawn the `atomic` CLI or use RPC mode.