fix(secrets): validate user-secret rotations

Require rotation payloads to include literal material or an external reference, apply the stricter schema to current-user secret rotation routes and OpenAPI, and let optional user-secret bindings quietly omit disabled definitions during runtime resolution.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

packages/shared/src/index.ts

@@ -1293,6 +1293,7 @@ export {
   updateUserSecretDefinitionSchema,
   createUserSecretValueSchema,
   updateUserSecretValueSchema,
+  rotateUserSecretValueSchema,
   createUserSecretDeclarationSchema,
   createSecretProviderConfigSchema,
   updateSecretProviderConfigSchema,

packages/shared/src/validators/index.ts

@@ -471,6 +471,7 @@ export {
   updateUserSecretDefinitionSchema,
   createUserSecretValueSchema,
   updateUserSecretValueSchema,
+  rotateUserSecretValueSchema,
   createUserSecretDeclarationSchema,
   createSecretProviderConfigSchema,
   updateSecretProviderConfigSchema,

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({

packages/shared/src/validators/secret.ts

@@ -79,12 +79,25 @@ export const createSecretSchema = z.object({
 
 export type CreateSecret = z.infer<typeof createSecretSchema>;
 
+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<typeof rotateSecretSchema>;
 
@@ -176,6 +189,15 @@ export const updateUserSecretValueSchema = z.object({
 
 export type UpdateUserSecretValue = z.infer<typeof updateUserSecretValueSchema>;
 
+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<typeof rotateUserSecretValueSchema>;
+
 export const createUserSecretDeclarationSchema = secretBindingTargetSchema.extend({
   definitionKey: secretKeySchema,
   envKey: z.string().trim().min(1),

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",

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",

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 },
 });

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;

server/src/services/secrets.ts

@@ -2253,11 +2253,13 @@ export function secretService(db: Db) {
     ): Promise<RuntimeSecretResolution | null> => {
       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,