[WRONG BRANCH] fix(qoder): keep system/developer prompts out of child-process argv by using prompt file - #478
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ 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. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
![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: 04c209734b
ℹ️ 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".
| promptDir = system ? await mkdtemp(join(tmpdir(), "ocx-qoder-prompt-")) : undefined; | ||
| promptFile = promptDir ? join(promptDir, "system-prompt.txt") : undefined; | ||
| if (promptFile) await writeFile(promptFile, system!, { encoding: "utf8", mode: 0o600 }); |
There was a problem hiding this comment.
On Windows, mode: 0o600 does not remove inherited ACEs, as documented by the repository's existing src/lib/windows-secret-acl.ts:4-8. When %TEMP% is shared or inherits access for Users/Authenticated Users, another local account can enumerate or read this file, whose path is also exposed through --append-system-prompt-file, defeating the security purpose of this change. Harden the temporary directory before writing and/or the file before spawning with the existing required async Windows ACL helpers, and fail closed if hardening fails.
AGENTS.md reference: AGENTS.md:L357-L363
Useful? React with 👍 / 👎.
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
1 participant
Motivation
argv, which exposes confidential prompt content to other local users via process inspection on common OSes.Description
buildQoderArgs()to accept an optionalsystemPromptFileand use--append-system-prompt-fileinstead of embedding the prompt text intoargv.createQoderAdapter().runTurn, write the folded system+developer prompt to a private temporary file created withmkdtemp, written with mode0600, and pass the file path tobuildQoderArgs()so the CLI reads the prompt from the file.rm(..., { recursive: true, force: true }).catch(() => {})).tests/providers/qoder-adapter.test.tsthat asserts prompt text is not present inargv, that the CLI receives the prompt via the file, and that the temporary file is deleted after the turn.Testing
bun test tests/providers/qoder-adapter.test.ts, which passed (6 tests, 0 failures).bun run typecheckwhich succeeded.bun run privacy:scanwhich succeeded.bun run testand observed unrelated environment-sensitive failures in the full suite (worker panic and a few Codex shim process-group tests); the focused Qoder change passed in isolation.Codex Task