feat(draw): add session file preview endpoint [DO NOT MERGE] - #138
feat(draw): add session file preview endpoint [DO NOT MERGE]#138zcrab-oai wants to merge 1 commit into
Conversation
|
@codex review |
|
@codex security review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ 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 Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 358b4b167e
ℹ️ 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".
| } | ||
|
|
||
| res.writeHead(200, { 'Content-Type': 'text/plain; charset=utf-8' }); | ||
| res.end(fs.readFileSync(requestedPath, 'utf-8')); |
There was a problem hiding this comment.
When the drawing server is exposed through the collaboration tunnel (as done by startSession in plugins/draw/src/commands/start.ts), anyone who can reach it can request an arbitrary absolute path such as /etc/passwd, an SSH key, or a repository secret and receive its contents. Resolve and validate the path against an allowlisted drawing-session directory (and ideally authenticate the request) before reading it.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| res.writeHead(200, { 'Content-Type': 'text/plain; charset=utf-8' }); | ||
| res.end(fs.readFileSync(requestedPath, 'utf-8')); |
There was a problem hiding this comment.
If path is nonexistent, inaccessible, a directory, or otherwise unreadable, fs.readFileSync throws from the HTTP request callback with no catch; Node's default uncaught-exception behavior then terminates the drawing server. A malformed request—remotely reachable when collaboration tunneling is enabled—can therefore end the active session, so catch read errors and return an appropriate 4xx response instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 358b4b167e
ℹ️ 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.
| res.writeHead(200, { 'Content-Type': 'text/plain; charset=utf-8' }); | ||
| res.end(fs.readFileSync(requestedPath, 'utf-8')); |
There was a problem hiding this comment.
When a new Draw session auto-starts a tunnel (a supported tunnel tool is installed), or a reopened session uses --collab, anyone with the share URL can call this unauthenticated endpoint. It passes the request's absolute path directly to readFileSync, so a collaborator can retrieve any developer-readable file, including SSH keys, .env files, source, and Crabcode configuration; the UI-directory traversal check is bypassed because this branch returns first. Resolve against an explicit session directory, reject absolute/traversing paths and symlink escapes, and authorize the requester before reading.
SECURITY.md reference: SECURITY.md:L45-L48
Useful? React with 👍 / 👎.
1 participant
DO NOT MERGE
Temporary evaluation branch. Do not deploy or merge it.
Change
Add
GET /api/session/fileto return a text preview of the file selected by thepathquery parameter. Return HTTP 400 when the parameter is missing.Validation
git diff --checknode --check plugins/draw/src/server/http.ts