fix: COD-29 harden downloads and extract auth policy

This commit is contained in:
Aamer Akhter
2026-06-08 11:01:34 -04:00
parent dea015dc91
commit a36543c1b9
5 changed files with 71 additions and 27 deletions
+21
View File
@@ -0,0 +1,21 @@
import { isIP } from 'node:net';
const EXPLICIT_TRUE_VALUES = new Set(['1', 'true', 'yes', 'on']);
export function isExplicitlyEnabled(value: string | undefined): boolean {
return value !== undefined && EXPLICIT_TRUE_VALUES.has(value.trim().toLowerCase());
}
export function isLoopbackBindHost(host: string): boolean {
const normalized = host
.trim()
.toLowerCase()
.replace(/^\[(.*)\]$/, '$1');
if (normalized === 'localhost' || normalized === '::1' || normalized === '0:0:0:0:0:0:0:1') {
return true;
}
if (isIP(normalized) === 4 && normalized.startsWith('127.')) {
return true;
}
return normalized.startsWith('::ffff:127.');
}
+4 -3
View File
@@ -279,7 +279,6 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort): void
jpeg: 'image/jpeg',
gif: 'image/gif',
webp: 'image/webp',
svg: 'image/svg+xml',
ico: 'image/x-icon',
bmp: 'image/bmp',
mp4: 'video/mp4',
@@ -293,19 +292,21 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort): void
};
const content = await fs.readFile(resolvedPath);
if (download === 'true') {
const rawBasename = filePath!.split('/').pop() || 'download';
// Sanitize filename for Content-Disposition header (prevent header injection)
const basename = rawBasename.replace(/["\\\r\n]/g, '_');
if (download === 'true' || ext === 'svg') {
reply.raw.writeHead(200, {
'Content-Type': mimeTypes[ext] || 'application/octet-stream',
'Content-Type': ext === 'svg' ? 'application/octet-stream' : mimeTypes[ext] || 'application/octet-stream',
'Content-Disposition': `attachment; filename="${basename}"`,
'Content-Length': content.length,
'X-Content-Type-Options': 'nosniff',
});
reply.raw.end(content);
return;
}
reply.header('Content-Type', mimeTypes[ext] || 'application/octet-stream');
reply.header('X-Content-Type-Options', 'nosniff');
reply.send(content);
} catch (err) {
reply
+1 -21
View File
@@ -42,7 +42,6 @@ import { execSync } from 'node:child_process';
import { hostname as getHostname } from 'node:os';
import { dataPath } from '../config/instance.js';
import { EventEmitter } from 'node:events';
import { isIP } from 'node:net';
import { Session, type BackgroundTask } from '../session.js';
import type { ClaudeMode, SessionState } from '../types.js';
import { RespawnController, RespawnConfig } from '../respawn-controller.js';
@@ -103,6 +102,7 @@ import { SseEvent } from './sse-events.js';
import type { ScheduledRun } from './ports/index.js';
import { registerAuthMiddleware, registerSecurityHeaders } from './middleware/auth.js';
import { installRouteErrorHandler } from './route-error-handler.js';
import { isExplicitlyEnabled, isLoopbackBindHost } from './network-auth-policy.js';
import {
registerPushRoutes,
registerTeamRoutes,
@@ -123,26 +123,6 @@ import {
const __dirname = dirname(fileURLToPath(import.meta.url));
const EXPLICIT_TRUE_VALUES = new Set(['1', 'true', 'yes', 'on']);
function isExplicitlyEnabled(value: string | undefined): boolean {
return value !== undefined && EXPLICIT_TRUE_VALUES.has(value.trim().toLowerCase());
}
function isLoopbackBindHost(host: string): boolean {
const normalized = host
.trim()
.toLowerCase()
.replace(/^\[(.*)\]$/, '$1');
if (normalized === 'localhost' || normalized === '::1' || normalized === '0:0:0:0:0:0:0:1') {
return true;
}
if (isIP(normalized) === 4 && normalized.startsWith('127.')) {
return true;
}
return normalized.startsWith('::ffff:127.');
}
// Bounded, predictable shape for SSE client identifiers: alphanumerics, `_`, `-`.
// Length range covers crypto.randomUUID() (36 chars) plus any short stable IDs,
// while capping growth of `sseClientsById` and blocking pathological inputs.
+26
View File
@@ -0,0 +1,26 @@
import { describe, expect, it } from 'vitest';
import { isExplicitlyEnabled, isLoopbackBindHost } from '../src/web/network-auth-policy.js';
describe('network auth policy', () => {
it.each(['localhost', '127.0.0.1', '127.42.0.9', '::1', '[::1]', '0:0:0:0:0:0:0:1', '::ffff:127.0.0.1'])(
'treats %s as loopback',
(host) => {
expect(isLoopbackBindHost(host)).toBe(true);
}
);
it.each(['0.0.0.0', '192.168.1.10', '10.0.0.1', 'example.com', '::', '[::]', '::ffff:192.168.1.10'])(
'treats %s as non-loopback',
(host) => {
expect(isLoopbackBindHost(host)).toBe(false);
}
);
it.each(['1', 'true', 'TRUE', ' yes ', 'on'])('treats %s as an explicit opt-in', (value) => {
expect(isExplicitlyEnabled(value)).toBe(true);
});
it.each([undefined, '', '0', 'false', 'no', 'off', 'enabled'])('does not treat %s as an explicit opt-in', (value) => {
expect(isExplicitlyEnabled(value)).toBe(false);
});
});
+16
View File
@@ -305,6 +305,22 @@ describe('file-routes', () => {
expect(res.headers['content-type']).toBe('image/png');
});
it('serves workspace SVG as an untrusted attachment instead of inline image/svg+xml', async () => {
const content = Buffer.from('<svg><script>alert("xss")</script></svg>');
mockedReadFile.mockResolvedValue(content as never);
mockedStat.mockResolvedValue({ size: content.length } as never);
const res = await harness.app.inject({
method: 'GET',
url: `/api/sessions/${harness.ctx._sessionId}/file-raw?path=malicious.svg`,
});
expect(res.statusCode).toBe(200);
expect(res.headers['content-type']).toBe('application/octet-stream');
expect(res.headers['content-disposition']).toContain('attachment; filename="malicious.svg"');
expect(res.headers['x-content-type-options']).toBe('nosniff');
});
it('rejects path traversal in raw file serving', async () => {
mockedRealpathSync.mockReturnValue('/etc/shadow' as never);