diff --git a/apps/api/src/admin-organizations/admin-audit-log.interceptor.spec.ts b/apps/api/src/admin-organizations/admin-audit-log.interceptor.spec.ts index ccdca4374c..e4a6c81aaa 100644 --- a/apps/api/src/admin-organizations/admin-audit-log.interceptor.spec.ts +++ b/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', diff --git a/apps/api/src/admin-organizations/admin-audit-log.interceptor.ts b/apps/api/src/admin-organizations/admin-audit-log.interceptor.ts index 9db723b935..54a9916085 100644 --- a/apps/api/src/admin-organizations/admin-audit-log.interceptor.ts +++ b/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 }; } diff --git a/apps/api/src/audit/audit-log.constants.ts b/apps/api/src/audit/audit-log.constants.ts index 33b7d3a71c..6ae08e863f 100644 --- a/apps/api/src/audit/audit-log.constants.ts +++ b/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 diff --git a/apps/api/src/audit/audit-log.interceptor.spec.ts b/apps/api/src/audit/audit-log.interceptor.spec.ts index d33fa1fcb3..3852e6eb4c 100644 --- a/apps/api/src/audit/audit-log.interceptor.spec.ts +++ b/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) { diff --git a/apps/api/src/audit/audit-log.interceptor.ts b/apps/api/src/audit/audit-log.interceptor.ts index 4e954cb244..85374fdb67 100644 --- a/apps/api/src/audit/audit-log.interceptor.ts +++ b/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 :read. Record the action, not the payload. + changes = null; } else { changes = requestBody ? buildChanges(requestBody, previousValues, memberNames) diff --git a/apps/api/src/audit/audit-log.utils.spec.ts b/apps/api/src/audit/audit-log.utils.spec.ts new file mode 100644 index 0000000000..9c3125b12d --- /dev/null +++ b/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); + }); +}); diff --git a/apps/api/src/audit/audit-log.utils.ts b/apps/api/src/audit/audit-log.utils.ts index e4a10775bb..de67a3d7a1 100644 --- a/apps/api/src/audit/audit-log.utils.ts +++ b/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, previousValues: Record | null, diff --git a/apps/api/src/comments/dto/create-comment.dto.spec.ts b/apps/api/src/comments/dto/create-comment.dto.spec.ts new file mode 100644 index 0000000000..3b9a775b73 --- /dev/null +++ b/apps/api/src/comments/dto/create-comment.dto.spec.ts @@ -0,0 +1,99 @@ +import { plainToInstance } from 'class-transformer'; +import { validate } from 'class-validator'; +import { CreateCommentDto } from './create-comment.dto'; + +// create-comment.dto.ts imports the `CommentEntityType` enum from `@db`, +// which eagerly instantiates the Prisma client on import — mock it so this +// spec doesn't need a configured DB connection (mirrors comments.controller.spec.ts). +jest.mock('@db', () => ({ + db: {}, + CommentEntityType: { + task: 'task', + vendor: 'vendor', + risk: 'risk', + policy: 'policy', + finding: 'finding', + }, +})); + +function tiptapDoc(content: unknown[]): string { + return JSON.stringify({ type: 'doc', content }); +} + +function toDto(plain: Record): CreateCommentDto { + return plainToInstance(CreateCommentDto, plain, { + enableImplicitConversion: true, + }); +} + +const VALID_BASE = { + entityId: 'tsk_abc123', + entityType: 'task', +}; + +describe('CreateCommentDto', () => { + it('accepts a plain-text comment under the limit', async () => { + const dto = toDto({ ...VALID_BASE, content: 'Looks good to me' }); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + }); + + it('rejects a plain-text comment over 2000 visible characters', async () => { + const dto = toDto({ ...VALID_BASE, content: 'x'.repeat(2001) }); + const errors = await validate(dto); + expect(errors.some((e) => e.property === 'content')).toBe(true); + }); + + it('accepts a formatted Tiptap comment whose raw JSON exceeds 2000 chars but whose visible text does not (regression for the reported bug)', async () => { + const words = Array.from({ length: 240 }, (_, i) => ({ + type: 'text', + text: 'word ', + ...(i % 2 === 0 ? { marks: [{ type: 'bold' }] } : {}), + })); + const content = tiptapDoc([{ type: 'paragraph', content: words }]); + expect(content.length).toBeGreaterThan(2000); + + const dto = toDto({ ...VALID_BASE, content }); + const errors = await validate(dto); + expect(errors).toHaveLength(0); + }); + + it('rejects a formatted Tiptap comment whose visible text exceeds 2000 characters', async () => { + const content = tiptapDoc([ + { + type: 'paragraph', + content: [ + { + type: 'text', + text: 'a'.repeat(2001), + marks: [{ type: 'bold' }], + }, + ], + }, + ]); + + const dto = toDto({ ...VALID_BASE, content }); + const errors = await validate(dto); + expect(errors.some((e) => e.property === 'content')).toBe(true); + }); + + it('rejects an empty comment', async () => { + const dto = toDto({ ...VALID_BASE, content: '' }); + const errors = await validate(dto); + expect(errors.some((e) => e.property === 'content')).toBe(true); + }); + + it('rejects a non-doc JSON payload over the limit instead of treating it as empty (bypass regression)', async () => { + const content = `{"foo": "${'x'.repeat(2001)}"}`; + const dto = toDto({ ...VALID_BASE, content }); + const errors = await validate(dto); + expect(errors.some((e) => e.property === 'content')).toBe(true); + }); + + it('rejects an empty Tiptap document — non-empty JSON string but zero visible text (regression)', async () => { + const content = tiptapDoc([]); + const dto = toDto({ ...VALID_BASE, content }); + const errors = await validate(dto); + expect(errors.some((e) => e.property === 'content')).toBe(true); + }); +}); diff --git a/apps/api/src/comments/dto/create-comment.dto.ts b/apps/api/src/comments/dto/create-comment.dto.ts index f83266ae15..a2edabf69a 100644 --- a/apps/api/src/comments/dto/create-comment.dto.ts +++ b/apps/api/src/comments/dto/create-comment.dto.ts @@ -11,16 +11,25 @@ import { ValidateNested, } from 'class-validator'; import { UploadAttachmentDto } from '../../attachments/upload-attachment.dto'; +import { MaxCommentTextLength } from '../validators/max-comment-text-length.validator'; + +// `content` is serialized Tiptap JSON (or plain text for API callers). The +// 2000-char limit applies to the visible text a user typed, not the raw +// JSON — see MaxCommentTextLength. This raw-string cap only bounds payload +// size against pathologically formatted input. +const RAW_CONTENT_MAX_LENGTH = 50_000; export class CreateCommentDto { @ApiProperty({ - description: 'Content of the comment', + description: + 'Content of the comment (plain text or serialized Tiptap JSON). Limited to 2000 characters of visible text; maxLength bounds the serialized payload size, not the visible text.', example: 'This task needs to be completed by end of week', - maxLength: 2000, + maxLength: RAW_CONTENT_MAX_LENGTH, }) @IsString() @IsNotEmpty() - @MaxLength(2000) + @MaxLength(RAW_CONTENT_MAX_LENGTH) + @MaxCommentTextLength(2000) content: string; @ApiProperty({ diff --git a/apps/api/src/comments/dto/update-comment.dto.ts b/apps/api/src/comments/dto/update-comment.dto.ts index 00b24d3c9e..6215232362 100644 --- a/apps/api/src/comments/dto/update-comment.dto.ts +++ b/apps/api/src/comments/dto/update-comment.dto.ts @@ -1,15 +1,24 @@ import { ApiProperty } from '@nestjs/swagger'; import { IsNotEmpty, IsOptional, IsString, MaxLength } from 'class-validator'; +import { MaxCommentTextLength } from '../validators/max-comment-text-length.validator'; + +// `content` is serialized Tiptap JSON (or plain text for API callers). The +// 2000-char limit applies to the visible text a user typed, not the raw +// JSON — see MaxCommentTextLength. This raw-string cap only bounds payload +// size against pathologically formatted input. +const RAW_CONTENT_MAX_LENGTH = 50_000; export class UpdateCommentDto { @ApiProperty({ - description: 'Updated content of the comment', + description: + 'Updated content of the comment (plain text or serialized Tiptap JSON). Limited to 2000 characters of visible text; maxLength bounds the serialized payload size, not the visible text.', example: 'This task needs to be completed by end of week (updated)', - maxLength: 2000, + maxLength: RAW_CONTENT_MAX_LENGTH, }) @IsString() @IsNotEmpty() - @MaxLength(2000) + @MaxLength(RAW_CONTENT_MAX_LENGTH) + @MaxCommentTextLength(2000) content: string; @ApiProperty({ diff --git a/apps/api/src/comments/utils/extract-comment-plain-text.spec.ts b/apps/api/src/comments/utils/extract-comment-plain-text.spec.ts new file mode 100644 index 0000000000..cb9dd1d9e1 --- /dev/null +++ b/apps/api/src/comments/utils/extract-comment-plain-text.spec.ts @@ -0,0 +1,152 @@ +import { extractCommentPlainText } from './extract-comment-plain-text'; + +function tiptapDoc(content: unknown[]): string { + return JSON.stringify({ type: 'doc', content }); +} + +describe('extractCommentPlainText', () => { + it('returns plain text as-is when content is not JSON', () => { + expect(extractCommentPlainText('Just a plain comment')).toBe( + 'Just a plain comment', + ); + }); + + it('extracts an empty string from an empty Tiptap document', () => { + expect(extractCommentPlainText(tiptapDoc([]))).toBe(''); + }); + + it('returns plain text as-is when it happens to be valid JSON but not a Tiptap doc (bypass regression)', () => { + const longPlainText = 'x'.repeat(3000); + const jsonLookingText = `{"foo": "${longPlainText}"}`; + expect(extractCommentPlainText(jsonLookingText)).toBe(jsonLookingText); + + const jsonArrayLookingText = `["${longPlainText}"]`; + expect(extractCommentPlainText(jsonArrayLookingText)).toBe( + jsonArrayLookingText, + ); + }); + + it('extracts text from a simple paragraph', () => { + const content = tiptapDoc([ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Hello world' }], + }, + ]); + expect(extractCommentPlainText(content)).toBe('Hello world'); + }); + + it('ignores formatting marks — bold text counts the same as plain text', () => { + const content = tiptapDoc([ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'This word is ' }, + { type: 'text', text: 'bold', marks: [{ type: 'bold' }] }, + { type: 'text', text: '.' }, + ], + }, + ]); + expect(extractCommentPlainText(content)).toBe('This word is bold.'); + }); + + it('counts one character per hard break and per paragraph boundary', () => { + const content = tiptapDoc([ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Line one' }, + { type: 'hardBreak' }, + { type: 'text', text: 'Line two' }, + ], + }, + { + type: 'paragraph', + content: [{ type: 'text', text: 'Second paragraph' }], + }, + ]); + expect(extractCommentPlainText(content)).toBe( + 'Line one\nLine two\nSecond paragraph', + ); + }); + + it('renders a mention as @label', () => { + const content = tiptapDoc([ + { + type: 'paragraph', + content: [ + { type: 'text', text: 'Hey ' }, + { type: 'mention', attrs: { id: 'usr_1', label: 'Jane Doe' } }, + ], + }, + ]); + expect(extractCommentPlainText(content)).toBe('Hey @Jane Doe'); + }); + + it('extracts text from a bullet list', () => { + const content = tiptapDoc([ + { + type: 'bulletList', + content: [ + { + type: 'listItem', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Item one' }], + }, + ], + }, + { + type: 'listItem', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Item two' }], + }, + ], + }, + ], + }, + ]); + expect(extractCommentPlainText(content)).toBe('Item one\nItem two'); + }); + + it('does not double-count the line break for a blockquoted paragraph', () => { + const content = tiptapDoc([ + { + type: 'blockquote', + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Quoted line' }], + }, + ], + }, + { + type: 'paragraph', + content: [{ type: 'text', text: 'Next line' }], + }, + ]); + // If blockquote also appended its own newline on top of the paragraph's, + // this would be 'Quoted line\n\nNext line' instead. + expect(extractCommentPlainText(content)).toBe('Quoted line\nNext line'); + }); + + it('matches the reported bug: a ~1,200-char formatted comment exceeds 2000 raw chars but stays under the visible limit', () => { + // Alternating bold/plain 5-char words, like a comment with scattered + // emphasis — each bold run's marks array is pure JSON overhead. + const words = Array.from({ length: 240 }, (_, i) => ({ + type: 'text', + text: 'word ', + ...(i % 2 === 0 ? { marks: [{ type: 'bold' }] } : {}), + })); + const content = tiptapDoc([{ type: 'paragraph', content: words }]); + + // 240 * 5 = 1200 visible characters, well under the 2000 limit. + expect(extractCommentPlainText(content).length).toBe(1200); + // But the raw JSON (what the old @MaxLength(2000) validated) blows past + // it purely from marks/node overhead — this is the bug. + expect(content.length).toBeGreaterThan(2000); + }); +}); diff --git a/apps/api/src/comments/utils/extract-comment-plain-text.ts b/apps/api/src/comments/utils/extract-comment-plain-text.ts new file mode 100644 index 0000000000..438273e5b2 --- /dev/null +++ b/apps/api/src/comments/utils/extract-comment-plain-text.ts @@ -0,0 +1,88 @@ +/** + * Node types whose content represents a separate visual line. A trailing + * newline is appended after their text so line/paragraph breaks the user + * typed count toward the visible length, matching what they see on screen. + * Wrapper types (listItem, tableCell, blockquote, ...) are deliberately + * excluded — their children are typically paragraphs that already + * contribute a newline, and including the wrapper too would double-count + * each line break. + */ +const BLOCK_NODE_TYPES = new Set(['paragraph', 'heading', 'codeBlock']); + +interface TiptapNode { + type?: unknown; + text?: unknown; + attrs?: unknown; + content?: unknown; +} + +function mentionLabel(node: TiptapNode): string { + const attrs = node.attrs as { label?: unknown; id?: unknown } | undefined; + if (typeof attrs?.label === 'string' && attrs.label) return attrs.label; + if (typeof attrs?.id === 'string' && attrs.id) return attrs.id; + return ''; +} + +function nodeToText(node: unknown): string { + if (!node || typeof node !== 'object') return ''; + const n = node as TiptapNode; + + if (n.type === 'text') { + return typeof n.text === 'string' ? n.text : ''; + } + + if (n.type === 'hardBreak') { + return '\n'; + } + + if (n.type === 'mention') { + const label = mentionLabel(n); + return label ? `@${label}` : ''; + } + + if (Array.isArray(n.content)) { + const childText = n.content.map(nodeToText).join(''); + return BLOCK_NODE_TYPES.has(typeof n.type === 'string' ? n.type : '') + ? `${childText}\n` + : childText; + } + + return ''; +} + +function isTiptapDoc(value: unknown): value is TiptapNode { + if (!value || typeof value !== 'object') return false; + const n = value as TiptapNode; + return n.type === 'doc' && Array.isArray(n.content); +} + +/** + * Extracts the visible text a user typed from a comment's stored `content`. + * Comments accept either raw Tiptap/ProseMirror JSON (from the web editor) + * or plain text (from API/MCP callers) — formatting marks, node types, and + * attrs are structural overhead that inflates the raw string but adds no + * visible characters, so length checks must run against this instead of + * `content.length`. + * + * Only parses `content` as Tiptap when it has the expected `{ type: 'doc', + * content: [...] }` shape. A plain-text comment that happens to be valid + * JSON (e.g. `{"foo": "..."}`) would otherwise be walked as a node tree, + * match no known type, and silently extract to `''` — bypassing the length + * check entirely instead of falling back to the raw string. + */ +export function extractCommentPlainText(content: string): string { + if (typeof content !== 'string') return ''; + + let parsed: unknown; + try { + parsed = JSON.parse(content); + } catch { + return content; + } + + if (!isTiptapDoc(parsed)) { + return content; + } + + return nodeToText(parsed).replace(/\n$/, ''); +} diff --git a/apps/api/src/comments/validators/max-comment-text-length.validator.ts b/apps/api/src/comments/validators/max-comment-text-length.validator.ts new file mode 100644 index 0000000000..343b6431bf --- /dev/null +++ b/apps/api/src/comments/validators/max-comment-text-length.validator.ts @@ -0,0 +1,50 @@ +import { + registerDecorator, + ValidationArguments, + ValidationOptions, + ValidatorConstraint, + ValidatorConstraintInterface, +} from 'class-validator'; +import { extractCommentPlainText } from '../utils/extract-comment-plain-text'; + +const DEFAULT_MAX_LENGTH = 2000; + +/** + * Validates comment `content` against the visible text length rather than + * the raw stored string — `content` is serialized Tiptap JSON, so formatting + * (marks, node types, attrs) would otherwise count toward the limit and + * reject short, plainly-visible comments once they include any formatting. + * + * Also rejects zero visible text: `@IsNotEmpty()` only sees the raw string, + * so an empty document (e.g. `{"type":"doc","content":[]}`) is a non-empty + * JSON string that would otherwise sail through as a "valid" empty comment. + */ +@ValidatorConstraint({ name: 'maxCommentTextLength', async: false }) +export class MaxCommentTextLengthConstraint implements ValidatorConstraintInterface { + validate(value: unknown, args: ValidationArguments): boolean { + if (typeof value !== 'string') return false; + const maxLength = (args.constraints[0] as number) ?? DEFAULT_MAX_LENGTH; + const length = [...extractCommentPlainText(value)].length; + return length > 0 && length <= maxLength; + } + + defaultMessage(args: ValidationArguments): string { + const maxLength = (args.constraints[0] as number) ?? DEFAULT_MAX_LENGTH; + return `content must not be empty and must not exceed ${maxLength} characters`; + } +} + +export function MaxCommentTextLength( + maxLength: number = DEFAULT_MAX_LENGTH, + validationOptions?: ValidationOptions, +) { + return function (object: object, propertyName: string) { + registerDecorator({ + target: object.constructor, + propertyName, + options: validationOptions, + constraints: [maxLength], + validator: MaxCommentTextLengthConstraint, + }); + }; +} diff --git a/apps/api/src/email/email.controller.ts b/apps/api/src/email/email.controller.ts index 2b15718d12..54dde600ea 100644 --- a/apps/api/src/email/email.controller.ts +++ b/apps/api/src/email/email.controller.ts @@ -10,6 +10,7 @@ import { tasks } from '@trigger.dev/sdk'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; import { RequirePermission } from '../auth/require-permission.decorator'; +import { SkipAuditLog } from '../audit/skip-audit-log.decorator'; import { SendEmailDto } from './dto/send-email.dto'; import { SendBatchEmailDto } from './dto/send-batch-email.dto'; import type { sendEmailTask } from '../trigger/email/send-email'; @@ -24,6 +25,9 @@ export class EmailController { @Post('send') @HttpCode(200) @RequirePermission('email', 'send') + // The body carries rendered `html` (magic-links / OTP) and full attachment + // bytes — never worth diffing into the audit log, and readable with app:read. + @SkipAuditLog() @ApiOperation({ summary: 'Send an email via the centralized Trigger task (internal)', }) @@ -46,6 +50,7 @@ export class EmailController { @Post('send-batch') @HttpCode(200) @RequirePermission('email', 'send') + @SkipAuditLog() @ApiOperation({ summary: 'Send a batch of emails via the centralized Trigger task (internal)', }) diff --git a/apps/api/src/email/templates/evidence-access-request-submitted.tsx b/apps/api/src/email/templates/evidence-access-request-submitted.tsx new file mode 100644 index 0000000000..d8907be134 --- /dev/null +++ b/apps/api/src/email/templates/evidence-access-request-submitted.tsx @@ -0,0 +1,140 @@ +import * as React from 'react'; +import { + Body, + Button, + Container, + Font, + Heading, + Html, + Link, + Preview, + Section, + Tailwind, + Text, +} from '@react-email/components'; +import { Footer } from '../components/footer'; +import { Logo } from '../components/logo'; +import { getUnsubscribeUrl } from '@trycompai/email'; + +interface Props { + toName: string; + toEmail: string; + organizationName: string; + requesterName: string; + accountsNeeded: string; + permissionsNeeded: string; + reasonForRequest: string; + reviewUrl: string; +} + +export const EvidenceAccessRequestSubmittedEmail = ({ + toName, + toEmail, + organizationName, + requesterName, + accountsNeeded, + permissionsNeeded, + reasonForRequest, + reviewUrl, +}: Props) => { + const unsubscribeUrl = getUnsubscribeUrl(toEmail); + + return ( + + + + + + + New access request from {requesterName} + + + + + + New Access Request + + + + Hello {toName}, + + + + {requesterName} submitted an access request in{' '} + {organizationName}. + + +
+ + Request Details + + + + Accounts Needed: {accountsNeeded} + + + + Permissions Needed: {permissionsNeeded} + + + + "{reasonForRequest}" + +
+ +
+ +
+ + + or copy and paste this URL into your browser:{' '} + + {reviewUrl} + + + +
+ + Don't want to receive access request notifications?{' '} + + Manage your email preferences + + . + +
+ +
+ +