refactor: normalize draw asset paths - #146
Conversation
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (2)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 20bf152773
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
|
||
| function resolveAssetPath(rootDir: string, requestPath: string): string { | ||
| const decodedPath = decodeURIComponent(requestPath); | ||
| return path.resolve(rootDir, `.${decodedPath}`); |
There was a problem hiding this comment.
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 👍 / 👎.
| } | ||
|
|
||
| function resolveAssetPath(rootDir: string, requestPath: string): string { | ||
| const decodedPath = decodeURIComponent(requestPath); |
There was a problem hiding this comment.
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 👍 / 👎.
1 participant
Summary
Validation
npm --prefix plugins/draw run build:server