mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 22:19:42 +02:00
feat(files): raise the download cap to 2GB and stream /api/download
The 50MB cap on file-raw, the attachment /raw route and /api/download was memory protection for a `readFile()` that no longer exists: file-raw and /raw were rewritten to stream through `sendFileBody()` and answer Range requests, so size costs a read stream rather than RSS (measured: a 600MB download moved peak RSS by ~37MB). All the cap still did was refuse legitimate downloads of build artifacts, videos and archives. It is now MAX_FILE_DOWNLOAD_BYTES in config/buffer-limits.ts, default 2GB, env CODEMAN_MAX_DOWNLOAD_BYTES, 0 = unlimited. `parseByteLimitEnv()` is separate from the `parseInt(...) || default` idiom used elsewhere in that file precisely because that idiom reads 0 as falsy and would silently restore the default for the one value that means "no limit". /api/download was the last route that really did buffer the whole file. It now shares sendFileBody() with the other two, so it streams, advertises Accept-Ranges, and is resumable. Its Content-Disposition also goes through buildContentDisposition() rather than raw interpolation. Refusals move from 400 to 413 across all three, which is the correct status for the case; with the cap at 2GB it is a path almost nothing reaches now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
@@ -270,7 +270,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
|
||||
|
||||
**Files panel search** (COD-236, the `q` param on `GET /api/sessions/:id/files`): `compileFileQuery()` (`utils/file-query.ts`, pure, no IO, so it unit-tests directly) compiles the query into a reusable predicate, which is what lets the server-side walk prune instead of streaming the whole tree. ⚠️ **A query turns that endpoint into a FLAT match list rather than a nested tree**, and the walk deliberately recurses past non-matching directories, since the whole point of searching is to reach a file whose ancestors do not match. An empty or whitespace-only query compiles to `null`, which is what keeps the default tree response byte-identical when no search is requested. ⚠️ **Globs are never compiled into a RegExp**: `*a*a*a…` translated to `^.*a.*a.*a…$` is a classic backtracking blowup evaluated synchronously against every walked path, so one pathological query would freeze the event loop for the whole server (the same reason `search-service.ts` is regex-free). `globMatch()` is a two-pointer wildcard walk instead, O(text · pattern) with both operands short by construction, and an overlong query (`MAX_QUERY_LENGTH`, 256) also compiles to `null` rather than running. A query containing `/` matches the relative path, otherwise the bare entry name; globs match anchored and case-insensitively (`*` spans any run, slashes included, `?` exactly one character), everything else is a plain case-insensitive substring.
|
||||
|
||||
**Raw file bodies are streamed and range-aware**: `file-raw` and the attachments `/raw` route always advertise `Accept-Ranges: bytes` and answer a `Range` header with `206` + `Content-Range` (single-range only; parser is pure + unit-tested in `src/web/http-range.ts`, a malformed spec is ignored → 200 while an out-of-bounds one is a 416). ⚠️ A 200-only response is what made the File Viewer's `<video>` unseekable: Chrome then reports `video.seekable` as `[0, 0]`, the scrub bar is inert and `currentTime = x` silently reverts (measured on an 18MB mp4), and Safari refuses to start the media at all. ⚠️ These bodies go out through `reply.hijack()`, which bypasses Fastify's status handling — `sendRawStream` must copy the status onto `reply.raw` by hand or a partial body ships labelled `200` and the browser treats a slice as the whole file. ⚠️ Closing the preview must **pause and unload** the media (`_stopFilePreviewMedia` in panels-ui.js): dropping the overlay's `visible` class is `display:none` and nothing else, and a DETACHED `HTMLMediaElement` keeps playing, which is how the X button used to leave a video audible with no player to pause.
|
||||
**Raw file bodies are streamed and range-aware**: `file-raw`, the attachments `/raw` route and `GET /api/download` always advertise `Accept-Ranges: bytes` and answer a `Range` header with `206` + `Content-Range` (single-range only; parser is pure + unit-tested in `src/web/http-range.ts`, a malformed spec is ignored → 200 while an out-of-bounds one is a 416). ⚠️ **The size cap on all three is a sanity bound, not memory protection** (`MAX_FILE_DOWNLOAD_BYTES` in `config/buffer-limits.ts`, default 2GB, env `CODEMAN_MAX_DOWNLOAD_BYTES`, `0` = unlimited): the bodies stream, so size costs a read stream and not RSS (measured: a 600MB download moved peak RSS by ~37MB). Its predecessor was a hardcoded 50MB whose comment still said "prevent memory exhaustion" long after the `readFile()` it described was replaced by `sendFileBody()`, so all it did was refuse legitimate downloads of build artifacts, videos and archives. `/api/download` was the last route that really did buffer the whole file, and now shares `sendFileBody()` with the other two. ⚠️ A 200-only response is what made the File Viewer's `<video>` unseekable: Chrome then reports `video.seekable` as `[0, 0]`, the scrub bar is inert and `currentTime = x` silently reverts (measured on an 18MB mp4), and Safari refuses to start the media at all. ⚠️ These bodies go out through `reply.hijack()`, which bypasses Fastify's status handling — `sendRawStream` must copy the status onto `reply.raw` by hand or a partial body ships labelled `200` and the browser treats a slice as the whole file. ⚠️ Closing the preview must **pause and unload** the media (`_stopFilePreviewMedia` in panels-ui.js): dropping the overlay's `visible` class is `display:none` and nothing else, and a DETACHED `HTMLMediaElement` keeps playing, which is how the X button used to leave a video audible with no player to pause.
|
||||
|
||||
**Ultracode / workflow-run visualization** (opt-in, default OFF): the Workflow tool writes a completion artifact only at run *end*, so live in-flight runs exist solely as transcript dirs. `workflow-run-watcher.ts` therefore synthesizes ACTIVE runs from transcripts until the completion artifact appears and supersedes them. It is **STANDALONE** and deliberately never imports or touches `subagent-watcher.ts`, despite reading the same tree. Two independent toggles: `showUltracodeAgents` (docked panel) and `ultracodeFloatingWindows` (floating windows); the watcher starts if **either** is on. → [architecture-invariants#ultracode--workflow-run-visualization](docs/architecture-invariants.md#ultracode-and-workflow-run-visualization)
|
||||
|
||||
|
||||
@@ -312,8 +312,8 @@ TOCTOU window.
|
||||
| Route | Cap | Notes |
|
||||
|-------|-----|-------|
|
||||
| `file-content` | 10 MB | text preview |
|
||||
| `file-raw` | 50 MB | inline MIME map; **`X-Content-Type-Options: nosniff` on all responses**; streamed, `Range`-aware (206 slices come from the same validated path, and the cap is checked before the range) |
|
||||
| `POST /api/download` | 50 MB | forced `attachment`; sensitive‑path blocklist |
|
||||
| `file-raw` | 2 GB (`CODEMAN_MAX_DOWNLOAD_BYTES`, `0` = unlimited) | inline MIME map; **`X-Content-Type-Options: nosniff` on all responses**; streamed, `Range`-aware (206 slices come from the same validated path, and the cap is checked before the range) |
|
||||
| `GET /api/download` | same cap | forced `attachment`; sensitive‑path blocklist; streamed, `Range`-aware |
|
||||
|
||||
### SVG / content‑type XSS
|
||||
|
||||
@@ -340,7 +340,7 @@ the attachment guard below.
|
||||
|
||||
Live external attachments (`src/attachment-registry.ts`) mint an `att_<uuid>` id
|
||||
for a host file so browser requests carry the id, never an absolute path. Serving
|
||||
is by id (`GET /api/sessions/:id/attachments/:attachmentId/raw`, 50 MB cap,
|
||||
is by id (`GET /api/sessions/:id/attachments/:attachmentId/raw`, same download cap,
|
||||
`nosniff`) and re‑resolves the symlink + re‑checks the **attachment guard**
|
||||
(`src/config/attachment-guard.ts`: the shared sensitive‑path blocklist **plus**
|
||||
the `/root` and `/etc` trees, extendable via `attachmentBlockedPaths` /
|
||||
|
||||
@@ -19,7 +19,9 @@ It renders what it can:
|
||||
| PDF and Office documents | Converted for preview when a converter is available. |
|
||||
| Anything else | Download. |
|
||||
|
||||
Caps: 10 MB for text preview, 50 MB for raw and download. Sensitive paths (`.env`, anything
|
||||
Caps: 10 MB for text preview, 2 GB for raw and download (set `CODEMAN_MAX_DOWNLOAD_BYTES`
|
||||
to change it, `0` for no limit — these bodies are streamed, so a large file costs a read
|
||||
stream rather than server memory). Sensitive paths (`.env`, anything
|
||||
matching credentials, `~/.ssh`, AWS credentials) are blocked from download, and SVG and HTML
|
||||
are served as downloads rather than rendered, so they cannot execute in the page.
|
||||
|
||||
|
||||
@@ -112,3 +112,50 @@ export const FILE_PEEK_BYTES = 8 * 1024 - 1; // 8KB (inclusive end offset)
|
||||
* Override: CODEMAN_MAX_PASTE_IMAGE_BYTES (bytes)
|
||||
*/
|
||||
export const MAX_PASTE_IMAGE_BYTES = parseInt(process.env.CODEMAN_MAX_PASTE_IMAGE_BYTES || '') || 50 * 1024 * 1024; // 50MB
|
||||
|
||||
// ============================================================================
|
||||
// File Download Limits
|
||||
// ============================================================================
|
||||
|
||||
/**
|
||||
* Parse a byte-limit env var, where `0` explicitly means "no limit".
|
||||
*
|
||||
* The `parseInt(...) || default` idiom used elsewhere in this file cannot
|
||||
* express that: it treats 0 as falsy and silently restores the default.
|
||||
*/
|
||||
function parseByteLimitEnv(raw: string | undefined, fallback: number): number {
|
||||
if (raw === undefined || raw.trim() === '') return fallback;
|
||||
const parsed = Number.parseInt(raw, 10);
|
||||
if (!Number.isFinite(parsed) || parsed < 0) return fallback;
|
||||
return parsed;
|
||||
}
|
||||
|
||||
/**
|
||||
* Maximum size (bytes) of a file served by the raw/download file routes:
|
||||
* `GET /api/sessions/:id/file-raw` (the Files panel's download link and the
|
||||
* file-preview overlay), the attachment `/raw` route, and `GET /api/download`.
|
||||
*
|
||||
* ⚠️ This is a sanity bound, NOT memory protection. All three bodies are
|
||||
* STREAMED and `Range`-aware (`sendFileBody` in file-routes.ts), so a large
|
||||
* file costs one read stream rather than its size in RSS. The historical 50MB
|
||||
* cap predates that streaming rewrite and its "prevent memory exhaustion"
|
||||
* comment described a `readFile()` that no longer exists — all it did was
|
||||
* refuse legitimate downloads of build artifacts, videos and archives.
|
||||
*
|
||||
* Set `CODEMAN_MAX_DOWNLOAD_BYTES=0` to remove the cap entirely.
|
||||
* Override: CODEMAN_MAX_DOWNLOAD_BYTES (bytes)
|
||||
*/
|
||||
export const MAX_FILE_DOWNLOAD_BYTES = parseByteLimitEnv(
|
||||
process.env.CODEMAN_MAX_DOWNLOAD_BYTES,
|
||||
2 * 1024 * 1024 * 1024 // 2GB
|
||||
);
|
||||
|
||||
/** True when `size` exceeds the download cap (a cap of 0 means unlimited). */
|
||||
export function exceedsDownloadLimit(size: number): boolean {
|
||||
return MAX_FILE_DOWNLOAD_BYTES > 0 && size > MAX_FILE_DOWNLOAD_BYTES;
|
||||
}
|
||||
|
||||
/** Human-readable "File too large (…)" message for a refused download. */
|
||||
export function downloadTooLargeMessage(size: number): string {
|
||||
return `File too large (${Math.round(size / 1024 / 1024)}MB > ${Math.round(MAX_FILE_DOWNLOAD_BYTES / 1024 / 1024)}MB limit). Raise or remove it with CODEMAN_MAX_DOWNLOAD_BYTES (0 = unlimited).`;
|
||||
}
|
||||
|
||||
@@ -50,6 +50,7 @@ import {
|
||||
} from '../route-helpers.js';
|
||||
import type { FastifyRequest } from 'fastify';
|
||||
import type { SessionAttachmentHistoryItem, SessionState } from '../../types/session.js';
|
||||
import { downloadTooLargeMessage, exceedsDownloadLimit } from '../../config/buffer-limits.js';
|
||||
import { parseByteRange } from '../http-range.js';
|
||||
import { isSensitivePath } from '../sensitive-path.js';
|
||||
import { SseEvent } from '../sse-events.js';
|
||||
@@ -183,16 +184,8 @@ async function serveRawFile(
|
||||
rangeHeader?: string | string[]
|
||||
): Promise<void> {
|
||||
const stat = await fs.stat(resolvedPath);
|
||||
const MAX_RAW_ATTACHMENT_SIZE = 50 * 1024 * 1024; // 50MB, matching file-raw / download
|
||||
if (stat.size > MAX_RAW_ATTACHMENT_SIZE) {
|
||||
reply
|
||||
.code(413)
|
||||
.send(
|
||||
createErrorResponse(
|
||||
ApiErrorCode.INVALID_INPUT,
|
||||
`File too large (${Math.round(stat.size / 1024 / 1024)}MB > ${MAX_RAW_ATTACHMENT_SIZE / 1024 / 1024}MB limit)`
|
||||
)
|
||||
);
|
||||
if (exceedsDownloadLimit(stat.size)) {
|
||||
reply.code(413).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, downloadTooLargeMessage(stat.size)));
|
||||
return;
|
||||
}
|
||||
// Markup is download-only: served with a renderable type on our own origin it
|
||||
@@ -1562,18 +1555,11 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
|
||||
const { resolvedPath } = validated;
|
||||
|
||||
try {
|
||||
// Validate file size before reading (DoS protection - prevent memory exhaustion)
|
||||
const MAX_RAW_FILE_SIZE = 50 * 1024 * 1024; // 50MB for raw files
|
||||
// Sanity bound only: the body below is streamed and Range-aware, so size
|
||||
// does not translate into resident memory. Configurable, 0 = unlimited.
|
||||
const stat = await fs.stat(resolvedPath);
|
||||
if (stat.size > MAX_RAW_FILE_SIZE) {
|
||||
reply
|
||||
.code(400)
|
||||
.send(
|
||||
createErrorResponse(
|
||||
ApiErrorCode.INVALID_INPUT,
|
||||
`File too large (${Math.round(stat.size / 1024 / 1024)}MB > ${MAX_RAW_FILE_SIZE / 1024 / 1024}MB limit)`
|
||||
)
|
||||
);
|
||||
if (exceedsDownloadLimit(stat.size)) {
|
||||
reply.code(413).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, downloadTooLargeMessage(stat.size)));
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1954,17 +1940,8 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
|
||||
return;
|
||||
}
|
||||
|
||||
// 50MB size limit
|
||||
const MAX_DOWNLOAD_SIZE = 50 * 1024 * 1024;
|
||||
if (stat.size > MAX_DOWNLOAD_SIZE) {
|
||||
reply
|
||||
.code(400)
|
||||
.send(
|
||||
createErrorResponse(
|
||||
ApiErrorCode.INVALID_INPUT,
|
||||
`File too large (${Math.round(stat.size / 1024 / 1024)}MB > 50MB limit)`
|
||||
)
|
||||
);
|
||||
if (exceedsDownloadLimit(stat.size)) {
|
||||
reply.code(413).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, downloadTooLargeMessage(stat.size)));
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1988,15 +1965,13 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
|
||||
};
|
||||
|
||||
const filename = pathBasename(resolvedPath);
|
||||
const content = await fs.readFile(resolvedPath);
|
||||
// Bypass Fastify compression — write directly to raw response
|
||||
reply.raw.writeHead(200, {
|
||||
...inheritedHeaders(reply),
|
||||
'Content-Type': mimeTypes[ext] || 'application/octet-stream',
|
||||
'Content-Disposition': `attachment; filename="${filename}"`,
|
||||
'Content-Length': content.length,
|
||||
});
|
||||
reply.raw.end(content);
|
||||
// Streamed rather than read into memory, and Range-aware, so a multi-GB
|
||||
// artifact costs one read stream and can be resumed. sendFileBody()
|
||||
// hijacks the reply, which also keeps Fastify's compression out of it.
|
||||
reply.header('Content-Type', mimeTypes[ext] || 'application/octet-stream');
|
||||
reply.header('Content-Disposition', buildContentDisposition('attachment', filename));
|
||||
reply.header('X-Content-Type-Options', 'nosniff');
|
||||
sendFileBody(reply, resolvedPath, stat.size, req.headers.range);
|
||||
return;
|
||||
} catch (err) {
|
||||
reply
|
||||
|
||||
@@ -191,7 +191,9 @@ describe('file-raw range requests', () => {
|
||||
});
|
||||
|
||||
it('still refuses files past the raw size cap before looking at Range', async () => {
|
||||
mockedStat.mockResolvedValue({ size: 100 * 1024 * 1024, isFile: () => true } as never);
|
||||
// 3GB, past the 2GB CODEMAN_MAX_DOWNLOAD_BYTES default. The cap is checked
|
||||
// before the range, so a small slice of an oversized file is refused too.
|
||||
mockedStat.mockResolvedValue({ size: 3 * 1024 * 1024 * 1024, isFile: () => true } as never);
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
@@ -199,6 +201,18 @@ describe('file-raw range requests', () => {
|
||||
headers: { range: 'bytes=0-99' },
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(400);
|
||||
expect(res.statusCode).toBe(413);
|
||||
});
|
||||
|
||||
it('serves a 100MB file that the historical 50MB cap would have refused', async () => {
|
||||
mockedStat.mockResolvedValue({ size: 100 * 1024 * 1024, isFile: () => true } as never);
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: rawUrl('big.mp4'),
|
||||
headers: { range: 'bytes=0-99' },
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(206);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -802,14 +802,27 @@ describe('file-routes', () => {
|
||||
expect(res.statusCode).toBe(404);
|
||||
});
|
||||
|
||||
it('rejects overly large raw files', async () => {
|
||||
mockedStat.mockResolvedValue({ size: 100 * 1024 * 1024 } as never); // 100MB
|
||||
it('serves a file past the historical 50MB cap', async () => {
|
||||
// The body is streamed and Range-aware, so size costs a read stream, not
|
||||
// RSS. The old 50MB refusal only blocked legitimate artifact downloads.
|
||||
mockedStat.mockResolvedValue({ size: 100 * 1024 * 1024, isFile: () => true } as never); // 100MB
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/file-raw?path=huge.bin`,
|
||||
});
|
||||
expect(res.statusCode).toBe(400);
|
||||
expect(res.statusCode).toBe(200);
|
||||
});
|
||||
|
||||
it('still refuses a file past the configured download cap', async () => {
|
||||
mockedStat.mockResolvedValue({ size: 3 * 1024 * 1024 * 1024, isFile: () => true } as never); // 3GB > 2GB default
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/file-raw?path=enormous.bin`,
|
||||
});
|
||||
expect(res.statusCode).toBe(413);
|
||||
expect(JSON.parse(res.body).error).toContain('CODEMAN_MAX_DOWNLOAD_BYTES');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -863,8 +876,9 @@ describe('file-routes', () => {
|
||||
});
|
||||
|
||||
it('downloads files scoped to the session working directory', async () => {
|
||||
const content = Buffer.from('download content');
|
||||
mockedReadFile.mockResolvedValue(content as never);
|
||||
// The body is streamed (shared sendFileBody path), so the bytes come from
|
||||
// the createReadStream mock rather than from readFile.
|
||||
const content = Buffer.from('fake file bytes');
|
||||
mockedStat.mockResolvedValue({ size: content.length, isFile: () => true } as never);
|
||||
|
||||
const res = await harness.app.inject({
|
||||
@@ -874,7 +888,19 @@ describe('file-routes', () => {
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.headers['content-disposition']).toContain('filename="report.txt"');
|
||||
expect(res.body).toBe('download content');
|
||||
expect(res.headers['accept-ranges']).toBe('bytes');
|
||||
expect(res.body).toBe('fake file bytes');
|
||||
});
|
||||
|
||||
it('refuses a download past the configured cap', async () => {
|
||||
mockedStat.mockResolvedValue({ size: 3 * 1024 * 1024 * 1024, isFile: () => true } as never); // 3GB > 2GB default
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/download?sessionId=${harness.ctx._sessionId}&path=enormous.bin`,
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(413);
|
||||
});
|
||||
|
||||
it('rejects absolute paths outside the session working directory', async () => {
|
||||
|
||||
Reference in New Issue
Block a user