[comp] Production Deploy

apps/api/src/admin-organizations/admin-audit-log.interceptor.spec.ts

@@ -54,6 +54,8 @@ jest.mock('@db', () => ({
 jest.mock('../audit/audit-log.constants', () => ({
   MUTATION_METHODS: new Set(['POST', 'PATCH', 'PUT', 'DELETE']),
   SENSITIVE_KEYS: new Set(['password', 'token']),
+  SENSITIVE_KEY_PATTERN:
+    /secret|password|passphrase|credential|token|api[_-]?key|private[_-]?key|totp|access[_-]?key/i,
 }));
 
 function buildContext(overrides: {
@@ -261,6 +263,28 @@ describe('AdminAuditLogInterceptor', () => {
     });
   });
 
+  it('sanitizes credential-shaped keys the exact list misses (clientSecret)', (done) => {
+    mockPolicyFind.mockResolvedValue({ name: 'Test' });
+
+    const ctx = buildContext({
+      method: 'PATCH',
+      url: '/v1/admin/organizations/org_1/policies/pol_1',
+      params: { orgId: 'org_1' },
+      body: { status: 'published', clientSecret: 'leak_me' },
+    });
+
+    interceptor.intercept(ctx, nextHandler).subscribe({
+      complete: () => {
+        setTimeout(() => {
+          const changes = mockCreate.mock.calls[0][0].data.data.changes;
+          expect(changes.status).toBeDefined();
+          expect(changes.clientSecret).toBeUndefined();
+          done();
+        }, 50);
+      },
+    });
+  });
+
   it('should handle DELETE for invitations', (done) => {
     const ctx = buildContext({
       method: 'DELETE',

apps/api/src/admin-organizations/admin-audit-log.interceptor.ts

@@ -8,7 +8,11 @@ import {
 import { AuditLogEntityType, db, Prisma } from '@db';
 import { Reflector } from '@nestjs/core';
 import { Observable, tap } from 'rxjs';
-import { MUTATION_METHODS, SENSITIVE_KEYS } from '../audit/audit-log.constants';
+import {
+  MUTATION_METHODS,
+  SENSITIVE_KEYS,
+  SENSITIVE_KEY_PATTERN,
+} from '../audit/audit-log.constants';
 import { SKIP_ADMIN_AUDIT_LOG_KEY } from './skip-admin-audit-log.decorator';
 
 const SEGMENT_TO_RESOURCE: Record<
@@ -261,7 +265,15 @@ export class AdminAuditLogInterceptor implements NestInterceptor {
     const changes: Changes = {};
 
     for (const [key, value] of Object.entries(body)) {
-      if (value === undefined || SENSITIVE_KEYS.has(key)) continue;
+      // Skip exact-match sensitive keys AND credential-shaped names the exact
+      // list misses (clientSecret, secretAccessKey, …) — shared with the global
+      // interceptor so both audit paths redact consistently.
+      if (
+        value === undefined ||
+        SENSITIVE_KEYS.has(key) ||
+        SENSITIVE_KEY_PATTERN.test(key)
+      )
+        continue;
       changes[key] = { previous: null, current: value };
     }
 

apps/api/src/audit/audit-log.constants.ts

@@ -24,6 +24,24 @@ export const SENSITIVE_KEYS = new Set([
   'totpCode',
 ]);
 
+/**
+ * Fallback pattern for credential-ish field names the exact-match set misses
+ * (e.g. `clientSecret`, `secretAccessKey`, `aws_secret_access_key`). Matched
+ * case-insensitively against key names anywhere in the audited body, so new
+ * credential fields are redacted without having to enumerate every name.
+ */
+export const SENSITIVE_KEY_PATTERN =
+  /secret|password|passphrase|credential|token|api[_-]?key|private[_-]?key|totp|access[_-]?key/i;
+
+/**
+ * Resources whose request body must never be diffed into the audit log at all —
+ * the field carrying the secret is generically named (e.g. the secret manager's
+ * `value`) so key-based redaction can't catch it, and reading audit logs needs
+ * only `app:read`. For these we log the action (Created/Updated/Deleted) with no
+ * payload, keeping plaintext out of a store that bypasses `secret:read`.
+ */
+export const REDACT_BODY_RESOURCES = new Set(['secret']);
+
 export const RESOURCE_TO_ENTITY_TYPE: Record<
   string,
   AuditLogEntityType | null

apps/api/src/audit/audit-log.interceptor.spec.ts

@@ -381,6 +381,107 @@ describe('AuditLogInterceptor', () => {
     });
   });
 
+  it('should skip read endpoints that use a mutation verb (POST with read-only permission)', (done) => {
+    // e.g. POST /v1/trust-portal/documents/list — a list/status read that uses
+    // POST to carry a filter body. It declares `read`, so it must not be logged
+    // as "Created trust".
+    jest.spyOn(reflector, 'getAllAndOverride').mockImplementation((key) => {
+      if (key === PERMISSIONS_KEY) {
+        return [{ resource: 'trust', actions: ['read'] }];
+      }
+      if (key === SKIP_AUDIT_LOG_KEY) return false;
+      return undefined;
+    });
+
+    const context = createMockExecutionContext({
+      method: 'POST',
+      url: '/v1/trust-portal/documents/list',
+      params: {},
+      body: { organizationId: 'org_123' },
+    });
+    const handler = createMockCallHandler([]);
+
+    interceptor.intercept(context, handler).subscribe({
+      next: () => {
+        setTimeout(() => {
+          expect(mockCreate).not.toHaveBeenCalled();
+          done();
+        }, 50);
+      },
+    });
+  });
+
+  it('still logs when only ONE of several declared permissions is read-only', (done) => {
+    // A POST that declares [read, create] is a real mutation — the read
+    // requirement must not suppress the audit entry.
+    jest.spyOn(reflector, 'getAllAndOverride').mockImplementation((key) => {
+      if (key === PERMISSIONS_KEY) {
+        return [
+          { resource: 'trust', actions: ['read'] },
+          { resource: 'policy', actions: ['create'] },
+        ];
+      }
+      if (key === SKIP_AUDIT_LOG_KEY) return false;
+      return undefined;
+    });
+
+    const context = createMockExecutionContext({
+      method: 'POST',
+      url: '/v1/something',
+      params: {},
+      body: { organizationId: 'org_123' },
+    });
+    const handler = createMockCallHandler({ id: 'ent_new' });
+
+    interceptor.intercept(context, handler).subscribe({
+      next: () => {
+        setTimeout(() => {
+          expect(mockCreate).toHaveBeenCalled();
+          done();
+        }, 50);
+      },
+    });
+  });
+
+  it('logs the action but never the payload for the secret resource', (done) => {
+    // Reading audit logs needs only app:read; the secret manager's plaintext
+    // `value` must not be diffed into the log where an auditor (no secret:read)
+    // could read it.
+    jest.spyOn(reflector, 'getAllAndOverride').mockImplementation((key) => {
+      if (key === PERMISSIONS_KEY) {
+        return [{ resource: 'secret', actions: ['create'] }];
+      }
+      if (key === SKIP_AUDIT_LOG_KEY) return false;
+      return undefined;
+    });
+
+    const context = createMockExecutionContext({
+      method: 'POST',
+      url: '/v1/secrets',
+      params: {},
+      body: { name: 'STRIPE_KEY', value: 'sk_live_super_secret' },
+    });
+    const handler = createMockCallHandler({ id: 'sec_1' });
+
+    interceptor.intercept(context, handler).subscribe({
+      next: () => {
+        setTimeout(() => {
+          expect(mockCreate).toHaveBeenCalled();
+          // The action is recorded...
+          expect(mockCreate).toHaveBeenCalledWith(
+            expect.objectContaining({
+              data: expect.objectContaining({ description: 'Created secret' }),
+            }),
+          );
+          // ...but the plaintext value never appears anywhere in the row.
+          const persisted = JSON.stringify(mockCreate.mock.calls[0][0]);
+          expect(persisted).not.toContain('sk_live_super_secret');
+          done();
+        }, 50);
+      },
+    });
+  });
+
   it('should skip requests without userId', (done) => {
     jest.spyOn(reflector, 'getAllAndOverride').mockImplementation((key) => {
       if (key === PERMISSIONS_KEY) {

apps/api/src/audit/audit-log.interceptor.ts

@@ -15,6 +15,7 @@ import { AUDIT_READ_KEY, SKIP_AUDIT_LOG_KEY } from './skip-audit-log.decorator';
 import {
   MEMBER_REF_FIELDS,
   MUTATION_METHODS,
+  REDACT_BODY_RESOURCES,
   RESOURCE_TO_ENTITY_TYPE,
 } from './audit-log.constants';
 import {
@@ -76,6 +77,24 @@ export class AuditLogInterceptor implements NestInterceptor {
     }
 
     const { resource, actions } = requiredPermissions[0];
+
+    // Read-only endpoints that use a mutation HTTP verb (e.g. `POST .../list` or
+    // `POST .../status` that carry a filter body) declare exactly `['read']`.
+    // The method-derived verb below would log them as "Created X" on every page
+    // load, so skip them — a required permission of only `read` is definitionally
+    // not a mutation. `@AuditRead` opts a read endpoint back into logging.
+    // Only skip when EVERY declared permission is read-only: an endpoint with
+    // multiple requirements (e.g. [read, create]) still performs a mutation.
+    if (
+      !isAuditRead &&
+      requiredPermissions.every(
+        (permission) =>
+          permission.actions.length === 1 && permission.actions[0] === 'read',
+      )
+    ) {
+      return next.handle();
+    }
+
     // Derive the actual action from the HTTP method rather than using the first
     // permission action. This is important when a controller declares multiple
     // actions (e.g. ['create','read','update','delete']) at the class level.
@@ -250,6 +269,12 @@ export class AuditLogInterceptor implements NestInterceptor {
               } else if (relationMappingResult) {
                 changes = relationMappingResult.changes;
                 descriptionOverride ??= relationMappingResult.description;
+              } else if (REDACT_BODY_RESOURCES.has(resource)) {
+                // Credential resources (e.g. the secret manager) carry their
+                // secret in a generically-named field. Diffing the body would
+                // land plaintext in a store readable with only app:read, which
+                // bypasses <resource>:read. Record the action, not the payload.
+                changes = null;
               } else {
                 changes = requestBody
                   ? buildChanges(requestBody, previousValues, memberNames)

apps/api/src/audit/audit-log.utils.spec.ts

@@ -0,0 +1,66 @@
+// buildChanges → constants pull Prisma enums at module load; stub @db so the
+// pure redaction logic can be tested without a real client.
+jest.mock('@db', () => ({
+  db: {},
+  AuditLogEntityType: new Proxy({}, { get: (_t, p) => p }),
+  CommentEntityType: new Proxy({}, { get: (_t, p) => p }),
+}));
+
+import { buildChanges } from './audit-log.utils';
+
+describe('buildChanges — redaction', () => {
+  it('redacts the existing exact-match sensitive keys', () => {
+    const changes = buildChanges(
+      { password: 'p', apiKey: 'k', name: 'ok' },
+      null,
+      {},
+    );
+    expect(changes?.password.current).toBe('[REDACTED]');
+    expect(changes?.apiKey.current).toBe('[REDACTED]');
+    expect(changes?.name.current).toBe('ok');
+  });
+
+  it('redacts credential-named keys the exact list misses (clientSecret, secretAccessKey)', () => {
+    const changes = buildChanges(
+      {
+        clientSecret: 'abc',
+        secretAccessKey: 'xyz',
+        aws_secret_access_key: 'q',
+        name: 'ok',
+      },
+      null,
+      {},
+    );
+    expect(changes?.clientSecret.current).toBe('[REDACTED]');
+    expect(changes?.secretAccessKey.current).toBe('[REDACTED]');
+    expect(changes?.aws_secret_access_key.current).toBe('[REDACTED]');
+    expect(changes?.name.current).toBe('ok');
+  });
+
+  it('summarizes object elements inside arrays so secrets in generic fields cannot leak', () => {
+    // e.g. browserbase `extraFields: [{ label, value }]` — `value` is a secret
+    // under a non-credential key; the whole element is hidden as [Object].
+    const changes = buildChanges(
+      { extraFields: [{ label: 'workspace', value: 'sekret' }] },
+      null,
+      {},
+    );
+    expect(changes?.extraFields.current).toEqual(['[Object]']);
+  });
+
+  it('keeps primitive array elements (ids, scopes, tags) visible', () => {
+    const changes = buildChanges({ scopes: ['read', 'write'] }, null, {});
+    expect(changes?.scopes.current).toEqual(['read', 'write']);
+  });
+
+  it('keeps summarizing nested plain objects as [Object]', () => {
+    const changes = buildChanges({ config: { a: 1, token: 't' } }, null, {});
+    expect(changes?.config.current).toBe('[Object]');
+  });
+
+  it('leaves ordinary values untouched', () => {
+    const changes = buildChanges({ status: 'active', count: 3 }, null, {});
+    expect(changes?.status.current).toBe('active');
+    expect(changes?.count.current).toBe(3);
+  });
+});

apps/api/src/audit/audit-log.utils.ts

@@ -4,6 +4,7 @@ import {
   COMMENT_ENTITY_TYPE_MAP,
   MEMBER_REF_FIELDS,
   SENSITIVE_KEYS,
+  SENSITIVE_KEY_PATTERN,
 } from './audit-log.constants';
 
 export type AuditContextOverride = {
@@ -328,14 +329,29 @@ export function buildDescription(
   }
 }
 
+function isSensitiveKey(key: string): boolean {
+  return SENSITIVE_KEYS.has(key) || SENSITIVE_KEY_PATTERN.test(key);
+}
+
 function sanitizeValue(key: string, value: unknown): unknown {
-  if (SENSITIVE_KEYS.has(key)) return '[REDACTED]';
+  if (isSensitiveKey(key)) return '[REDACTED]';
   if (value instanceof Date) return value.toISOString();
-  if (value && typeof value === 'object' && !Array.isArray(value))
-    return '[Object]';
+  // Arrays are logged rather than summarized, so a secret in a generically-named
+  // field inside an array element (e.g. `extraFields: [{ label, value }]`) would
+  // otherwise land in the log verbatim. Keep primitive elements (ids, scopes,
+  // tags) but summarize object/array elements as '[Object]' — the same way a
+  // nested object is hidden below.
+  if (Array.isArray(value)) return value.map(summarizeArrayItem);
+  if (value && typeof value === 'object') return '[Object]';
   return value;
 }
 
+function summarizeArrayItem(item: unknown): unknown {
+  if (item instanceof Date) return item.toISOString();
+  if (item && typeof item === 'object') return '[Object]';
+  return item;
+}
+
 export function buildChanges(
   body: Record<string, unknown>,
   previousValues: Record<string, unknown> | null,