Security: harden local runtime and document handling#1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ded5e9512d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await proxyBuffered({ request, response, socketPath, internalToken, url, body }); | ||
| return; | ||
| } | ||
| await proxyBuffered({ request, response, socketPath, internalToken, url, body: Buffer.alloc(0) }); |
There was a problem hiding this comment.
For browser/Tauri dev via npm run serve, the page's <script src="./app-config.js"> reaches this catch-all proxy. The upstream createLocalServer special-cases /app-config.js, so the response is not the new bootstrap wrapper in public/app-config.js; it sets SCHEMA_DOCS_API_BASE_URL to http://schema-docs-internal and exposes the private socket token, after which ensureLocalApiConfig() treats config as present and all API calls go to an unresolvable host. Handle /app-config.js specially before this proxy path, e.g. by serving the static bootstrap script or a public-safe config.
Useful? React with 👍 / 👎.
| throw new HttpSecurityError(403, "external_markdown_route_disabled", "Direct external Markdown access is disabled. Import the file into a workspace first."); | ||
| } | ||
| if (OUTPUT_PATH_ROUTES.has(route) || /^\/api\/export\/(?:md|docx|pdf|html)$/.test(route)) { | ||
| if (!isSafeWorkspaceRelativePath(body.outputRelativePath)) { |
There was a problem hiding this comment.
This blanket workspace-relative check breaks the existing desktop save/export flows when a user uses the native picker: saveCurrentNoteWithDialog() and the Word/PDF/HTML export pickers populate outputRelativePath with an absolute path, then call /api/markdown/export, so the request now fails with 400 unsafe_output_path even for a user-selected destination. Either convert in-workspace absolute selections to relative/update the UI to stop offering external destinations, or keep a safe external export path that matches the picker behavior.
Useful? React with 👍 / 👎.
|
|
||
| export const HTTP_SECURITY_LIMITS = Object.freeze({ | ||
| jsonBytes: 8 * 1024 * 1024, | ||
| uploadBytes: 256 * 1024 * 1024 |
There was a problem hiding this comment.
With this new 256MB proxy limit, drag-and-drop uploads for documents between 256MB and the UI's advertised 1024MB limit pass the client check in public/importUploadPanel.js but are rejected by the secure proxy with 413 request_too_large. Either lower the client-side document limit/message to match this server cap or keep the proxy limit at the same size the upload UI accepts.
Useful? React with 👍 / 👎.
1 participant
Summary
This PR fixes all vulnerabilities confirmed during the security review of v0.1.1:
script-src 'unsafe-inline'from the Tauri CSPSecurity behavior change
Direct external Markdown read/write and API-driven absolute-path export are now denied. Files must be imported into a workspace and all server-side output paths must remain workspace-relative. This intentionally removes the unsafe arbitrary-path capability.
Validation
Executed locally on Node.js 22.16.0:
Result: 10 passed, 0 failed.
The checks cover traversal without directory side effects, valid workspace writes, symlink-parent escape, normal ZIP parsing, ZIP compression bombs, per-entry/total ZIP limits, trusted loopback contexts, unsafe export paths, disabled external Markdown routes, and declared/streamed request-size limits.
Also executed syntax checks for every modified JavaScript file and a dynamic secure-proxy integration test covering:
Review notes
npm testdiscovery so the repository's audited public-preview test-count metadata remains unchanged; run it with the explicit command above.