fix(git_operations): stop -c diff.external= from breaking every diff

[YellowSnnowmann]

Closes #5979.

The bug

hardened_git injects every NEUTRALISED_CONFIG entry as a -c override, and one of them was diff.external=. An empty value does not disable an external diff driver — git tries to execute the empty string — so every diff this tool ran died with:

error: cannot run : No such file or directory
fatal: external diff died, stopping at <file>

That is not hardening; it is an outage wearing hardening's clothes. GitOperationsTool's diff operation is broken on main for every repository, with or without config of its own — this is a user-facing break, not only a test failure.

Reproduced standalone, no OpenHuman involved:

$ git init -q . && echo a > f.txt && git add f.txt && git commit -qm init && echo b >> f.txt

$ git -c diff.external= diff -- f.txt
error: cannot run : No such file or directory
fatal: external diff died, stopping at f.txt

$ git -c diff.external= diff --no-ext-diff -- f.txt
diff --git a/f.txt b/f.txt          # correct

The fix

diff.external cannot be neutralised by a -c value — there is no value meaning "none". Two things replace it, and together they are strictly stronger than what they replace:

• The allowlist already refuses it. diff.external is not on ALLOWED_REPO_CONFIG, so a repository that sets it is refused outright by first_disallowed_repo_config_key. That is the fail-closed guarantee, and the -c entry never carried it.
• --no-ext-diff on the command covers what the -c entry was actually reaching for: a key written into the repository in the gap between that inspection and the command itself — the race the NEUTRALISED_CONFIG doc comment describes.

Verified directly, against a repository whose diff.external names a script that touches a marker file: without the flag the marker appears and the driver's output is used; with it the diff is correct and the marker never appears. GIT_EXTERNAL_DIFF was already cleared by suppress_ambient_git_config, so the env half was never open.

git_log passes --pretty=format: and never produces a patch, so it has no external diff to refuse; diff is the only affected operation.

Why it wasn't caught

The coverage lane derives which raw-coverage modules to run from the changed paths, so tools_network_channels_raw_coverage_e2e only runs when the diff reaches into that area. It surfaced on #5955 — a PR that touches no git code at all.

Test plan

• an_ordinary_repository_still_produces_a_diff — the regression: a repository with no config of its own can produce a diff
• a_repository_naming_an_external_diff_driver_never_gets_it_run — the hardening property still holds; the planted driver is proven to run before it is planted, so a pass means refusal, not an inert fixture
• diff_external_is_not_neutralised_by_a_config_override — pins that the entry is never re-added, since the value that looks correct there is the one that breaks the tool
• tools_network_channels_raw_coverage_e2e::git_operations_cover_read_write_markdown_and_safety_rejections — the CI test that exposed this now passes
• cargo clippy (product feature set) clean at -D warnings; cargo fmt --check clean
• 15/15 git_operations::config_tests

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.

---

_{Comment @coderabbitai help to get the list of available commands.}

[YellowSnnowmann]

Closing — the fix is moving into #5955 instead, where the CI failure it repairs actually surfaced. Same commit, same tests; #5979 stays open until it lands there.