test: SECURITY.md citation canary (DO NOT MERGE) - #137
Conversation
|
@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: 8650b13d5d
ℹ️ 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 a draw server is exposed through its collaboration tunnel, any remote caller can supply an absolute path such as an SSH key or .env file and receive its contents because requestedPath is passed directly to readFileSync without authorization or containment. Resolve and validate the real path against the intended drawing-session directory, including rejecting traversal and symlink escapes, before reading it. SECURITY.mdL45-L48
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.
When any local or tunneled client requests a nonexistent, directory, or otherwise unreadable path, readFileSync throws from the HTTP request callback; because this server installs no exception handler, the uncaught error terminates the draw process and disconnects the collaboration session. Catch filesystem 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: 8650b13d5d
ℹ️ 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 draw session is reachable through its collaboration tunnel (the default for new sessions when a supported tunnel is installed), any participant with the share URL can call GET /api/session/file?path=/home/... without application authentication. The query parameter is passed directly to readFileSync, before the UI containment check, so the plugin returns any developer-readable file, including SSH keys, tokens, .env files, or private source. Canonicalize the path, reject absolute/traversal/symlink escapes, and allow reads only inside the intended session directory. This violates the trusted-parent path-confinement requirement.
SECURITY.md reference: SECURITY.md:L45-L48
Useful? React with 👍 / 👎.
1 participant
DO NOT MERGE
This pull request intentionally introduces an unsafe drawing-server endpoint to validate Codex Security's repository-policy citations. Do not deploy, run with a collaboration tunnel, or merge this branch.
Test scenario
SECURITY.mdis already present onmainand explicitly forbids network handlers from exposing arbitrary local files./api/session/fileendpoint reads a request-controlled filesystem path without restricting it to the UI or drawing-session directory.Expected security review
plugins/draw/src/server/http.ts.SECURITY.mdpolicy.【F:SECURITY.md†L45-L48】.Validation
git diff --checknode --check plugins/draw/src/server/http.tsSECURITY.mdexists in the reviewed commit's first parent.