From 20853c196a7e2ade6571ebfac60c2ad0f21d5609 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 19 Jun 2026 20:07:33 -0700 Subject: [PATCH] Add environment env var runtime precedence --- .../environment-instance-routes.test.ts | 48 +++++++++++++++++ .../src/__tests__/environment-routes.test.ts | 19 ++++++- .../__tests__/heartbeat-project-env.test.ts | 54 ++++++++++++++++--- server/src/routes/environments.ts | 29 +++++++++- server/src/services/heartbeat.ts | 47 ++++++++++++++++ 5 files changed, 188 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/environment-instance-routes.test.ts b/server/src/__tests__/environment-instance-routes.test.ts index 7e7ac811b4c..a0aafd1eb95 100644 --- a/server/src/__tests__/environment-instance-routes.test.ts +++ b/server/src/__tests__/environment-instance-routes.test.ts @@ -30,8 +30,10 @@ const mockLogActivity = vi.hoisted(() => vi.fn()); const mockSecretService = vi.hoisted(() => ({ create: vi.fn(), + normalizeEnvBindingsForPersistence: vi.fn(), listBindingCompanyIdsForTarget: vi.fn(), resolveSecretValueForEphemeralAccess: vi.fn(), + syncEnvBindingsForTarget: vi.fn(), syncSecretRefsForTarget: vi.fn(), })); @@ -98,13 +100,18 @@ describe("environment instance routes", () => { mockExecutionWorkspaceService.clearEnvironmentSelection.mockReset(); mockLogActivity.mockReset(); mockSecretService.create.mockReset(); + mockSecretService.normalizeEnvBindingsForPersistence.mockReset(); mockSecretService.listBindingCompanyIdsForTarget.mockReset(); mockSecretService.resolveSecretValueForEphemeralAccess.mockReset(); + mockSecretService.syncEnvBindingsForTarget.mockReset(); mockSecretService.syncSecretRefsForTarget.mockReset(); mockInstanceSettingsService.listCompanyIds.mockResolvedValue(["company-1", "company-2"]); mockEnvironmentService.list.mockResolvedValue([]); mockEnvironmentService.create.mockResolvedValue(createEnvironment()); + mockSecretService.normalizeEnvBindingsForPersistence.mockImplementation(async (_companyId, env) => env ?? {}); + mockSecretService.listBindingCompanyIdsForTarget.mockResolvedValue([]); + mockSecretService.syncEnvBindingsForTarget.mockResolvedValue([]); mockSecretService.syncSecretRefsForTarget.mockResolvedValue([]); }); @@ -193,10 +200,51 @@ describe("environment instance routes", () => { { targetType: "environment", targetId: "env-1" }, [], ); + expect(mockSecretService.syncEnvBindingsForTarget).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("normalizes and syncs environment envVars on create", async () => { + const envVars = { + ANTHROPIC_API_KEY: { type: "secret_ref", secretId: "11111111-1111-4111-8111-111111111111", version: "latest" }, + }; + mockSecretService.normalizeEnvBindingsForPersistence.mockResolvedValue(envVars); + mockEnvironmentService.create.mockResolvedValue(createEnvironment({ envVars })); + 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: {}, + envVars, + }); + + expect(res.status).toBe(201); + expect(mockSecretService.normalizeEnvBindingsForPersistence).toHaveBeenCalledWith( + "company-1", + envVars, + expect.objectContaining({ fieldPath: "envVars" }), + ); + expect(mockEnvironmentService.create).toHaveBeenCalledWith(expect.objectContaining({ envVars })); + expect(mockSecretService.syncEnvBindingsForTarget).toHaveBeenCalledWith( + "company-1", + { targetType: "environment", targetId: "env-1" }, + envVars, + ); + }); + it("returns full environment details for an instance admin", async () => { mockEnvironmentService.getById.mockResolvedValue(createEnvironment({ config: { shell: "zsh" } })); const app = createApp({ diff --git a/server/src/__tests__/environment-routes.test.ts b/server/src/__tests__/environment-routes.test.ts index 733a44e83c9..6ac1b2b47d7 100644 --- a/server/src/__tests__/environment-routes.test.ts +++ b/server/src/__tests__/environment-routes.test.ts @@ -23,6 +23,10 @@ const mockProjectService = vi.hoisted(() => ({ getById: vi.fn(), })); +const mockInstanceSettingsService = vi.hoisted(() => ({ + listCompanyIds: vi.fn(), +})); + const mockEnvironmentService = vi.hoisted(() => ({ list: vi.fn(), getById: vi.fn(), @@ -36,8 +40,11 @@ const mockLogActivity = vi.hoisted(() => vi.fn()); const mockProbeEnvironment = vi.hoisted(() => vi.fn()); const mockSecretService = vi.hoisted(() => ({ create: vi.fn(), + normalizeEnvBindingsForPersistence: vi.fn(), + listBindingCompanyIdsForTarget: vi.fn(), resolveSecretValue: vi.fn(), resolveSecretValueForEphemeralAccess: vi.fn(), + syncEnvBindingsForTarget: vi.fn(), syncSecretRefsForTarget: vi.fn(), remove: vi.fn(), })); @@ -48,9 +55,8 @@ const mockResolvePluginSandboxProviderDriverByKey = vi.hoisted(() => vi.fn()); const mockExecutionWorkspaceService = vi.hoisted(() => ({})); vi.mock("../services/index.js", () => ({ - accessService: () => mockAccessService, - agentService: () => mockAgentService, issueService: () => mockIssueService, + instanceSettingsService: () => mockInstanceSettingsService, environmentService: () => mockEnvironmentService, logActivity: mockLogActivity, projectService: () => mockProjectService, @@ -89,6 +95,7 @@ function createEnvironment() { driver: "local", status: "active" as const, config: { shell: "zsh" }, + envVars: {}, metadata: { source: "manual" }, createdAt: now, updatedAt: now, @@ -148,6 +155,7 @@ describe("environment routes", () => { mockAgentService.getById.mockReset(); mockIssueService.getById.mockReset(); mockProjectService.getById.mockReset(); + mockInstanceSettingsService.listCompanyIds.mockReset(); mockEnvironmentService.list.mockReset(); mockEnvironmentService.list.mockResolvedValue([]); mockEnvironmentService.getById.mockReset(); @@ -158,13 +166,20 @@ describe("environment routes", () => { mockLogActivity.mockReset(); mockProbeEnvironment.mockReset(); mockSecretService.create.mockReset(); + mockSecretService.normalizeEnvBindingsForPersistence.mockReset(); + mockSecretService.listBindingCompanyIdsForTarget.mockReset(); mockSecretService.resolveSecretValue.mockReset(); mockSecretService.resolveSecretValueForEphemeralAccess.mockReset(); + mockSecretService.syncEnvBindingsForTarget.mockReset(); mockSecretService.syncSecretRefsForTarget.mockReset(); mockSecretService.remove.mockReset(); mockSecretService.create.mockResolvedValue({ id: "11111111-1111-1111-1111-111111111111", }); + mockInstanceSettingsService.listCompanyIds.mockResolvedValue(["company-1"]); + mockSecretService.normalizeEnvBindingsForPersistence.mockImplementation(async (_companyId, env) => env ?? {}); + mockSecretService.listBindingCompanyIdsForTarget.mockResolvedValue([]); + mockSecretService.syncEnvBindingsForTarget.mockResolvedValue([]); mockSecretService.syncSecretRefsForTarget.mockResolvedValue([]); mockSecretService.remove.mockResolvedValue(null); mockSecretService.resolveSecretValueForEphemeralAccess.mockResolvedValue("resolved-provider-key"); diff --git a/server/src/__tests__/heartbeat-project-env.test.ts b/server/src/__tests__/heartbeat-project-env.test.ts index 2fde8594c64..ab02b46fc3d 100644 --- a/server/src/__tests__/heartbeat-project-env.test.ts +++ b/server/src/__tests__/heartbeat-project-env.test.ts @@ -8,7 +8,7 @@ import { } from "../services/heartbeat.ts"; describe("resolveExecutionRunAdapterConfig", () => { - it("overlays project and routine env on top of agent env and unions secret keys", async () => { + it("overlays environment, project, and routine env on top of agent env and unions secret keys", async () => { const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({ config: { env: { @@ -32,6 +32,24 @@ describe("resolveExecutionRunAdapterConfig", () => { }); const resolveEnvBindings = vi .fn() + .mockResolvedValueOnce({ + env: { + SHARED_KEY: "environment", + ENV_ONLY: "environment-only", + }, + secretKeys: new Set(["ENV_SECRET"]), + manifest: [ + { + configPath: "env.ENV_SECRET", + envKey: "ENV_SECRET", + secretId: "secret-environment", + secretKey: "environment-secret", + version: 1, + provider: "local_encrypted", + outcome: "success", + }, + ], + }) .mockResolvedValueOnce({ env: { SHARED_KEY: "project", @@ -72,6 +90,8 @@ describe("resolveExecutionRunAdapterConfig", () => { const result = await resolveExecutionRunAdapterConfig({ companyId: "company-1", executionRunConfig: { env: { SHARED_KEY: "agent" } }, + environmentId: "environment-1", + environmentEnv: { SHARED_KEY: "environment" }, projectEnv: { SHARED_KEY: "project" }, routineEnv: { SHARED_KEY: "routine" }, routineId: "routine-1", @@ -85,27 +105,34 @@ describe("resolveExecutionRunAdapterConfig", () => { other: "value", env: { SHARED_KEY: "routine", + ENV_ONLY: "environment-only", AGENT_ONLY: "agent-only", PROJECT_ONLY: "project-only", ROUTINE_ONLY: "routine-only", }, }); - expect(Array.from(result.secretKeys).sort()).toEqual(["AGENT_SECRET", "PROJECT_SECRET", "ROUTINE_SECRET"]); + expect(Array.from(result.secretKeys).sort()).toEqual(["AGENT_SECRET", "ENV_SECRET", "PROJECT_SECRET", "ROUTINE_SECRET"]); expect(result.secretManifest.map((entry) => entry.secretId).sort()).toEqual([ "secret-agent", + "secret-environment", "secret-project", "secret-routine", ]); expect(JSON.stringify(result.secretManifest)).not.toContain("agent-only"); + expect(JSON.stringify(result.secretManifest)).not.toContain("environment-only"); expect(JSON.stringify(result.secretManifest)).not.toContain("project-only"); expect(JSON.stringify(result.secretManifest)).not.toContain("routine-only"); - expect(resolveEnvBindings.mock.calls[1]?.[2]).toMatchObject({ + expect(resolveEnvBindings.mock.calls[0]?.[2]).toMatchObject({ + consumerType: "environment", + consumerId: "environment-1", + }); + expect(resolveEnvBindings.mock.calls[2]?.[2]).toMatchObject({ consumerType: "routine", consumerId: "routine-1", }); }); - it("drops Paperclip runtime-owned env before resolving agent, project, and routine overlays", async () => { + it("drops Paperclip runtime-owned env before resolving environment, agent, project, and routine overlays", async () => { const resolveAdapterConfigForRuntime = vi.fn(async (_companyId, config: Record) => ({ config: { ...config, @@ -125,6 +152,12 @@ describe("resolveExecutionRunAdapterConfig", () => { const result = await resolveExecutionRunAdapterConfig({ companyId: "company-1", agentId: "agent-1", + environmentId: "environment-1", + environmentEnv: { + PAPERCLIP_API_KEY: "environment-api-key", + PAPERCLIP_AGENT_ID: "environment-agent", + ENV_ONLY: "environment-only", + }, executionRunConfig: { env: { PAPERCLIP_API_KEY: { type: "secret_ref", secretId: "secret-api-key", version: "latest" }, @@ -149,18 +182,22 @@ describe("resolveExecutionRunAdapterConfig", () => { } as any, }); + expect(resolveEnvBindings.mock.calls[0]?.[1]).toEqual({ + ENV_ONLY: "environment-only", + }); expect(resolveAdapterConfigForRuntime.mock.calls[0]?.[1]).toEqual({ env: { AGENT_ONLY: "agent-only", }, }); - expect(resolveEnvBindings.mock.calls[0]?.[1]).toEqual({ + expect(resolveEnvBindings.mock.calls[1]?.[1]).toEqual({ PROJECT_ONLY: "project-only", }); - expect(resolveEnvBindings.mock.calls[1]?.[1]).toEqual({ + expect(resolveEnvBindings.mock.calls[2]?.[1]).toEqual({ ROUTINE_ONLY: "routine-only", }); expect(result.resolvedConfig.env).toEqual({ + ENV_ONLY: "environment-only", AGENT_ONLY: "agent-only", PROJECT_ONLY: "project-only", ROUTINE_ONLY: "routine-only", @@ -208,9 +245,11 @@ describe("resolveExecutionRunAdapterConfig", () => { agentId: "agent-1", issueId: "issue-1", heartbeatRunId: "run-1", + environmentId: "environment-1", projectId: "project-1", routineId: "routine-1", executionRunConfig: { env: {} }, + environmentEnv: { ENVIRONMENT_FLAG: "plain" }, projectEnv: { PROJECT_FLAG: "plain" }, routineEnv: { ROUTINE_FLAG: "plain" }, trustPreset: { @@ -239,6 +278,9 @@ describe("resolveExecutionRunAdapterConfig", () => { expect(resolveEnvBindings.mock.calls[1]?.[2]).toMatchObject({ allowedBindingIds: ["binding-1"], }); + expect(resolveEnvBindings.mock.calls[2]?.[2]).toMatchObject({ + allowedBindingIds: ["binding-1"], + }); }); it("rejects inline sensitive env values for low-trust runs", async () => { diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 5316164d3a9..22e7b13759d 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -42,6 +42,7 @@ export function environmentRoutes( const instanceSettings = instanceSettingsService(db); const projects = projectService(db); const secrets = secretService(db); + const strictSecretsMode = process.env.PAPERCLIP_SECRETS_STRICT_MODE === "true"; function parseObject(value: unknown): Record { return value && typeof value === "object" && !Array.isArray(value) @@ -203,6 +204,11 @@ export function environmentRoutes( const actor = getActorInfo(req); const input = { ...req.body, + envVars: await secrets.normalizeEnvBindingsForPersistence( + companyId, + req.body.envVars, + { strictMode: strictSecretsMode, fieldPath: "envVars" }, + ), config: await normalizeEnvironmentConfigForPersistence({ db, companyId, @@ -223,6 +229,11 @@ export function environmentRoutes( { targetType: "environment", targetId: environment.id }, await collectEnvironmentSecretRefs({ db, environment }), ); + await secrets.syncEnvBindingsForTarget( + companyId, + { targetType: "environment", targetId: environment.id }, + environment.envVars, + ); await logInstanceEnvironmentActivity({ actor, action: "environment.created", @@ -280,7 +291,7 @@ export function environmentRoutes( const nextDriver = req.body.driver ?? existing.driver; const nextName = req.body.name ?? existing.name; const companyIdForSecrets = - req.body.config !== undefined || req.body.driver !== undefined + req.body.config !== undefined || req.body.driver !== undefined || req.body.envVars !== undefined ? await resolveEnvironmentSecretContextCompanyId(req, existing.id, { required: true }) : null; const configSource = @@ -296,6 +307,15 @@ export function environmentRoutes( : existing.config; const patch = { ...req.body, + ...(req.body.envVars !== undefined + ? { + envVars: await secrets.normalizeEnvBindingsForPersistence( + companyIdForSecrets!, + req.body.envVars, + { strictMode: strictSecretsMode, fieldPath: "envVars" }, + ), + } + : {}), ...(req.body.config !== undefined || req.body.driver !== undefined ? { config: await normalizeEnvironmentConfigForPersistence({ @@ -326,6 +346,13 @@ export function environmentRoutes( await collectEnvironmentSecretRefs({ db, environment }), ); } + if (patch.envVars !== undefined) { + await secrets.syncEnvBindingsForTarget( + companyIdForSecrets!, + { targetType: "environment", targetId: environment.id }, + environment.envVars, + ); + } await logInstanceEnvironmentActivity({ actor, action: "environment.updated", diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index fde4b8a0d19..fb0470c0d99 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -457,6 +457,8 @@ export async function resolveExecutionRunAdapterConfig(input: { agentId?: string | null; issueId?: string | null; heartbeatRunId?: string | null; + environmentId?: string | null; + environmentEnv?: unknown; projectId?: string | null; routineId?: string | null; executionRunConfig: Record; @@ -466,12 +468,14 @@ export async function resolveExecutionRunAdapterConfig(input: { trustPreset?: TrustPresetResolution; }) { const executionRunConfig = stripPaperclipRuntimeEnvFromAdapterConfig(input.executionRunConfig); + const environmentEnv = stripPaperclipRuntimeEnvBindings(input.environmentEnv); const projectEnv = stripPaperclipRuntimeEnvBindings(input.projectEnv); const routineEnv = stripPaperclipRuntimeEnvBindings(input.routineEnv); const lowTrustAllowedBindingIds = input.trustPreset?.kind === "low_trust_review" ? input.trustPreset.boundary.allowedSecretBindingIds ?? [] : undefined; if (input.trustPreset?.kind === "low_trust_review") { + assertLowTrustEnvConfigAllowed(environmentEnv, "environment.env"); assertLowTrustEnvConfigAllowed(executionRunConfig.env, "agent.env"); assertLowTrustEnvConfigAllowed(projectEnv, "project.env"); assertLowTrustEnvConfigAllowed(routineEnv, "routine.env"); @@ -482,6 +486,15 @@ export async function resolveExecutionRunAdapterConfig(input: { // dispatched-then-failed run (which previously surfaced as opaque setup_failed). if (typeof input.secretsSvc.collectMissingRuntimeBindings === "function") { const missingBindings: MissingRuntimeBinding[] = []; + if (environmentEnv && input.environmentId) { + missingBindings.push( + ...(await input.secretsSvc.collectMissingRuntimeBindings( + input.companyId, + environmentEnv, + { consumerType: "environment", consumerId: input.environmentId }, + )), + ); + } if (input.agentId) { missingBindings.push( ...(await input.secretsSvc.collectMissingRuntimeBindings( @@ -524,6 +537,23 @@ export async function resolveExecutionRunAdapterConfig(input: { }); } } + const environmentEnvResolution = environmentEnv + ? await input.secretsSvc.resolveEnvBindings( + input.companyId, + environmentEnv, + input.environmentId + ? { + consumerType: "environment", + consumerId: input.environmentId, + actorType: "agent", + actorId: input.agentId ?? null, + issueId: input.issueId ?? null, + heartbeatRunId: input.heartbeatRunId ?? null, + ...(lowTrustAllowedBindingIds !== undefined ? { allowedBindingIds: lowTrustAllowedBindingIds } : {}), + } + : undefined, + ) + : { env: {}, secretKeys: new Set(), manifest: [] }; const { config: resolvedConfig, secretKeys, manifest } = await input.secretsSvc.resolveAdapterConfigForRuntime( input.companyId, executionRunConfig, @@ -539,6 +569,15 @@ export async function resolveExecutionRunAdapterConfig(input: { } : undefined, ); + if (Object.keys(environmentEnvResolution.env).length > 0) { + resolvedConfig.env = { + ...environmentEnvResolution.env, + ...parseObject(resolvedConfig.env), + }; + for (const key of environmentEnvResolution.secretKeys) { + secretKeys.add(key); + } + } const projectEnvResolution = projectEnv ? await input.secretsSvc.resolveEnvBindings( input.companyId, @@ -595,6 +634,7 @@ export async function resolveExecutionRunAdapterConfig(input: { resolvedConfig, secretKeys, secretManifest: [ + ...(environmentEnvResolution.manifest ?? []), ...(manifest ?? []), ...(projectEnvResolution.manifest ?? []), ...(routineEnvResolution.manifest ?? []), @@ -8587,11 +8627,18 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) }); const configSnapshot = buildExecutionWorkspaceConfigSnapshot(mergedConfig, selectedEnvironmentId); const executionRunConfig = stripWorkspaceRuntimeFromExecutionRunConfig(mergedConfig); + const selectedEnvironmentForConfig = selectedEnvironmentId === localEnvironment.id + ? localEnvironment + : selectedEnvironmentId + ? await environmentsSvc.getById(selectedEnvironmentId) + : null; const { resolvedConfig, secretKeys, secretManifest } = await resolveExecutionRunAdapterConfig({ companyId: agent.companyId, agentId: agent.id, issueId, heartbeatRunId: run.id, + environmentId: selectedEnvironmentForConfig?.id ?? null, + environmentEnv: selectedEnvironmentForConfig?.envVars ?? null, projectId: projectContext?.id ?? null, routineId: routineEnvContext.routineId, executionRunConfig,