Skip to content

Commit

Permalink
Restrict bazelrc-set flag aliases to the "build" command.
Browse files Browse the repository at this point in the history
This prevents being able to build with an alias that "$ bazel
canonicalize-flags" doesn't understand. This is a safety check for build and CI
pipelines that process invocation flags through canonicalize-flags.

Example scenario this fixes:

$ cat .bazelrc
test --flag_alias=myalias=//foo
build:myconfig --flag_alias=configalias=//foo
build:myconfig --configalias=5

$ bazel test //:mytest --myalias=blah
<succeeds>
$ bazel canonicalize-flags --for_command=test -- --myalias=blah
ERROR: Unrecognized option: --myalias=blah

$ bazel build //:mybuild --config=myconfig
<succeeds>
$ bazel canonicalize-flags -- --configalias=4
ERROR: Unrecognized option: --configalias=4

We could potentially expand CanonicalizeCommand.java to inherit from
TestCommand.class instead of BuildCommand.class if there was a use case for
loosening this restriction.

PiperOrigin-RevId: 374867565
  • Loading branch information
gregestren authored and copybara-github committed May 20, 2021
1 parent 9732d23 commit 329b22a
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.common.options.Converters;
import com.google.devtools.common.options.InvocationPolicyEnforcer;
import com.google.devtools.common.options.OptionDefinition;
import com.google.devtools.common.options.OptionPriority.PriorityCategory;
Expand Down Expand Up @@ -428,7 +429,8 @@ static ListMultimap<String, RcChunkOfArgs> structureRcOptionsAndConfigs(
EventHandler eventHandler,
List<String> rcFiles,
List<ClientOptions.OptionOverride> rawOverrides,
Set<String> validCommands) {
Set<String> validCommands)
throws OptionsParsingException {
ListMultimap<String, RcChunkOfArgs> commandToRcArgs = ArrayListMultimap.create();

String lastRcFile = null;
Expand All @@ -440,6 +442,21 @@ static ListMultimap<String, RcChunkOfArgs> structureRcOptionsAndConfigs(
continue;
}
String rcFile = rcFiles.get(override.blazeRc);
// The canonicalize-flags command only inherits bazelrc "build" commands. Not "test", not
// "build:foo". Restrict --flag_alias accordingly to prevent building with flags that
// canonicalize-flags can't recognize.
if ((override.option.startsWith("--" + Converters.BLAZE_ALIASING_FLAG + "=")
|| override.option.equals("--" + Converters.BLAZE_ALIASING_FLAG))
// In production, "build" is always a valid command, but not necessarily in tests.
// Particularly C0Command, which some tests use for low-level options parsing logic. We
// don't want to interfere with those.
&& validCommands.contains("build")
&& !override.command.equals("build")) {
throw new OptionsParsingException(
String.format(
"%s: \"%s %s\" disallowed. --%s only supports the \"build\" command.",
rcFile, override.command, override.option, Converters.BLAZE_ALIASING_FLAG));
}
String command = override.command;
int index = command.indexOf(':');
if (index > 0) {
Expand Down
68 changes: 67 additions & 1 deletion src/test/shell/integration/starlark_configurations_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ if "$is_windows"; then
export MSYS2_ARG_CONV_EXCL="*"
fi

add_to_bazelrc "build --package_path=%workspace%"
function set_up() {
write_default_bazelrc
add_to_bazelrc "build --package_path=%workspace%"
}

#### HELPER FXNS #######################################################

Expand Down Expand Up @@ -635,4 +638,67 @@ function test_rc_flag_alias_canonicalizes() {
expect_log "--//$pkg:type=coffee"
}

function test_rc_flag_alias_unsupported_under_test_command() {
local -r pkg=$FUNCNAME
mkdir -p $pkg

add_to_bazelrc "test --flag_alias=drink=//$pkg:type"
write_build_setting_bzl

bazel canonicalize-flags -- --drink=coffee \
>& "$TEST_log" && fail "Expected failure"
expect_log "--flag_alias=drink=//$pkg:type\" disallowed. --flag_alias only "\
"supports the \"build\" command."

bazel build //$pkg:my_drink >& "$TEST_log" && fail "Expected failure"
expect_log "--flag_alias=drink=//$pkg:type\" disallowed. --flag_alias only "\
"supports the \"build\" command."

# Post-test cleanup_workspace() calls "bazel clean", which would also fail
# unless we reset the bazelrc.
write_default_bazelrc
}

function test_rc_flag_alias_unsupported_under_conditional_build_command() {
local -r pkg=$FUNCNAME
mkdir -p $pkg

add_to_bazelrc "build:foo --flag_alias=drink=//$pkg:type"
write_build_setting_bzl

bazel canonicalize-flags -- --drink=coffee \
>& "$TEST_log" && fail "Expected failure"
expect_log "--flag_alias=drink=//$pkg:type\" disallowed. --flag_alias only "\
"supports the \"build\" command."

bazel build //$pkg:my_drink >& "$TEST_log" && fail "Expected failure"
expect_log "--flag_alias=drink=//$pkg:type\" disallowed. --flag_alias only "\
"supports the \"build\" command."

# Post-test cleanup_workspace() calls "bazel clean", which would also fail
# unless we reset the bazelrc.
write_default_bazelrc
}

function test_rc_flag_alias_unsupported_with_space_assignment_syntax() {
local -r pkg=$FUNCNAME
mkdir -p $pkg

add_to_bazelrc "test --flag_alias drink=//$pkg:type"
write_build_setting_bzl

bazel canonicalize-flags -- --drink=coffee \
>& "$TEST_log" && fail "Expected failure"
expect_log "--flag_alias\" disallowed. --flag_alias only "\
"supports the \"build\" command."

bazel build //$pkg:my_drink >& "$TEST_log" && fail "Expected failure"
expect_log "--flag_alias\" disallowed. --flag_alias only "\
"supports the \"build\" command."

# Post-test cleanup_workspace() calls "bazel clean", which would also fail
# unless we reset the bazelrc.
write_default_bazelrc
}

run_suite "${PRODUCT_NAME} starlark configurations tests"

0 comments on commit 329b22a

Please sign in to comment.