fix: claude-lane/task_1780281408931_wl61hi99e-20260602145932 - #31
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Review Summary by QodoSecure OpenCode API key from child process exposure
WalkthroughsDescription• Prevent API key exposure via environment variables to child processes • Write API key to temporary file with restrictive permissions (0600) • Use mkdtempSync to create secure, unguessable temp directory • Pass file path instead of key value in environment variable Diagramflowchart LR
A["API Key in Config"] --> B["Write to Temp File<br/>mode 0600"]
B --> C["chmod 0600<br/>Belt-and-suspenders"]
C --> D["Pass File Path<br/>in Env Var"]
D --> E["Child Process<br/>Reads from File"]
F["Prevents /proc/pid/environ<br/>Exposure"] -.-> D
File Changes1. src/lib/opencode/config.ts
|
Code Review by Qodo
1. API key still in env
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances security by writing the API key to a temporary file with restrictive permissions (0600) instead of passing it directly via environment variables. The review feedback highlights critical improvements: explicitly setting the temporary directory permissions to 0700 to prevent local traversal, implementing a cleanup mechanism on process exit to avoid leaking sensitive files, and addressing a security bypass in server.ts where the raw API key is still exposed in the environment through a stringified config object.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const dir = mkdtempSync(join(tmpdir(), "codeflow-opencode-")); | ||
| const keyFile = join(dir, "api_key"); | ||
| writeFileSync(keyFile, config.apiKey, { mode: 0o600 }); | ||
| // Belt-and-suspenders: explicitly chmod in case the umask interfered | ||
| // with the mode option (writeFileSync's mode is masked by process.umask). | ||
| chmodSync(keyFile, 0o600); | ||
| env[providerConfig.apiKeyEnvVar] = keyFile; |
There was a problem hiding this comment.
There are two important security/robustness improvements here:
- Directory Permissions: While the API key file itself is written with
0o600permissions, the parent directory created bymkdtempSyncis subject to the process's umask (often resulting in0o755or0o700). To ensure other local users cannot traverse or list the directory, we should explicitly set the directory permissions to0o700usingchmodSync(dir, 0o700). - Cleanup Tracking: Push the created directory to
createdTempDirsso it can be cleaned up on process exit.
⚠️ Critical Security Warning
While this function goes to great lengths to avoid placing the API key directly in the environment, this protection is completely bypassed in src/lib/opencode/server.ts.
In server.ts (line 44), the entire config object is stringified and placed in the environment:
OPENCODE_CONFIG_CONTENT: JSON.stringify(config),Since config contains the raw apiKey, the API key is still exposed in the environment variables of the spawned child process. To fix this, you should sanitize the config object in server.ts before stringifying it (e.g., by deleting or replacing the apiKey property).
const dir = mkdtempSync(join(tmpdir(), "codeflow-opencode-"));
chmodSync(dir, 0o700);
createdTempDirs.push(dir);
const keyFile = join(dir, "api_key");
writeFileSync(keyFile, config.apiKey, { mode: 0o600 });
// Belt-and-suspenders: explicitly chmod in case the umask interfered
// with the mode option (writeFileSync's mode is masked by process.umask).
chmodSync(keyFile, 0o600);
env[providerConfig.apiKeyEnvVar] = keyFile;| import { writeFileSync, mkdtempSync, chmodSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
| import { tmpdir } from "node:os"; |
There was a problem hiding this comment.
To prevent leaking temporary directories and sensitive API key files on disk indefinitely, we should track the created temporary directories and clean them up when the process exits.
We can import rmSync and register a synchronous process.on('exit') handler to delete these directories.
import { writeFileSync, mkdtempSync, chmodSync, rmSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
const createdTempDirs: string[] = [];
if (typeof process !== "undefined") {
process.on("exit", () => {
for (const dir of createdTempDirs) {
try {
rmSync(dir, { recursive: true, force: true });
} catch {}
}
});
}![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: 1e5457e44c
ℹ️ 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".
| // Belt-and-suspenders: explicitly chmod in case the umask interfered | ||
| // with the mode option (writeFileSync's mode is masked by process.umask). | ||
| chmodSync(keyFile, 0o600); | ||
| env[providerConfig.apiKeyEnvVar] = keyFile; |
There was a problem hiding this comment.
When starting OpenCode with a hosted provider, this now sets ANTHROPIC_API_KEY/OPENAI_API_KEY/etc. to a temp-file path instead of the API key itself. OpenCode’s config docs distinguish {env:ANTHROPIC_API_KEY} as substituting the environment variable value from {file:...} as reading file contents, so a provider launched through src/lib/opencode/server.ts will receive /tmp/codeflow-opencode-.../api_key as the credential and authentication will fail rather than reading the file.
Useful? React with 👍 / 👎.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| * Convert OpencodeConfig to environment variables for OpenCode CLI. | ||
| * | ||
| * SECURITY: The API key is NEVER placed directly in the environment. | ||
| * Environment variables are readable via `/proc/<pid>/environ` on Linux | ||
| * by any local user, which would expose the key to local privilege | ||
| * escalation. Instead, the key is written to a temp file with mode | ||
| * 0600 (readable/writable only by the owner) and the file path is | ||
| * passed as the env var value. The child process is expected to read | ||
| * the key from the file. |
There was a problem hiding this comment.
1. Api key still in env 🐞 Bug ⛨ Security
Despite the new claim that the API key is never placed in environment variables, OpencodeServer.start() still sets OPENCODE_CONFIG_CONTENT to JSON.stringify(config), which includes config.apiKey and therefore exposes the key in the child process environment.
Agent Prompt
### Issue description
`OPENCODE_CONFIG_CONTENT` is set to `JSON.stringify(config)` and `config` includes `apiKey`, so the spawned `opencode` process still receives the API key via environment variables, contradicting the new security guarantee.
### Issue Context
The PR moved provider API key env vars to a temp file, but `OPENCODE_CONFIG_CONTENT` continues to serialize and export the entire config object.
### Fix Focus Areas
- src/lib/opencode/server.ts[40-45]
- src/lib/opencode/types.ts[19-28]
### Suggested fix
- Build a sanitized config for `OPENCODE_CONFIG_CONTENT` that omits `apiKey` (or replaces it with a non-secret reference if truly required).
- Example: `const { apiKey, ...safeConfig } = config; OPENCODE_CONFIG_CONTENT: JSON.stringify(safeConfig)`
- Ensure any other env var or logging path does not include the raw key.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const dir = mkdtempSync(join(tmpdir(), "codeflow-opencode-")); | ||
| const keyFile = join(dir, "api_key"); | ||
| writeFileSync(keyFile, config.apiKey, { mode: 0o600 }); | ||
| // Belt-and-suspenders: explicitly chmod in case the umask interfered | ||
| // with the mode option (writeFileSync's mode is masked by process.umask). | ||
| chmodSync(keyFile, 0o600); | ||
| env[providerConfig.apiKeyEnvVar] = keyFile; |
There was a problem hiding this comment.
2. Temp key file not cleaned 🐞 Bug ☼ Reliability
configToEnv() writes the API key into a newly created temp directory but provides no cleanup path, so API keys persist on disk and repeated starts/restarts will accumulate secret-bearing files under the system temp directory.
Agent Prompt
### Issue description
A temp directory and key file are created for every `configToEnv()` call, but nothing deletes them. This leaves API keys at rest on disk indefinitely and can also create unbounded growth in the temp directory across restarts.
### Issue Context
`configToEnv()` is used by `OpencodeServer.start()` to construct the environment for the spawned `opencode` process. `OpencodeServer.stop()` kills the process but does not remove any temp files.
### Fix Focus Areas
- src/lib/opencode/config.ts[65-90]
- src/lib/opencode/server.ts[29-86]
- src/lib/opencode/server.ts[88-118]
### Suggested fix
- Change `configToEnv()` (or its caller) to return both:
1) the env object, and
2) a cleanup function (or list of created paths) that removes the temp directory.
- In `OpencodeServer.start()`, register cleanup on:
- normal stop (`stop()`), and
- child process `exit`/`error` handlers.
- Use best-effort removal: `rmSync(dir, { recursive: true, force: true })` (or async equivalent) and ignore errors.
- (Optional hardening) explicitly ensure the temp directory permissions are owner-only before writing the key file.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Pull request overview
This PR modifies the OpenCode configuration helper to change how OpencodeConfig is converted into environment variables for the spawned OpenCode CLI process, with the intent of avoiding placing API keys directly in env vars.
Changes:
- Added Node.js filesystem/path/os usage to write the API key into a temp file with restrictive permissions.
- Updated
configToEnvto set the provider API key env var to the temp file path instead of the API key string. - Expanded the function’s doc comment with security rationale for the change.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { writeFileSync, mkdtempSync, chmodSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
| import { tmpdir } from "node:os"; |
| // Write the API key to a unique temp file with restrictive permissions. | ||
| // Using mkdtempSync guarantees an unguessable, exclusive directory name, | ||
| // avoiding symlink attacks in the shared /tmp directory. | ||
| const dir = mkdtempSync(join(tmpdir(), "codeflow-opencode-")); | ||
| const keyFile = join(dir, "api_key"); |
| * Convert OpencodeConfig to environment variables for OpenCode CLI. | ||
| * | ||
| * SECURITY: The API key is NEVER placed directly in the environment. | ||
| * Environment variables are readable via `/proc/<pid>/environ` on Linux | ||
| * by any local user, which would expose the key to local privilege | ||
| * escalation. Instead, the key is written to a temp file with mode | ||
| * 0600 (readable/writable only by the owner) and the file path is | ||
| * passed as the env var value. The child process is expected to read | ||
| * the key from the file. |
|
Closing as superseded. The cleanup commit No action needed on this branch. |
Automated DevPulse recovery — see commit history.
Recovered by recover_unprd_tasks.py