From 8fed992735391e2203a55e415691e58b1f2a4960 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 16 May 2026 20:50:40 -0500 Subject: [PATCH] Fix routine secret security blockers --- .../__tests__/heartbeat-project-env.test.ts | 63 ++++++++++++++++++ server/src/__tests__/secrets-service.test.ts | 65 +++++++++++++++++++ server/src/services/heartbeat.ts | 33 ++++++++-- server/src/services/secrets.ts | 53 +++++++++++++-- 4 files changed, 203 insertions(+), 11 deletions(-) diff --git a/server/src/__tests__/heartbeat-project-env.test.ts b/server/src/__tests__/heartbeat-project-env.test.ts index 707f87f63ce..7ab6ed24b98 100644 --- a/server/src/__tests__/heartbeat-project-env.test.ts +++ b/server/src/__tests__/heartbeat-project-env.test.ts @@ -104,6 +104,69 @@ describe("resolveExecutionRunAdapterConfig", () => { }); }); + it("drops Paperclip runtime-owned env before resolving agent, project, and routine overlays", async () => { + const resolveAdapterConfigForRuntime = vi.fn(async (_companyId, config: Record) => ({ + config: { + ...config, + env: { ...(config.env as Record) }, + }, + secretKeys: new Set(), + manifest: [], + })); + const resolveEnvBindings = vi.fn(async (_companyId, env: Record) => ({ + env: Object.fromEntries( + Object.entries(env).filter((entry): entry is [string, string] => typeof entry[1] === "string"), + ), + secretKeys: new Set(), + manifest: [], + })); + + const result = await resolveExecutionRunAdapterConfig({ + companyId: "company-1", + agentId: "agent-1", + executionRunConfig: { + env: { + PAPERCLIP_API_KEY: { type: "secret_ref", secretId: "secret-api-key", version: "latest" }, + PAPERCLIP_AGENT_ID: "spoofed-agent", + AGENT_ONLY: "agent-only", + }, + }, + projectEnv: { + PAPERCLIP_API_KEY: "project-api-key", + PAPERCLIP_COMPANY_ID: "spoofed-company", + PROJECT_ONLY: "project-only", + }, + routineEnv: { + PAPERCLIP_API_KEY: "routine-api-key", + PAPERCLIP_RUN_ID: "spoofed-run", + ROUTINE_ONLY: "routine-only", + }, + routineId: "routine-1", + secretsSvc: { + resolveAdapterConfigForRuntime, + resolveEnvBindings, + } as any, + }); + + expect(resolveAdapterConfigForRuntime.mock.calls[0]?.[1]).toEqual({ + env: { + AGENT_ONLY: "agent-only", + }, + }); + expect(resolveEnvBindings.mock.calls[0]?.[1]).toEqual({ + PROJECT_ONLY: "project-only", + }); + expect(resolveEnvBindings.mock.calls[1]?.[1]).toEqual({ + ROUTINE_ONLY: "routine-only", + }); + expect(result.resolvedConfig.env).toEqual({ + AGENT_ONLY: "agent-only", + PROJECT_ONLY: "project-only", + ROUTINE_ONLY: "routine-only", + }); + expect(JSON.stringify(result.resolvedConfig.env)).not.toContain("PAPERCLIP_"); + }); + it("skips project env resolution when the project has no bindings", async () => { const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({ config: { env: { AGENT_ONLY: "agent-only" } }, diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index de4e11e54d5..d01b677707b 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -250,6 +250,71 @@ describeEmbeddedPostgres("secretService", () => { expect(JSON.stringify(events)).not.toContain("routine-super-secret"); }); + it("records stable redacted failure codes for routine env secret resolution", async () => { + const companyId = await seedCompany(); + const svc = secretService(db); + const secret = await svc.create(companyId, { + name: `routine-failure-codes-${randomUUID()}`, + provider: "local_encrypted", + value: "routine-super-secret", + }); + const env = { + ROUTINE_API_KEY: { type: "secret_ref" as const, secretId: secret.id, version: "latest" as const }, + }; + const context = { + consumerType: "routine" as const, + consumerId: "routine-1", + actorType: "agent" as const, + actorId: "agent-1", + }; + await svc.syncEnvBindingsForTarget(companyId, { targetType: "routine", targetId: "routine-1" }, env); + + await expect( + svc.resolveEnvBindings(companyId, env, { ...context, consumerId: "routine-2" }), + ).rejects.toThrow(/not bound/i); + + await db.update(companySecrets).set({ status: "disabled" }).where(eq(companySecrets.id, secret.id)); + await expect(svc.resolveEnvBindings(companyId, env, context)).rejects.toThrow(/not active/i); + + await db.update(companySecrets).set({ status: "active" }).where(eq(companySecrets.id, secret.id)); + await expect( + svc.resolveSecretValue(companyId, secret.id, 999, { + ...context, + configPath: "env.ROUTINE_API_KEY", + }), + ).rejects.toThrow(/version not found/i); + + await db + .update(companySecretVersions) + .set({ status: "disabled" }) + .where(eq(companySecretVersions.secretId, secret.id)); + await expect(svc.resolveEnvBindings(companyId, env, context)).rejects.toThrow(/version is not active/i); + + await db + .update(companySecretVersions) + .set({ status: "current" }) + .where(eq(companySecretVersions.secretId, secret.id)); + vi.spyOn(localEncryptedProvider, "resolveVersion").mockRejectedValueOnce( + new Error("provider leaked value routine-super-secret"), + ); + await expect(svc.resolveEnvBindings(companyId, env, context)).rejects.toThrow(/provider leaked value/i); + + await db.update(companySecrets).set({ status: "deleted" }).where(eq(companySecrets.id, secret.id)); + await expect(svc.resolveEnvBindings(companyId, env, context)).rejects.toThrow(/not found/i); + + const events = await svc.listAccessEvents(companyId, secret.id); + expect(events.map((event) => event.errorCode).sort()).toEqual([ + "binding_missing", + "provider_error", + "secret_deleted", + "secret_inactive", + "version_inactive", + "version_missing", + ]); + expect(JSON.stringify(events)).not.toContain("routine-super-secret"); + expect(JSON.stringify(events)).not.toContain("provider leaked value"); + }); + it("scopes env binding sync deletes to the env path prefix", async () => { const companyId = await seedCompany(); const svc = secretService(db); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index be7aeba5525..06906e0f00a 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -331,6 +331,26 @@ type RuntimeConfigSecretResolver = Pick< "resolveAdapterConfigForRuntime" | "resolveEnvBindings" >; +function isPaperclipRuntimeEnvKey(key: string) { + return key.startsWith("PAPERCLIP_"); +} + +function stripPaperclipRuntimeEnvBindings(envValue: unknown): Record | null { + const record = parseObject(envValue); + const filtered = Object.fromEntries( + Object.entries(record).filter(([key]) => !isPaperclipRuntimeEnvKey(key)), + ); + return Object.keys(filtered).length > 0 ? filtered : null; +} + +function stripPaperclipRuntimeEnvFromAdapterConfig(config: Record): Record { + if (!Object.prototype.hasOwnProperty.call(config, "env")) return config; + return { + ...config, + env: stripPaperclipRuntimeEnvBindings(config.env) ?? {}, + }; +} + export async function resolveExecutionRunAdapterConfig(input: { companyId: string; agentId?: string | null; @@ -343,9 +363,12 @@ export async function resolveExecutionRunAdapterConfig(input: { routineEnv?: unknown; secretsSvc: RuntimeConfigSecretResolver; }) { + const executionRunConfig = stripPaperclipRuntimeEnvFromAdapterConfig(input.executionRunConfig); + const projectEnv = stripPaperclipRuntimeEnvBindings(input.projectEnv); + const routineEnv = stripPaperclipRuntimeEnvBindings(input.routineEnv); const { config: resolvedConfig, secretKeys, manifest } = await input.secretsSvc.resolveAdapterConfigForRuntime( input.companyId, - input.executionRunConfig, + executionRunConfig, input.agentId ? { consumerType: "agent", @@ -357,10 +380,10 @@ export async function resolveExecutionRunAdapterConfig(input: { } : undefined, ); - const projectEnvResolution = input.projectEnv + const projectEnvResolution = projectEnv ? await input.secretsSvc.resolveEnvBindings( input.companyId, - input.projectEnv, + projectEnv, input.projectId ? { consumerType: "project", @@ -382,10 +405,10 @@ export async function resolveExecutionRunAdapterConfig(input: { secretKeys.add(key); } } - const routineEnvResolution = input.routineEnv + const routineEnvResolution = routineEnv ? await input.secretsSvc.resolveEnvBindings( input.companyId, - input.routineEnv, + routineEnv, input.routineId ? { consumerType: "routine", diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 17f13cef3d2..226e40f7bd3 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -195,6 +195,14 @@ type RuntimeSecretResolution = { manifestEntry: RuntimeSecretManifestEntry; }; +type SecretResolutionErrorCode = + | "binding_missing" + | "secret_deleted" + | "secret_inactive" + | "version_missing" + | "version_inactive" + | "provider_error"; + function asRecord(value: unknown): Record | null { if (typeof value !== "object" || value === null || Array.isArray(value)) return null; return value as Record; @@ -238,6 +246,33 @@ function defaultProviderConfigStatus(provider: SecretProvider): SecretProviderCo return COMING_SOON_SECRET_PROVIDERS.has(provider) ? "coming_soon" : "ready"; } +function secretResolutionErrorCode(error: unknown): SecretResolutionErrorCode { + if (isSecretProviderClientError(error)) return "provider_error"; + if (error instanceof HttpError) { + const details = asRecord(error.details); + switch (details?.code) { + case "binding_missing": + case "secret_deleted": + case "secret_inactive": + case "version_missing": + case "version_inactive": + case "provider_error": + return details.code; + } + if (error.message === "Secret is not active") return "secret_inactive"; + if (error.message === "Secret version not found") return "version_missing"; + if (error.message === "Secret version is not active") return "version_inactive"; + if ( + error.message === "Secret resolution requires a binding config path" || + error.message.startsWith("Secret is not bound to ") + ) { + return "binding_missing"; + } + if (error.status >= 500) return "provider_error"; + } + return "provider_error"; +} + function assertSelectableProviderConfig(config: { provider: string; status: string; @@ -321,7 +356,7 @@ export function secretService(db: Db) { ) { if (!context) return; if (!context.configPath) { - throw unprocessable("Secret resolution requires a binding config path"); + throw unprocessable("Secret resolution requires a binding config path", { code: "binding_missing" }); } const binding = await getBinding({ companyId, @@ -333,6 +368,7 @@ export function secretService(db: Db) { if (!binding) { throw unprocessable( `Secret is not bound to ${context.consumerType}:${context.consumerId} at ${context.configPath}`, + { code: "binding_missing" }, ); } } @@ -495,19 +531,24 @@ export function secretService(db: Db) { version: number | "latest", context?: SecretConsumerContext, ): Promise { - const secret = await assertSecretInCompany(companyId, secretId); + const secret = await getById(secretId); + if (!secret) throw notFound("Secret not found"); + if (secret.companyId !== companyId) throw unprocessable("Secret must belong to same company"); const resolvedVersion = version === "latest" ? secret.latestVersion : version; const providerId = secret.provider as SecretProvider; const configPath = context?.configPath ?? null; try { + if (secret.status === "deleted") { + throw new HttpError(404, "Secret not found", { code: "secret_deleted" }); + } if (secret.status !== "active") { - throw unprocessable("Secret is not active"); + throw unprocessable("Secret is not active", { code: "secret_inactive" }); } await assertBindingContext(companyId, secret.id, context); const versionRow = await getSecretVersion(secret.id, resolvedVersion); - if (!versionRow) throw notFound("Secret version not found"); + if (!versionRow) throw new HttpError(404, "Secret version not found", { code: "version_missing" }); if (versionRow.status === "disabled" || versionRow.status === "destroyed" || versionRow.revokedAt) { - throw unprocessable("Secret version is not active"); + throw unprocessable("Secret version is not active", { code: "version_inactive" }); } const provider = getSecretProvider(providerId); const providerConfig = await getSelectableRuntimeProviderConfig({ @@ -555,7 +596,7 @@ export function secretService(db: Db) { }, }; } catch (err) { - const errorCode = err instanceof Error ? err.message.slice(0, 120) : "resolution_failed"; + const errorCode = secretResolutionErrorCode(err); await recordAccessEvent({ companyId, secretId: secret.id,