From ce2fd09269d5e0eae17a328d0e3c44d4d48e42ab Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sat, 20 Jun 2026 01:46:55 -0700 Subject: [PATCH] fix(server): harden environment read and probe paths --- .../src/__tests__/environment-routes.test.ts | 88 +++++++++++++++++++ ...at-accepted-plan-workspace-refresh.test.ts | 14 ++- server/src/routes/environments.ts | 40 ++++++++- 3 files changed, 137 insertions(+), 5 deletions(-) diff --git a/server/src/__tests__/environment-routes.test.ts b/server/src/__tests__/environment-routes.test.ts index 83a1884308e..208e3b5b0f0 100644 --- a/server/src/__tests__/environment-routes.test.ts +++ b/server/src/__tests__/environment-routes.test.ts @@ -247,6 +247,51 @@ describe("environment routes", () => { }); }); + it("redacts environment config for non-admin board readers", async () => { + mockEnvironmentService.list.mockResolvedValue([createEnvironment()]); + const app = createApp({ + type: "board", + userId: "user-2", + source: "session", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "member" }], + isInstanceAdmin: false, + }); + + const res = await request(app).get("/api/companies/company-1/environments"); + + expect(res.status).toBe(200); + expect(res.body[0]).toMatchObject({ + id: "env-1", + name: "Local", + config: {}, + envVars: {}, + metadata: null, + }); + }); + + it("redacts environment detail config for non-admin board readers", async () => { + mockEnvironmentService.getById.mockResolvedValue(createEnvironment()); + const app = createApp({ + type: "board", + userId: "user-2", + source: "session", + companyIds: ["company-1"], + memberships: [{ companyId: "company-1", status: "active", membershipRole: "member" }], + isInstanceAdmin: false, + }); + + const res = await request(app).get("/api/environments/env-1"); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + id: "env-1", + config: {}, + envVars: {}, + metadata: null, + }); + }); + it("returns provider capabilities for the company", async () => { const app = createApp({ type: "board", @@ -1262,6 +1307,49 @@ describe("environment routes", () => { ); }); + it("requires explicit companyId when probing a secret-backed environment without inferable secret context", async () => { + const environment = { + ...createEnvironment(), + driver: "ssh" as const, + config: { + host: "ssh.example.test", + port: 22, + username: "ssh-user", + remoteWorkspacePath: "/srv/paperclip/workspace", + privateKey: null, + privateKeySecretRef: { + type: "secret_ref", + secretId: "11111111-1111-4111-8111-111111111111", + version: "latest", + }, + knownHosts: null, + strictHostKeyChecking: true, + }, + }; + mockEnvironmentService.getById.mockResolvedValue(environment); + mockSecretService.listBindingCompanyIdsForTarget.mockResolvedValue([]); + const app = createApp({ + type: "board", + userId: "user-1", + source: "session", + companyIds: ["company-1", "company-2"], + memberships: [ + { companyId: "company-1", status: "active", membershipRole: "member" }, + { companyId: "company-2", status: "active", membershipRole: "member" }, + ], + isInstanceAdmin: true, + runId: "run-1", + }); + + const res = await request(app) + .post(`/api/environments/${environment.id}/probe`) + .send({}); + + expect(res.status).toBe(422); + expect(res.body.error).toContain("explicit companyId"); + expect(mockProbeEnvironment).not.toHaveBeenCalled(); + }); + it("probes a sandbox environment and logs the result", async () => { const environment = { ...createEnvironment(), diff --git a/server/src/__tests__/heartbeat-accepted-plan-workspace-refresh.test.ts b/server/src/__tests__/heartbeat-accepted-plan-workspace-refresh.test.ts index 11f917854ff..fcde58f3e99 100644 --- a/server/src/__tests__/heartbeat-accepted-plan-workspace-refresh.test.ts +++ b/server/src/__tests__/heartbeat-accepted-plan-workspace-refresh.test.ts @@ -114,9 +114,17 @@ describeEmbeddedPostgres("accepted plan workspace refresh", () => { await db.delete(documents); await db.delete(agentTaskSessions); await db.delete(executionWorkspaces); - await db.delete(activityLog); - await db.delete(heartbeatRunEvents); - await db.delete(heartbeatRuns); + for (let attempt = 0; attempt < 5; attempt += 1) { + await db.delete(activityLog); + await db.delete(heartbeatRunEvents); + try { + await db.delete(heartbeatRuns); + break; + } catch (error) { + if (attempt === 4) throw error; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } await db.delete(issueComments); await db.delete(issues); await db.delete(projectWorkspaces); diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index f0fb87bb978..80e6d491761 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -62,6 +62,34 @@ export function environmentRoutes( assertBoardOrgAccess(req); } + function canReadFullInstanceEnvironment(req: Request) { + return req.actor.type === "board" + && (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin); + } + + function redactEnvironmentForRestrictedView | null; + envVars?: Record | null; + metadata: Record | null; + }>(environment: T): T { + return { + ...environment, + config: {}, + ...(Object.prototype.hasOwnProperty.call(environment, "envVars") ? { envVars: {} } : {}), + metadata: null, + }; + } + + function presentEnvironmentForRead | null; + envVars?: Record | null; + metadata: Record | null; + }>(req: Request, environment: T): T { + return canReadFullInstanceEnvironment(req) + ? environment + : redactEnvironmentForRestrictedView(environment); + } + async function assertCanReadSecretsForDraftProbe(req: Request, companyId: string) { assertCanAccessInstanceEnvironments(req); return companyId; @@ -164,7 +192,7 @@ export function environmentRoutes( status: req.query.status as string | undefined, driver: req.query.driver as string | undefined, }); - res.json(rows); + res.json(rows.map((row) => presentEnvironmentForRead(req, row))); }); router.get("/companies/:companyId/environments/capabilities", async (req, res) => { @@ -258,7 +286,7 @@ export function environmentRoutes( return; } assertCanReadInstanceEnvironments(req); - res.json(environment); + res.json(presentEnvironmentForRead(req, environment)); }); router.get("/environments/:id/leases", async (req, res) => { @@ -413,6 +441,14 @@ export function environmentRoutes( assertCanAccessInstanceEnvironments(req); const actor = getActorInfo(req); const companyIdForSecrets = await resolveEnvironmentSecretContextCompanyId(req, environment.id, { required: false }); + if (!companyIdForSecrets) { + const secretRefs = await collectEnvironmentSecretRefs({ db, environment }); + if (secretRefs.length > 0) { + throw unprocessable( + "Environment probe requires an explicit companyId to resolve secret-backed config for this environment.", + ); + } + } const probe = await probeEnvironment(db, environment, { companyId: companyIdForSecrets, pluginWorkerManager: options.pluginWorkerManager,