fix(cli): name the check that refused, and a remedy that can remove it

A reporter brought a full ProcMon trace to #1856 and still could not say which
check refused their install. That is not their failing: the binary discards the
answer before printing it.

cli_activation_production_context_init() validates the cache, rendezvous and
log directories -- it is the emitter MOST likely to hold a useful detail -- and
it printed CLI_ACTIVATION_REFUSED_MESSAGE bare, through the raw sink rather
than cli_activation_diagnostic(). So the reader got "Check the errors above"
with nothing above. That is the exact dead end #1416 and #1537 already fixed;
the property was repaired on the paths that had tests and left broken on this
sibling, which had none. Route it through the attributing helper: the refusal
now names the transaction refusal note or cbm_daemon_ipc_validation_detail().

The remedy was also wrong for the reported shape. `icacls <dir> /remove:g <sid>`
cannot remove an INHERITED ACE, and the stock C:\ grant for Authenticated Users
reaches every new child directory exactly that way -- so a reporter following
our advice watches the command succeed and the refusal persist. The refusal now
says whether the ACE was inherited, and the advice names
`icacls <dir> /inheritance:r /grant:r "%USERNAME%":(OI)(CI)F` for that case.

No safety check is relaxed. An install directory writable by other accounts is
still refused; it is now refused in terms the reader can act on.

The new contract test guards the CLASS rather than this one call site: any
emitter of a refusal constant that bypasses the attributing helper fails it,
which is what would have caught this. It fails on the previous tree naming
src/cli/cli.c:750, plus the missing inherited-ACE report and the missing
/inheritance:r remedy.

Local: cli 314 passed; activation_transaction daemon_ipc daemon_bootstrap
daemon_version 110 passed, 1 skipped (user namespaces are Linux-only, runs on
the Linux leg); make -f Makefile.cbm lint-ci clean.

Reported by zaferavci1.

Refs #1856
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>

scripts/test.sh

@@ -215,6 +215,9 @@ bash "$ROOT/tests/test_soak_daemon_recovery_contract.sh"
 echo "=== Step 0e: Windows launcher bundle contract ==="
 bash "$ROOT/tests/test_windows_bundle_contract.sh"
 
+echo "=== Step 0e2: activation refusal diagnostic contract ==="
+bash "$ROOT/tests/test_activation_diagnostic_contract.sh"
+
 echo "=== Step 0f: tree-sitter runtime Makefile dependencies ==="
 bash "$ROOT/tests/test_makefile_ts_runtime_dependencies.sh"
 

src/cli/activation_transaction.c

@@ -712,9 +712,17 @@ static bool activation_windows_acl_check(HANDLE handle, DWORD tolerated_untruste
         if (!trusted) {
             char label[128];
             LPSTR sid_text = NULL;
-            (void)snprintf(label, sizeof(label), "acl-grants-cross-account-mutation to %s",
+            /* #1856: say whether the grant is INHERITED. The remedy differs and
+             * the wrong one silently does nothing: `icacls <dir> /remove:g <sid>`
+             * cannot remove an inherited ACE -- that needs `/inheritance:r` --
+             * and the stock `C:\` ACE for Authenticated Users reaches every new
+             * child directory exactly this way. A reporter following the
+             * generic advice sees the command succeed and the refusal persist. */
+            bool inherited = (header->AceFlags & INHERITED_ACE) != 0;
+            (void)snprintf(label, sizeof(label), "acl-grants-cross-account-mutation to %s%s",
                            ConvertSidToStringSidA(sid, &sid_text) && sid_text ? sid_text
-                                                                              : "unparsable-sid");
+                                                                              : "unparsable-sid",
+                           inherited ? " (inherited from a parent directory)" : "");
             if (sid_text) {
                 (void)LocalFree(sid_text);
             }

src/cli/cli.c

@@ -240,7 +240,9 @@ static void cli_activation_diagnostic(const cbm_cli_activation_ops_t *ops, const
                        "error: activation was refused by a filesystem safety check before any "
                        "change was made: %s\n"
                        "error: this is not a session problem. If the flagged directory is one you "
-                       "trust, remove the flagged permission grant (icacls <dir> /remove:g <sid>) "
+                       "trust, remove the flagged permission grant (icacls <dir> /remove:g <sid>; "
+                       "an INHERITED grant needs icacls <dir> /inheritance:r /grant:r "
+                       "\"%%USERNAME%%\":(OI)(CI)F instead, because /remove:g cannot delete one) "
                        "or use an owner-private directory for --dir/CBM_CACHE_DIR, then retry.",
                        note);
         diagnostic = attributed;
@@ -747,7 +749,15 @@ static int cli_activation_guard(cbm_daemon_runtime_activation_action_t action,
     cli_activation_production_context_t context;
     if (!cli_activation_production_context_init(&context, action, target_version, target_build)) {
         cli_activation_production_context_close(&context);
-        cli_activation_production_diagnostic(NULL, CLI_ACTIVATION_REFUSED_MESSAGE);
+        /* #1856: this refusal used to print the generic text BARE. Every other
+         * emitter routes through cli_activation_diagnostic, which appends the
+         * transaction refusal note or cbm_daemon_ipc_validation_detail() and so
+         * names the check that actually refused. Here the reader got "Check the
+         * errors above" with nothing above -- exactly the dead end #1537/#1416
+         * fixed on the other paths and missed on this one. Context init is
+         * where the cache, rendezvous and log directories are validated, so it
+         * is the emitter MOST likely to hold a detail worth showing. */
+        cli_activation_diagnostic(NULL, CLI_ACTIVATION_REFUSED_MESSAGE);
         return CLI_TRUE;
     }
     printf("Stopping active CBM sessions and operations for %s...\n",

tests/test_activation_diagnostic_contract.sh

@@ -0,0 +1,108 @@
+#!/usr/bin/env bash
+# Activation-refusal diagnostics must never point at evidence they do not show.
+#
+# #1416 and #1537 both landed because a refusal printed generic text and told
+# the reader to "check the errors above" when nothing was above. The fix was
+# cli_activation_diagnostic(): it appends the transaction refusal note, or
+# cbm_daemon_ipc_validation_detail(), so the message names the check that
+# actually refused.
+#
+# #1856 showed the fix was incomplete in the way that matters: the property was
+# repaired on the paths that had tests and left broken on a sibling call site
+# that had none. cli_activation_production_context_init() -- the emitter that
+# validates the cache, rendezvous and log directories, i.e. the one MOST likely
+# to hold a useful detail -- printed the constant bare. A reporter with a full
+# ProcMon trace still could not tell which check refused, because the binary
+# discarded the answer before printing.
+#
+# So the guard here is the CLASS, not the one call site: every emitter of the
+# refusal constants goes through the attributing helper. A future emitter that
+# bypasses it fails this test instead of reaching a reporter.
+set -euo pipefail
+
+ROOT="$(cd "$(dirname "$0")/.." && pwd)"
+
+python3 - "$ROOT" <<'PY'
+from __future__ import annotations
+
+import pathlib
+import re
+import sys
+
+root = pathlib.Path(sys.argv[1])
+failures: list[str] = []
+
+
+def require(condition: bool, message: str) -> None:
+    if not condition:
+        failures.append(message)
+
+
+cli = (root / "src" / "cli" / "cli.c").read_text(encoding="utf-8")
+
+# 1. Every call that passes a refusal constant to a diagnostic sink must use the
+#    attributing helper. cli_activation_production_diagnostic is the raw sink --
+#    legitimate as an ops->visible_diagnostic member (it receives the ALREADY
+#    attributed string), never as a direct emitter of the constant.
+ATTRIBUTING = "cli_activation_diagnostic"
+CONSTANTS = ("CLI_ACTIVATION_REFUSED_MESSAGE", "CLI_ACTIVATION_BUSY_MESSAGE")
+
+for call in re.finditer(r"(\w*diagnostic)\s*\(([^;]*?)\)\s*;", cli, re.S):
+    callee, arguments = call.group(1), call.group(2)
+    if not any(constant in arguments for constant in CONSTANTS):
+        continue
+    if callee == ATTRIBUTING:
+        continue
+    line = cli.count("\n", 0, call.start()) + 1
+    require(
+        False,
+        f"src/cli/cli.c:{line}: {callee}() emits a refusal constant directly. "
+        f"Route it through {ATTRIBUTING}() so the message names the check that "
+        "refused instead of pointing at errors that were never printed (#1856)",
+    )
+
+# 2. The helper must still be the one that appends both attribution channels --
+#    deleting either turns the message back into a dead end.
+helper = re.search(
+    r"static void cli_activation_diagnostic\((?:.|\n)*?\n\}\n", cli
+)
+require(helper is not None, "cli_activation_diagnostic() not found in src/cli/cli.c")
+body = helper.group(0) if helper else ""
+require(
+    "cbm_activation_transaction_refusal_note()" in body,
+    "cli_activation_diagnostic() must append the transaction refusal note (#1416)",
+)
+require(
+    "cbm_daemon_ipc_validation_detail()" in body,
+    "cli_activation_diagnostic() must append the daemon validation detail (#1537)",
+)
+
+# 3. The remedy must be one that works. `icacls /remove:g` silently does nothing
+#    to an INHERITED ACE -- and the stock C:\ grant for Authenticated Users
+#    reaches every new child directory exactly that way, which is the shape
+#    #1856 was reported against. Advice that cannot fix the reported case is
+#    worse than none: the command succeeds and the refusal persists.
+require(
+    "/inheritance:r" in body,
+    "the refusal remedy must name icacls /inheritance:r for an inherited grant "
+    "-- /remove:g cannot remove one, so the advice fails on the stock C:\\ ACE "
+    "that #1856 reported",
+)
+
+# 4. And the refusal itself must say which of the two shapes it hit, or the
+#    reader cannot choose between those commands.
+transaction = (root / "src" / "cli" / "activation_transaction.c").read_text(encoding="utf-8")
+require(
+    re.search(r"AceFlags\s*&\s*INHERITED_ACE", transaction) is not None,
+    "src/cli/activation_transaction.c must report whether the refusing ACE was "
+    "inherited, so the printed remedy matches the grant (#1856)",
+)
+
+if failures:
+    print("Activation diagnostic contract FAILED:")
+    for failure in failures:
+        print(f"  - {failure}")
+    sys.exit(1)
+
+print("Activation diagnostic contract passed")
+PY