From f88f94617b025c2fb1058bd9860be8d9abd63bd0 Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 5 Nov 2026 13:42:27 -0600 Subject: [PATCH] fix(secrets): authorize company creation before body validation Co-Authored-By: Paperclip --- maps/agent-access-map/secrets.md | 2 + server/src/__tests__/secrets-routes.test.ts | 47 +++++++++++++++++++++ server/src/routes/secrets.ts | 9 +++- 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/maps/agent-access-map/secrets.md b/maps/agent-access-map/secrets.md index c1242b56fd1..07f9c18d401 100644 --- a/maps/agent-access-map/secrets.md +++ b/maps/agent-access-map/secrets.md @@ -64,6 +64,8 @@ API: `present` — `POST /api/companies/{companyId}/secrets`. Agent access: `restricted`; board API/CLI `secrets create -C ... --name ... --value-env ` or `secrets link -C ... --name ... --provider ... --external-ref ...`. Agent alternative is proposed secret creation, not direct write. +Create authorization runs before body validation. A same-company agent receives HTTP 403 with a human Settings → Secrets handoff and a warning against sending values in chat or tool calls. Authorized board callers still receive HTTP 400 for malformed bodies. Cross-company callers remain denied. Route regression tests cover these boundaries; a live agent handoff remains unverified. + Policy: same company, same-provider selectable vault, valid unique path/name/key. Managed values require value and prohibit externalRef; external_reference requires supported provider/ref and stores provider reference metadata without fetching plaintext. Defaults use configured provider; GCP/Vault placeholders are not operational providers. New remote writes reserve/persist provider material with cleanup on DB failure; do not describe failed remote create as necessarily having no external effect. Evidence: `source-only` — Reviewed at `3bbc2104b65caf9f7763938a01260a5673e12f7a`. [routes/secrets.ts:887](../../server/src/routes/secrets.ts); [services/secrets.ts:2176-2395](../../server/src/services/secrets.ts); [cli/.../secrets.ts:415,443](../../cli/src/commands/client/secrets.ts). diff --git a/server/src/__tests__/secrets-routes.test.ts b/server/src/__tests__/secrets-routes.test.ts index ffd4c3893f6..b054e4588dd 100644 --- a/server/src/__tests__/secrets-routes.test.ts +++ b/server/src/__tests__/secrets-routes.test.ts @@ -155,6 +155,53 @@ describe("secret routes", () => { }); }); + it.each([undefined, "", "agent-supplied-value"])("denies agent secret creation before validating value %j", async (value) => { + const res = await request(createApp({ + type: "agent", agentId: "agent-1", companyId: "company-1", source: "agent_jwt", + })).post("/api/companies/company-1/secrets").send({ name: "Deployment key", value }); + + expect(res.status).toBe(403); + expect(res.body.error).toContain("Settings → Secrets"); + expect(res.body.error).toContain("human"); + expect(res.body.error).toContain("Do not send secret values in chat"); + expect(res.body).not.toHaveProperty("details"); + expect(mockSecretService.create).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it.each([undefined, ""])("validates a board caller's missing or empty managed value %j", async (value) => { + const res = await request(createApp()).post("/api/companies/company-1/secrets") + .send({ name: "Deployment key", value }); + + expect(res.status).toBe(400); + expect(res.body.details).toEqual(expect.arrayContaining([expect.objectContaining({ path: ["value"] })])); + expect(mockSecretService.create).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + + it("keeps authorized board creation and its human activity attribution", async () => { + mockSecretService.create.mockResolvedValue({ id: "secret-1", name: "Deployment key", provider: "local_encrypted" }); + const res = await request(createApp()).post("/api/companies/company-1/secrets") + .send({ name: "Deployment key", value: "board-supplied-value" }); + + expect(res.status).toBe(201); + expect(mockSecretService.create).toHaveBeenCalledWith("company-1", + expect.objectContaining({ name: "Deployment key", value: "board-supplied-value" }), + { userId: "user-1", agentId: null }); + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ + companyId: "company-1", actorType: "user", actorId: "user-1", action: "secret.created", entityId: "secret-1", + })); + }); + + it.each([undefined, "board-supplied-value"])("denies cross-company board creation before body validation %j", async (value) => { + const res = await request(createApp()).post("/api/companies/company-2/secrets") + .send({ name: "Deployment key", value }); + + expect(res.status).toBe(403); + expect(mockSecretService.create).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + it("rejects managed secret creation when externalRef is supplied", async () => { const res = await request(createApp()).post("/api/companies/company-1/secrets").send({ name: "OpenAI API Key", diff --git a/server/src/routes/secrets.ts b/server/src/routes/secrets.ts index 3d3ca863f22..cf77a56a9a3 100644 --- a/server/src/routes/secrets.ts +++ b/server/src/routes/secrets.ts @@ -884,9 +884,16 @@ export function secretRoutes(db: Db, deps: SecretRoutesDeps = {}) { res.json({ ok: true }); }); - router.post("/companies/:companyId/secrets", validate(createSecretSchema), async (req, res) => { + router.post("/companies/:companyId/secrets", (req, _res, next) => { const companyId = req.params.companyId as string; + if (req.actor.type === "agent") { + assertCompanyAccess(req, companyId); + throw forbidden("Company secret creation requires a human board operator. Ask the human to enter the value in Settings → Secrets for this company. Do not send secret values in chat or tool calls."); + } assertCompanySecretWrite(req, companyId); + next(); + }, validate(createSecretSchema), async (req, res) => { + const companyId = req.params.companyId as string; const created = await svc.create( companyId,