From dc55ee5778ae77db6e87bef43ea2492f714602f5 Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 8 Oct 2026 13:28:43 -0500 Subject: [PATCH] Verify Copilot admission on the actual selected execution environment --- .../agent-test-environment-routes.test.ts | 44 ++++++++++++++++++- server/src/__tests__/ai-connections.test.ts | 37 ++++++++++++++++ .../copilot-connection-probe.test.ts | 27 ++++++++++++ server/src/routes/agents.ts | 12 ++++- .../src/services/copilot-connection-probe.ts | 2 +- 5 files changed, 118 insertions(+), 4 deletions(-) diff --git a/server/src/__tests__/agent-test-environment-routes.test.ts b/server/src/__tests__/agent-test-environment-routes.test.ts index 2c2499e4f34..8904af21bef 100644 --- a/server/src/__tests__/agent-test-environment-routes.test.ts +++ b/server/src/__tests__/agent-test-environment-routes.test.ts @@ -32,6 +32,7 @@ const mockEnvironmentService = vi.hoisted(() => ({ releaseLease: vi.fn(), listBoundCompanyIds: vi.fn(async () => [] as string[]), findManagedSandboxEnvironment: vi.fn(async () => null as Record | null), + findKubernetesEnvironment: vi.fn(async () => null as Record | null), })); const mockReleaseRunLease = vi.hoisted(() => vi.fn(async () => undefined)); @@ -46,7 +47,7 @@ const mockEnvironmentRuntime = vi.hoisted(() => ({ const mockResolveEnvironmentExecutionTarget = vi.hoisted(() => vi.fn()); const mockInstanceSettingsService = vi.hoisted(() => ({ get: vi.fn(async () => ({ defaultEnvironmentId: null as string | null })), - getGeneral: vi.fn(async () => ({ censorUsernameInLogs: false })), + getGeneral: vi.fn(async () => ({ censorUsernameInLogs: false, executionMode: "local" })), getExperimental: vi.fn(async () => ({ enableManagedSandboxOnly: false })), })); @@ -121,6 +122,10 @@ vi.mock("../routes/ai-connections.js", async (importOriginal) => ({ validateAiApiKey: mockValidateAiApiKey, })); const mockMarkAuthenticationFailed = vi.hoisted(() => vi.fn(async () => undefined)); +const mockCopilotProbe = vi.hoisted(() => vi.fn(async () => ({ status: "verified", promptSent: false }))); +vi.mock("../services/copilot-connection-probe.js", async importOriginal => ({ + ...(await importOriginal()), probeCopilotExecutionTarget: mockCopilotProbe, +})); vi.mock("../services/ai-connections.js", async (importOriginal) => ({ ...(await importOriginal()), aiConnectionService: () => ({ markAuthenticationFailed: mockMarkAuthenticationFailed }), @@ -192,6 +197,8 @@ describe("agent test-environment route", () => { vi.resetModules(); vi.clearAllMocks(); mockInstanceSettingsService.get.mockResolvedValue({ defaultEnvironmentId: null }); + mockInstanceSettingsService.getGeneral.mockResolvedValue({ censorUsernameInLogs: false, executionMode: "local" }); + mockEnvironmentService.findKubernetesEnvironment.mockResolvedValue(null); mockInstanceSettingsService.getExperimental.mockResolvedValue({ enableManagedSandboxOnly: false }); mockEnvironmentService.findManagedSandboxEnvironment.mockResolvedValue(null); mockAccessService.decide.mockResolvedValue({ @@ -327,6 +334,41 @@ describe("agent test-environment route", () => { expect(testEnvironmentSpy).toHaveBeenCalledWith(expect.objectContaining({ executionTarget: target })); }); + it.each([null, "33333333-3333-4333-8333-333333333333"])("verifies Copilot on forced Kubernetes before the host selection (%s)", async environmentId => { + const kubernetesId = "11111111-1111-4111-8111-111111111111"; + mockInstanceSettingsService.getGeneral.mockResolvedValue({ censorUsernameInLogs: false, executionMode: "kubernetes" }); + mockEnvironmentService.findKubernetesEnvironment.mockResolvedValue({ id: kubernetesId }); + const target = { kind: "remote", transport: "sandbox", remoteCwd: "/workspace", providerKey: "kubernetes", runner: { execute: vi.fn() } }; + mockResolveEnvironmentExecutionTarget.mockResolvedValue(target); + mockPrepareManagedAiRuntime.mockImplementation(async (_db, input) => ({ + config: { ...input.config, env: { COPILOT_GITHUB_TOKEN: "private-fixture" } }, + attribution: { provider: "github", method: "api_key" }, cleanup: vi.fn(), + })); + const app = await createApp(); + const response = await request(app).post("/api/companies/company-1/adapters/paperclip_runner/test-environment").send({ + environmentId, adapterConfig: { provider: "acpx", acpxAgent: "copilot", model: "gpt-5.6-luna" }, + aiConnection: { provider: "github", method: "api_key", mode: "responsible_user" }, + }); + expect(response.status, JSON.stringify(response.body)).toBe(200); + expect(response.body.status).toBe("pass"); + expect(mockEnvironmentRuntime.acquireRunLease).toHaveBeenCalledWith(expect.objectContaining({ environment: expect.objectContaining({ id: kubernetesId }) })); + expect(mockCopilotProbe).toHaveBeenCalledExactlyOnceWith("private-fixture", target, "gpt-5.6-luna"); + expect(mockReleaseRunLease).toHaveBeenCalledOnce(); + }); + + it("refuses a Copilot test before credentials when forced Kubernetes is unavailable", async () => { + mockInstanceSettingsService.getGeneral.mockResolvedValue({ censorUsernameInLogs: false, executionMode: "kubernetes" }); + const app = await createApp(); + const response = await request(app).post("/api/companies/company-1/adapters/paperclip_runner/test-environment").send({ + adapterConfig: { provider: "acpx", acpxAgent: "copilot", model: "gpt-5.6-luna" }, + aiConnection: { provider: "github", method: "api_key", mode: "responsible_user" }, + }); + expect(response.status).toBe(422); + expect(response.body.details.code).toBe("copilot_environment_unavailable"); + expect(mockPrepareManagedAiRuntime).not.toHaveBeenCalled(); + expect(mockCopilotProbe).not.toHaveBeenCalled(); + }); + it("checks the inherited environment's company before resolving credentials", async () => { mockInstanceSettingsService.get.mockResolvedValue({ defaultEnvironmentId: "11111111-1111-4111-8111-111111111111" }); mockEnvironmentService.listBoundCompanyIds.mockResolvedValue(["company-2"]); diff --git a/server/src/__tests__/ai-connections.test.ts b/server/src/__tests__/ai-connections.test.ts index bcbe1e1d1a5..75b702c134f 100644 --- a/server/src/__tests__/ai-connections.test.ts +++ b/server/src/__tests__/ai-connections.test.ts @@ -32,6 +32,7 @@ import request from "supertest"; import { aiConnectionRoutes, canInstallSharedAiConnectionForNewAgent, responsibleUserForAiRequest } from "../routes/ai-connections.js"; import { validateAiApiKey } from "../routes/ai-connections.js"; import * as copilotProbe from "../services/copilot-connection-probe.js"; +import * as settingsModule from "../services/instance-settings.js"; import { agentRoutes } from "../routes/agents.js"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; vi.mock("../services/local-ai-browser-login.js", () => ({ @@ -63,6 +64,42 @@ beforeAll(async () => { afterAll(async () => { await database?.cleanup(); vi.unstubAllEnvs(); if (home) await rm(home, { recursive: true, force: true }); }); describe("managed AI connections", () => { + it.each(["create", "hire", "edit"])("refuses Copilot %s before probing or persisting when forced Kubernetes is missing", async action => { + const { errorHandler } = await import("../middleware/index.js"); + const owner = `copilot-kubernetes-${action}`, id = randomUUID(); + const config = { provider: "acpx", acpxAgent: "copilot", model: "gpt-5.6-luna" }; + const aiConnection = { provider: "github", method: "api_key", mode: "responsible_user" } as const; + await db.insert(companyMemberships).values({ companyId, principalId: owner, principalType: "user", status: "active", membershipRole: "member" }); + await db.insert(agents).values({ id, companyId, name: "Kubernetes fixture", adapterType: "paperclip_runner", adapterConfig: config, runtimeConfig: { aiConnection } }); + const account = await service.save(companyId, owner, { provider: "github", method: "api_key", ownership: "personal", name: "Kubernetes fixture", apiKey: "github_pat_fixture_kubernetes", agentIds: [id], allAgents: false }, "github_pat_fixture_kubernetes"); + await service.setDefault(companyId, owner, account.grantId); + const [company] = await db.select().from(companies).where(eq(companies.id, companyId)); + await db.update(companies).set({ requireBoardApprovalForNewAgents: false }).where(eq(companies.id, companyId)); + const before = await db.select({ id: agents.id }).from(agents).where(eq(agents.companyId, companyId)); + const originalSettings = settingsModule.instanceSettingsService; + const settings = vi.spyOn(settingsModule, "instanceSettingsService").mockImplementation(database => { + const service = originalSettings(database); + return { ...service, getGeneral: async () => ({ ...await service.getGeneral(), executionMode: "kubernetes" as const }) }; + }); + const probe = vi.spyOn(copilotProbe, "probeCopilotExecutionTarget"); + const app = express(); app.use(express.json()); + app.use((req, _res, next) => { req.actor = { type: "board", source: "local_implicit", userId: owner, companyIds: [companyId], isInstanceAdmin: true }; next(); }); + app.use("/api", agentRoutes(db)); app.use(errorHandler); + try { + const changedConfig = { ...config, model: "prospective-model" }; + const response = action === "edit" + ? await request(app).patch(`/api/agents/${id}`).send({ adapterConfig: changedConfig }) + : await request(app).post(`/api/companies/${companyId}/${action === "hire" ? "agent-hires" : "agents"}`).send({ name: "Forced Kubernetes", role: "general", adapterType: "paperclip_runner", adapterConfig: changedConfig, runtimeConfig: { aiConnection } }); + expect(response.status, JSON.stringify(response.body)).toBe(422); + expect(response.body.details.code).toBe("copilot_environment_unavailable"); + expect(probe).not.toHaveBeenCalled(); + expect((await db.select().from(agents).where(eq(agents.id, id)))[0].adapterConfig).toEqual(config); + expect(await db.select({ id: agents.id }).from(agents).where(eq(agents.companyId, companyId))).toHaveLength(before.length); + } finally { + probe.mockRestore(); settings.mockRestore(); + await db.update(companies).set({ requireBoardApprovalForNewAgents: company.requireBoardApprovalForNewAgents }).where(eq(companies.id, companyId)); + } + }); it.each(["create", "hire", "edit"].flatMap(action => [true, false].map(connectionProvided => ({ action, connectionProvided }))))( "refuses unavailable Copilot models before $action persists configuration (connection binding: $connectionProvided)", async ({ action, connectionProvided }) => { const { unprocessable } = await import("../errors.js"); diff --git a/server/src/__tests__/copilot-connection-probe.test.ts b/server/src/__tests__/copilot-connection-probe.test.ts index 2fbd55f1c9f..9ae392d88ef 100644 --- a/server/src/__tests__/copilot-connection-probe.test.ts +++ b/server/src/__tests__/copilot-connection-probe.test.ts @@ -23,6 +23,33 @@ beforeEach(() => { mocks.command.mockResolvedValue({ timedOut: false, stdout: JSON.stringify(verified), exitCode: 0 }); mocks.metadata.mockResolvedValue(verified); }); describe("Copilot connection verification", () => { + it.each(["ssh", "sandbox", "plugin"])("keeps an explicitly selected %s environment under managed-only execution", async driver => { + mocks.experimental.mockResolvedValue({ enableManagedSandboxOnly: true }); + mocks.environment.mockResolvedValue({ id: "selected", driver, status: "active", config: { provider: "daytona" } }); + expect(await probeCopilotConnection({} as Db, "company", "private-token", "selected")).toEqual(verified); + expect(mocks.managed).not.toHaveBeenCalled(); + expect(mocks.resolve).toHaveBeenCalledWith(expect.objectContaining({ environment: expect.objectContaining({ id: "selected", driver }) })); + expect(mocks.metadata).not.toHaveBeenCalled(); + expect(mocks.command).toHaveBeenCalledOnce(); + }); + it.each([null, "local"])("redirects only a local selection (%s) onto the managed environment", async selection => { + mocks.experimental.mockResolvedValue({ enableManagedSandboxOnly: true }); + mocks.managed.mockResolvedValue({ id: "managed" }); + mocks.environment.mockImplementation(async id => ({ id, driver: id === "local" ? "local" : "sandbox", status: "active", config: { provider: "daytona" } })); + expect(await probeCopilotConnection({} as Db, "company", "private-token", selection)).toEqual(verified); + expect(mocks.resolve).toHaveBeenCalledWith(expect.objectContaining({ environment: expect.objectContaining({ id: "managed" }) })); + expect(mocks.metadata).not.toHaveBeenCalled(); + }); + it("gives forced Kubernetes precedence over an explicitly selected remote and managed-only policy", async () => { + mocks.general.mockResolvedValue({ executionMode: "kubernetes" }); + mocks.experimental.mockResolvedValue({ enableManagedSandboxOnly: true }); + mocks.kubernetes.mockResolvedValue({ id: "kubernetes" }); + mocks.environment.mockImplementation(async id => ({ id, driver: "sandbox", status: "active", config: { provider: "kubernetes" } })); + expect(await probeCopilotConnection({} as Db, "company", "private-token", "selected")).toEqual(verified); + expect(mocks.resolve).toHaveBeenCalledWith(expect.objectContaining({ environment: expect.objectContaining({ id: "kubernetes" }) })); + expect(mocks.managed).not.toHaveBeenCalled(); + expect(mocks.metadata).not.toHaveBeenCalled(); + }); it("probes the selected environment and releases its owned lease", async () => { const db = {} as Db; const pluginWorkerManager = {} as PluginWorkerManager; diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 7c7037d9388..56ed7c939fe 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1225,7 +1225,14 @@ export function agentRoutes( // A null agent override inherits the instance default, just like dispatch. // Resolve this before secrets or probes so a default remote environment can // never accidentally validate the account on the control-plane host. - async function resolveAdapterTestEnvironmentId(companyId: string, environmentId: string | null | undefined) { + async function resolveAdapterTestEnvironmentId(companyId: string, environmentId: string | null | undefined, copilot = false) { + // Copilot admission must verify the same forced environment as dispatch, + // even when the agent explicitly selects the controller or another host. + if (copilot && (await instanceSettings.getGeneral()).executionMode === "kubernetes") { + const kubernetes = await environmentsSvc.findKubernetesEnvironment(companyId); + if (!kubernetes) throw unprocessable("The Kubernetes execution environment is unavailable.", { code: "copilot_environment_unavailable" }); + return kubernetes.id; + } if (environmentId) return environmentId; const settings = await instanceSettings.get(); if (settings.defaultEnvironmentId) return settings.defaultEnvironmentId; @@ -3603,7 +3610,7 @@ export function agentRoutes( // packaged runtime before creation/adoption even when a token is saved. // This metadata-only path sends no model prompt. if (test || binding.provider === "github") { - const testEnvironmentId = await resolveAdapterTestEnvironmentId(companyId, environmentId); + const testEnvironmentId = await resolveAdapterTestEnvironmentId(companyId, environmentId, binding.provider === "github"); if (testEnvironmentId) await assertAdapterTestEnvironmentForCompany(companyId, testEnvironmentId); const target = await resolveAdapterTestExecutionContext({ companyId, adapterType, environmentId: testEnvironmentId }); try { @@ -3661,6 +3668,7 @@ export function agentRoutes( req.body.environmentId === undefined ? savedAgent?.defaultEnvironmentId : asNonEmptyString(req.body.environmentId), + aiBinding?.provider === "github", ); // Fail closed on a foreign environment before any secret resolution, env // merge, target resolution, sandbox lease, or adapter test runs. diff --git a/server/src/services/copilot-connection-probe.ts b/server/src/services/copilot-connection-probe.ts index 12860f67c7e..b7fce1978b6 100644 --- a/server/src/services/copilot-connection-probe.ts +++ b/server/src/services/copilot-connection-probe.ts @@ -19,7 +19,7 @@ export async function probeCopilotConnection(db: Db, companyId: string, token: s const kubernetes = await environments.findKubernetesEnvironment(companyId); if (!kubernetes) throw unprocessable("The Kubernetes execution environment is unavailable.", { code: "copilot_environment_unavailable" }); environmentId = kubernetes.id; - } else if (experimental.enableManagedSandboxOnly) { + } else if (experimental.enableManagedSandboxOnly && (!environmentId || (await environments.getById(environmentId))?.driver === "local")) { const managed = await environments.findManagedSandboxEnvironment(companyId); if (!managed) throw unprocessable("The managed environment is unavailable.", { code: "copilot_environment_unavailable" }); environmentId = managed.id;