Hacker News Reader

Comment by chanon | Hacker News Reader

I'm starting to think these 'Github Apps' are a bad idea. Even if CodeRabbit didn't have this vulnerability, what guarantee do we have that they will always be good actors? That their internal security measures will ensure that none of their employees may do any malicious things?

Taking care of private user data in a typical SaaS is one thing, but here you have the keys to make targetted supply chain attacks that could really wreak havoc.

View on HN

Correct me if I'm wrong, but the problem here is not with GitHub Apps, instead CodeRabbit violated the principle of least privilege: ideally the private key of their app should never end up in the environment of a job for a client but rather a short lived token should be minted from it (for just a single repo (for which the job is running)) so it never gets anywhere near those areas where one of their clients has any influence over what runs.

I agree, this seems like straight up bad design from a security perspective.

But at the same time, me as a customer of Github, would prefer if Github made it harder for vendors like CodeRabbit to make misstakes like this.

If you have an app with access to more than 1M repos, it would make sense for Github to require a short lived token to access a given repository and only allow the "master" private key to update the app info or whatever.

And/or maybe design mechanisms that only allow minting of these tokens for the repo whenever a certain action is run (i.e not arbitrarily).

But at the end of the day, yes, it's impossible for Github to both allow users to grant full access to whatever app and at the same time ensure stuff like this doesn't happen.

The private key isn’t a key in the “API KEY” sense, it’s a key in the “public/private key pair” sense. It’s not sent to github and there’s no way for them to know if the signing of the token used to make the call happened in a secure manner or not, because github doesn’t receive the key as part of the request at all.

GH Apps already use short-lived tokens that can be scoped per repo. You mint a token using your private key and exchange it for a token via API. Then you use that token and dispose of it. That's the only way to use GH Apps (User Access Tokens which are the same thing, but require user interaction) Those tokens always expire.

I'd rather GitHub finally fix their registry to allow these GH Apps to push/pull with that instead of PAT.

That's...literally the way it already works.

There is a master private key that mints expiring limited-use tokens.

The problem was leaking the master private key.

There's also no reason why they needed to have write access to post code review comments. But for some reason they ask for it and you can't deny that part when hooking up their thing.

That stood out to me as well. It comes across as greedy - "some of you must suffer, but that is a price I am willing to pay".

The bunny will often include patches in its replies that the PR author can commit. I've never been clear as to which of us is doing the committing but that could be the need for write access. (I always do it myself but I can see how some might prefer the convenience.)

They should really mass revoke that privilege because I can't see any upside to it. Unless they have a plan for some future state where they will want write access?

Just don't grant write access to random apps and you are safe.

Reply on news.ycombinator.com