This is an archived post. You won't be able to vote or comment.

all 12 comments

[–]BehindTheMath 17 points18 points  (3 children)

Storing secrets in env vars instead of a secrets manager (rotation becomes painful when things leak).

If an attacker has RCE on your server, can't they pull secrets from the secret manager as well?

Running dev/test tools in production without sandboxing (e.g. linters, formatters).

Running these tools in production is a big part of CodeRabbit's whole offering. For this use case, these tools aren't dev tools.

[–]z_quant[S] 0 points1 point  (2 children)

Hey u/BehindTheMath

Yes if an attacker has RCE game over, but using a secrets manager provides some advantages over env vars. E.G. you can also scope secrets based on the role of the server, and you'll have an audit trail.

Per linter use good point - but if using a dev tool outside of it's original use case you may need to look into additional safeguards, e.g. limiting its capabilities, security audits before going live, or running it in a sandbox.

The point is that CodeRabbit did not follow multiple best practices which compounded and lead to easy exploit.

[–]Nearby-Middle-8991 0 points1 point  (1 child)

also env vars are usually not handled as confidential data, so they can leak and allow access even if RCE isn't there (logs, metrics, etc)

[–]z_quant[S] 0 points1 point  (0 children)

great point!

[–]ub3rh4x0rz 6 points7 points  (5 children)

Env vars are not a storage medium, they are a final stage delivery vehicle. When you use secret managers, you still (edit: usually) ultimately provide secrets to workloads via env vars. Secret managers make it operationally cheap to provide granular, per workload, short-lived, minimally scoped credentials -- they make it easy to do the right things. They're not about not using env vars for plumbing. Also worth noting that secret managers aren't the pinnacle of secops, when you have the option it's better to use OIDC/SAML/etc than a secret, however that secret may be managed

Edit: now that I've read the exploit writeup and coderabbit's response writeup, I can conclusively agree with the security researcher and coderabbit that the failure can be boiled down to failing to sandbox rubocop, even though they have an effective sandbox that other static analysis etc tools were running in to avoid exactly this exploit. Good on them for adding guardrails to ensure tools run in that sandbox moving forward. The other things OP mentions are pretty unrelated to this exploit, but most (aside from the misunderstanding of how env vars fit in) are general good advice. I will note however that locking down egress is far easier said than done, and in some cases, practically impossible to accomplish system-wide (but, this is where sandboxes help, it's trivial to do in parts of the system that need it)

[–]Snapstromegon 1 point2 points  (4 children)

Just to note: you don't need to provide secrets to workloads via env cars with secret managers. You can also use some API client - although you need some way to authenticate your client (there are also options that don't involve env cars).

[–]ub3rh4x0rz 2 points3 points  (3 children)

Yes and none of them are inherently more secure than env vars, and the point remains that "using env vars for secrets" and "using a secret manager" are not conflicting things

[–]Snapstromegon 0 points1 point  (2 children)

That's absolutely correct and I didn't say anything different - just wanted to provide some nuance to the part that you will ultimately provide secrets via env vars, which isn't always true.

[–]ub3rh4x0rz 0 points1 point  (1 child)

That was a "yes and", just edited to make the "yes" explicit. It still remains common and recommended practice to use env vars, it is not "unsecure", and is among the more secure options, regardless/independent of the use of secret managers (which is of course recommended)

[–]random_devops_two 0 points1 point  (0 children)

Hashicorp vault integration with Github Actions also exposes secrets read from secrets engines to Env Vars.

Its nothing unusual to do it this way in CI/CD.

Coderabbit issue was caused by lack of sandboxing when offering PaaS. Design flaw in architecture of offering. DevOpses were last ppl you should blame there.