bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
22.97k stars 4.03k forks source link

Bazel silently exits with status 37 when using --credential_helper with PATH containing an empty element #23305

Closed jayconrod closed 3 weeks ago

jayconrod commented 1 month ago

Description of the bug:

The --credential_helper flag lets the user specify a binary that Bazel should call to obtain credentials for a remote service. The binary may be an absolute path, workspace-relative path, or a name to be resolved with PATH.

The user can set PATH to something that doesn't necessarily make sense. I had an empty element in mine, which causes Bazel to exit with status 37 (internal error). Most tools ignore empty elements.

I briefly looked through the code to figure out what was going on: at CommandLinePathFactory.java:137, it calls fileSystem.getPath with an unsanitized path element. That seems to expect an absolute, normalized path, so the empty string triggers an assertion.

Which category does this issue belong to?

CLI

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

  1. Set PATH with an empty element, like /bin::/usr/bin.
  2. Run any bazel command with --credential_helper=anything. anything doesn't need to exist.
  3. Observe bazel exits with status 37.

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 7.3.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

n/a

What's the output of git remote get-url origin; git rev-parse HEAD ?

n/a

If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.

n/a

Have you found anything relevant by searching the web?

n/a

Any other information, logs, or outputs that you want to share?

bazel info server_log didn't contain any related error message. It took me a while to figure out what was causing this.

tjgq commented 1 month ago

@jayconrod Can you send a PR to ignore empty elements in PATH?

jayconrod commented 1 month ago

Yes, will do.

jayconrod commented 4 weeks ago

23310 should fix this.