Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/api/src/attachments/attachments.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { randomBytes } from 'crypto';
import { AttachmentResponseDto } from '../tasks/dto/task-responses.dto';
import { UploadAttachmentDto } from './upload-attachment.dto';
import { s3Client } from '@/app/s3';
import { validateFileContent } from '../utils/file-type-validation';

@Injectable()
export class AttachmentsService {
Expand Down Expand Up @@ -123,6 +124,9 @@ export class AttachmentsService {
);
}

// Validate file content matches declared MIME type
validateFileContent(fileBuffer, uploadDto.fileType, uploadDto.fileName);

// Generate unique file key
const fileId = randomBytes(16).toString('hex');
const sanitizedFileName = this.sanitizeFileName(uploadDto.fileName);
Expand Down
25 changes: 14 additions & 11 deletions apps/api/src/auth/api-key.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,19 @@ export class ApiKeyService {
expiresAt?: string,
scopes?: string[],
) {
// Validate scopes if provided
const validatedScopes = scopes?.length ? scopes : [];
if (validatedScopes.length > 0) {
const availableScopes = this.getAvailableScopes();
const invalid = validatedScopes.filter((s) => !availableScopes.includes(s));
if (invalid.length > 0) {
throw new BadRequestException(
`Invalid scopes: ${invalid.join(', ')}`,
);
}
// New keys must have explicit scopes — no more legacy empty-scope keys
if (!scopes || scopes.length === 0) {
throw new BadRequestException(
'API keys must have at least one scope. Use the "Full Access" preset to grant all permissions.',
);
}
// Validate all scopes against the allowlist
const availableScopes = this.getAvailableScopes();
const invalid = scopes.filter((s) => !availableScopes.includes(s));
if (invalid.length > 0) {
throw new BadRequestException(
`Invalid scopes: ${invalid.join(', ')}`,
);
}

const apiKey = this.generateApiKey();
Expand Down Expand Up @@ -103,7 +106,7 @@ export class ApiKeyService {
salt,
expiresAt: expirationDate,
organizationId,
scopes: validatedScopes,
scopes,
},
select: {
id: true,
Expand Down
61 changes: 61 additions & 0 deletions apps/api/src/auth/auth-server-origins.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* Tests for the getTrustedOrigins logic.
*
* Because auth.server.ts has side effects at module load time (better-auth
* initialization, DB connections, validateSecurityConfig), we test the logic
* in isolation rather than importing the module directly.
*/

function getTrustedOriginsLogic(authTrustedOrigins: string | undefined): string[] {
if (authTrustedOrigins) {
return authTrustedOrigins.split(',').map((o) => o.trim());
}

return [
'http://localhost:3000',
'http://localhost:3002',
'http://localhost:3333',
'https://app.trycomp.ai',
'https://portal.trycomp.ai',
'https://api.trycomp.ai',
'https://app.staging.trycomp.ai',
'https://portal.staging.trycomp.ai',
'https://api.staging.trycomp.ai',
'https://dev.trycomp.ai',
];
}

describe('getTrustedOrigins', () => {
it('should return env-configured origins when AUTH_TRUSTED_ORIGINS is set', () => {
const origins = getTrustedOriginsLogic('https://a.com, https://b.com');
expect(origins).toEqual(['https://a.com', 'https://b.com']);
});

it('should return hardcoded origins when AUTH_TRUSTED_ORIGINS is not set', () => {
const origins = getTrustedOriginsLogic(undefined);
expect(origins).toContain('https://app.trycomp.ai');
});

it('should never include wildcard origin', () => {
const origins = getTrustedOriginsLogic(undefined);
expect(origins.every((o: string) => o !== '*' && o !== 'true')).toBe(true);
});

it('should trim whitespace from comma-separated origins', () => {
const origins = getTrustedOriginsLogic(' https://a.com , https://b.com ');
expect(origins).toEqual(['https://a.com', 'https://b.com']);
});

it('main.ts should use getTrustedOrigins instead of origin: true', () => {
// Validate the CORS config change was made correctly by checking file content
const fs = require('fs');
const path = require('path');
const mainTs = fs.readFileSync(
path.join(__dirname, '..', 'main.ts'),
'utf-8',
) as string;
expect(mainTs).not.toContain('origin: true');
expect(mainTs).toContain('origin: getTrustedOrigins()');
expect(mainTs).toContain("import { getTrustedOrigins } from './auth/auth.server'");
});
});
2 changes: 1 addition & 1 deletion apps/api/src/auth/auth.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function getCookieDomain(): string | undefined {
/**
* Get trusted origins for CORS/auth
*/
function getTrustedOrigins(): string[] {
export function getTrustedOrigins(): string[] {
const origins = process.env.AUTH_TRUSTED_ORIGINS;
if (origins) {
return origins.split(',').map((o) => o.trim());
Expand Down
151 changes: 151 additions & 0 deletions apps/api/src/auth/origin-check.middleware.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import { originCheckMiddleware } from './origin-check.middleware';

// Mock getTrustedOrigins
jest.mock('./auth.server', () => ({
getTrustedOrigins: () => [
'http://localhost:3000',
'http://localhost:3002',
'https://app.trycomp.ai',
'https://portal.trycomp.ai',
],
}));

function createMockReq(
method: string,
path: string,
origin?: string,
): Record<string, unknown> {
return {
method,
path,
headers: origin ? { origin } : {},
};
}

function createMockRes(): Record<string, unknown> & { statusCode?: number; body?: unknown } {
const res: Record<string, unknown> & { statusCode?: number; body?: unknown } = {};
res.status = jest.fn().mockImplementation((code: number) => {
res.statusCode = code;
return res;
});
res.json = jest.fn().mockImplementation((body: unknown) => {
res.body = body;
return res;
});
return res;
}

describe('originCheckMiddleware', () => {
it('should allow GET requests regardless of origin', () => {
const req = createMockReq('GET', '/v1/controls', 'http://evil.com');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).toHaveBeenCalled();
});

it('should allow HEAD requests regardless of origin', () => {
const req = createMockReq('HEAD', '/v1/health', 'http://evil.com');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).toHaveBeenCalled();
});

it('should allow OPTIONS requests regardless of origin', () => {
const req = createMockReq('OPTIONS', '/v1/controls', 'http://evil.com');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).toHaveBeenCalled();
});

it('should allow POST from trusted origin', () => {
const req = createMockReq('POST', '/v1/organization/api-keys', 'http://localhost:3000');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).toHaveBeenCalled();
});

it('should block POST from untrusted origin', () => {
const req = createMockReq('POST', '/v1/organization/transfer-ownership', 'http://evil.com');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(403);
});

it('should block DELETE from untrusted origin', () => {
const req = createMockReq('DELETE', '/v1/organization', 'http://evil.com');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(403);
});

it('should block PATCH from untrusted origin', () => {
const req = createMockReq('PATCH', '/v1/members/123/role', 'http://evil.com');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).not.toHaveBeenCalled();
expect(res.status).toHaveBeenCalledWith(403);
});

it('should allow POST without Origin header (API key / service token)', () => {
const req = createMockReq('POST', '/v1/controls', undefined);
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).toHaveBeenCalled();
});

it('should allow POST to /api/auth routes (better-auth exempt)', () => {
const req = createMockReq('POST', '/api/auth/sign-in', 'http://evil.com');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).toHaveBeenCalled();
});

it('should allow POST to health check', () => {
const req = createMockReq('POST', '/v1/health', 'http://evil.com');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).toHaveBeenCalled();
});

it('should allow production origins', () => {
const req = createMockReq('POST', '/v1/organization/api-keys', 'https://app.trycomp.ai');
const res = createMockRes();
const next = jest.fn();

originCheckMiddleware(req as any, res as any, next);

expect(next).toHaveBeenCalled();
});
});
65 changes: 65 additions & 0 deletions apps/api/src/auth/origin-check.middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import type { Request, Response, NextFunction } from 'express';
import { getTrustedOrigins } from './auth.server';

const SAFE_METHODS = new Set(['GET', 'HEAD', 'OPTIONS']);

/**
* Paths exempt from Origin validation (webhooks, public endpoints).
* These are called by external services that don't send browser Origin headers.
*/
const EXEMPT_PATH_PREFIXES = [
'/api/auth', // better-auth handles its own CSRF
'/v1/health', // health check
'/api/docs', // swagger
];

/**
* Express middleware that validates the Origin header on state-changing requests.
*
* This is defense-in-depth against CSRF attacks that bypass CORS:
* - HTML form submissions (Content-Type: application/x-www-form-urlencoded)
* don't trigger CORS preflight, so CORS alone doesn't block them.
* - This middleware rejects any state-changing request whose Origin header
* doesn't match a trusted origin.
*
* API keys and service tokens (which don't come from browsers) typically
* don't send an Origin header, so requests without an Origin are allowed
* — they'll be authenticated by HybridAuthGuard instead.
*/
export function originCheckMiddleware(
req: Request,
res: Response,
next: NextFunction,
): void {
// Allow safe (read-only) methods
if (SAFE_METHODS.has(req.method)) {
return next();
}

// Allow exempt paths (webhooks, auth, etc.)
const isExempt = EXEMPT_PATH_PREFIXES.some((prefix) =>
req.path.startsWith(prefix),
);
if (isExempt) {
return next();
}

const origin = req.headers['origin'] as string | undefined;

// No Origin header = not a browser request (API key, service token, curl, etc.)
// These are authenticated via HybridAuthGuard, not cookies, so no CSRF risk.
if (!origin) {
return next();
}

// Validate Origin against trusted origins
const trustedOrigins = getTrustedOrigins();
if (trustedOrigins.includes(origin)) {
return next();
}

res.status(403).json({
statusCode: 403,
message: 'Forbidden',
});
}
Loading
Loading