I think that Security fuckups of this disastrous scale should get classified as "breaches" or "incidents" and be required to be publicly disclosed by the news media, in order to protect consumers.
Here is a tool with 1 million customers which was breached with an exploit a clever 11 year old could created.
When the exploit is so simple, I find it likely that bots or Black Hats or APTs had already found a way in and established persistence before the White Hat researchers reported the issue. If this is the case, patching the issue might prevent NEW bad actors from penetrating CodeRabbit's environment, but it might not evict any bad actors which might now be lurking in their environment.
I know Security is hard, but come on guys
ketzo · 1h ago
> While running the exploit, CodeRabbit would still review our pull request and post a comment on the GitHub PR saying that it detected a critical security risk, yet the application would happily execute our code because it wouldn’t understand that this was actually running on their production system.
What a bizarre world we're living in, where computers can talk about how they're being hacked while it's happening.
Also, this is pretty worrisome:
> Being quick to respond and remediate, as the CodeRabbit team was, is a critical part of addressing vulnerabilities in modern, fast-moving environments. Other vendors we contacted never responded at all, and their products are still vulnerable. [emphasis mine]
Props to the CodeRabbit team, and, uh, watch yourself out there otherwise!
progforlyfe · 1h ago
Beautiful that CodeRabbit reviewed an exploit on its own system!
curuinor · 19m ago
hey, this is Howon from CodeRabbit here. we wish to note that this RCE was reported and fixed in January. it was entirely prospective and no customer data was affected. we have extensive sandboxing for basically any execution of anything now, including any and every tool and all generated code of any kind under the CodeRabbit umbrella.
How can you guarantee that nobody ripped the private key before the researcher told you about the issue though?
KingOfCoders · 15m ago
Or has a backdoor installed somewhere?
KingOfCoders · 16m ago
The chuzpe to use this as PR.
tadfisher · 13m ago
But do you still store your GH API private key in environment variables?
cleverwebb · 5m ago
how do you know that no customer data was affected? did you work with github and scan all uses of your keys? how do you know if a use of your github key was authentic or not? did you check with anthroipic/openai/etc to scan logs usage?
It's really hard to trust a "hey we got this guys" statement after a fuckup this big
thyrfa · 17m ago
It is incredibly bad practice that their "become the github app as you desire" keys to the kingdom private key was just sitting in the environment variables. Anybody can get hacked, but that's just basic secrets management, that doesn't have to be there. Github LITERALLY SAYS on their doc that storing it in an environment variable is a bad idea. Just day 1 stuff. https://docs.github.com/en/apps/creating-github-apps/authent...
binarydreams · 22m ago
I've noticed CodeRabbit at times does reviews that are super. It is able to catch bugs that even claude code misses on our Github PRs. Blows my mind at times tbh.
Based on the env vars seems like they're using anthropic, openai, etc. only?
tnolet · 13m ago
Interesting. We removed it as it was mostly too verbose, catching too many false positives and never really added anything useful.
hahn-kev · 42m ago
Why does CodeRabbit need write access to the git repo? Why doesn't Github let me limit it's access?
dpcx · 38m ago
Because it has the ability to write tests for the PR in question.
tadfisher · 24m ago
Then it should open a PR for those tests so it can go through the normal CI and review process.
dpcx · 20m ago
It updates the existing PR with the tests, I believe. They'd still get reviewed and go through CI.
tadfisher · 17m ago
Right, the downside being that the app needs write access to your repository.
brainless · 1h ago
I did not understand something: why did CodeRabbit run external tools on external code within its own set of environment variables? Why are these variables needed for this entire tooling?
tadfisher · 20m ago
> Why are these variables needed for this entire tooling?
They are not. The Github API secret key should never be exposed in the environment, period; you're supposed to keep the key in an HSM and only use it to sign the per-repo access token. Per the GH docs [0]:
> The private key is the single most valuable secret for a GitHub App. Consider storing the key in a key vault, such as Azure Key Vault, and making it sign-only. This helps ensure that you can't lose the private key. Once the private key is uploaded to the key vault, it can never be read from there. It can only be used to sign things, and access to the private key is determined by your infrastructure rules.
> Alternatively, you can store the key as an environment variable. This is not as strong as storing the key in a key vault. If an attacker gains access to the environment, they can read the private key and gain persistent authentication as the GitHub App.
It sounds like they were putting these processes in a chroot jail or something and not allowing them to access the parent process env vars. There's a continuum of ways to isolate child processes in Linux that don't necessarily involve containers or docker.
The_Fox · 56m ago
Their own tools would need the various API keys, of course, and they did build a method to filter out those variables and managed most user code through it, but it sounds like they forgot to put Rubocop through the special method.
So this researcher may have gotten lucky in choosing to dig into the tool that CodeRabbit got unlucky in forgetting.
chuckadams · 47m ago
It sounds like a pretty bad approach in general to have to "filter out the bad stuff" on a case-by-case basis. It should be as simple as launching everything from a sanitized parent environment, and making it impossible to launch any tool otherwise. Or better, make that sanitized environment the default and make privileged operations be the thing that jumps through hoops to talk to a bastion/enclave/whatever that holds the actual keys.
The_Fox · 25m ago
Yes although somewhere there will be an `if` statement to determine if the process being started should get the complete environment or a key to get the other keys or whatever. Best to make that `if` at the highest level of the architecture as possible and wrapped in something that makes it obvious, like a `DangerousUserCodeProcess` class.
The only other safety I can think of is a whitelist, perhaps of file pathnames. This helps to maintain a safe-by-default posture. Taking it further, the whitelist could be specified in config and require change approval from a second team.
elpakal · 56m ago
presuming they take the output of running these linters and pass it for interpretation to Claude or OpenAI
edm0nd · 12m ago
No bounty was paid for this?
elpakal · 1h ago
> After responsibly disclosing this critical vulnerability to the CodeRabbit team, we learned from them that they had an isolation mechanism in place, but Rubocop somehow was not running inside it.
Curious what this (isolation mechanism) means if anyone knows.
diggan · 51m ago
> Curious what this (isolation mechanism) means if anyone knows.
If they're anything like the typical web-startup "developing fast but failing faster", they probably are using docker containers for "security isolation".
No comments yet
kachapopopow · 49m ago
you could say that they have vibe forgotten to sandbox it.
(likely asked AI to implement x and ai completely disregarded the need to sandbox).
thewisenerd · 17m ago
global scoped installations or keys always scare me for this reason
i believe the answer here was to exchange the token for something scoped to the specific repo coderabbit is running in, but alas, that doesn't remove the "RCE" _on_ the repo
tadfisher · 10m ago
They do that, this is how GH apps work. There is no reason to expose the app's private key in the environment for the code that actually runs on the PR.
cleverwebb · 10m ago
I had a visceral and (quite audible) reaction when I got to the environment variable listing.
kachapopopow · 52m ago
Unrelated to the article, but the first time I saw them was in a twitter ad with a completely comically bull** suggestion. I cannot take a company seriously that had something like that inside an ad that is supposed to show the best they're capable of.
Here is a tool with 1 million customers which was breached with an exploit a clever 11 year old could created.
When the exploit is so simple, I find it likely that bots or Black Hats or APTs had already found a way in and established persistence before the White Hat researchers reported the issue. If this is the case, patching the issue might prevent NEW bad actors from penetrating CodeRabbit's environment, but it might not evict any bad actors which might now be lurking in their environment.
I know Security is hard, but come on guys
What a bizarre world we're living in, where computers can talk about how they're being hacked while it's happening.
Also, this is pretty worrisome:
> Being quick to respond and remediate, as the CodeRabbit team was, is a critical part of addressing vulnerabilities in modern, fast-moving environments. Other vendors we contacted never responded at all, and their products are still vulnerable. [emphasis mine]
Props to the CodeRabbit team, and, uh, watch yourself out there otherwise!
if you want to learn how CodeRabbit does the isolation, here's a blog post about how: https://cloud.google.com/blog/products/ai-machine-learning/h...
It's really hard to trust a "hey we got this guys" statement after a fuckup this big
Based on the env vars seems like they're using anthropic, openai, etc. only?
They are not. The Github API secret key should never be exposed in the environment, period; you're supposed to keep the key in an HSM and only use it to sign the per-repo access token. Per the GH docs [0]:
> The private key is the single most valuable secret for a GitHub App. Consider storing the key in a key vault, such as Azure Key Vault, and making it sign-only. This helps ensure that you can't lose the private key. Once the private key is uploaded to the key vault, it can never be read from there. It can only be used to sign things, and access to the private key is determined by your infrastructure rules.
> Alternatively, you can store the key as an environment variable. This is not as strong as storing the key in a key vault. If an attacker gains access to the environment, they can read the private key and gain persistent authentication as the GitHub App.
[0]: https://docs.github.com/en/apps/creating-github-apps/authent...
So this researcher may have gotten lucky in choosing to dig into the tool that CodeRabbit got unlucky in forgetting.
The only other safety I can think of is a whitelist, perhaps of file pathnames. This helps to maintain a safe-by-default posture. Taking it further, the whitelist could be specified in config and require change approval from a second team.
Curious what this (isolation mechanism) means if anyone knows.
If they're anything like the typical web-startup "developing fast but failing faster", they probably are using docker containers for "security isolation".
No comments yet
(likely asked AI to implement x and ai completely disregarded the need to sandbox).
i believe the answer here was to exchange the token for something scoped to the specific repo coderabbit is running in, but alas, that doesn't remove the "RCE" _on_ the repo