Avoiding accidental secret leaks in the BEP

[jmmv]

Publishing build results via the BEP can result in the accidental disclosure of secrets to a remote service. The primary way in which this happens is due to the presence of secrets in environment variables because Bazel captures the full environment as part of the invocation. But this isn’t the only way secrets might be leaked.

While the BEP is not a security boundary—a determined user will be able to embed secrets in the BEP—Bazel should do its best to not leak secrets by accident when a typical user runs Bazel against a remote service. This is important because common tools today (e.g. the aws CLI) encourage users to store secrets in their environment, so we must assume that their environment is sensitive.

I've put together a design document that describes the problem in more detail and proposes changes to make using Bazel safer against accidental secret leaks. I talked to a few of you during BazelCon about this issue and I think we agreed that this was a problem worth addressing, so here is the design document for proper review: https://docs.google.com/document/d/1-ou6dLV9xsjSSrKf3uJdZKZo-BUlTqbf0OAmKoe_W1s/edit. Feel free to add comments in the document, or here.

[sluongng]

This is a great proposal.

Over at BuildBuddy, we also had to go through similar secrets scrubbing on the server side https://www.buildbuddy.io/docs/guide-metadata#environment-variable-redacting to protect our customers from leaking sensitive data. It would be nice to have these configurable on the client side as well with a sane default.

Personally, very often I encountered bazel run //some/cli -- --some-secret=abc123 being leaked to BEP accidentally, which caused me to disable BES entirely for the bazel run sub-command.

# user.bazelrc
run --bes_results_url=
run --bes_backend=

It would be nice if the users could define their custom scrubbing logic instead of having to disable the feature entirely.

[michaeledgar]

I'll be reviewing this proposal - at first glance it makes a lot of sense to have a well-defined story here. Thanks for writing this up!

At Google we've historically disabled BEP for the "run" command for the same reasons you've given here. But very recently we've made some changes to support loosening this restriction, including adding new ExecRequest event for the "run" command:

• Commit 9a047de5f6cd5c2f1204b2389746d5a9862f10c8: Introduces ExecRequest and adds a flag for controlling whether the argv ("residue") is preserved.
• Commit dab6383ecef2d3996a67b2b1e4760369abc08de7: Renames flag for preserving/redacting command-line residue to --experimental_run_bep_event_include_residue
• Commits 92d19ee3c346fd904bb5be68026c2be4914db340 and b72cec8e6ef8e9b00447168710a856bb73b9c682: Fixing bugs with producing the argv when --experimental_run_bep_event_include_residue=true.

We haven't done anything (or planned to do anything) regarding implicit/explicit flags, environment variables, or console outputs. Since the proposal focuses on argv and implicit/explicit flags, I'll focus my attention on the latter.

Thanks again for sharing this!

[sebastianv1]

👋 Curious if there's been any progress on this issue? At minimum, I'm wondering if it's possible to at least flag this in the documentation with a warning that the whole environment is leaked into the BEP and users should take care of what they do with the file.

[jmmv]

Just FYI, the code we've been using to implement this proposal for ~2 years now is in Snowflake-Labs@4870460 . It'd need to be tweaked to make the whitelists parameterized, but the foundations are there.

(We've also noticed that secrets are leaked to the java.log and we have the hack Snowflake-Labs@a5aa12b to mitigate, but that's kinda orthogonal to this proposal.)