From 9136678ef08aafd974be350e93b7a4d0cd323721 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 28 Sep 2026 12:17:10 -0500 Subject: [PATCH] Reject missing Copilot credentials before provider admission Use only the explicitly sanitized COPILOT_GITHUB_TOKEN binding and preserve typed authentication diagnostics. Exercise missing-key cleanup and subsequent explicit credential admission, and make the packaged initialize smoke hermetic with a metadata-only loopback fixture. Co-Authored-By: Paperclip --- .../scripts/copilot-provider-pack-smoke.mjs | 33 ++++++++++++---- .../src/drivers/acpx/copilot-profile.ts | 10 +++++ .../src/drivers/acpx/copilot-registry.test.ts | 9 ++++- .../drivers/acpx/copilot-runtime-host.test.ts | 38 +++++++++++++++++++ .../src/drivers/acpx/profile-installation.ts | 8 ++-- 5 files changed, 87 insertions(+), 11 deletions(-) create mode 100644 packages/paperclip-runner/src/drivers/acpx/copilot-runtime-host.test.ts diff --git a/packages/paperclip-runner/scripts/copilot-provider-pack-smoke.mjs b/packages/paperclip-runner/scripts/copilot-provider-pack-smoke.mjs index eec37a86924..774cda0b9dc 100644 --- a/packages/paperclip-runner/scripts/copilot-provider-pack-smoke.mjs +++ b/packages/paperclip-runner/scripts/copilot-provider-pack-smoke.mjs @@ -1,6 +1,7 @@ import assert from "node:assert/strict"; import { createHash } from "node:crypto"; import { once } from "node:events"; +import { createServer } from "node:http"; import { mkdir, mkdtemp, readFile, realpath, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; @@ -16,20 +17,32 @@ process.env.PAPERCLIP_ACPX_PROVIDER_PACKAGE_MANIFEST = join(pack, "package.json" const candidate = manifest.payload.candidateProviders?.copilot; assert.equal(candidate?.version, "1.0.88"); assert.equal(candidate.qualification, "pending"); -const { verifyAcpxProfileInstallation } = await import(pathToFileURL(join(pack, "dist/drivers/acpx/profile-installation.js"))); +const { assertAcpxProfileEnvironment, verifyAcpxProfileInstallation } = await import(pathToFileURL(join(pack, "dist/drivers/acpx/profile-installation.js"))); const { resolveQualifiedAcpxProfile } = await import(pathToFileURL(join(pack, "dist/drivers/acpx/qualified-profiles.js"))); const { COPILOT_ACP_CLIENT_CAPABILITIES } = await import(pathToFileURL(join(pack, "dist/drivers/acpx/copilot-events.js"))); // This explicit value exercises profile admission only; no model is selected // or used by initialize, and this is not a claim of model availability. const profile = resolveQualifiedAcpxProfile("copilot", "gpt-4.1"); +assert.throws(() => assertAcpxProfileEnvironment("copilot", {}), { code: "COPILOT_AUTH_REQUIRED" }); const installation = await verifyAcpxProfileInstallation(profile); assert.equal(installation.commandDigest, candidate.profileDigest); const lease = await installation.openCommand(); const root = await mkdtemp(join(tmpdir(), "paperclip-copilot-pack-smoke-")); let child; let exited; +const fixtureRequests = []; +const fixture = createServer((request, response) => { + fixtureRequests.push({ method: request.method, path: request.url?.split("?")[0] }); + response.writeHead(request.method === "GET" ? 200 : 503, { "content-type": "application/json" }); + response.end(JSON.stringify(request.method === "GET" ? { object: "list", data: [{ id: "gpt-4.1", object: "model", owned_by: "fixture" }] } : { error: "This initialize-only fixture cannot run inference" })); +}); +fixture.listen(0, "127.0.0.1"); +await once(fixture, "listening"); try { - const environment = { PATH: "/usr/bin:/bin", COPILOT_OFFLINE: "true", COPILOT_AUTO_UPDATE: "false", NO_COLOR: "1" }; + const environment = { PATH: "/usr/bin:/bin", COPILOT_OFFLINE: "true", COPILOT_AUTO_UPDATE: "false", NO_COLOR: "1", + COPILOT_PROVIDER_BASE_URL: `http://127.0.0.1:${fixture.address().port}`, COPILOT_PROVIDER_TYPE: "openai", + COPILOT_PROVIDER_MODEL_ID: "gpt-4.1", COPILOT_MODEL: "gpt-4.1", + }; for (const name of ["HOME", "XDG_CONFIG_HOME", "XDG_CACHE_HOME", "XDG_DATA_HOME", "COPILOT_HOME", "COPILOT_CACHE_HOME"]) { environment[name] = join(root, name.toLowerCase()); await mkdir(environment[name], { mode: 0o700 }); @@ -39,11 +52,12 @@ try { exited = once(child, "exit"); const initialized = new Promise((done, reject) => { let buffer = ""; + let stderr = ""; let total = 0; - const timeout = setTimeout(() => reject(new Error("Copilot ACP initialize timed out")), 20_000); + const timeout = setTimeout(() => reject(new Error(`Copilot ACP initialize timed out: ${stderr.replaceAll(root, "/fixture/workspace").replaceAll(pack, "/fixture/provider-pack").slice(0, 4000)}`)), 20_000); timeout.unref(); child.once("error", reject); - child.once("exit", () => reject(new Error("Copilot exited before initialize"))); + child.once("exit", () => reject(new Error(`Copilot exited before initialize: ${stderr.replaceAll(root, "/fixture/workspace").replaceAll(pack, "/fixture/provider-pack").slice(0, 4000)}`))); child.stdout.on("data", chunk => { total += chunk.length; if (total > 1024 * 1024) { reject(new Error("Copilot ACP output exceeded bound")); return; } @@ -61,7 +75,7 @@ try { } } }); - child.stderr.resume(); // Never persist diagnostics that may contain paths. + child.stderr.on("data", chunk => { stderr = (stderr + chunk.toString("utf8")).slice(-16_384); }); }); child.stdin.write(`${JSON.stringify({ jsonrpc: "2.0", id: 0, method: "initialize", params: { protocolVersion: 1, clientInfo: { name: "paperclip-pack-offline-smoke", version: "1" }, clientCapabilities: COPILOT_ACP_CLIENT_CAPABILITIES } })}\n`); const result = await initialized; @@ -73,14 +87,17 @@ try { clearTimeout(timer); assert.equal(exitCode, 0, "Copilot did not exit cleanly after stdin EOF"); assert.equal(signal, null); + assert.equal(fixtureRequests.some(request => request.method !== "GET"), false, "Unexpected model inference request"); + const safeInitialize = JSON.parse(JSON.stringify(result, (key, value) => + key === "command" && typeof value === "string" && value.endsWith("/distribution/copilot") ? "/fixture/verified/copilot" : value)); const executable = await readFile(join(pack, candidate.path, "copilot")); process.stdout.write(`${JSON.stringify({ schema: "paperclip.copilot-provider-pack-smoke/v1", sourceRevision: manifest.payload.runnerSourceRevision, providerPackDigest: manifest.digest, platform: process.platform, architecture: process.arch, candidate, executableSha256: `sha256:${createHash("sha256").update(executable).digest("hex")}`, registryLaunch: "verifyAcpxProfileInstallation/openCommand/spawn", initializeRequestId: 0, - initialize: result, cleanExit: true, inheritedCredentials: false, promptSent: false, - networkMode: "COPILOT_OFFLINE=true", costUsd: 0, + initialize: safeInitialize, missingCredentialPreflight: "COPILOT_AUTH_REQUIRED", cleanExit: true, inheritedCredentials: false, promptSent: false, + networkMode: "COPILOT_OFFLINE=true; loopback metadata-only provider", fixtureRequests, protocolFixtureModel: "gpt-4.1 (not a qualified GitHub model)", costUsd: 0, qualification: "pending: no credential, entitlement, model execution or Daytona", }, null, 2)}\n`); } finally { @@ -89,6 +106,8 @@ try { child.kill("SIGTERM"); await within(exited, 5_000); } + fixture.closeAllConnections(); + await new Promise(resolve => fixture.close(resolve)); await lease.close(); await rm(root, { recursive: true, force: true }); } diff --git a/packages/paperclip-runner/src/drivers/acpx/copilot-profile.ts b/packages/paperclip-runner/src/drivers/acpx/copilot-profile.ts index ed67c5e700a..3b7f713375f 100644 --- a/packages/paperclip-runner/src/drivers/acpx/copilot-profile.ts +++ b/packages/paperclip-runner/src/drivers/acpx/copilot-profile.ts @@ -59,6 +59,16 @@ export type CopilotFailureCode = | "COPILOT_MODEL_UNAVAILABLE" | "COPILOT_REQUEST_FAILED"; +/** The runner binds this credential explicitly; ambient gh login is never used. */ +export function assertCopilotCredentials(environment: Readonly): void { + const token = environment.COPILOT_GITHUB_TOKEN; + if (typeof token !== "string" || token.trim().length === 0 || token.includes("\0")) { + throw Object.assign(new Error("Bind a valid COPILOT_GITHUB_TOKEN credential for this runner."), { + code: "COPILOT_AUTH_REQUIRED", retryable: false, + }); + } +} + /** Classify, but never echo provider text that may contain tokens or user data. */ export function classifyCopilotFailure(error: unknown): { code: CopilotFailureCode; message: string } { const source = error instanceof Error ? error.message diff --git a/packages/paperclip-runner/src/drivers/acpx/copilot-registry.test.ts b/packages/paperclip-runner/src/drivers/acpx/copilot-registry.test.ts index 32c6712d795..39365f1158b 100644 --- a/packages/paperclip-runner/src/drivers/acpx/copilot-registry.test.ts +++ b/packages/paperclip-runner/src/drivers/acpx/copilot-registry.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import type { CanonicalProviderEvent } from "../../provider-events.js"; import { COPILOT_ACP_CLIENT_CAPABILITIES, COPILOT_ACP_EVENT_METHOD } from "./copilot-events.js"; import { acpxProfileClientCapabilities, bindAcpxExtensionTurn, createAcpxProfileExtensionAdapter } from "./profile-extensions.js"; -import { classifyAcpxProfileError, verifyAcpxProfileInstallation } from "./profile-installation.js"; +import { assertAcpxProfileEnvironment, classifyAcpxProfileError, verifyAcpxProfileInstallation } from "./profile-installation.js"; import { resolveQualifiedAcpxProfile } from "./qualified-profiles.js"; import { verifyCopilotInstallation } from "./copilot-installation.js"; @@ -10,6 +10,13 @@ vi.mock("./copilot-installation.js", () => ({ verifyCopilotInstallation: vi.fn(a const context = { workspacePath: "/workspace", sessionId: "backend-1", turnId: "turn-1" }; describe("Copilot provider registry conformance", () => { + it("requires only explicitly bound Copilot credentials before starting the provider", () => { + for (const environment of [{}, { COPILOT_GITHUB_TOKEN: " " }, { COPILOT_GITHUB_TOKEN: "bad\0token" }, { GITHUB_TOKEN: "ambient-secret", GH_TOKEN: "ambient-secret" }]) { + expect(() => assertAcpxProfileEnvironment("copilot", environment)).toThrow(expect.objectContaining({ code: "COPILOT_AUTH_REQUIRED", retryable: false })); + } + expect(() => assertAcpxProfileEnvironment("copilot", { COPILOT_GITHUB_TOKEN: "fixture-explicit-token" })).not.toThrow(); + expect(() => assertAcpxProfileEnvironment("codex", {})).not.toThrow(); + }); it("selects only the Copilot native installer and preserves the exact caller-selected profile", async () => { const profile = resolveQualifiedAcpxProfile("copilot", "explicit-exact-model"); expect(await verifyAcpxProfileInstallation(profile)).toMatchObject({ commandDigest: "verified-by-native-factory" }); diff --git a/packages/paperclip-runner/src/drivers/acpx/copilot-runtime-host.test.ts b/packages/paperclip-runner/src/drivers/acpx/copilot-runtime-host.test.ts new file mode 100644 index 00000000000..88443b8af62 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/acpx/copilot-runtime-host.test.ts @@ -0,0 +1,38 @@ +import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { expect, it, vi } from "vitest"; +import { AcpxRuntimeHost, type AcpxRuntimeHostDependencies, type AcpxRuntimePortOpenOptions } from "./runtime-host.js"; + +it("reports missing Copilot authentication before spawn and releases admission ownership for retry", async () => { + const root = await mkdtemp(join(tmpdir(), "paperclip-copilot-auth-admission-")); + const runtimeDirectory = join(root, "runtime"); + const workingDirectory = join(root, "workspace"); + await Promise.all([mkdir(runtimeDirectory), mkdir(workingDirectory), mkdir(join(root, "provider"))]); + const openRuntime = vi.fn(async (_options: AcpxRuntimePortOpenOptions): Promise => { + throw new Error("fixture stopped before provider spawn"); + }); + const close = vi.fn(async () => {}); + const openCommand = vi.fn(async () => ({ spawn: () => { throw new Error("must not spawn"); }, close })); + const dependencies: AcpxRuntimeHostDependencies = { + verifyInstallation: async profile => ({ commandDigest: profile.commandDigest, + agentServerPackageJsonPath: join(root, "provider/package.json"), agentRuntimePackageJsonPath: null, openCommand }), + openRuntime, reportRetainedCleanupFailure: () => {}, + }; + const options = { agent: "copilot" as const, model: "explicit-model", normalizedSessionId: "copilot-auth-test", + runtimeDirectory, workingDirectory, providerPolicy: { readOnly: false } }; + try { + await expect(AcpxRuntimeHost.open({ ...options, environment: { GH_TOKEN: "ambient-forbidden", GITHUB_TOKEN: "ambient-forbidden" } }, dependencies)) + .rejects.toMatchObject({ code: "COPILOT_AUTH_REQUIRED", retryable: false }); + expect(openCommand).not.toHaveBeenCalled(); + expect(openRuntime).not.toHaveBeenCalled(); + await expect(AcpxRuntimeHost.open({ ...options, environment: { COPILOT_GITHUB_TOKEN: "explicit-fixture-only", GH_TOKEN: "ambient-forbidden" } }, dependencies)) + .rejects.toThrow("fixture stopped before provider spawn"); + expect(openRuntime).toHaveBeenCalledOnce(); + const environment = openRuntime.mock.calls[0]![0].launchEnvironment; + expect(environment.COPILOT_GITHUB_TOKEN).toBe("explicit-fixture-only"); + expect(environment.GH_TOKEN).toBeUndefined(); + expect(environment.GITHUB_TOKEN).toBeUndefined(); + expect(close).toHaveBeenCalled(); + } finally { await rm(root, { recursive: true, force: true }); } +}); diff --git a/packages/paperclip-runner/src/drivers/acpx/profile-installation.ts b/packages/paperclip-runner/src/drivers/acpx/profile-installation.ts index 779a4e8ae05..ec4d458da9a 100644 --- a/packages/paperclip-runner/src/drivers/acpx/profile-installation.ts +++ b/packages/paperclip-runner/src/drivers/acpx/profile-installation.ts @@ -1,4 +1,4 @@ -import { classifyCopilotFailure } from "./copilot-profile.js"; +import { assertCopilotCredentials, classifyCopilotFailure } from "./copilot-profile.js"; import { verifyCopilotInstallation } from "./copilot-installation.js"; import type { QualifiedAcpxAgent, QualifiedAcpxProfile } from "./qualified-profiles.js"; import { verifyQualifiedAcpxInstallation, type VerifiedAcpxInstallation } from "./installation-integrity.js"; @@ -15,8 +15,10 @@ export async function verifyAcpxProfileInstallation(profile: QualifiedAcpxProfil /** Provider policy admission is repeated immediately before each process launch. */ export async function assertAcpxProfileWorkspace(_agent: QualifiedAcpxAgent, _workspace: string): Promise {} -/** Candidate branches validate only explicitly bound, sanitized launch credentials. */ -export function assertAcpxProfileEnvironment(_agent: QualifiedAcpxAgent, _environment: Readonly): void {} +/** Called with the sanitized launch environment, never ambient process.env. */ +export function assertAcpxProfileEnvironment(agent: QualifiedAcpxAgent, environment: Readonly): void { + if (agent === "copilot") assertCopilotCredentials(environment); +} /** Provider admission diagnostics expose no raw provider strings or credentials. */ export function classifyAcpxProfileError(agent: QualifiedAcpxAgent, error: unknown): Error | null {