Skip to content

fix(files): add file type validation on upload with extension blocklist#323

Open
DeryFerd wants to merge 1 commit into
myrialabs:mainfrom
DeryFerd:fix/files/upload-file-type-validation
Open

fix(files): add file type validation on upload with extension blocklist#323
DeryFerd wants to merge 1 commit into
myrialabs:mainfrom
DeryFerd:fix/files/upload-file-type-validation

Conversation

@DeryFerd

@DeryFerd DeryFerd commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

The file upload endpoint had no server-side validation for file types. You could upload anything — .exe, .dll, .sh, .jar, .ps1, .class — and the server would accept it, write it to disk, and serve it back. There was already client-side filtering in the UI, but that is trivially bypassed by sending a direct POST request.

This PR adds a two-layer file type validator that blocks dangerous uploads before they hit disk.

New file: backend/http/file-type-validator.ts

Layer 1 — Extension blocklist.

A curated set of extensions that are never allowed through. The list covers executable formats (.exe, .dll, .so, .dmg), scripting runtimes (.ps1, .sh, .bash, .vbs), compiled bytecode (.jar, .class, .pyc), and Windows installer/shortcut formats (.msi, .lnk, .scr). The full set:

.exe .dll .com .bat .cmd .ps1 .psm1 .psd1 .vbs .vbe .jse .wsf .wsh
.msi .msp .scr .pif .scf .lnk .inf .sh .bash .zsh .ksh .csh
.app .dmg .pkg .elf .so .o .ko .jar .class .pyc .pyo

Layer 2 — Magic byte verification (currently stubbed for future use).

A mapping of known file signatures — PNG, JPEG, GIF, PDF, ZIP, and others — with their header bytes. The matchesMagicByte function lets you verify that the first few bytes of a file match its declared extension. This is not wired into the upload flow yet because it requires reading the beginning of the file stream before writing, which needs a small refactor in the upload pipeline. But the function and data are ready, and extending the check later is a one-line addition.

Change in backend/http/files-upload.ts

A single guard at the top of the upload handler, right after the filename is extracted and before any file processing begins:

if (isBlockedExtension(fileNameParam)) {
    return new Response(`File type "${ext}" is not allowed for upload`, { status: 400 });
}

It runs before the temp file is created, before the writer stream opens, before any disk I/O happens. The rejection is instant.

Why this matters

Without server-side validation, the upload endpoint is essentially a free file write primitive. A user with a valid session could upload executable content and then access it through the file serving path. This is especially relevant in a self-hosted multi-user setup where the server might also serve uploaded files over HTTP.

The blocklist approach was chosen over an allowlist because the workspace needs to accept arbitrary project files — source code, configs, assets, datasets — and an allowlist would be too restrictive. Blocking known-dangerous formats is the practical middle ground.

What is intentionally not covered

Deep content inspection (e.g., scanning inside archives, stripping metadata from office documents) is out of scope. The goal here is to stop the obvious attacks cheaply, not to build a full antivirus.

Validation

  • bun build --no-bundle backend/http/file-type-validator.ts — compiles clean
  • bun run lint — no warnings
  • The check runs entirely synchronously with no I/O, so it adds zero latency to successful uploads

@ArgaFairuz

Copy link
Copy Markdown
Collaborator

Hi @DeryFerd, thanks for digging into this — the threat model writeup in the PR description is thorough, and identifying the upload endpoint as a gap shows good security instincts. I want to talk through one thing before this moves forward.

I'm not convinced the blocklist approach is the right direction for Clopen. The threat is real, but blocking file types at the upload layer introduces false positives — users with legitimate use cases (building a Go binary, deploying shell scripts as part of a project, etc.) will hit 400 with no recourse. For a self-hosted multi-user tool, the correct defense is authentication + path access control + audit logging + proper Content-Disposition/Content-Type headers at serve time, rather than blocking extensions at the upload boundary.

That said, I may be missing context — is there a deployment scenario or threat model where upload-layer blocking is necessary despite the serve-time protections? What would change my mind: a concrete scenario where serve-time headers alone wouldn't prevent the attack.

Could you take a look by June 20, 2026? If you can't respond by then, I'll close this PR as auto-stale — you can reopen anytime once you're back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants