bazel-contrib / publish-to-bcr

A GitHub app that mirrors releases of your Bazel ruleset to the Central Registry
Apache License 2.0
21 stars 9 forks source link

consider alternative design ideas #157

Open junyer opened 5 months ago

junyer commented 5 months ago

In light of CVE-2024-3094, would it make sense to consider alternative design ideas at this time? It's not that I distrust this app. It's that I don't really want to add any app to each project's repository... and to my account's fork of the BCR repository.

For example, would it be possible to trigger off "commands" specified in issues filed to BCR? It would be useful to provide an action to facilitate issue filing, I suppose, but it would be crucial to confine everything else to the BCR side.

Thanks in advance for your consideration.

kormide commented 5 months ago

I appreciate the desire here to reduce potential attack vectors, but alternative designs that I've considered have a number of drawbacks in terms of developer experience.

For example, the app listens for release events so that it create a PR to publish the entry right away. If there's a manual step to comment on an issue in the BCR, that will result in some releases being delayed because people will forget.

The BCR does not give write access to non-Googlers, and because the only way to submit an entry is to create a PR, for the vast majority of ruleset owners this has to be from a branch in a fork, and the app needs write permission to that fork to push an entry. I could make a centralized BCR fork perhaps under bazel-contrib that all entries are pushed to, but then we'd need to give write access to the public which could allow malicious code to be inserted. If we only allow the app to push to that fork and deny write access to ruleset maintainers, then they wouldn't be able to push edits to the PR in response to review feedback.

Perhaps the publish workflow could be reworked as a series of GitHub Actions where authorization is done via a user PAT, but that's a very different design and a project that would require funding. I'm open to more ideas on improving security though, so if you have any concrete proposals please send them.

junyer commented 5 months ago

Thanks for the response! Point taken about needing to add the app to my account's fork of the BCR repository. Is it at all possible for the release event to trigger the PR creation without having added the app to the project's repository? I'm not too fussed about the specifics so long as explicit actuation as part of a workflow is straightforward. Regarding funding, I could contribute changes if that would help? :)

kormide commented 5 months ago

Thanks for the response! Point taken about needing to add the app to my account's fork of the BCR repository. Is it at all possible for the release event to trigger the PR creation without having added the app to the project's repository? I'm not too fussed about the specifics so long as explicit actuation as part of a workflow is straightforward. Regarding funding, I could contribute changes if that would help? :)

Is your concern that the app requires write access to the repository? It's unfortunate that it requires write access just for the BCR fork and not for the actual project repositories. One solution is to use a separate app, one that only listens for release events which gets installed to the project, and the other with write access that gets installed to a BCR fork. However, there are already two apps: one that can create PRs on the BCR, and the one that users use. It exists for the same reason---the canonical BCR didn't want to grant write access. Adding a second app for users to install could be confusing.

A second approach is for the app to poll repositories it knows about for new releases (perhaps you commit to a config file in this repo to add your project), then users only need to install the write-access app to their BCR fork.

A third approach is just to offer a GitHub actions style workflow that uses a PAT for authenticating all of the operations.

I think we'd need to see more demand for this to dedicate some SIG funding to it. You're the first person to raise any concern over the app permissions. Perhaps you could start a discussion on the bazel slack #bzlmod channel?

Regarding funding, I could contribute changes if that would help? :)

For sure that would be helpful if we move forward with something :).

junyer commented 5 months ago

Is your concern that the app requires write access to the repository?

Yes, that's the showstopper for me. If nobody else feels particularly worried about it, 🤷, but I can't unsee it. :(

A third approach is just to offer a GitHub actions style workflow that uses a PAT for authenticating all of the operations.

Indeed, if it isn't feasible to signal from a workflow running in the project's repository to the app running in my account's fork of the BCR repository, then I'm going to have to hack together a workflow based on the third approach.

kormide commented 5 months ago

Is your concern that the app requires write access to the repository?

Yes, that's the showstopper for me. If nobody else feels particularly worried about it, 🤷, but I can't unsee it. :(

I think that's a very reasonable concern.

A third approach is just to offer a GitHub actions style workflow that uses a PAT for authenticating all of the operations.

Indeed, if it isn't feasible to signal from a workflow running in the project's repository to the app running in my account's fork of the BCR repository, then I'm going to have to hack together a workflow based on the third approach.

I could see a GitHub action approach being offered as an alternative to the app. I've tried to keep the core domain logic separate from the application logic (trying to follow Domain Driven Design), so the GitHub action could be a different orchestration in the application layer but use the same domain code.

Would you be interested in helping to build this out? It would be best to hold hold off until I've done some planned refactoring. I'm planning to add NestJS to the app to handle all the dependencies in a cleaner way.

junyer commented 5 months ago

I would be keen to help do the needful to make - uses: bazel-contrib/gh-action-publish-to-bcr@vX.Y.Z happen. I started looking at something quick and dirty with Perl one-liners for metadata.json and source.json, but holding off and doing things properly is grand.

kormide commented 4 months ago

We had a discussion about this in the SIG meeting. The approach everyone seemed to prefer was to just make a CLI. You could use it in a GitHub workflow to perform your release and it address https://github.com/bazel-contrib/publish-to-bcr/issues/9.

junyer commented 4 months ago

Thanks for the update! That's great to hear. Looking forward to seeing how the CLI can be packaged/distributed and subsequently used. Also, please feel free to close this issue as a duplicate of that issue. :)

davidben commented 2 days ago

Any updates on sorting out the security concerns with this app? I would like to automate BCR publishing for BoringSSL, but giving a third-party app, even one officially recommended by Bazel, write access to a core, security-critical library is definitely a concern.

kormide commented 8 hours ago

Any updates on sorting out the security concerns with this app? I would like to automate BCR publishing for BoringSSL, but giving a third-party app, even one officially recommended by Bazel, write access to a core, security-critical library is definitely a concern.

We (Aspect) have a contract with Google to implement provenance for the BCR. A likely outcome from that work is that Publish to BCR will become a GitHub Action action giving you more control over what it does via workflows. However, we likely won't start on it until the next quarter.

mering commented 7 hours ago

@kormide GitHub provides a JWT claim to verify a request originates from a certain repo and branch. Every module could set additional filters like only publishing from certain protected branches or tags. This should be enough to implement provenance.