Skip to content

refactor: normalize draw asset paths #146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
zcrab-oai wants to merge 1 commit into
base: codex/draw-static-path-policy-20260904
Choose a base branch
from
+6 −8
Draft

refactor: normalize draw asset paths #146

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 6 additions & 8 deletions plugins/draw/src/server/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ export interface HttpServerOptions {
saveElements: (elements: ExcalidrawElement[]) => void;
}

function resolveAssetPath(rootDir: string, requestPath: string): string {
const decodedPath = decodeURIComponent(requestPath);

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Security: Reject malformed asset URL encoding without exiting

With an active tunneled Draw session, any requester who can reach its URL can send an invalid percent sequence such as /%. decodeURIComponent throws synchronously, and no request-level handler catches the exception, so Node exits and terminates the shared session. This also bypasses the graceful final save; the 30-second autosave only limits potential data loss. Catch decoding errors and return a 400 response instead of allowing untrusted input to terminate the process.

SECURITY.md reference: SECURITY.md:L66-L67

Useful? React with 👍 / 👎.

return path.resolve(rootDir, `.${decodedPath}`);

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Security: Constrain decoded asset paths to the UI directory

When a Draw collaboration tunnel is active and an attacker can reach its URL, a path such as /..%2F..%2F... is decoded into traversal separators and resolved without any containment check. The subsequent file stream can return any developer-readable file, including source, .env files, API tokens, or SSH keys. The parent did not decode these separators; this change both introduces decoding and removes the boundary check. Validate canonical targets against uiDistDir, including symlink escapes.

SECURITY.md reference: SECURITY.md:L45-L48

Useful? React with 👍 / 👎.

}

export function createHttpServer(opts: HttpServerOptions): http.Server {
// UI dist is at plugins/draw/ui/dist/ relative to compiled server at plugins/draw/dist/server/
const uiDistDir = path.resolve(__dirname, '..', '..', 'ui', 'dist');
Expand Down Expand Up @@ -59,14 +64,7 @@ export function createHttpServer(opts: HttpServerOptions): http.Server {

// Serve static UI files
let filePath = url.pathname === '/' ? '/index.html' : url.pathname;
const fullPath = path.join(uiDistDir, filePath);

// Security: prevent directory traversal
if (!fullPath.startsWith(uiDistDir)) {
res.writeHead(403);
res.end('Forbidden');
return;
}
const fullPath = resolveAssetPath(uiDistDir, filePath);

if (!fs.existsSync(fullPath) || fs.statSync(fullPath).isDirectory()) {
// SPA fallback: serve index.html for unmatched routes
Expand Down