Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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'");
});
});
10 changes: 9 additions & 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 Expand Up @@ -150,6 +150,14 @@ export const auth = betterAuth({
database: {
generateId: false,
},
// Prevent cookie collisions between environments.
// Production keeps the default 'better-auth' prefix (unchanged).
...(cookieDomain === '.staging.trycomp.ai' && {
cookiePrefix: 'staging',
}),
...(!cookieDomain && {
cookiePrefix: 'local',
}),
...(cookieDomain && {
crossSubDomainCookies: {
enabled: true,
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