diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index ea3c5dd6673..ccdcd455e7b 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -1293,6 +1293,7 @@ export { updateUserSecretDefinitionSchema, createUserSecretValueSchema, updateUserSecretValueSchema, + rotateUserSecretValueSchema, createUserSecretDeclarationSchema, createSecretProviderConfigSchema, updateSecretProviderConfigSchema, diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index 6ff08e8cd75..167ac5e581c 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -471,6 +471,7 @@ export { updateUserSecretDefinitionSchema, createUserSecretValueSchema, updateUserSecretValueSchema, + rotateUserSecretValueSchema, createUserSecretDeclarationSchema, createSecretProviderConfigSchema, updateSecretProviderConfigSchema, diff --git a/packages/shared/src/validators/secret.test.ts b/packages/shared/src/validators/secret.test.ts index 18cdf193d9d..4c2cfe02901 100644 --- a/packages/shared/src/validators/secret.test.ts +++ b/packages/shared/src/validators/secret.test.ts @@ -7,6 +7,8 @@ import { envBindingUserSecretRefSchema, remoteSecretImportPreviewSchema, remoteSecretImportSchema, + rotateSecretSchema, + rotateUserSecretValueSchema, secretProviderConfigDiscoveryPreviewSchema, secretProviderConfigPayloadSchema, updateUserSecretDefinitionSchema, @@ -61,6 +63,14 @@ describe("secret validators", () => { }); }); + it("requires secret rotation payloads to include new material", () => { + expect(() => rotateSecretSchema.parse({})).toThrow(/requires value or externalRef/); + expect(() => rotateUserSecretValueSchema.parse({})).toThrow(/requires value or externalRef/); + expect(rotateUserSecretValueSchema.parse({ value: "new-secret" })).toEqual({ + value: "new-secret", + }); + }); + it("rejects externalRef on managed secrets", () => { expect(() => createSecretSchema.parse({ diff --git a/packages/shared/src/validators/secret.ts b/packages/shared/src/validators/secret.ts index ec939fae542..01f75b09f42 100644 --- a/packages/shared/src/validators/secret.ts +++ b/packages/shared/src/validators/secret.ts @@ -79,12 +79,25 @@ export const createSecretSchema = z.object({ export type CreateSecret = z.infer; +function requireSecretRotationMaterial( + value: { value?: string | null; externalRef?: string | null }, + ctx: z.RefinementCtx, +) { + if (!value.value?.trim() && !value.externalRef?.trim()) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["value"], + message: "Secret rotation requires value or externalRef", + }); + } +} + export const rotateSecretSchema = z.object({ value: z.string().min(1).optional().nullable(), externalRef: z.string().optional().nullable(), providerVersionRef: z.string().optional().nullable(), providerConfigId: z.string().uuid().optional().nullable(), -}); +}).superRefine(requireSecretRotationMaterial); export type RotateSecret = z.infer; @@ -176,6 +189,15 @@ export const updateUserSecretValueSchema = z.object({ export type UpdateUserSecretValue = z.infer; +export const rotateUserSecretValueSchema = z.object({ + value: z.string().min(1).optional().nullable(), + externalRef: z.string().optional().nullable(), + providerVersionRef: z.string().optional().nullable(), + providerConfigId: z.string().uuid().optional().nullable(), +}).superRefine(requireSecretRotationMaterial); + +export type RotateUserSecretValue = z.infer; + export const createUserSecretDeclarationSchema = secretBindingTargetSchema.extend({ definitionKey: secretKeySchema, envKey: z.string().trim().min(1), diff --git a/server/src/__tests__/secrets-routes.test.ts b/server/src/__tests__/secrets-routes.test.ts index b7531c6d972..450dba3a305 100644 --- a/server/src/__tests__/secrets-routes.test.ts +++ b/server/src/__tests__/secrets-routes.test.ts @@ -156,6 +156,16 @@ describe("secret routes", () => { expect(JSON.stringify(mockLogActivity.mock.calls)).not.toContain("secret-value"); }); + it("rejects empty current-user secret rotation payloads", async () => { + const res = await request(createApp()) + .post("/api/companies/company-1/me/user-secrets/secret-1/rotate") + .send({}); + + expect(res.status).toBe(400); + expect(JSON.stringify(res.body)).toMatch(/requires value or externalRef/); + expect(mockSecretService.rotateCurrentUserSecretValue).not.toHaveBeenCalled(); + }); + it("hides user-scoped secrets from legacy company secret mutation routes", async () => { mockSecretService.getById.mockResolvedValue({ id: "secret-1", diff --git a/server/src/__tests__/secrets-service.test.ts b/server/src/__tests__/secrets-service.test.ts index e51606e451c..26817f8f7d9 100644 --- a/server/src/__tests__/secrets-service.test.ts +++ b/server/src/__tests__/secrets-service.test.ts @@ -517,6 +517,27 @@ describeEmbeddedPostgres("secretService", () => { manifest: [], }); + await db + .update(userSecretDefinitions) + .set({ status: "disabled" }) + .where(eq(userSecretDefinitions.id, definition.id)); + await expect( + svc.resolveEnvBindings(companyId, optionalEnv, { + consumerType: "agent", + consumerId: "agent-optional", + actorType: "agent", + actorId: "agent-optional", + responsibleUserId: "user-2", + }), + ).resolves.toMatchObject({ + env: {}, + manifest: [], + }); + await db + .update(userSecretDefinitions) + .set({ status: "active" }) + .where(eq(userSecretDefinitions.id, definition.id)); + const resolved = await svc.resolveEnvBindings(companyId, env, { consumerType: "agent", consumerId: "agent-1", diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 70888026361..4194cf8ddbc 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -52,6 +52,7 @@ import { createSecretSchema, updateSecretSchema, rotateSecretSchema, + rotateUserSecretValueSchema, createUserSecretDefinitionSchema, updateUserSecretDefinitionSchema, createUserSecretValueSchema, @@ -2287,7 +2288,7 @@ registry.registerPath({ summary: "Rotate my user secret value", request: { params: z.object({ companyId: z.string(), secretId: z.string() }), - body: jsonBody(updateUserSecretValueSchema), + body: jsonBody(rotateUserSecretValueSchema), }, responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden, 404: r.notFound }, }); diff --git a/server/src/routes/secrets.ts b/server/src/routes/secrets.ts index 12267a05f1c..401a54f57b8 100644 --- a/server/src/routes/secrets.ts +++ b/server/src/routes/secrets.ts @@ -8,6 +8,7 @@ import { remoteSecretImportPreviewSchema, remoteSecretImportSchema, rotateSecretSchema, + rotateUserSecretValueSchema, secretProviderConfigDiscoveryPreviewSchema, updateSecretProviderConfigSchema, updateSecretSchema, @@ -507,7 +508,7 @@ export function secretRoutes(db: Db) { router.post( "/companies/:companyId/me/user-secrets/:secretId/rotate", - validate(updateUserSecretValueSchema), + validate(rotateUserSecretValueSchema), async (req, res) => { assertBoard(req); const companyId = req.params.companyId as string; diff --git a/server/src/services/secrets.ts b/server/src/services/secrets.ts index 31ee9204fe3..23f8449a2b9 100644 --- a/server/src/services/secrets.ts +++ b/server/src/services/secrets.ts @@ -2253,11 +2253,13 @@ export function secretService(db: Db) { ): Promise => { const responsibleUserId = input.responsibleUserId ?? context?.responsibleUserId ?? null; const definition = await resolveUserSecretDefinition(companyId, input); + const optionalBinding = input.allowMissingOverride || input.required === false; if (definition.status !== "active") { + if (optionalBinding) return null; throw unprocessable("User secret definition is not active"); } if (!responsibleUserId?.trim()) { - if (input.allowMissingOverride || input.required === false) return null; + if (optionalBinding) return null; throw unprocessable("Responsible user is required for user secret resolution", { code: "responsible_user_missing", }); @@ -2297,7 +2299,7 @@ export function secretService(db: Db) { definitionId: definition.id, }); if (!secret) { - if (input.allowMissingOverride || input.required === false) return null; + if (optionalBinding) return null; throw unprocessable("User secret value is not configured", { code: "user_secret_missing", definitionId: definition.id,