Validate that bazel_args holds flags, not a command

SKILL.md

@@ -112,9 +112,12 @@ whether its frontend reads it:
 ```
 
 Fields:
-- **`bazel_args`** — the extra args aquery must run with to mirror the real
-  build (see [Extract the Bazel action graph](#extract-bazel-graph)). Recorded so
-  the comparison is reproducible; read by the operator, not the diff.
+- **`bazel_args`** — the extra **flags** aquery must run with to mirror the
+  real build (see [Extract the Bazel action graph](#extract-bazel-graph)), e.g.
+  `["--config=release"]` or `["-c", "opt"]`; never a command
+  (`["aquery", "mnemonic(...)", ...]` is wrong). Recorded so the comparison is
+  reproducible; `diff.py` refuses an entry that is neither a flag nor a flag's
+  value.
 - **`target_map`** — `reference_name → bazel_name` for intentionally renamed
   **executables**. Bazel targets are keyed by full label, so the value is
   usually `:name` (e.g. `"bssl": ":bssl"`). Libraries need no mapping.

scripts/config.py

@@ -100,9 +100,12 @@ class MigrationConfig:
     # Unlike `ignore` (flags/defines), this is the only lever for missing_tu /
     # missing_target on whole subtrees. Each entry is a reviewable decision.
     exclude_targets: Set[str] = field(default_factory=set)
-    # The bazel aquery invocation this migration is compared against, recorded
+    # The FLAGS appended to `bazel build` / `bazel aquery` so the Bazel side
+    # mirrors the real build (e.g. "--config=release", or "-c" "opt"), recorded
     # so the comparison is reproducible. Read by the skill/operator, not by the
-    # diff itself (the diff consumes the already-extracted model).
+    # diff itself (the diff consumes the already-extracted model); validated at
+    # load (see validate_bazel_args) because anything that consumes it splices
+    # it into a bazel command line.
     bazel_args: tuple = ()
     # Opt-in: also diff TEST targets (role=test). OFF by default because test
     # diffing requires BOTH models to be extracted with tests enabled and the
@@ -161,6 +164,38 @@ def target_excluded(self, name: str) -> bool:
         return name in self.exclude_targets
 
 
+def validate_bazel_args(args) -> tuple:
+    """Check that `bazel_args` is a list of flags, and return it as a tuple.
+
+    Each entry must be a flag (starts with '-') or the value of the flag just
+    before it, when that flag has no '=' (so ["-c", "opt"] and
+    ["--config", "release"] are fine). Anything else -- most often a whole
+    command such as ["aquery", "mnemonic(...)", "--output=jsonproto"] -- is
+    refused, naming the entry: spliced into `bazel aquery <args> <query>` it
+    becomes a second query expression and breaks the command. A boolean flag
+    followed by a stray word slips through; the check is a guard against the
+    command-in-the-field mistake, not a Bazel option parser."""
+    if isinstance(args, str) or not isinstance(args, (list, tuple)):
+        raise ValueError(
+            f"bazel_args must be a list of flags, got {type(args).__name__}")
+    value_allowed = False
+    for a in args:
+        if not isinstance(a, str):
+            raise ValueError(f"bazel_args entry {a!r} is not a string")
+        if a.startswith("-"):
+            value_allowed = "=" not in a
+            continue
+        if value_allowed:
+            value_allowed = False
+            continue
+        raise ValueError(
+            f"bazel_args entry {a!r} is not a flag or a flag's value. "
+            "bazel_args holds only the flags appended to `bazel build` / "
+            "`bazel aquery` to mirror the real build (e.g. [\"--config=release\"] "
+            "or [\"-c\", \"opt\"]), never the command or query itself.")
+    return tuple(args)
+
+
 def load(path: str) -> MigrationConfig:
     """Load config from an explicit file path, or return empty if absent."""
     if not path or not os.path.isfile(path):
@@ -182,7 +217,7 @@ def load(path: str) -> MigrationConfig:
         source_map=tuple(
             (e["from"], e["to"]) for e in ig.get("source_map", [])),
         exclude_targets=set(obj.get("exclude_targets", [])),
-        bazel_args=tuple(obj.get("bazel_args", [])),
+        bazel_args=validate_bazel_args(obj.get("bazel_args", [])),
         include_tests=bool(obj.get("include_tests", False)),
     )
 

scripts/diff.py

@@ -519,5 +519,8 @@ def summarize(discs: List[Discrepancy],
     from serialize import load_model
     a = load_model(sys.argv[1])
     b = load_model(sys.argv[2])
-    cfg = config_mod.load(sys.argv[3]) if len(sys.argv) > 3 else MigrationConfig()
+    try:
+        cfg = config_mod.load(sys.argv[3]) if len(sys.argv) > 3 else MigrationConfig()
+    except ValueError as e:
+        sys.exit(f"diff.py: {sys.argv[3]}: {e}")
     print(json.dumps(summarize(diff_models(a, b, cfg), a, b, cfg), indent=2))

tests/test_engine.py

@@ -640,6 +640,62 @@ def test_config_loads_all_fields_from_json():
     assert cfg.map_include("external/absl+/base") == "@absl/base"  # rewrite
 
 
+def test_bazel_args_accepts_flags_and_flag_values():
+    import config as config_mod
+    for ok in ([], ["--config=release"], ["-c", "opt"],
+               ["--config", "release", "--copt=-O3"],
+               ["--compilation_mode=opt", "--cxxopt=-std=c++17", "--force_pic"]):
+        assert config_mod.validate_bazel_args(ok) == tuple(ok), ok
+
+
+def test_bazel_args_rejects_a_command_line():
+    # A whole aquery command in the field (seen in practice): spliced into
+    # `bazel aquery <args> <query>` it yields two query expressions.
+    import config as config_mod
+    bad_lists = (
+        ["aquery", 'mnemonic("CppCompile|CppLink|CppArchive", //...)',
+         "--output=jsonproto"],
+        ["--config=release", "build"],        # '=' flag takes no separate value
+        ["-c", "opt", "//..."],               # one value per flag
+        "--config=release",                   # a string, not a list
+    )
+    for bad in bad_lists:
+        try:
+            config_mod.validate_bazel_args(bad)
+        except ValueError as e:
+            if isinstance(bad, list):
+                assert repr(bad[-1] if bad[0].startswith("-") else bad[0]) in str(e), e
+        else:
+            raise AssertionError(f"accepted {bad!r}")
+
+
+def test_bazel_args_rejects_a_non_string_entry():
+    import config as config_mod
+    try:
+        config_mod.validate_bazel_args(["-c", 3])
+    except ValueError as e:
+        assert "3" in str(e) and "not a string" in str(e), e
+    else:
+        raise AssertionError("accepted a non-string entry")
+
+
+def test_config_load_refuses_malformed_bazel_args():
+    import json
+    import tempfile
+    import config as config_mod
+    with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as f:
+        json.dump({"bazel_args": ["aquery", "//...", "--output=jsonproto"]}, f)
+        path = f.name
+    try:
+        config_mod.load(path)
+    except ValueError as e:
+        assert "'aquery'" in str(e), e
+    else:
+        raise AssertionError("malformed bazel_args loaded without error")
+    finally:
+        os.unlink(path)
+
+
 def _test_models(cmake_test_srcs, bazel_test_srcs, n_cmake_bins=1, n_bazel_bins=1):
     """Build a/b models with a shared production lib plus test targets carrying
     the given sources, split across n_*_bins test executables."""