From 63e26e9147a695414f6d10b41e82e7263af686d6 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 19 Jun 2026 19:50:39 -0700 Subject: [PATCH] Make environment APIs instance-scoped --- .../environment-instance-routes.test.ts | 214 +++++++++++++++ .../src/__tests__/environment-service.test.ts | 32 +-- .../instance-settings-service.test.ts | 1 + server/src/routes/agents.ts | 8 +- server/src/routes/environment-selection.ts | 5 +- server/src/routes/environments.ts | 258 ++++++++---------- server/src/services/environment-probe.ts | 18 +- .../services/environment-run-orchestrator.ts | 6 - server/src/services/environments.ts | 106 ++++--- server/src/services/instance-settings.ts | 2 + server/src/services/secrets.ts | 16 +- 11 files changed, 451 insertions(+), 215 deletions(-) create mode 100644 server/src/__tests__/environment-instance-routes.test.ts diff --git a/server/src/__tests__/environment-instance-routes.test.ts b/server/src/__tests__/environment-instance-routes.test.ts new file mode 100644 index 00000000000..7e7ac811b4c --- /dev/null +++ b/server/src/__tests__/environment-instance-routes.test.ts @@ -0,0 +1,214 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { environmentRoutes } from "../routes/environments.js"; +import { errorHandler } from "../middleware/index.js"; + +const mockIssueService = vi.hoisted(() => ({ + clearExecutionWorkspaceEnvironmentSelection: vi.fn(), +})); + +const mockProjectService = vi.hoisted(() => ({ + clearExecutionWorkspaceEnvironmentSelection: vi.fn(), +})); + +const mockInstanceSettingsService = vi.hoisted(() => ({ + listCompanyIds: vi.fn(), +})); + +const mockEnvironmentService = vi.hoisted(() => ({ + list: vi.fn(), + getById: vi.fn(), + create: vi.fn(), +})); + +const mockExecutionWorkspaceService = vi.hoisted(() => ({ + clearEnvironmentSelection: vi.fn(), +})); + +const mockLogActivity = vi.hoisted(() => vi.fn()); + +const mockSecretService = vi.hoisted(() => ({ + create: vi.fn(), + listBindingCompanyIdsForTarget: vi.fn(), + resolveSecretValueForEphemeralAccess: vi.fn(), + syncSecretRefsForTarget: vi.fn(), +})); + +vi.mock("../services/index.js", () => ({ + issueService: () => mockIssueService, + instanceSettingsService: () => mockInstanceSettingsService, + logActivity: mockLogActivity, + projectService: () => mockProjectService, +})); + +vi.mock("../services/environments.js", () => ({ + environmentService: () => mockEnvironmentService, +})); + +vi.mock("../services/execution-workspaces.js", () => ({ + executionWorkspaceService: () => mockExecutionWorkspaceService, +})); + +vi.mock("../services/secrets.js", () => ({ + secretService: () => mockSecretService, +})); + +vi.mock("../services/plugin-environment-driver.js", () => ({ + listReadyPluginEnvironmentDrivers: vi.fn(async () => []), +})); + +function createEnvironment(overrides: Record = {}) { + const now = new Date("2026-06-20T00:00:00.000Z"); + return { + id: "env-1", + name: "Local", + description: "Default execution environment", + driver: "local", + status: "active" as const, + config: {}, + envVars: {}, + metadata: { managedByPaperclip: true }, + createdAt: now, + updatedAt: now, + ...overrides, + }; +} + +function createApp(actor: Record) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as typeof req & { actor: Record }).actor = actor; + next(); + }); + app.use("/api", environmentRoutes({} as never)); + app.use(errorHandler); + return app; +} + +describe("environment instance routes", () => { + beforeEach(() => { + mockIssueService.clearExecutionWorkspaceEnvironmentSelection.mockReset(); + mockProjectService.clearExecutionWorkspaceEnvironmentSelection.mockReset(); + mockInstanceSettingsService.listCompanyIds.mockReset(); + mockEnvironmentService.list.mockReset(); + mockEnvironmentService.getById.mockReset(); + mockEnvironmentService.create.mockReset(); + mockExecutionWorkspaceService.clearEnvironmentSelection.mockReset(); + mockLogActivity.mockReset(); + mockSecretService.create.mockReset(); + mockSecretService.listBindingCompanyIdsForTarget.mockReset(); + mockSecretService.resolveSecretValueForEphemeralAccess.mockReset(); + mockSecretService.syncSecretRefsForTarget.mockReset(); + + mockInstanceSettingsService.listCompanyIds.mockResolvedValue(["company-1", "company-2"]); + mockEnvironmentService.list.mockResolvedValue([]); + mockEnvironmentService.create.mockResolvedValue(createEnvironment()); + mockSecretService.syncSecretRefsForTarget.mockResolvedValue([]); + }); + + it("lists the instance environment catalog for a local board actor", async () => { + mockEnvironmentService.list.mockResolvedValue([createEnvironment()]); + const app = createApp({ + type: "board", + userId: "board-1", + source: "local_implicit", + isInstanceAdmin: true, + }); + + const res = await request(app).get("/api/companies/company-1/environments?driver=local"); + + expect(res.status).toBe(200); + expect(res.body).toHaveLength(1); + expect(mockEnvironmentService.list).toHaveBeenCalledWith({ + status: undefined, + driver: "local", + }); + }); + + it("rejects company agents from enumerating the shared environment catalog", async () => { + const app = createApp({ + type: "agent", + agentId: "agent-1", + companyId: "company-1", + source: "agent_key", + runId: "run-1", + }); + + const res = await request(app).get("/api/companies/company-1/environments"); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("board operators"); + }); + + it("rejects non-admin signed-in board members from mutating instance environments", async () => { + const app = createApp({ + type: "board", + userId: "user-1", + source: "session", + companyIds: ["company-1"], + isInstanceAdmin: false, + }); + + const res = await request(app) + .post("/api/companies/company-1/environments") + .send({ + name: "Shared Local", + driver: "local", + config: {}, + }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Instance admin"); + expect(mockEnvironmentService.create).not.toHaveBeenCalled(); + }); + + it("creates an instance-scoped environment and logs the mutation to every company", async () => { + const app = createApp({ + type: "board", + userId: "board-1", + source: "local_implicit", + isInstanceAdmin: true, + }); + + const res = await request(app) + .post("/api/companies/company-1/environments") + .send({ + name: "Shared Local", + driver: "local", + config: {}, + }); + + expect(res.status).toBe(201); + expect(mockEnvironmentService.create).toHaveBeenCalledWith( + expect.objectContaining({ + name: "Shared Local", + driver: "local", + status: "active", + }), + ); + expect(mockSecretService.syncSecretRefsForTarget).toHaveBeenCalledWith( + "company-1", + { targetType: "environment", targetId: "env-1" }, + [], + ); + expect(mockLogActivity).toHaveBeenCalledTimes(2); + expect(mockLogActivity.mock.calls.map((call) => call[1].companyId)).toEqual(["company-1", "company-2"]); + }); + + it("returns full environment details for an instance admin", async () => { + mockEnvironmentService.getById.mockResolvedValue(createEnvironment({ config: { shell: "zsh" } })); + const app = createApp({ + type: "board", + userId: "board-1", + source: "local_implicit", + isInstanceAdmin: true, + }); + + const res = await request(app).get("/api/environments/env-1"); + + expect(res.status).toBe(200); + expect(res.body.config).toEqual({ shell: "zsh" }); + }); +}); diff --git a/server/src/__tests__/environment-service.test.ts b/server/src/__tests__/environment-service.test.ts index 37788d9ca79..dd15546750d 100644 --- a/server/src/__tests__/environment-service.test.ts +++ b/server/src/__tests__/environment-service.test.ts @@ -69,11 +69,15 @@ describeEmbeddedPostgres("environmentService leases", () => { }); await db.insert(environments).values({ id: environmentId, - companyId, - name: "Local", - driver: "local", + name: "Lease Fixture", + driver: "ssh", status: "active", - config: {}, + config: { + host: "fixture.example.test", + port: 22, + username: "fixture", + remoteWorkspacePath: "/srv/paperclip", + }, createdAt: new Date(), updatedAt: new Date(), }); @@ -158,7 +162,7 @@ describeEmbeddedPostgres("environmentService leases", () => { expect(created.driver).toBe("local"); expect(reused.id).toBe(created.id); - const rows = await db.select().from(environments).where(eq(environments.companyId, companyId)); + const rows = await db.select().from(environments).where(eq(environments.driver, "local")); expect(rows).toHaveLength(1); expect(rows[0]?.name).toBe("Local"); }); @@ -176,7 +180,6 @@ describeEmbeddedPostgres("environmentService leases", () => { const [existing] = await db .insert(environments) .values({ - companyId, name: "Archived Local", description: "Operator-managed local environment", driver: "local", @@ -195,7 +198,7 @@ describeEmbeddedPostgres("environmentService leases", () => { expect(ensured.status).toBe("archived"); expect(ensured.metadata).toEqual({ owner: "operator" }); - const rows = await db.select().from(environments).where(eq(environments.companyId, companyId)); + const rows = await db.select().from(environments).where(eq(environments.driver, "local")); expect(rows).toHaveLength(1); expect(rows[0]?.updatedAt.toISOString()).toBe(archivedAt.toISOString()); }); @@ -216,7 +219,7 @@ describeEmbeddedPostgres("environmentService leases", () => { expect(new Set(results.map((environment) => environment.id)).size).toBe(1); - const rows = await db.select().from(environments).where(eq(environments.companyId, companyId)); + const rows = await db.select().from(environments).where(eq(environments.driver, "local")); expect(rows).toHaveLength(1); expect(rows[0]?.driver).toBe("local"); expect(rows[0]?.status).toBe("active"); @@ -268,7 +271,7 @@ describeEmbeddedPostgres("environmentService leases", () => { const rows = await db .select() .from(environments) - .where(eq(environments.companyId, companyId)); + .where(eq(environments.driver, "sandbox")); expect(rows.filter((row) => row.driver === "sandbox")).toHaveLength(1); }); @@ -295,7 +298,7 @@ describeEmbeddedPostgres("environmentService leases", () => { const rows = await db .select() .from(environments) - .where(and(eq(environments.companyId, companyId), eq(environments.driver, "sandbox"))); + .where(eq(environments.driver, "sandbox")); expect(rows).toHaveLength(1); expect((rows[0]?.metadata as Record)?.managedKubernetesSandbox).toBe(true); }); @@ -312,7 +315,6 @@ describeEmbeddedPostgres("environmentService leases", () => { const now = new Date(); await db.insert(environments).values({ - companyId, name: "First", driver: "sandbox", status: "active", @@ -327,7 +329,6 @@ describeEmbeddedPostgres("environmentService leases", () => { // same company. This is the DB-level invariant that replaced the previous // application-side post-insert convergence loop. const secondInsert = db.insert(environments).values({ - companyId, name: "Second", driver: "sandbox", status: "active", @@ -346,12 +347,11 @@ describeEmbeddedPostgres("environmentService leases", () => { (error as { cause?: { constraint_name?: string } })?.cause?.constraint_name ?? "unknown"; } - expect(raisedConstraint).toBe("environments_company_managed_sandbox_idx"); + expect(raisedConstraint).toBe("environments_managed_sandbox_idx"); // Index does NOT cover tenant-created sandbox rows (no managedByPaperclip // marker) — operators must be able to keep multiple tenant sandbox envs. await db.insert(environments).values({ - companyId, name: "Tenant Sandbox", driver: "sandbox", status: "active", @@ -364,7 +364,7 @@ describeEmbeddedPostgres("environmentService leases", () => { const rows = await db .select() .from(environments) - .where(and(eq(environments.companyId, companyId), eq(environments.driver, "sandbox"))); + .where(eq(environments.driver, "sandbox")); expect(rows).toHaveLength(2); }); @@ -441,7 +441,7 @@ describeEmbeddedPostgres("environmentService leases", () => { expect(first.id).not.toBe(second.id); - const rows = await db.select().from(environments).where(eq(environments.companyId, companyId)); + const rows = await db.select().from(environments); expect(rows.filter((row) => row.driver === "ssh")).toHaveLength(2); }); }); diff --git a/server/src/__tests__/instance-settings-service.test.ts b/server/src/__tests__/instance-settings-service.test.ts index 13849e47279..dd83b47b379 100644 --- a/server/src/__tests__/instance-settings-service.test.ts +++ b/server/src/__tests__/instance-settings-service.test.ts @@ -19,6 +19,7 @@ describe("instance settings service", () => { enableIsolatedWorkspaces: true, enableStreamlinedLeftNavigation: false, enableConferenceRoomChat: false, + enableTaskWatchdogs: false, enableIssuePlanDecompositions: true, enableExperimentalFileViewer: true, enableTaskWatchdogs: true, diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 0ae5312bbd6..07b7ecf46c9 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -271,7 +271,7 @@ export function agentRoutes( } const environment = await environmentsSvc.getById(input.environmentId); - if (!environment || environment.companyId !== input.companyId) { + if (!environment) { return { executionTarget: null, environmentName: null, @@ -865,8 +865,8 @@ export function agentRoutes( ) { if (environmentId === undefined || environmentId === null) return; const environment = await environmentsSvc.getById(environmentId); - if (!environment || environment.companyId !== companyId) { - throw unprocessable("Selected environment must belong to the same company"); + if (!environment) { + throw unprocessable("Selected environment was not found"); } if (options?.allowedDrivers && !options.allowedDrivers.includes(environment.driver)) { throw unprocessable(`Environment driver "${environment.driver}" is not allowed here`); @@ -1592,7 +1592,7 @@ export function agentRoutes( : false; const environmentId = asNonEmptyString(req.query.environmentId); const environment = environmentId ? await environmentsSvc.getById(environmentId) : null; - if (environmentId && (!environment || environment.companyId !== companyId)) { + if (environmentId && !environment) { res.status(404).json({ error: "Environment not found" }); return; } diff --git a/server/src/routes/environment-selection.ts b/server/src/routes/environment-selection.ts index f278f88bfa6..eba3d8772c3 100644 --- a/server/src/routes/environment-selection.ts +++ b/server/src/routes/environment-selection.ts @@ -4,13 +4,12 @@ export async function assertEnvironmentSelectionForCompany( environmentsSvc: { getById(environmentId: string): Promise<{ id: string; - companyId: string; driver: string; status?: string | null; config: Record | null; } | null>; }, - companyId: string, + _companyId: string, environmentId: string | null | undefined, options?: { allowedDrivers?: string[]; @@ -19,7 +18,7 @@ export async function assertEnvironmentSelectionForCompany( ) { if (environmentId === undefined || environmentId === null) return; const environment = await environmentsSvc.getById(environmentId); - if (!environment || environment.companyId !== companyId) { + if (!environment) { throw unprocessable("Environment not found."); } if (environment.status === "archived") { diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 8899988feb7..5316164d3a9 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -7,12 +7,11 @@ import { probeEnvironmentConfigSchema, updateEnvironmentSchema, } from "@paperclipai/shared"; -import { conflict, forbidden } from "../errors.js"; +import { conflict, forbidden, unprocessable } from "../errors.js"; import { validate } from "../middleware/validate.js"; import { - accessService, - agentService, issueService, + instanceSettingsService, logActivity, projectService, } from "../services/index.js"; @@ -20,7 +19,6 @@ import { collectEnvironmentSecretRefs, normalizeEnvironmentConfigForPersistence, normalizeEnvironmentConfigForProbe, - parseEnvironmentDriverConfig, readSshEnvironmentPrivateKeySecretId, type ParsedEnvironmentConfig, } from "../services/environment-config.js"; @@ -28,7 +26,7 @@ import { probeEnvironment } from "../services/environment-probe.js"; import { secretService } from "../services/secrets.js"; import { listReadyPluginEnvironmentDrivers } from "../services/plugin-environment-driver.js"; import { getConfiguredSecretProvider } from "../secrets/configured-provider.js"; -import { assertCompanyAccess, getActorInfo } from "./authz.js"; +import { getActorInfo } from "./authz.js"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; import { environmentService } from "../services/environments.js"; import { executionWorkspaceService } from "../services/execution-workspaces.js"; @@ -38,11 +36,10 @@ export function environmentRoutes( options: { pluginWorkerManager?: PluginWorkerManager } = {}, ) { const router = Router(); - const agents = agentService(db); - const access = accessService(db); const svc = environmentService(db); const executionWorkspaces = executionWorkspaceService(db); const issues = issueService(db); + const instanceSettings = instanceSettingsService(db); const projects = projectService(db); const secrets = secretService(db); @@ -52,77 +49,74 @@ export function environmentRoutes( : {}; } - function canCreateAgents(agent: { permissions: Record | null | undefined }) { - if (!agent.permissions || typeof agent.permissions !== "object") return false; - return Boolean((agent.permissions as Record).canCreateAgents); - } - - async function assertCanMutateEnvironments(req: Request, companyId: string) { - assertCompanyAccess(req, companyId); - - if (req.actor.type === "board") { - if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) return; - const allowed = await access.canUser(companyId, req.actor.userId, "environments:manage"); - if (!allowed) { - throw forbidden("Missing permission: environments:manage"); - } - return; + function assertCanAccessInstanceEnvironments(req: Request) { + if (req.actor.type !== "board") { + throw forbidden("Instance environment management is restricted to board operators"); } - - if (!req.actor.agentId) { - throw forbidden("Agent authentication required"); - } - - const actorAgent = await agents.getById(req.actor.agentId); - if (!actorAgent || actorAgent.companyId !== companyId) { - throw forbidden("Agent key cannot access another company"); - } - - const allowedByGrant = await access.hasPermission(companyId, "agent", actorAgent.id, "environments:manage"); - if (allowedByGrant || canCreateAgents(actorAgent)) { - return; - } - - throw forbidden("Missing permission: environments:manage"); + if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) return; + throw forbidden("Instance admin access required"); } async function assertCanReadSecretsForDraftProbe(req: Request, companyId: string) { - const decision = await access.decide({ - actor: req.actor, - action: "secrets:read", - resource: { type: "company", companyId }, + assertCanAccessInstanceEnvironments(req); + return companyId; + } + + async function logInstanceEnvironmentActivity(input: { + actor: ReturnType; + action: string; + entityId: string; + details: Record; + }) { + const companyIds = await instanceSettings.listCompanyIds(); + await Promise.all( + companyIds.map((companyId) => + logActivity(db, { + companyId, + actorType: input.actor.actorType, + actorId: input.actor.actorId, + agentId: input.actor.agentId, + runId: input.actor.runId, + action: input.action, + entityType: "environment", + entityId: input.entityId, + details: input.details, + }) + ), + ); + } + + async function resolveEnvironmentSecretContextCompanyId( + req: Request, + environmentId: string, + options: { required: boolean }, + ): Promise { + const routeCompanyId = + typeof req.params.companyId === "string" && req.params.companyId.trim().length > 0 + ? req.params.companyId.trim() + : typeof req.query.companyId === "string" && req.query.companyId.trim().length > 0 + ? req.query.companyId.trim() + : null; + const bindingCompanyIds = await secrets.listBindingCompanyIdsForTarget({ + targetType: "environment", + targetId: environmentId, }); - if (!decision.allowed) { - throw forbidden(decision.explanation); + if (routeCompanyId && bindingCompanyIds.length > 0 && !bindingCompanyIds.includes(routeCompanyId)) { + throw conflict("Environment secret bindings already use a different company context."); } - } - - async function actorCanReadEnvironmentConfigurations(req: Request, companyId: string) { - assertCompanyAccess(req, companyId); - - if (req.actor.type === "board") { - if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) return true; - return access.canUser(companyId, req.actor.userId, "environments:manage"); + if (routeCompanyId) return routeCompanyId; + if (bindingCompanyIds.length === 1) return bindingCompanyIds[0] ?? null; + if (bindingCompanyIds.length > 1) { + throw conflict("Environment secret bindings span multiple companies and require explicit companyId context."); } - - if (!req.actor.agentId) return false; - const actorAgent = await agents.getById(req.actor.agentId); - if (!actorAgent || actorAgent.companyId !== companyId) return false; - const allowedByGrant = await access.hasPermission(companyId, "agent", actorAgent.id, "environments:manage"); - return allowedByGrant || canCreateAgents(actorAgent); - } - - function redactEnvironmentForRestrictedView; - metadata: Record | null; - }>(environment: T): T & { configRedacted: true; metadataRedacted: true } { - return { - ...environment, - config: {}, - metadata: null, - configRedacted: true, - metadataRedacted: true, - }; + if (req.actor.type === "agent" && req.actor.companyId) return req.actor.companyId; + if (req.actor.type === "board" && Array.isArray(req.actor.companyIds) && req.actor.companyIds.length === 1) { + return req.actor.companyIds[0] ?? null; + } + if (!options.required) return null; + throw unprocessable( + "Environment secret management requires a companyId context during the instance-scoped transition.", + ); } function summarizeEnvironmentUpdate( @@ -160,23 +154,16 @@ export function environmentRoutes( } router.get("/companies/:companyId/environments", async (req, res) => { - const companyId = req.params.companyId as string; - assertCompanyAccess(req, companyId); - const rows = await svc.list(companyId, { + assertCanAccessInstanceEnvironments(req); + const rows = await svc.list({ status: req.query.status as string | undefined, driver: req.query.driver as string | undefined, }); - const canReadConfigs = await actorCanReadEnvironmentConfigurations(req, companyId); - if (canReadConfigs) { - res.json(rows); - return; - } - res.json(rows.map((environment) => redactEnvironmentForRestrictedView(environment))); + res.json(rows); }); router.get("/companies/:companyId/environments/capabilities", async (req, res) => { - const companyId = req.params.companyId as string; - assertCompanyAccess(req, companyId); + assertCanAccessInstanceEnvironments(req); const pluginDrivers = await listReadyPluginEnvironmentDrivers({ db, workerManager: options.pluginWorkerManager, @@ -206,11 +193,11 @@ export function environmentRoutes( router.post("/companies/:companyId/environments", validate(createEnvironmentSchema), async (req, res) => { const companyId = req.params.companyId as string; - await assertCanMutateEnvironments(req, companyId); + assertCanAccessInstanceEnvironments(req); if (req.body.driver === "local") { - const existingLocal = await svc.list(companyId, { driver: "local" }); + const existingLocal = await svc.list({ driver: "local" }); if (existingLocal.length > 0) { - throw conflict("A local environment already exists for this company."); + throw conflict("A local environment already exists for this instance."); } } const actor = getActorInfo(req); @@ -230,20 +217,15 @@ export function environmentRoutes( pluginWorkerManager: options.pluginWorkerManager, }), }; - const environment = await svc.create(companyId, input); + const environment = await svc.create(input); await secrets.syncSecretRefsForTarget( companyId, { targetType: "environment", targetId: environment.id }, await collectEnvironmentSecretRefs({ db, environment }), ); - await logActivity(db, { - companyId, - actorType: actor.actorType, - actorId: actor.actorId, - agentId: actor.agentId, - runId: actor.runId, + await logInstanceEnvironmentActivity({ + actor, action: "environment.created", - entityType: "environment", entityId: environment.id, details: { name: environment.name, @@ -260,13 +242,8 @@ export function environmentRoutes( res.status(404).json({ error: "Environment not found" }); return; } - assertCompanyAccess(req, environment.companyId); - const canReadConfigs = await actorCanReadEnvironmentConfigurations(req, environment.companyId); - if (canReadConfigs) { - res.json(environment); - return; - } - res.json(redactEnvironmentForRestrictedView(environment)); + assertCanAccessInstanceEnvironments(req); + res.json(environment); }); router.get("/environments/:id/leases", async (req, res) => { @@ -275,11 +252,7 @@ export function environmentRoutes( res.status(404).json({ error: "Environment not found" }); return; } - assertCompanyAccess(req, environment.companyId); - const canReadConfigs = await actorCanReadEnvironmentConfigurations(req, environment.companyId); - if (!canReadConfigs) { - throw forbidden("Missing permission: environments:manage"); - } + assertCanAccessInstanceEnvironments(req); const leases = await svc.listLeases(environment.id, { status: req.query.status as string | undefined, }); @@ -292,11 +265,7 @@ export function environmentRoutes( res.status(404).json({ error: "Environment lease not found" }); return; } - assertCompanyAccess(req, lease.companyId); - const canReadConfigs = await actorCanReadEnvironmentConfigurations(req, lease.companyId); - if (!canReadConfigs) { - throw forbidden("Missing permission: environments:manage"); - } + assertCanAccessInstanceEnvironments(req); res.json(lease); }); @@ -306,10 +275,14 @@ export function environmentRoutes( res.status(404).json({ error: "Environment not found" }); return; } - await assertCanMutateEnvironments(req, existing.companyId); + assertCanAccessInstanceEnvironments(req); const actor = getActorInfo(req); const nextDriver = req.body.driver ?? existing.driver; const nextName = req.body.name ?? existing.name; + const companyIdForSecrets = + req.body.config !== undefined || req.body.driver !== undefined + ? await resolveEnvironmentSecretContextCompanyId(req, existing.id, { required: true }) + : null; const configSource = req.body.config !== undefined ? req.body.driver !== undefined && req.body.driver !== existing.driver @@ -327,7 +300,7 @@ export function environmentRoutes( ? { config: await normalizeEnvironmentConfigForPersistence({ db, - companyId: existing.companyId, + companyId: companyIdForSecrets!, environmentName: nextName, driver: nextDriver, secretProvider: getConfiguredSecretProvider(), @@ -348,19 +321,14 @@ export function environmentRoutes( } if (patch.config !== undefined || patch.driver !== undefined) { await secrets.syncSecretRefsForTarget( - environment.companyId, + companyIdForSecrets!, { targetType: "environment", targetId: environment.id }, await collectEnvironmentSecretRefs({ db, environment }), ); } - await logActivity(db, { - companyId: environment.companyId, - actorType: actor.actorType, - actorId: actor.actorId, - agentId: actor.agentId, - runId: actor.runId, + await logInstanceEnvironmentActivity({ + actor, action: "environment.updated", - entityType: "environment", entityId: environment.id, details: summarizeEnvironmentUpdate(patch as Record, environment), }); @@ -373,12 +341,15 @@ export function environmentRoutes( res.status(404).json({ error: "Environment not found" }); return; } - await assertCanMutateEnvironments(req, existing.companyId); - await Promise.all([ - executionWorkspaces.clearEnvironmentSelection(existing.companyId, existing.id), - issues.clearExecutionWorkspaceEnvironmentSelection(existing.companyId, existing.id), - projects.clearExecutionWorkspaceEnvironmentSelection(existing.companyId, existing.id), - ]); + assertCanAccessInstanceEnvironments(req); + const companyIds = await instanceSettings.listCompanyIds(); + await Promise.all( + companyIds.flatMap((companyId) => [ + executionWorkspaces.clearEnvironmentSelection(companyId, existing.id), + issues.clearExecutionWorkspaceEnvironmentSelection(companyId, existing.id), + projects.clearExecutionWorkspaceEnvironmentSelection(companyId, existing.id), + ]), + ); const removed = await svc.remove(existing.id); if (!removed) { res.status(404).json({ error: "Environment not found" }); @@ -389,14 +360,9 @@ export function environmentRoutes( await secrets.remove(secretId); } const actor = getActorInfo(req); - await logActivity(db, { - companyId: existing.companyId, - actorType: actor.actorType, - actorId: actor.actorId, - agentId: actor.agentId, - runId: actor.runId, + await logInstanceEnvironmentActivity({ + actor, action: "environment.deleted", - entityType: "environment", entityId: removed.id, details: { name: removed.name, @@ -413,19 +379,16 @@ export function environmentRoutes( res.status(404).json({ error: "Environment not found" }); return; } - await assertCanMutateEnvironments(req, environment.companyId); + assertCanAccessInstanceEnvironments(req); const actor = getActorInfo(req); + const companyIdForSecrets = await resolveEnvironmentSecretContextCompanyId(req, environment.id, { required: false }); const probe = await probeEnvironment(db, environment, { + companyId: companyIdForSecrets, pluginWorkerManager: options.pluginWorkerManager, }); - await logActivity(db, { - companyId: environment.companyId, - actorType: actor.actorType, - actorId: actor.actorId, - agentId: actor.agentId, - runId: actor.runId, + await logInstanceEnvironmentActivity({ + actor, action: "environment.probed", - entityType: "environment", entityId: environment.id, details: { driver: environment.driver, @@ -441,7 +404,7 @@ export function environmentRoutes( validate(probeEnvironmentConfigSchema), async (req, res) => { const companyId = req.params.companyId as string; - await assertCanMutateEnvironments(req, companyId); + assertCanAccessInstanceEnvironments(req); if (req.body.driver === "sandbox") { // Draft sandbox probes can resolve unbound secret refs, so require // the same company-scoped secret-read capability before normalization. @@ -469,25 +432,22 @@ export function environmentRoutes( driver: req.body.driver, status: "active" as const, config: normalizedConfig, + envVars: {}, metadata: req.body.metadata ?? null, createdAt: new Date(), updatedAt: new Date(), }; const probe = await probeEnvironment(db, environment, { + companyId, pluginWorkerManager: options.pluginWorkerManager, resolvedConfig: { driver: req.body.driver, config: normalizedConfig, } as ParsedEnvironmentConfig, }); - await logActivity(db, { - companyId, - actorType: actor.actorType, - actorId: actor.actorId, - agentId: actor.agentId, - runId: actor.runId, + await logInstanceEnvironmentActivity({ + actor, action: "environment.probed_unsaved", - entityType: "environment", entityId: "unsaved", details: { driver: environment.driver, diff --git a/server/src/services/environment-probe.ts b/server/src/services/environment-probe.ts index 83c881829dd..8c0e39b31a2 100644 --- a/server/src/services/environment-probe.ts +++ b/server/src/services/environment-probe.ts @@ -2,6 +2,7 @@ import type { Environment, EnvironmentProbeResult } from "@paperclipai/shared"; import type { Db } from "@paperclipai/db"; import { ensureSshWorkspaceReady } from "@paperclipai/adapter-utils/ssh"; import { + parseEnvironmentDriverConfig, resolveEnvironmentDriverConfigForRuntime, type ParsedEnvironmentConfig, } from "./environment-config.js"; @@ -13,9 +14,18 @@ import type { PluginWorkerManager } from "./plugin-worker-manager.js"; export async function probeEnvironment( db: Db, environment: Environment, - options: { pluginWorkerManager?: PluginWorkerManager; resolvedConfig?: ParsedEnvironmentConfig } = {}, + options: { + companyId?: string | null; + pluginWorkerManager?: PluginWorkerManager; + resolvedConfig?: ParsedEnvironmentConfig; + } = {}, ): Promise { - const parsed = options.resolvedConfig ?? await resolveEnvironmentDriverConfigForRuntime(db, environment.companyId, environment); + const resolvedCompanyId = options.companyId ?? null; + const parsed = options.resolvedConfig ?? ( + resolvedCompanyId + ? await resolveEnvironmentDriverConfigForRuntime(db, resolvedCompanyId, environment) + : parseEnvironmentDriverConfig(environment) + ); if (parsed.driver === "local") { return { @@ -44,7 +54,7 @@ export async function probeEnvironment( return await probePluginSandboxProviderDriver({ db, workerManager: options.pluginWorkerManager, - companyId: environment.companyId, + companyId: resolvedCompanyId ?? "instance", environmentId: environment.id, provider: parsed.config.provider, config: parsed.config as unknown as Record, @@ -68,7 +78,7 @@ export async function probeEnvironment( return await probePluginEnvironmentDriver({ db, workerManager: options.pluginWorkerManager, - companyId: environment.companyId, + companyId: resolvedCompanyId ?? "instance", environmentId: environment.id, config: parsed.config, }); diff --git a/server/src/services/environment-run-orchestrator.ts b/server/src/services/environment-run-orchestrator.ts index 693b57c69c4..4884e6111a7 100644 --- a/server/src/services/environment-run-orchestrator.ts +++ b/server/src/services/environment-run-orchestrator.ts @@ -182,12 +182,6 @@ export function environmentRunOrchestrator( }); } - if (environment.companyId !== input.companyId) { - throw new EnvironmentRunError("environment_not_found", `Environment "${environmentId}" does not belong to this company.`, { - environmentId, - }); - } - if (environment.status !== "active") { throw new EnvironmentRunError("environment_inactive", `Environment "${environment.name}" is not active (status: ${environment.status}).`, { environmentId: environment.id, diff --git a/server/src/services/environments.ts b/server/src/services/environments.ts index 69aaa5c8bbe..4b05f72d225 100644 --- a/server/src/services/environments.ts +++ b/server/src/services/environments.ts @@ -73,16 +73,53 @@ function readEnum(value: string | null, allowed: readonly T[], function toEnvironment(row: EnvironmentRow): Environment { return { id: row.id, - companyId: row.companyId, name: row.name, description: row.description ?? null, driver: readEnum(row.driver, ENVIRONMENT_DRIVERS, "environment driver") ?? "local", status: readEnum(row.status, ENVIRONMENT_STATUSES, "environment status") ?? "active", config: cloneRecord(row.config, {}) ?? {}, + envVars: cloneRecord(row.envVars, {}) ?? {}, metadata: cloneRecord(row.metadata), createdAt: row.createdAt, updatedAt: row.updatedAt, - }; + } as unknown as Environment; +} + +type EnvironmentListFilters = { + status?: string; + driver?: string; +}; + +function resolveListFilters( + companyIdOrFilters?: string | EnvironmentListFilters, + maybeFilters?: EnvironmentListFilters, +): EnvironmentListFilters { + if (typeof companyIdOrFilters === "string") { + return maybeFilters ?? {}; + } + return companyIdOrFilters ?? {}; +} + +function resolveCreateInput( + companyIdOrInput: string | CreateEnvironment, + maybeInput?: CreateEnvironment, +): CreateEnvironment { + if (typeof companyIdOrInput === "string") { + if (!maybeInput) throw new Error("Create environment input is required"); + return maybeInput; + } + return companyIdOrInput; +} + +function resolveKubernetesConfig( + companyIdOrConfig: string | KubernetesEnvironmentConfigInput, + maybeConfig?: KubernetesEnvironmentConfigInput, +): KubernetesEnvironmentConfigInput { + if (typeof companyIdOrConfig === "string") { + if (!maybeConfig) throw new Error("Kubernetes environment config is required"); + return maybeConfig; + } + return companyIdOrConfig; } function toEnvironmentLease(row: EnvironmentLeaseRow): EnvironmentLease { @@ -116,19 +153,17 @@ function toEnvironmentLease(row: EnvironmentLeaseRow): EnvironmentLease { export function environmentService(db: Db) { return { list: async ( - companyId: string, - filters: { - status?: string; - driver?: string; - } = {}, + companyIdOrFilters?: string | EnvironmentListFilters, + maybeFilters?: EnvironmentListFilters, ): Promise => { - const conditions = [eq(environments.companyId, companyId)]; + const filters = resolveListFilters(companyIdOrFilters, maybeFilters); + const conditions = []; if (filters.status) conditions.push(eq(environments.status, filters.status)); if (filters.driver) conditions.push(eq(environments.driver, filters.driver)); const rows = await db .select() .from(environments) - .where(and(...conditions)) + .where(conditions.length > 0 ? and(...conditions) : undefined) .orderBy(desc(environments.updatedAt), desc(environments.createdAt)); return rows.map(toEnvironment); }, @@ -147,26 +182,26 @@ export function environmentService(db: Db) { return row ? toEnvironmentLease(row) : null; }, - ensureLocalEnvironment: async (companyId: string): Promise => { + ensureLocalEnvironment: async (_companyId?: string): Promise => { const now = new Date(); const row = await db .insert(environments) .values({ - companyId, name: DEFAULT_LOCAL_ENVIRONMENT_NAME, description: DEFAULT_LOCAL_ENVIRONMENT_DESCRIPTION, driver: "local", status: "active", config: {}, + envVars: {}, metadata: { managedByPaperclip: true, - defaultForCompany: true, + defaultForInstance: true, }, createdAt: now, updatedAt: now, }) .onConflictDoNothing({ - target: [environments.companyId, environments.driver], + target: [environments.driver], where: sql`${environments.driver} = 'local'`, }) .returning() @@ -176,7 +211,7 @@ export function environmentService(db: Db) { const existing = await db .select() .from(environments) - .where(and(eq(environments.companyId, companyId), eq(environments.driver, "local"))) + .where(eq(environments.driver, "local")) .then((rows) => rows[0] ?? null); if (!existing) { throw new Error("Failed to ensure local environment"); @@ -186,7 +221,7 @@ export function environmentService(db: Db) { /** * Idempotently ensure a managed Kubernetes sandbox environment exists for a - * company, configured from instance/operator-supplied config. Mirrors + * instance, configured from instance/operator-supplied config. Mirrors * `ensureLocalEnvironment`, but there is no DB unique index for sandbox * drivers, so idempotency is by metadata marker + driver lookup. * @@ -196,9 +231,10 @@ export function environmentService(db: Db) { * update egress/runtimeClass via gitops without recreating the row). */ ensureKubernetesEnvironment: async ( - companyId: string, - config: KubernetesEnvironmentConfigInput, + companyIdOrConfig: string | KubernetesEnvironmentConfigInput, + maybeConfig?: KubernetesEnvironmentConfigInput, ): Promise => { + const config = resolveKubernetesConfig(companyIdOrConfig, maybeConfig); const desiredConfig: Record = { ...config, provider: KUBERNETES_PROVIDER_KEY, @@ -211,7 +247,7 @@ export function environmentService(db: Db) { const existing = await db .select() .from(environments) - .where(and(eq(environments.companyId, companyId), eq(environments.driver, "sandbox"))) + .where(eq(environments.driver, "sandbox")) .then((rows) => rows.find( (row) => @@ -235,27 +271,27 @@ export function environmentService(db: Db) { return toEnvironment(updated); } - // The partial unique index `environments_company_managed_sandbox_idx` - // (added in migration 0102) enforces "at most one Paperclip-managed - // sandbox row per company" at the DB level. Use ON CONFLICT DO NOTHING - // keyed on that index so concurrent callers can race the INSERT — only - // one succeeds; losers re-read the surviving row. + // The partial unique index `environments_managed_sandbox_idx` enforces + // "at most one Paperclip-managed sandbox row per instance" at the DB + // level. Use ON CONFLICT DO NOTHING keyed on that index so concurrent + // callers can race the INSERT; losers re-read the surviving row. const inserted = await db .insert(environments) .values({ - companyId, name: DEFAULT_KUBERNETES_ENVIRONMENT_NAME, description: DEFAULT_KUBERNETES_ENVIRONMENT_DESCRIPTION, driver: "sandbox", status: "active", config: desiredConfig, + envVars: {}, metadata: desiredMetadata, createdAt: now, updatedAt: now, }) .onConflictDoNothing({ - target: [environments.companyId], - where: sql`${environments.driver} = 'sandbox' AND (${environments.metadata} ->> 'managedByPaperclip')::boolean = true`, + target: [environments.driver], + where: + sql`${environments.driver} = 'sandbox' AND (${environments.metadata} ->> 'managedByPaperclip')::boolean = true`, }) .returning() .then((rows) => rows[0] ?? null); @@ -264,7 +300,7 @@ export function environmentService(db: Db) { const winner = await db .select() .from(environments) - .where(and(eq(environments.companyId, companyId), eq(environments.driver, "sandbox"))) + .where(eq(environments.driver, "sandbox")) .then( (rows) => rows.find( @@ -281,17 +317,16 @@ export function environmentService(db: Db) { }, /** - * Find an active Kubernetes sandbox environment for a company, if one + * Find the active managed Kubernetes sandbox environment, if one * exists. Read-only counterpart to `ensureKubernetesEnvironment` used by the * per-run execution guard (which must not silently create config-less envs). */ - findKubernetesEnvironment: async (companyId: string): Promise => { + findKubernetesEnvironment: async (_companyId?: string): Promise => { const rows = await db .select() .from(environments) .where( and( - eq(environments.companyId, companyId), eq(environments.driver, "sandbox"), eq(environments.status, "active"), ), @@ -304,17 +339,21 @@ export function environmentService(db: Db) { return match ? toEnvironment(match) : null; }, - create: async (companyId: string, input: CreateEnvironment): Promise => { + create: async ( + companyIdOrInput: string | CreateEnvironment, + maybeInput?: CreateEnvironment, + ): Promise => { + const input = resolveCreateInput(companyIdOrInput, maybeInput); const now = new Date(); const row = await db .insert(environments) .values({ - companyId, name: input.name, description: input.description ?? null, driver: input.driver, status: input.status ?? "active", config: input.config ?? {}, + envVars: (input as CreateEnvironment & { envVars?: Record }).envVars ?? {}, metadata: input.metadata ?? null, createdAt: now, updatedAt: now, @@ -336,6 +375,9 @@ export function environmentService(db: Db) { if (patch.driver !== undefined) values.driver = patch.driver; if (patch.status !== undefined) values.status = patch.status; if (patch.config !== undefined) values.config = patch.config; + if ("envVars" in patch && patch.envVars !== undefined) { + values.envVars = (patch.envVars ?? {}) as Record; + } if (patch.metadata !== undefined) values.metadata = patch.metadata ?? null; const row = await db diff --git a/server/src/services/instance-settings.ts b/server/src/services/instance-settings.ts index 3260d16a991..7d9e0e6cd34 100644 --- a/server/src/services/instance-settings.ts +++ b/server/src/services/instance-settings.ts @@ -47,6 +47,7 @@ export function normalizeExperimentalSettings(raw: unknown): InstanceExperimenta enableIsolatedWorkspaces: parsed.data.enableIsolatedWorkspaces ?? false, enableStreamlinedLeftNavigation: parsed.data.enableStreamlinedLeftNavigation ?? false, enableConferenceRoomChat: parsed.data.enableConferenceRoomChat ?? false, + enableTaskWatchdogs: (parsed.data as Record).enableTaskWatchdogs === true, enableIssuePlanDecompositions: parsed.data.enableIssuePlanDecompositions ?? false, enableExperimentalFileViewer: parsed.data.enableExperimentalFileViewer ?? false, enableTaskWatchdogs: parsed.data.enableTaskWatchdogs ?? false, @@ -63,6 +64,7 @@ export function normalizeExperimentalSettings(raw: unknown): InstanceExperimenta enableIsolatedWorkspaces: false, enableStreamlinedLeftNavigation: false, enableConferenceRoomChat: false, + enableTaskWatchdogs: false, enableIssuePlanDecompositions: false, enableExperimentalFileViewer: false, enableTaskWatchdogs: false, diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index a86051156cf..01a1e81b082 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -865,7 +865,7 @@ export function secretService(db: Db) { status: environments.status, }) .from(environments) - .where(and(eq(environments.companyId, companyId), inArray(environments.id, environmentIds))); + .where(inArray(environments.id, environmentIds)); for (const row of rows) { setTarget({ type: "environment", @@ -2181,6 +2181,20 @@ export function secretService(db: Db) { return normalizedRefs; }, + listBindingCompanyIdsForTarget: async ( + target: { targetType: SecretBindingTargetType; targetId: string }, + ): Promise => + db + .select({ companyId: companySecretBindings.companyId }) + .from(companySecretBindings) + .where( + and( + eq(companySecretBindings.targetType, target.targetType), + eq(companySecretBindings.targetId, target.targetId), + ), + ) + .then((rows) => [...new Set(rows.map((row) => row.companyId))]), + syncEnvBindingsForTarget: async ( companyId: string, target: { targetType: SecretBindingTargetType; targetId: string; pathPrefix?: string },