diff --git a/doc/plans/2026-04-26-plugin-secret-ref-company-scope.md b/doc/plans/2026-04-26-plugin-secret-ref-company-scope.md new file mode 100644 index 00000000000..ca689e1958d --- /dev/null +++ b/doc/plans/2026-04-26-plugin-secret-ref-company-scope.md @@ -0,0 +1,86 @@ +# Plugin Secret Refs: Company Scope Reintroduction Plan + +Date: 2026-04-26 +Status: follow-up after fail-closed mitigation +Related issue: PAP-2394 + +## Current state + +`PAP-2394` now fails closed: + +- `POST /api/plugins/:pluginId/config` rejects any config containing plugin secret refs. +- `ctx.secrets.resolve()` is disabled for plugin workers. + +This removes the release-blocking cross-company exposure path, but it also disables plugin secret-ref support until the runtime carries company scope end to end. + +## Vulnerability summary + +The original design mixed an instance-global config store with company-scoped secret bindings: + +- [server/src/routes/plugins.ts](/Users/dotta/paperclip/.paperclip/worktrees/PAP-2339-secrets-make-a-plan/server/src/routes/plugins.ts:1898) saved one global plugin config row, then wrote bindings into `company_secret_bindings` grouped by each referenced secret's owning company. +- [packages/db/src/schema/plugin_config.ts](/Users/dotta/paperclip/.paperclip/worktrees/PAP-2339-secrets-make-a-plan/packages/db/src/schema/plugin_config.ts:15) stored one config row per plugin, with no company dimension. +- [packages/db/src/schema/company_secret_bindings.ts](/Users/dotta/paperclip/.paperclip/worktrees/PAP-2339-secrets-make-a-plan/packages/db/src/schema/company_secret_bindings.ts:5) already modeled bindings as company-scoped. +- [server/src/services/plugin-secrets-handler.ts](/Users/dotta/paperclip/.paperclip/worktrees/PAP-2339-secrets-make-a-plan/server/src/services/plugin-secrets-handler.ts:212) resolved by `pluginId` + secret UUID, with no active company context from the bridge call. +- [packages/plugins/sdk/src/worker-rpc-host.ts](/Users/dotta/paperclip/.paperclip/worktrees/PAP-2339-secrets-make-a-plan/packages/plugins/sdk/src/worker-rpc-host.ts:384) exposed `ctx.config.get()` and `ctx.secrets.resolve()` without a company parameter. + +This violated Least Privilege, Complete Mediation, and Secure Defaults. + +## Recommended end state + +Re-enable plugin secret refs only after both of these are true: + +1. Plugin config reads/writes are company-scoped. +2. Runtime secret resolution carries explicit company context and enforces it at resolution time. + +## Implementation plan + +### 1. Make plugin config company-scoped + +- Add `company_id` to `plugin_config`, with a unique index on `(plugin_id, company_id)`. +- Update registry helpers to require `companyId` for `getConfig`, `upsertConfig`, `patchConfig`, and `deleteConfig`. +- Update plugin config routes to require `companyId` and call `assertCompanyAccess(req, companyId)`. +- Keep instance-global plugin lifecycle state separate from company-scoped plugin config. + +### 2. Propagate company context through the worker runtime + +- Extend the SDK so `ctx.config.get()` and `ctx.secrets.resolve()` can receive or derive `companyId`. +- Introduce worker request context storage for handlers that already run with company scope: + - `getData` + - `performAction` + - scoped API routes + - tool executions + - environment driver calls +- Fail closed when plugin code tries to read company-scoped config or secrets outside an active company context. + +### 3. Rebind secrets by `(companyId, pluginId, configPath)` + +- On config save, validate every referenced secret belongs to the authorized company. +- Store bindings only for that company. +- Resolve secrets only by the current company-scoped binding, never by bare plugin ID plus UUID. +- Treat stale bindings as invalid and remove them on config replacement. + +### 4. Prevent cross-company config disclosure + +- When returning config to the UI, only materialize the selected company's secret refs. +- Never expose another company's secret UUIDs through the global plugin config surface. + +## Required regression coverage + +- Company A board user cannot save plugin config that references a Company B secret. +- Company A plugin execution cannot resolve a Company B secret even if the same plugin is configured for Company B. +- Company-scoped config reads only return the selected company's secret bindings. +- Config replacement removes stale bindings for the same `(companyId, pluginId)` target. +- Runtime calls without company context fail closed. + +## Migration notes + +- Existing `plugin_config` rows need a migration strategy before re-enable. +- Safest default: do not auto-assume a company for historical secret refs. +- Prefer one of: + - explicit admin migration per company, or + - import existing rows as non-secret config only and require re-entry of secret refs. + +## Release posture + +- Keep plugin secret refs disabled until all steps above land. +- Do not restore the feature behind a soft warning; the insecure path must remain unavailable by default. diff --git a/server/src/__tests__/environment-routes.test.ts b/server/src/__tests__/environment-routes.test.ts index 3c9ecdb4e07..6d74de22df9 100644 --- a/server/src/__tests__/environment-routes.test.ts +++ b/server/src/__tests__/environment-routes.test.ts @@ -36,10 +36,13 @@ const mockProbeEnvironment = vi.hoisted(() => vi.fn()); const mockSecretService = vi.hoisted(() => ({ create: vi.fn(), resolveSecretValue: vi.fn(), + syncSecretRefsForTarget: vi.fn(), + remove: vi.fn(), })); const mockValidatePluginEnvironmentDriverConfig = vi.hoisted(() => vi.fn()); const mockValidatePluginSandboxProviderConfig = vi.hoisted(() => vi.fn()); const mockListReadyPluginEnvironmentDrivers = vi.hoisted(() => vi.fn()); +const mockResolvePluginSandboxProviderDriverByKey = vi.hoisted(() => vi.fn()); const mockExecutionWorkspaceService = vi.hoisted(() => ({})); vi.mock("../services/index.js", () => ({ @@ -69,6 +72,7 @@ vi.mock("../services/execution-workspaces.js", () => ({ vi.mock("../services/plugin-environment-driver.js", () => ({ listReadyPluginEnvironmentDrivers: mockListReadyPluginEnvironmentDrivers, + resolvePluginSandboxProviderDriverByKey: mockResolvePluginSandboxProviderDriverByKey, validatePluginEnvironmentDriverConfig: mockValidatePluginEnvironmentDriverConfig, validatePluginSandboxProviderConfig: mockValidatePluginSandboxProviderConfig, })); @@ -96,6 +100,7 @@ let currentActor: Record = { source: "local_implicit", }; const routeOptions: Record = {}; +const originalSecretsProviderEnv = process.env.PAPERCLIP_SECRETS_PROVIDER; function createApp(actor: Record, options: Record = {}) { currentActor = actor; @@ -119,6 +124,11 @@ function createApp(actor: Record, options: Record { afterAll(async () => { + if (originalSecretsProviderEnv === undefined) { + delete process.env.PAPERCLIP_SECRETS_PROVIDER; + } else { + process.env.PAPERCLIP_SECRETS_PROVIDER = originalSecretsProviderEnv; + } if (!server) return; await new Promise((resolve, reject) => { server?.close((err) => { @@ -145,9 +155,14 @@ describe("environment routes", () => { mockProbeEnvironment.mockReset(); mockSecretService.create.mockReset(); mockSecretService.resolveSecretValue.mockReset(); + mockSecretService.syncSecretRefsForTarget.mockReset(); + mockSecretService.remove.mockReset(); mockSecretService.create.mockResolvedValue({ id: "11111111-1111-1111-1111-111111111111", }); + mockSecretService.syncSecretRefsForTarget.mockResolvedValue([]); + mockSecretService.remove.mockResolvedValue(null); + delete process.env.PAPERCLIP_SECRETS_PROVIDER; mockValidatePluginEnvironmentDriverConfig.mockReset(); mockValidatePluginEnvironmentDriverConfig.mockImplementation(async ({ config }) => config); mockValidatePluginSandboxProviderConfig.mockReset(); @@ -162,6 +177,29 @@ describe("environment routes", () => { configSchema: { type: "object" }, }, })); + mockResolvePluginSandboxProviderDriverByKey.mockReset(); + mockResolvePluginSandboxProviderDriverByKey.mockImplementation(async ({ driverKey }) => ( + driverKey === "secure-plugin" + ? { + pluginId: "plugin-secure", + pluginKey: "acme.secure-sandbox-provider", + driver: { + driverKey: "secure-plugin", + kind: "sandbox_provider", + displayName: "Secure Sandbox", + configSchema: { + type: "object", + properties: { + template: { type: "string" }, + apiKey: { type: "string", format: "secret-ref" }, + timeoutMs: { type: "number" }, + reuseLease: { type: "boolean" }, + }, + }, + }, + } + : null + )); mockListReadyPluginEnvironmentDrivers.mockReset(); mockListReadyPluginEnvironmentDrivers.mockResolvedValue([]); }); @@ -555,6 +593,59 @@ describe("environment routes", () => { ); }); + it("uses the configured provider for SSH private key secret materialization", async () => { + process.env.PAPERCLIP_SECRETS_PROVIDER = "aws_secrets_manager"; + const environment = { + ...createEnvironment(), + id: "env-ssh", + name: "SSH Fixture", + 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-1111-1111-111111111111", + version: "latest", + }, + knownHosts: null, + strictHostKeyChecking: true, + }, + }; + mockEnvironmentService.create.mockResolvedValue(environment); + const app = createApp({ + type: "board", + userId: "user-1", + source: "local_implicit", + }); + + const res = await request(app) + .post("/api/companies/company-1/environments") + .send({ + name: "SSH Fixture", + driver: "ssh", + config: { + host: "ssh.example.test", + username: "ssh-user", + remoteWorkspacePath: "/srv/paperclip/workspace", + privateKey: "super-secret-key", + }, + }); + + expect(res.status).toBe(201); + expect(mockSecretService.create).toHaveBeenCalledWith( + "company-1", + expect.objectContaining({ + provider: "aws_secrets_manager", + value: "super-secret-key", + }), + expect.any(Object), + ); + }); + it("rejects persisted fake sandbox environments", async () => { const app = createApp({ type: "board", @@ -732,6 +823,78 @@ describe("environment routes", () => { ); }); + it("uses the configured provider for schema-driven sandbox secret fields", async () => { + process.env.PAPERCLIP_SECRETS_PROVIDER = "aws_secrets_manager"; + const environment = { + ...createEnvironment(), + id: "env-sandbox-secure-plugin", + name: "Secure Sandbox", + driver: "sandbox" as const, + config: { + provider: "secure-plugin", + template: "base", + apiKey: "11111111-1111-1111-1111-111111111111", + timeoutMs: 450000, + reuseLease: true, + }, + }; + mockEnvironmentService.create.mockResolvedValue(environment); + mockValidatePluginSandboxProviderConfig.mockResolvedValue({ + normalizedConfig: { + template: "base", + apiKey: "test-provider-key", + timeoutMs: 450000, + reuseLease: true, + }, + pluginId: "plugin-secure", + pluginKey: "acme.secure-sandbox-provider", + driver: { + driverKey: "secure-plugin", + kind: "sandbox_provider", + displayName: "Secure Sandbox", + configSchema: { + type: "object", + properties: { + template: { type: "string" }, + apiKey: { type: "string", format: "secret-ref" }, + timeoutMs: { type: "number" }, + reuseLease: { type: "boolean" }, + }, + }, + }, + }); + const pluginWorkerManager = {}; + const app = createApp({ + type: "board", + userId: "user-1", + source: "local_implicit", + }, { pluginWorkerManager }); + + const res = await request(app) + .post("/api/companies/company-1/environments") + .send({ + name: "Secure Sandbox", + driver: "sandbox", + config: { + provider: "secure-plugin", + template: "base", + apiKey: "test-provider-key", + timeoutMs: "450000", + reuseLease: true, + }, + }); + + expect(res.status).toBe(201); + expect(mockSecretService.create).toHaveBeenCalledWith( + "company-1", + expect.objectContaining({ + provider: "aws_secrets_manager", + value: "test-provider-key", + }), + expect.any(Object), + ); + }); + it("validates plugin environment config through the plugin driver host", async () => { const environment = { ...createEnvironment(), diff --git a/server/src/__tests__/plugin-routes-authz.test.ts b/server/src/__tests__/plugin-routes-authz.test.ts index faf3487bf68..52e678b46dc 100644 --- a/server/src/__tests__/plugin-routes-authz.test.ts +++ b/server/src/__tests__/plugin-routes-authz.test.ts @@ -224,6 +224,30 @@ describe.sequential("plugin install and upgrade authz", () => { expect(mockLifecycle.disable).not.toHaveBeenCalled(); }, 20_000); + it("rejects plugin config saves that contain secret refs even for instance admins", async () => { + readyPlugin(); + + const { app } = await createApp({ + type: "board", + userId: "admin-1", + source: "session", + isInstanceAdmin: true, + companyIds: [companyA], + }); + + const res = await request(app) + .post(`/api/plugins/${pluginId}/config`) + .send({ + configJson: { + apiKeyRef: "77777777-7777-4777-8777-777777777777", + }, + }); + + expect(res.status).toBe(422); + expect(res.body.error).toMatch(/secret references are disabled/i); + expect(mockRegistry.upsertConfig).not.toHaveBeenCalled(); + }, 20_000); + it("allows instance admins to upgrade plugins", async () => { const pluginId = "11111111-1111-4111-8111-111111111111"; mockRegistry.getById.mockResolvedValue({ diff --git a/server/src/__tests__/plugin-secrets-handler.test.ts b/server/src/__tests__/plugin-secrets-handler.test.ts new file mode 100644 index 00000000000..ec89c8722c0 --- /dev/null +++ b/server/src/__tests__/plugin-secrets-handler.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { + createPluginSecretsHandler, + PLUGIN_SECRET_REFS_DISABLED_MESSAGE, +} from "../services/plugin-secrets-handler.js"; + +describe("createPluginSecretsHandler", () => { + it("fails closed for plugin secret resolution until company scoping lands", async () => { + const handler = createPluginSecretsHandler({ + db: {} as never, + pluginId: "11111111-1111-4111-8111-111111111111", + }); + + await expect( + handler.resolve({ secretRef: "77777777-7777-4777-8777-777777777777" }), + ).rejects.toThrow(PLUGIN_SECRET_REFS_DISABLED_MESSAGE); + }); + + it("still rejects malformed secret refs before the feature-disable guard", async () => { + const handler = createPluginSecretsHandler({ + db: {} as never, + pluginId: "11111111-1111-4111-8111-111111111111", + }); + + await expect( + handler.resolve({ secretRef: "not-a-uuid" }), + ).rejects.toThrow(/invalid secret reference/i); + }); +}); diff --git a/server/src/__tests__/routines-service.test.ts b/server/src/__tests__/routines-service.test.ts index 9da3270d37c..70fe9d0500f 100644 --- a/server/src/__tests__/routines-service.test.ts +++ b/server/src/__tests__/routines-service.test.ts @@ -1,6 +1,6 @@ import { createHmac, randomUUID } from "node:crypto"; import { eq } from "drizzle-orm"; -import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { activityLog, agents, @@ -26,10 +26,12 @@ import { } from "./helpers/embedded-postgres.js"; import { issueService } from "../services/issues.ts"; import { instanceSettingsService } from "../services/instance-settings.ts"; +import * as providerRegistry from "../secrets/provider-registry.ts"; import { routineService } from "../services/routines.ts"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; +const originalSecretsProviderEnv = process.env.PAPERCLIP_SECRETS_PROVIDER; if (!embeddedPostgresSupport.supported) { console.warn( @@ -47,6 +49,11 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { }, 20_000); afterEach(async () => { + if (originalSecretsProviderEnv === undefined) { + delete process.env.PAPERCLIP_SECRETS_PROVIDER; + } else { + process.env.PAPERCLIP_SECRETS_PROVIDER = originalSecretsProviderEnv; + } await db.delete(activityLog); await db.delete(issueInboxArchives); await db.delete(issueReadStates); @@ -1272,6 +1279,82 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { expect(run.linkedIssueId).toBeTruthy(); }); + it("uses the configured provider for generated webhook trigger secrets", async () => { + process.env.PAPERCLIP_SECRETS_PROVIDER = "aws_secrets_manager"; + const originalGetSecretProvider = providerRegistry.getSecretProvider; + const getSecretProviderSpy = vi.spyOn(providerRegistry, "getSecretProvider").mockImplementation((provider) => { + if (provider !== "aws_secrets_manager") { + return originalGetSecretProvider(provider); + } + return { + id: "aws_secrets_manager", + descriptor: () => ({ + id: "aws_secrets_manager", + label: "AWS Secrets Manager", + supportsManaged: true, + supportsExternalReference: true, + }), + validateConfig: async () => ({ ok: true, warnings: [] }), + createSecret: async ({ value }) => ({ + material: { source: "managed", secretId: "arn:aws:secretsmanager:stub", versionId: "v1" }, + valueSha256: `sha:${value}`, + fingerprintSha256: `sha:${value}`, + externalRef: "arn:aws:secretsmanager:stub", + providerVersionRef: "v1", + }), + createVersion: async ({ value }) => ({ + material: { source: "managed", secretId: "arn:aws:secretsmanager:stub", versionId: "v2" }, + valueSha256: `sha:${value}`, + fingerprintSha256: `sha:${value}`, + externalRef: "arn:aws:secretsmanager:stub", + providerVersionRef: "v2", + }), + linkExternalSecret: async ({ externalRef, providerVersionRef }) => ({ + material: { source: "external", secretId: externalRef, versionId: providerVersionRef ?? null }, + valueSha256: "external", + fingerprintSha256: "external", + externalRef, + providerVersionRef: providerVersionRef ?? null, + }), + resolveVersion: async () => "resolved-secret", + deleteOrArchive: async () => undefined, + healthCheck: async () => ({ + provider: "aws_secrets_manager", + status: "ok", + message: "stubbed", + }), + }; + }); + + try { + const { routine, svc } = await seedFixture(); + const { trigger } = await svc.createTrigger( + routine.id, + { + kind: "webhook", + signingMode: "hmac_sha256", + replayWindowSec: 300, + }, + {}, + ); + + const [secret] = await db + .select({ + id: companySecrets.id, + provider: companySecrets.provider, + }) + .from(companySecrets) + .where(eq(companySecrets.id, trigger.secretId!)); + + expect(secret).toMatchObject({ + id: trigger.secretId, + provider: "aws_secrets_manager", + }); + } finally { + getSecretProviderSpy.mockRestore(); + } + }); + it("accepts GitHub-style X-Hub-Signature-256 with github_hmac signing mode", async () => { const { routine, svc } = await seedFixture(); const { trigger, secretMaterial } = await svc.createTrigger( diff --git a/server/src/routes/environments.ts b/server/src/routes/environments.ts index 5d1e306960d..2c0daded970 100644 --- a/server/src/routes/environments.ts +++ b/server/src/routes/environments.ts @@ -27,6 +27,7 @@ import { 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 type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; import { environmentService } from "../services/environments.js"; @@ -203,6 +204,7 @@ export function environmentRoutes( companyId, environmentName: req.body.name, driver: req.body.driver, + secretProvider: getConfiguredSecretProvider(), config: req.body.config, actor: { agentId: actor.agentId, @@ -311,6 +313,7 @@ export function environmentRoutes( companyId: existing.companyId, environmentName: nextName, driver: nextDriver, + secretProvider: getConfiguredSecretProvider(), config: configSource, actor: { agentId: actor.agentId, diff --git a/server/src/routes/plugins.ts b/server/src/routes/plugins.ts index e9f5667bfb2..75a206b154b 100644 --- a/server/src/routes/plugins.ts +++ b/server/src/routes/plugins.ts @@ -73,8 +73,10 @@ import { requireLocalFolderDeclaration, setStoredLocalFolder, } from "../services/plugin-local-folders.js"; -import { extractSecretRefPathsFromConfig } from "../services/plugin-secrets-handler.js"; -import { secretService } from "../services/secrets.js"; +import { + extractSecretRefPathsFromConfig, + PLUGIN_SECRET_REFS_DISABLED_MESSAGE, +} from "../services/plugin-secrets-handler.js"; import { badRequest, forbidden, notFound, unauthorized, unprocessable } from "../errors.js"; /** UI slot declaration extracted from plugin manifest */ @@ -1943,32 +1945,15 @@ export function pluginRoutes( } try { + const secretRefsByPath = extractSecretRefPathsFromConfig(body.configJson, schema); + if (secretRefsByPath.size > 0) { + res.status(422).json({ error: PLUGIN_SECRET_REFS_DISABLED_MESSAGE }); + return; + } + const result = await registry.upsertConfig(plugin.id, { configJson: body.configJson, }); - const secretRefsByPath = extractSecretRefPathsFromConfig(body.configJson, schema); - const secrets = secretService(db); - const refsByCompany = new Map>(); - for (const secretRef of secretRefsByPath.keys()) { - const secret = await secrets.getById(secretRef); - if (!secret) continue; - const refs = refsByCompany.get(secret.companyId) ?? []; - for (const configPath of secretRefsByPath.get(secretRef) ?? []) { - refs.push({ - secretId: secretRef, - configPath, - versionSelector: "latest", - }); - } - refsByCompany.set(secret.companyId, refs); - } - for (const [companyId, refs] of refsByCompany) { - await secrets.syncSecretRefsForTarget( - companyId, - { targetType: "plugin", targetId: plugin.id }, - refs, - ); - } await logPluginMutationActivity(req, "plugin.config.updated", plugin.id, { pluginId: plugin.id, pluginKey: plugin.pluginKey, diff --git a/server/src/routes/secrets.ts b/server/src/routes/secrets.ts index 984d27070f4..b10e47bbd4e 100644 --- a/server/src/routes/secrets.ts +++ b/server/src/routes/secrets.ts @@ -1,8 +1,6 @@ import { Router } from "express"; import type { Db } from "@paperclipai/db"; import { - SECRET_PROVIDERS, - type SecretProvider, createSecretSchema, rotateSecretSchema, updateSecretSchema, @@ -10,16 +8,12 @@ import { import { validate } from "../middleware/validate.js"; import { assertBoard, assertCompanyAccess } from "./authz.js"; import { logActivity, secretService } from "../services/index.js"; +import { getConfiguredSecretProvider } from "../secrets/configured-provider.js"; export function secretRoutes(db: Db) { const router = Router(); const svc = secretService(db); - const configuredDefaultProvider = process.env.PAPERCLIP_SECRETS_PROVIDER; - const defaultProvider = ( - configuredDefaultProvider && SECRET_PROVIDERS.includes(configuredDefaultProvider as SecretProvider) - ? configuredDefaultProvider - : "local_encrypted" - ) as SecretProvider; + const defaultProvider = getConfiguredSecretProvider(); router.get("/companies/:companyId/secret-providers", (req, res) => { assertBoard(req); diff --git a/server/src/secrets/configured-provider.ts b/server/src/secrets/configured-provider.ts new file mode 100644 index 00000000000..25dd821e65a --- /dev/null +++ b/server/src/secrets/configured-provider.ts @@ -0,0 +1,8 @@ +import { SECRET_PROVIDERS, type SecretProvider } from "@paperclipai/shared"; + +export function getConfiguredSecretProvider(): SecretProvider { + const configuredProvider = process.env.PAPERCLIP_SECRETS_PROVIDER; + return configuredProvider && SECRET_PROVIDERS.includes(configuredProvider as SecretProvider) + ? configuredProvider as SecretProvider + : "local_encrypted"; +} diff --git a/server/src/services/environment-config.ts b/server/src/services/environment-config.ts index 79b0c3a5933..e95fe37f7f1 100644 --- a/server/src/services/environment-config.ts +++ b/server/src/services/environment-config.ts @@ -9,6 +9,7 @@ import type { PluginEnvironmentConfig, PluginSandboxEnvironmentConfig, SandboxEnvironmentConfig, + SecretProvider, SecretVersionSelector, SshEnvironmentConfig, } from "@paperclipai/shared"; @@ -166,6 +167,7 @@ async function createEnvironmentSecret(input: { environmentName: string; driver: EnvironmentDriver; field: string; + provider: SecretProvider; value: string; actor?: { userId?: string | null; agentId?: string | null }; }) { @@ -173,7 +175,7 @@ async function createEnvironmentSecret(input: { input.companyId, { name: secretName(input), - provider: "local_encrypted", + provider: input.provider, value: input.value, description: `Secret for ${input.environmentName} ${input.field}.`, }, @@ -191,6 +193,7 @@ async function persistConfigSecretRefs(input: { companyId: string; environmentName: string; driver: EnvironmentDriver; + secretProvider: SecretProvider; config: Record; schema: Record | null; actor?: { userId?: string | null; agentId?: string | null }; @@ -214,6 +217,7 @@ async function persistConfigSecretRefs(input: { environmentName: input.environmentName, driver: input.driver, field: path.replace(/[^a-z0-9]+/gi, "-").toLowerCase(), + provider: input.secretProvider, value: trimmed, actor: input.actor, }); @@ -383,6 +387,7 @@ export async function normalizeEnvironmentConfigForPersistence(input: { companyId: string; environmentName: string; driver: EnvironmentDriver; + secretProvider: SecretProvider; config: Record | null | undefined; actor?: { userId?: string | null; agentId?: string | null }; pluginWorkerManager?: PluginWorkerManager; @@ -404,6 +409,7 @@ export async function normalizeEnvironmentConfigForPersistence(input: { environmentName: input.environmentName, driver: input.driver, field: "private-key", + provider: input.secretProvider, value: privateKey, actor: input.actor, }); @@ -447,6 +453,7 @@ export async function normalizeEnvironmentConfigForPersistence(input: { companyId: input.companyId, environmentName: input.environmentName, driver: input.driver, + secretProvider: input.secretProvider, config: { provider: parsed.data.provider, ...validated.normalizedConfig, diff --git a/server/src/services/plugin-secrets-handler.ts b/server/src/services/plugin-secrets-handler.ts index 8a3ada13d77..ccc5878a004 100644 --- a/server/src/services/plugin-secrets-handler.ts +++ b/server/src/services/plugin-secrets-handler.ts @@ -33,31 +33,20 @@ * @see services/secrets.ts — secretService used by agent env bindings */ -import { eq } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; -import { companySecrets, pluginConfig } from "@paperclipai/db"; -import { pluginRegistryService } from "./plugin-registry.js"; -import { secretService } from "./secrets.js"; import { collectSecretRefPaths, isUuidSecretRef, readConfigValueAtPath, } from "./json-schema-secret-refs.js"; +export const PLUGIN_SECRET_REFS_DISABLED_MESSAGE = + "Plugin secret references are disabled until company-scoped plugin config lands"; + // --------------------------------------------------------------------------- // Error helpers // --------------------------------------------------------------------------- -/** - * Create a sanitised error that never leaks secret material. - * Only the ref identifier is included; never the resolved value. - */ -function secretNotFound(secretRef: string): Error { - const err = new Error(`Secret not found: ${secretRef}`); - err.name = "SecretNotFoundError"; - return err; -} - function invalidSecretRef(secretRef: string): Error { const err = new Error(`Invalid secret reference: ${secretRef}`); err.name = "InvalidSecretRefError"; @@ -210,17 +199,11 @@ function createRateLimiter(maxAttempts: number, windowMs: number) { export function createPluginSecretsHandler( options: PluginSecretsHandlerOptions, ): PluginSecretsService { - const { db, pluginId } = options; - const registry = pluginRegistryService(db); - const secrets = secretService(db); + const { pluginId } = options; // Rate limit: max 30 resolution attempts per plugin per minute const rateLimiter = createRateLimiter(30, 60_000); - let cachedAllowedRefs: Map> | null = null; - let cachedAllowedRefsExpiry = 0; - const CONFIG_CACHE_TTL_MS = 30_000; // 30 seconds, matches event bus TTL - return { async resolve(params: PluginSecretsResolveParams): Promise { const { secretRef } = params; @@ -247,53 +230,9 @@ export function createPluginSecretsHandler( throw invalidSecretRef(trimmedRef); } - // --------------------------------------------------------------- - // 1b. Scope check — only allow secrets referenced in this plugin's config - // --------------------------------------------------------------- - const now = Date.now(); - if (!cachedAllowedRefs || now > cachedAllowedRefsExpiry) { - const [configRow, plugin] = await Promise.all([ - db - .select() - .from(pluginConfig) - .where(eq(pluginConfig.pluginId, pluginId)) - .then((rows) => rows[0] ?? null), - registry.getById(pluginId), - ]); - - const schema = (plugin?.manifestJson as unknown as Record | null) - ?.instanceConfigSchema as Record | undefined; - cachedAllowedRefs = extractSecretRefPathsFromConfig(configRow?.configJson, schema); - cachedAllowedRefsExpiry = now + CONFIG_CACHE_TTL_MS; - } - - const allowedPaths = cachedAllowedRefs.get(trimmedRef); - if (!allowedPaths) { - // Return "not found" to avoid leaking whether the secret exists - throw secretNotFound(trimmedRef); - } - - // --------------------------------------------------------------- - // 2. Look up the secret record by UUID - // --------------------------------------------------------------- - const secret = await db - .select() - .from(companySecrets) - .where(eq(companySecrets.id, trimmedRef)) - .then((rows) => rows[0] ?? null); - - if (!secret) { - throw secretNotFound(trimmedRef); - } - - return await secrets.resolveSecretValue(secret.companyId, secret.id, "latest", { - consumerType: "plugin", - consumerId: pluginId, - actorType: "plugin", - actorId: pluginId, - pluginId, - configPath: [...allowedPaths][0] ?? "$", - }); + // Fail closed until plugin config and worker runtime both carry an + // explicit company scope for secret bindings and resolution. + throw new Error(PLUGIN_SECRET_REFS_DISABLED_MESSAGE); }, }; } diff --git a/server/src/services/routines.ts b/server/src/services/routines.ts index d40d62a8c23..837c838aa60 100644 --- a/server/src/services/routines.ts +++ b/server/src/services/routines.ts @@ -50,6 +50,7 @@ import { trackRoutineRun } from "@paperclipai/shared/telemetry"; import { conflict, forbidden, notFound, unauthorized, unprocessable } from "../errors.js"; import { logger } from "../middleware/logger.js"; import { getTelemetryClient } from "../telemetry.js"; +import { getConfiguredSecretProvider } from "../secrets/configured-provider.js"; import { issueService } from "./issues.js"; import { secretService } from "./secrets.js"; import { getSecretProvider } from "../secrets/provider-registry.js"; @@ -951,9 +952,10 @@ export function routineService( executor?: Db, ) { const secretValue = crypto.randomBytes(24).toString("hex"); + const providerId = getConfiguredSecretProvider(); const input = { name: `routine-${routineId}-${crypto.randomBytes(6).toString("hex")}`, - provider: "local_encrypted" as const, + provider: providerId, value: secretValue, description: `Webhook auth for routine ${routineId}`, };