madtea.lol

ADR 0022 - Hermetic test guard

  • Status: Accepted
  • Date: 2026-07-07
  • Tracking: internal pre-Codeberg tracker (2034)

Context

A handler test called issuesDevelop without sandboxing. Because serviceissue.Develop operates on the process working directory’s git repo and resolves credentials from the ambient environment, the test — run from the real madtea checkout — resolved real issue #42 through the live forge API, created and checked out a branch, and pushed it to origin. It did this silently, through a full green gate, twice (origin gained issue-42-phase-3-git-commands-migration).

Two ambient dependencies made that possible:

  1. Credential resolution reads real config. internal/config resolves the forge URL + token from the real global/per-checkout git config (or the secure backend keyed off it) whenever explicit env config is absent. A test that does not inject config inherits the developer’s live credentials.
  2. Repo discovery trusts the process cwd. internal/git’s IsGitRepo / GetRepoRoot resolve whatever repo the test happens to be cd’d into. Run from the real checkout, a mutating service op mutates real history.

Point fixes (sandboxing individual tests with t.Chdir(t.TempDir()), as was done for one develop test) are convention, not mechanism: the next unsandboxed test reintroduces the hazard, and the gate stays green while it does.

Decision

Two structural guards, both keyed on testing.Testing() (Go stdlib; false outside go test, so production binaries are untouched), make the two hazards impossible rather than merely discouraged.

Config guard (kills the network half)

Under go test, the AMBIENT credential-resolution paths in internal/config refuse with a loud, explanatory error:

  • getConfigFromGit (feeds GetConfig / GetConfigNoRepo),
  • GetCredentials (after its explicit-env short-circuit),
  • GetCredentialsWithSource (after its env short-circuit; having no error channel, it degrades to the all-empty “not configured” signal).

The EXPLICIT env-injection path (MADTEA_URL + MADTEA_TOKEN, via getConfigFromEnv and each caller’s own env short-circuit) is allowed — it is explicit, test-owned state, not ambient, and is how fake-server tests inject config today.

Git-mutation guard (kills the git half)

Under go test, service-layer repo discovery refuses when the resolved worktree root is NOT beneath the OS temp dir (i.e. it is the real checkout):

  • GetRepoRootDir returns the guard error;
  • IsGitRepoDir — the gate every mutating service op (branch create/delete, commit, stage, push, pull, fetch, worktree, remote) checks — reports false, so those ops fail closed at detection before any mutation;
  • serviceissue.Develop gates on GetRepoRoot, so the incident flow now fails at its first step with the guard error, before any issue fetch, branch creation, or push.

Tests operating in t.TempDir() repos resolve a root under os.TempDir() and are unaffected.

Structural allowances (facts about test state, never a switch)

Mirroring ADR 0009’s empty-repo carve-out, every allowance keys on a fact about test state, not on any settable configuration:

  • Config guard applies a single UNIFIED predicate: resolution is permitted iff BOTH config scopes are test-owned or neutralized —

    • the global scope: HOME under the OS temp dir OR GIT_CONFIG_GLOBAL under the OS temp dir OR GIT_CONFIG_GLOBAL=/dev/null, AND
    • the repo scope: the process cwd resolves to NO git worktree toplevel (nothing per-repo to leak) OR that toplevel resolves under the OS temp dir.

    Both halves are required. A temp-scoped HOME while the cwd is still the real checkout is refused — per-repo credentials (madtea.url/madtea.token in a checkout’s own .git/config) are the documented default setup, so a temp-HOME-with-real-cwd allowance would re-open the network half of the incident. The binding contract clause is “refuse anything reaching real user/global/checkout state,” not merely real global state.

  • Git-mutation guard permits any repo whose worktree root resolves under the OS temp dir.

Both use symlink-resolved path comparison so a /tmp vs /private/tmp divergence never produces a false negative.

Residual limitations (accepted, not bugs)

  • GetCredentialsWithSource degrades SILENTLY. Unlike GetCredentials / getConfigFromGit, this function has no error return channel (callers such as whoami only display the result and must distinguish “not configured” from a hard failure). Under the guard it therefore returns the all-empty “not configured” tuple rather than a loud guard error — the refusal is correct (no real credentials leak) but is not surfaced to the caller as an explanatory message. This is an accepted tradeoff of keeping the 4-string signature.
  • GIT_CONFIG_SYSTEM (/etc/gitconfig) is still readable. The config guard keys on HOME / GIT_CONFIG_GLOBAL and the cwd’s worktree — it does not neutralize the system-scope git config. In practice madtea credentials are never written to /etc/gitconfig, and a system-scope guard would add a subprocess and a maintenance surface for a hazard no real setup exhibits, so a system-scope guard is deliberately NOT implemented. Tests that need full isolation from a hostile /etc/gitconfig set GIT_CONFIG_SYSTEM=/dev/null themselves; this is noted here so the boundary is explicit.

The rejected env opt-in

The issue body proposed MADTEA_TEST_ALLOW_REAL_CONFIG=1. Rejected. Per ADR 0009 (Agent-facing guards are non-overridable — an escape the guarded party can reach defeats the guard) and the standing rule that enforcement gates get no agent-usable bypass (the human is the escape hatch), the allowance must be STRUCTURAL. A settable switch is exactly the foot-gun ADR 0009 removed from the hooks; re-adding one here would let a test (or an agent editing a test) turn the guard off the moment it is inconvenient. There is no bypass switch, and the error messages advertise none — they point only at the structural path forward (run in a temp-dir repo / inject explicit test config via env).

Consequences

  • The incident is now impossible to reproduce silently. The acceptance test TestIssuesDevelop_NoChdir_HitsGuard runs the exact incident flow against the real checkout and asserts the guard error with no branch created.
  • Latent ambient-config reliance is surfaced, not hidden. Two tests that relied on ambient credential resolution (TestIssuesDelete_MultipleNumbers, TestPrsIsMerged_MultipleNumbers) were made hermetic by injecting explicit env config — the guard’s own advice — rather than by weakening the guard.
  • No production impact. testing.Testing() is false outside go test, so every guard is a no-op in shipped binaries.
  • Lineage. This extends ADR 0009’s non-overridable-guard posture from the Bash hooks to the Go test process, and reuses its structural-carve-out shape.

Alternatives considered

  • A MADTEA_TEST_ALLOW_REAL_CONFIG env opt-in (the issue’s proposal). Rejected — see above; it is an agent-reachable escape, which ADR 0009 forbids.
  • Convention only (lint / TestMain sweep / reviewer discipline). Rejected: the incident passed a full green gate twice. A mechanism that fails closed is the only thing that makes the hazard structurally impossible.
  • Guarding IsGitRepoDir by panic to surface a guard error message. Rejected: a panic is unidiomatic and would crash the MCP server should it ever fire outside tests; instead IsGitRepoDir fails closed to false and the develop flow gates on the error-returning GetRepoRoot, so the guard error still surfaces where it matters.

Canonical source: docs/adr/0022-hermetic-test-guard.md in the madtea repo.