Warn about config-local setting overrides

CHANGELOG.md

@@ -11,6 +11,9 @@
   explicitly selected `--xffrc` files. Each permission pair governs only its matching skip flag.
 - Add config-selectable `--allow-xffrc` / `--no-allow-xffrc` controls for explicitly named config
   files, subject to the system policy gate and unavailable to the file being admitted.
+- Warn about overriding the same setting more than once within one logical config section while
+  preserving last-value-wins behavior. Deliberately accumulating settings, expression primaries,
+  separate sections and tiers, and all command-line repetitions remain unaffected.
 - Apply named configuration and explicitly loaded config files at each command-line
   selector's exact position instead of flattening them ahead of all CLI options.
 - Defer regex matcher binding until the final configuration has selected grammar
@@ -23,6 +26,10 @@
   while `--sort=global` sorts the operands and walks each tree in deterministic depth-first order.
   Clarify the ordering, directory read-ahead, result buffering, post-order, and score-ranking
   contracts throughout the generated reference and parallel-walk design.
+- Honor the selected path encoding in tree-comparison status records, including non-UTF-8 path
+  bytes, instead of emitting raw relative paths independently of the output configuration.
+- Apply every matching system-policy safety-class denial to mixed config lines, so a destructive
+  primary cannot be hidden from an `@destructive` rule by a sensitive primary on the same line.
 - Clarify concurrency and matching controls, including worker defaults and `-exec` timing,
   filesystem-native versus explicit case folding, block-size units, and regex grammar scope.
 

XFF.md

@@ -83,6 +83,8 @@ Every `--config=NAME` remains active, so multiple named blocks can apply. Among
 
 Configuration expands in application order. Automatic system/user defaults and the invocation selector come first; each command-line `--config` then activates newly matching lines at that exact position, and each `--xffrc` loads its currently matching lines where it appears. Because conflicting options usually use the last value, moving a selector can intentionally change the result. A config line is applied at most once.
 
+Within one logical config section, repeating an overriding setting keeps the normal last-value-wins result but emits a warning: the earlier value is locally redundant and is usually a copy/paste mistake. Options that deliberately accumulate (such as `--exclude`, `--summary`, and `--define`) may repeat without warning, as may expression primaries. Separate config sections and tiers remain independent. Command-line repetition is never warned about, so a pasted command can be adjusted by appending an override.
+
 ### Arming dangerous directives
 
 A dangerous directive (the exec family `-exec` / `-execdir` / `-ok` / `-capture`, or `-delete`) carried by an `--xffrc` file is inert unless `--allow-exec` is set from a trusted tier (the command line or the system/user config, never an `--xffrc` file itself). Unarmed lines are dropped with a warning; the root system `[policy]` can hard-deny even `--allow-exec`.

xff/cli/BUILD.bazel

@@ -41,6 +41,7 @@ cc_library(
     name = "main_cc",
     srcs = ["main.cc"],
     deps = [
+        ":config_validation_cc",
         ":globals_cc",
         ":help_backend_cc",
         ":help_build_cc",
@@ -73,6 +74,34 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "config_validation_cc",
+    srcs = ["config_validation.cc"],
+    hdrs = ["config_validation.h"],
+    deps = [
+        ":globals_cc",
+        "//xff/config:config_cc",
+        "//xff/config:xffrc_cc",
+        "//xff/registry:registry_cc",
+        "@abseil-cpp//absl/container:flat_hash_set",
+        "@abseil-cpp//absl/strings",
+        "@mboworks_mbo//mbo/types:optional_ref_cc",
+    ],
+)
+
+cc_test(
+    name = "config_validation_test",
+    size = "small",
+    srcs = ["config_validation_test.cc"],
+    deps = [
+        ":config_validation_cc",
+        "//xff/config:config_cc",
+        "//xff/config:xffrc_cc",
+        "@googletest//:gtest",
+        "@googletest//:gtest_main",
+    ],
+)
+
 # The stock lean binary: core only, no composable extras. This is the target every test and golden
 # runs against, and the one built by `//...`.
 cc_binary(

xff/cli/config_validation.cc

@@ -0,0 +1,134 @@
+// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors
+// SPDX-License-Identifier: Apache-2.0
+
+#include "xff/cli/config_validation.h"
+
+#include <cstddef>
+#include <string>
+#include <string_view>
+#include <vector>
+
+#include "absl/container/flat_hash_set.h"
+#include "absl/strings/str_cat.h"
+#include "mbo/types/optional_ref.h"
+#include "xff/cli/globals.h"
+#include "xff/config/config.h"
+#include "xff/config/xffrc.h"
+#include "xff/registry/descriptor.h"
+#include "xff/registry/registry.h"
+
+namespace xff::cli {
+namespace {
+
+struct SectionSettings {
+  std::string key;
+  absl::flat_hash_set<std::string> names;
+};
+
+std::string CanonicalName(const GlobalFlag& flag) {
+  constexpr std::string_view kNo = "--no-";
+  if (!flag.name.starts_with(kNo)) {
+    return std::string(flag.name);
+  }
+  const std::string positive = absl::StrCat("--", flag.name.substr(kNo.size()));
+  const mbo::types::OptionalRef<const GlobalFlag> counterpart = LookupGlobal(positive);
+  return std::string(
+      counterpart.has_value() && counterpart->repetition == GlobalFlag::Repetition::kOverride ? counterpart->name
+                                                                                              : flag.name);
+}
+
+std::string SettingName(std::string_view token, const GlobalFlag& flag) {
+  std::string name = CanonicalName(flag);
+  if (flag.repetition != GlobalFlag::Repetition::kKeyed) {
+    return name;
+  }
+  const std::string_view::size_type first = token.find('=');
+  if (first == std::string_view::npos) {
+    return name;
+  }
+  const std::string_view value = token.substr(first + 1);
+  const std::string_view key = value.substr(0, value.find('='));
+  absl::StrAppend(&name, "=", key);
+  return name;
+}
+
+void RecordSetting(
+    std::string_view token,
+    std::string_view location,
+    absl::flat_hash_set<std::string>& seen,
+    std::vector<std::string>& notices) {
+  const mbo::types::OptionalRef<const GlobalFlag> flag = LookupGlobalArgument(token);
+  if (!flag.has_value() || flag->repetition == GlobalFlag::Repetition::kAccumulate) {
+    return;
+  }
+  const std::string name = SettingName(token, *flag);
+  if (!seen.insert(name).second) {
+    notices.push_back(absl::StrCat("setting ", name, " is overridden within ", location));
+  }
+}
+
+std::string_view PrimaryName(std::string_view token) {
+  return token.substr(0, token.find(':'));
+}
+
+void FindOverrides(
+    const std::vector<std::string>& tokens,
+    std::string_view location,
+    absl::flat_hash_set<std::string>& seen,
+    std::vector<std::string>& notices) {
+  for (std::size_t pos = 0; pos < tokens.size(); ++pos) {
+    if (tokens[pos] == "--") {
+      break;
+    }
+    if (LookupGlobalArgument(tokens[pos]).has_value()) {
+      RecordSetting(tokens[pos], location, seen, notices);
+      continue;
+    }
+    const auto primary = registry::Lookup(PrimaryName(tokens[pos]));
+    if (!primary.has_value()) {
+      continue;
+    }
+    if (primary->arity < 0) {
+      while (++pos < tokens.size() && tokens[pos] != ";" && tokens[pos] != "+") {}
+    } else {
+      pos += static_cast<std::size_t>(primary->arity);
+    }
+  }
+}
+
+void FindRcOverrides(
+    const std::vector<config::RcLine>& lines,
+    std::string_view file,
+    std::vector<std::string>& notices) {
+  std::vector<SectionSettings> sections;
+  for (const config::RcLine& line : lines) {
+    const std::string key = absl::StrCat(line.base, ":", line.config);
+    auto section = sections.begin();
+    while (section != sections.end() && section->key != key) {
+      ++section;
+    }
+    if (section == sections.end()) {
+      sections.push_back({.key = key});
+      section = sections.end() - 1;
+    }
+    const std::string location = absl::StrCat(file, " section '", key, "'");
+    FindOverrides(line.flags, location, section->names, notices);
+  }
+}
+
+}  // namespace
+
+std::vector<std::string> ConfigOverrideNotices(const config::ConfigInputs& inputs) {
+  std::vector<std::string> notices;
+  absl::flat_hash_set<std::string> system_globals;
+  FindOverrides(inputs.system.globals, "system config globals", system_globals, notices);
+  absl::flat_hash_set<std::string> system_defaults;
+  FindOverrides(inputs.system.defaults, "system config [defaults]", system_defaults, notices);
+  FindRcOverrides(inputs.user, "user config", notices);
+  for (const config::ExplicitConfig& file : inputs.xffrc) {
+    FindRcOverrides(file.lines, absl::StrCat("--xffrc file ", file.path), notices);
+  }
+  return notices;
+}
+
+}  // namespace xff::cli

xff/cli/config_validation.h

@@ -0,0 +1,22 @@
+// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors
+// SPDX-License-Identifier: Apache-2.0
+
+#ifndef XFF_CLI_CONFIG_VALIDATION_H_
+#define XFF_CLI_CONFIG_VALIDATION_H_
+
+#include <string>
+#include <vector>
+
+#include "xff/config/config.h"
+
+namespace xff::cli {
+
+// Reports overriding global settings that occur more than once in one logical config section.
+// Positive/negative forms are one setting; aliases and valued forms use their canonical global
+// identity. Accumulating settings, expression primaries, separate selectors, and separate files
+// remain independent. Structurally invalid config controls are rejected separately by policy.
+std::vector<std::string> ConfigOverrideNotices(const config::ConfigInputs& inputs);
+
+}  // namespace xff::cli
+
+#endif  // XFF_CLI_CONFIG_VALIDATION_H_

xff/cli/config_validation_test.cc

@@ -0,0 +1,73 @@
+// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors
+// SPDX-License-Identifier: Apache-2.0
+
+#include "xff/cli/config_validation.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "xff/config/config.h"
+#include "xff/config/xffrc.h"
+
+namespace xff::cli {
+namespace {
+
+using ::testing::ElementsAre;
+using ::testing::HasSubstr;
+using ::testing::IsEmpty;
+
+struct ConfigValidationTest : ::testing::Test {};
+
+TEST_F(ConfigValidationTest, ReportsCanonicalAliasAndNegatedOverrides) {
+  config::ConfigInputs inputs;
+  inputs.system.defaults = {"--timezone=utc", "--tz=local"};
+  EXPECT_THAT(ConfigOverrideNotices(inputs), ElementsAre(HasSubstr("setting --timezone is overridden")));
+
+  inputs.system.defaults = {"--hidden", "--no-hidden"};
+  EXPECT_THAT(ConfigOverrideNotices(inputs), ElementsAre(HasSubstr("setting --hidden is overridden")));
+}
+
+TEST_F(ConfigValidationTest, PermitsAccumulatingSettings) {
+  config::ConfigInputs inputs;
+  inputs.system.defaults = {"--exclude=one", "--exclude=two", "--config=one", "--config=two"};
+  EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty());
+}
+
+TEST_F(ConfigValidationTest, KeyedSettingsAccumulateByNameAndReportSameNameOverrides) {
+  config::ConfigInputs inputs;
+  inputs.system.defaults = {"--define=A=one", "--define=B=two"};
+  EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty());
+
+  inputs.system.defaults.emplace_back("--define=A=three");
+  EXPECT_THAT(ConfigOverrideNotices(inputs), ElementsAre(HasSubstr("setting --define=A is overridden")));
+}
+
+TEST_F(ConfigValidationTest, CombinesRepeatedLogicalUserSections) {
+  config::ConfigInputs inputs;
+  inputs.user = config::ParseXffrc("debug: --sort=tree\nother: --sort=global\ndebug: --sort=none");
+  EXPECT_THAT(ConfigOverrideNotices(inputs), ElementsAre(HasSubstr("user config section 'debug:'")));
+}
+
+TEST_F(ConfigValidationTest, KeepsDifferentSectionsAndFilesIndependent) {
+  config::ConfigInputs inputs;
+  inputs.user = config::ParseXffrc("debug: --sort=tree\nother: --sort=global");
+  inputs.xffrc = {
+      {.path = "/one", .lines = config::ParseXffrc("common: --color=always")},
+      {.path = "/two", .lines = config::ParseXffrc("common: --color=never")},
+  };
+  EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty());
+}
+
+TEST_F(ConfigValidationTest, DoesNotTreatPrimaryArgumentsAsGlobalSettings) {
+  config::ConfigInputs inputs;
+  inputs.user = config::ParseXffrc("common: -exec echo --sort=tree ; --sort=global");
+  EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty());
+}
+
+TEST_F(ConfigValidationTest, StopsRecognizingSettingsAfterDoubleDash) {
+  config::ConfigInputs inputs;
+  inputs.user = config::ParseXffrc("common: --sort=tree -- --sort=global");
+  EXPECT_THAT(ConfigOverrideNotices(inputs), IsEmpty());
+}
+
+}  // namespace
+}  // namespace xff::cli

xff/cli/explain_test.sh

@@ -150,6 +150,16 @@ test::authorized_no_user_config_suppresses_user_defaults_but_keeps_explicit_xffr
   expect_matches 'xffrc[[:space:]]+--color=never' "${out}"
 }
 
+test::config_local_override_warns_but_last_value_still_wins() {
+  local cfg="${TEST_TMPDIR}/local_override"
+  printf 'common: --color=always\ncommon: --color=never\n' >"${cfg}"
+  local out
+  out="$(XFF_CONFIG="${cfg}" "$(_xff_bin)" --explain 2>&1)"
+  expect_output_contains 'warning: setting --color is overridden within user config section' "${out}"
+  expect_matches 'user[[:space:]]+--color=always' "${out}"
+  expect_matches 'user[[:space:]]+--color=never' "${out}"
+}
+
 test::xffrc_dangerous_line_is_inert_unless_armed() {
   # The --xffrc tier is non-arming: a sensitive -exec carried by the file is dropped (inert) with a
   # "needs --allow-exec" note unless --allow-exec is passed from a trusted tier (here, the CLI). A