test: isolate tests from ambient XDG and GOG_* path variables

[malob]

What

Makes the test suite immune to ambient path-environment variables. The layout resolver (internal/config/layout.go) honors GOG_HOME, GOG_{CONFIG,DATA,STATE,CACHE}_DIR, and the XDG base directories ahead of HOME-derived defaults, but tests isolate themselves with per-test t.Setenv("HOME", t.TempDir()) sandboxes. On any machine that exports one of these documented variables, go test ./... today both fails (cross-test contamination through the shared real directory) and writes test fixtures into the developer's real gogcli data — including live file-keyring entries, tracking.json, and gmail-watch state.

Four test-only changes, no runtime code touched:

• internal/cmd/testmain_test.go — the existing TestMain (which already redirects HOME and XDG_CONFIG_HOME to a temp root, from 2ca93e9) now also unsets the five GOG_* path overrides plus XDG_DATA_HOME/XDG_STATE_HOME/XDG_CACHE_HOME, restoring saved values afterward.
• internal/config/testmain_test.go, internal/secrets/testmain_test.go (new) — minimal TestMains unsetting all nine path variables; these packages' tests were exposed the same way.
• internal/googleapi/service_account_test.go — TestTokenSourceForServiceAccountScopesUsesInjectedStore deliberately writes an "ambient" fixture through the real resolver to prove the injected store wins. It already pins HOME/XDG_CONFIG_HOME/XDG_DATA_HOME per test but not GOG_*, so with GOG_HOME exported it wrote <GOG_HOME>/data/sa-YUBiLmNvbQ.json (contents: ambient) into the real directory while reporting ok — silently clobbering any real stored service-account key for that address. It now clears the GOG_* overrides too.

Unsetting rather than redirecting is deliberate: we tried redirecting the variables at a single shared package-level directory, and tests still cross-contaminate through it — the failures need no preexisting content, because writer tests fill the shared directory mid-run and reader tests then see their state (preexisting junk only changes which package the failures land in). That is also the precise reason CI has never seen this: GitHub runners export none of these variables, so every test falls back to its own t.Setenv("HOME", …) sandbox — had a runner exported XDG_DATA_HOME, even a pristine one, the same failures would appear. Unsetting reproduces that environment everywhere. Per-test t.Setenv of any of these variables keeps working (TestMain runs before m.Run), and the build-tagged integration suites that intentionally target the real layout are untouched.

Why

Measured at current main (45b5d76), on macOS (the resolver branches involved are not platform-gated, so Linux with the same variables exported is equally exposed):

• XDG_DATA_HOME/XDG_STATE_HOME exported → 19 failing tests across internal/cmd, internal/config, internal/secrets (the split varies with what's already in the shared directory), plus service-account stubs, a file keyring, tracking.json, and gmail-watch state written into the real $XDG_DATA_HOME/gogcli and $XDG_STATE_HOME/gogcli.
• GOG_HOME exported → 77 failing tests, same mechanism, higher resolver precedence — and GOG_HOME is gogcli's own documented relocation knob, so the population most at risk is gogcli developers who also use gogcli.
• Worst case, no failure at all: the internal/googleapi leak above stays green while overwriting real data.

This came out of a real diagnosis: on a Nix-managed dev machine (XDG variables exported globally), 19 tests failed on a clean checkout of main, and the real ~/.local/share/gogcli / ~/.local/state/gogcli had been silently accumulating test fixtures since June. VISION.md counts reliability improvements around keyring and credentials as wanted work; this protects contributors' actual credentials/state from go test.

Behavior changes (complete ledger)

• None at runtime. The diff touches only _test.go files.
• Test processes for the four packages no longer see ambient GOG_HOME, GOG_{CONFIG,DATA,STATE,CACHE}_DIR, XDG_DATA_HOME, XDG_STATE_HOME, XDG_CACHE_HOME (and, in internal/config/internal/secrets, XDG_CONFIG_HOME). Tests that set these per test are unaffected.
• One incidental effect: with XDG_CACHE_HOME unset, the go build subprocess in internal/cmd's slides-assets test derives its build cache under the sandboxed HOME on Linux (cold cache per run). No measurable runtime change on darwin; the unset is still wanted because gogcli genuinely resolves the cache path (internal/cmd/backup_gmail.go).

Proof

Self-contained TAP script, no credentials required — it runs the matrix against whatever checkout it's started from, so the same script demonstrates the bug on main and its absence here. It pins GOFLAGS and starts each scenario from all nine path variables unset (setting only that scenario's), so ambient environment on the machine running it cannot skew or vacuously pass the checks.

proof-isolation.sh (bash, stdlib only)

#!/usr/bin/env bash
# proof-isolation.sh - run from a gogcli checkout root (no credentials needed).
# TAP output. For the four packages that resolve the system path layout,
# verifies `go test` neither fails nor leaves filesystem entries outside its
# sandboxes when the documented path variables are exported, and that behavior
# with none of them set (the environment CI provides) is unchanged.
set -u
PKGS=(./internal/cmd/ ./internal/config/ ./internal/secrets/ ./internal/googleapi/)
PATHVARS=(GOG_HOME GOG_CONFIG_DIR GOG_DATA_DIR GOG_STATE_DIR GOG_CACHE_DIR
  XDG_CONFIG_HOME XDG_DATA_HOME XDG_STATE_HOME XDG_CACHE_HOME)
UNSET=(); for v in "${PATHVARS[@]}"; do UNSET+=(-u "$v"); done
export GOFLAGS= # an inherited -run/-exec/-short would make green runs vacuous
S=$(mktemp -d /tmp/gog-proof-XXXXXX) || exit 1
echo "# head=$(git rev-parse --short HEAD) $(go version | cut -d' ' -f3-4)"
n=0 status=0

check() { # check <pass:0|nonzero> <description>
  n=$((n + 1))
  if [ "$1" -eq 0 ]; then echo "ok $n - $2"; else echo "not ok $n - $2"; status=1; fi
}

# gotest <logname> [VAR=value]... - go test with ONLY the given path vars set
gotest() {
  log=$1
  shift
  env "${UNSET[@]}" "$@" go test -count=1 "${PKGS[@]}" >"$S/$log.log" 2>&1
}

# leakcheck <description> <dir>... - fail on any entry under the dirs, or on find error
leakcheck() {
  desc=$1
  shift
  files=$(find "$@" -mindepth 1 -print 2>&1)
  frc=$?
  rc=0
  [ "$frc" -ne 0 ] && rc=1
  [ -n "$files" ] && rc=1
  check "$rc" "$desc"
  [ -n "$files" ] && printf '%s\n' "$files" | sed "s|^$S/|# leaked: |; s|^[^#]|# find: &|"
}

# 1-2: XDG data/state exported (the report that started this)
mkdir -p "$S/xdg-data" "$S/xdg-state"
gotest xdg XDG_DATA_HOME="$S/xdg-data" XDG_STATE_HOME="$S/xdg-state"
check $? "tests pass with XDG_DATA_HOME/XDG_STATE_HOME exported (others unset)"
leakcheck "no filesystem entries under the exported XDG dirs" "$S/xdg-data" "$S/xdg-state"

# 3-4: GOG_HOME exported (higher precedence than XDG in the resolver)
mkdir -p "$S/goghome"
gotest gog GOG_HOME="$S/goghome"
check $? "tests pass with GOG_HOME exported (others unset)"
leakcheck "no filesystem entries under the exported GOG_HOME" "$S/goghome"

# 5: all nine path variables unset - must pass before and after (CI runs this way)
gotest bare
check $? "tests pass with no path variables set"

echo "1..$n"
for f in xdg gog; do
  if grep -q '^--- FAIL' "$S/$f.log"; then
    echo "# $f run: $(grep -c '^--- FAIL' "$S/$f.log") failing tests, e.g.:"
    grep '^--- FAIL' "$S/$f.log" | head -3 | sed 's/^/#   /'
  fi
done
exit $status

At current main (45b5d76):

# head=45b5d766 go1.26.6 darwin/arm64
not ok 1 - tests pass with XDG_DATA_HOME/XDG_STATE_HOME exported (others unset)
not ok 2 - no filesystem entries under the exported XDG dirs
# leaked: xdg-data/gogcli
# leaked: xdg-data/gogcli/keep-sa-dXNlckBleGFtcGxlLmNvbQ.json
# leaked: xdg-data/gogcli/sa-c3RkaW5AZXhhbXBsZS5jb20.json
# leaked: xdg-data/gogcli/keep-sa-YUBiLmNvbQ.json
# leaked: xdg-data/gogcli/keyring
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_c2hhcmVkL3NlY3JldA
# leaked: xdg-data/gogcli/keyring/.lock
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_dG9rZW4tc3ViOmRlZmF1bHQ6c3ViamVjdC0wNg
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS90cmFja2luZ19rZXk
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS90cmFja2luZ19rZXlfdjI
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_dG9rZW46ZGVmYXVsdDp1c2VyQGV4YW1wbGUuY29t
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_dGVzdC9rZXk
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS90cmFja2luZ19rZXlfdjM
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS9hZG1pbl9rZXk
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_dG9rZW46dXNlckBleGFtcGxlLmNvbQ
# leaked: xdg-data/gogcli/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS90cmFja2luZ19rZXlfdjE
# leaked: xdg-data/gogcli/sa-ZW52QGV4YW1wbGUuY29t.json
# leaked: xdg-data/gogcli/sa-YUBiLmNvbQ.json
# leaked: xdg-data/gogcli/sa-dXNlckBleGFtcGxlLmNvbQ.json
# leaked: xdg-state/gogcli
# leaked: xdg-state/gogcli/tracking.lock
# leaked: xdg-state/gogcli/tracking.json
# leaked: xdg-state/gogcli/gmail-watch
# leaked: xdg-state/gogcli/gmail-watch/.lock
# leaked: xdg-state/gogcli/gmail-watch/user_x_example_com.json
# leaked: xdg-state/gogcli/gmail-watch/me_example_com.json
# leaked: xdg-state/gogcli/gmail-watch/a_b_com.json
not ok 3 - tests pass with GOG_HOME exported (others unset)
not ok 4 - no filesystem entries under the exported GOG_HOME
# leaked: goghome/config
# leaked: goghome/config/keep-sa-dXNlckBleGFtcGxlLmNvbQ.json
# leaked: goghome/config/keep-sa-a@b.com.json
# leaked: goghome/config/credentials-example.com.json
# leaked: goghome/config/config.json
# leaked: goghome/config/sa-bGVnYWN5QGV4YW1wbGUuY29t.json
# leaked: goghome/config/keep-sa-victim@example.com.json
# leaked: goghome/config/keep-sa-User@Example.com.json
# leaked: goghome/config/credentials.json
# leaked: goghome/config/gmail-attachments
# leaked: goghome/config/gmail-attachments/m-draft-1_a-draft-_a.txt
# leaked: goghome/config/gmail-attachments/m1_a1_a.txt
# leaked: goghome/config/keep-sa-Other@Example.com.json
# leaked: goghome/config/sa-dXNlckBleGFtcGxlLmNvbQ.json
# leaked: goghome/config/credentials-work.json
# leaked: goghome/config/credentials-bad!.json
# leaked: goghome/state
# leaked: goghome/state/tracking.lock
# leaked: goghome/state/tracking.json
# leaked: goghome/state/gmail-watch
# leaked: goghome/state/gmail-watch/.lock
# leaked: goghome/state/gmail-watch/user_x_example_com.json
# leaked: goghome/state/gmail-watch/me_example_com.json
# leaked: goghome/state/gmail-watch/a_b_com.json
# leaked: goghome/data
# leaked: goghome/data/keep-sa-dXNlckBleGFtcGxlLmNvbQ.json
# leaked: goghome/data/sa-c3RkaW5AZXhhbXBsZS5jb20.json
# leaked: goghome/data/keep-sa-YUBiLmNvbQ.json
# leaked: goghome/data/keyring
# leaked: goghome/data/keyring/_gogcli_key_v1_c2hhcmVkL3NlY3JldA
# leaked: goghome/data/keyring/.lock
# leaked: goghome/data/keyring/_gogcli_key_v1_dG9rZW4tc3ViOmRlZmF1bHQ6c3ViamVjdC0wMA
# leaked: goghome/data/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS90cmFja2luZ19rZXk
# leaked: goghome/data/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS90cmFja2luZ19rZXlfdjI
# leaked: goghome/data/keyring/_gogcli_key_v1_dG9rZW46ZGVmYXVsdDp1c2VyQGV4YW1wbGUuY29t
# leaked: goghome/data/keyring/_gogcli_key_v1_dGVzdC9rZXk
# leaked: goghome/data/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS90cmFja2luZ19rZXlfdjM
# leaked: goghome/data/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS9hZG1pbl9rZXk
# leaked: goghome/data/keyring/_gogcli_key_v1_dG9rZW46dXNlckBleGFtcGxlLmNvbQ
# leaked: goghome/data/keyring/_gogcli_key_v1_dHJhY2tpbmcvYUBiLmNvbS90cmFja2luZ19rZXlfdjE
# leaked: goghome/data/sa-ZW52QGV4YW1wbGUuY29t.json
# leaked: goghome/data/sa-YUBiLmNvbQ.json
# leaked: goghome/data/sa-dXNlckBleGFtcGxlLmNvbQ.json
ok 5 - tests pass with no path variables set
1..5
# xdg run: 19 failing tests, e.g.:
#   --- FAIL: TestAuthList_JSON_ReportsUnreadableToken (0.05s)
#   --- FAIL: TestAuthListRemoveTokensListDelete_JSON (0.03s)
#   --- FAIL: TestAuthServiceAccountStatus_MissingTextHasHint (0.03s)
# gog run: 77 failing tests, e.g.:
#   --- FAIL: TestAuthList_JSON_ReportsUnreadableToken (0.03s)
#   --- FAIL: TestAuthListRemoveTokensListDelete_JSON (0.03s)
#   --- FAIL: TestAuthStatusCmd_JSONReportsLegacyCredentialsPath (0.00s)

The silent case in isolation, at main — the package reports ok while writing through the real resolver (sa-YUBiLmNvbQ.json is the service-account stub for a@b.com, base64url-encoded):

$ G=$(mktemp -d); GOG_HOME=$G go test ./internal/googleapi/
ok  	github.com/openclaw/gogcli/internal/googleapi	3.374s
$ find "$G" -type f | sed "s|$G/||"
data/sa-YUBiLmNvbQ.json
$ cat "$G/data/sa-YUBiLmNvbQ.json"; echo
ambient

On this branch:

# head=7d71fa55 go1.26.6 darwin/arm64
ok 1 - tests pass with XDG_DATA_HOME/XDG_STATE_HOME exported (others unset)
ok 2 - no filesystem entries under the exported XDG dirs
ok 3 - tests pass with GOG_HOME exported (others unset)
ok 4 - no filesystem entries under the exported GOG_HOME
ok 5 - tests pass with no path variables set
1..5

Scope notes: the proof exercises the four affected packages; a full go test ./... under each of the three environments also passes on this branch (that is how the affected set was established — no other package resolves the system layout outside build-tagged integration tests, which intentionally use the real one). Windows CI runs with none of these variables set, so it sees pure CI-parity behavior. One adjacent observation, deliberately out of scope for this PR: CI cannot detect removal of these TestMains (runners never export the variables), so the isolation is convention-guarded only. (The keyring-selection variables — GOG_KEYRING_BACKEND and friends — were audited separately and need no scrubbing here: every test that opens a secrets store already pins the backend to file per test, on main and on this branch alike.)

🤖 Generated with Claude Code

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 14, 2026, 8:23 PM ET / August 15, 2026, 00:23 UTC.

ClawSweeper review

What this changes

The PR updates four Go test files so ambient GOG and XDG path overrides cannot direct test fixtures into a developer’s persistent gogcli directories.

Merge readiness

✅ Ready for maintainer review

Current main still lets documented GOG and XDG overrides bypass test homes, while this focused test-only patch clears those inputs before affected tests resolve storage paths; it remains a useful, mergeable fix.

Priority: P2
Reviewed head: 7d71fa551a4b92f320878e1b1d8891268db62673

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused test-isolation repair with a concrete real-environment proof path and no correctness finding.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The PR provides a self-contained terminal proof matrix that exercises exported XDG and GOG paths, checks both test success and filesystem leakage, and reports the before/after behavior.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR provides a self-contained terminal proof matrix that exercises exported XDG and GOG paths, checks both test success and filesystem leakage, and reports the before/after behavior.
Evidence reviewed	6 items	Current resolver behavior: Current main reads all five GOG overrides and all four XDG variables from the process environment, and resolves explicit GOG paths before GOG_HOME, XDG, and defaults. Documented precedence: The path documentation confirms that per-kind GOG variables, GOG_HOME, and XDG variables are supported storage-location inputs in that precedence order. Concrete unsafe test path: On current main, the service-account test sets only HOME and XDG data/config before resolving a data path and writing an ambient fixture; a pre-existing GOG override therefore wins.
Findings	None	None.
Security	None	None.

How this fits together

gogcli’s layout resolver turns command options, GOG environment variables, XDG variables, and platform defaults into config, data, state, and cache locations. Tests use those locations for credentials, keyring entries, and Gmail state, so test setup must ensure the resolver selects temporary paths.

flowchart LR
A[Ambient GOG and XDG variables] --> B[Layout resolver]
C[Test setup] --> B
B --> D[Storage-path precedence]
D --> E[Temporary test directories]
D --> F[Developer persistent directories]
E --> G[Test fixtures]
F --> H[Unwanted fixture writes]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Test-only delta	production +0, tests +76 across 4 files	The patch changes no runtime behavior and confines the repair to package test setup plus one unsafe fixture test.

Technical review

Best possible solution:

Merge the narrow test-process isolation while retaining individual tests’ ability to set an explicit path variable for resolver coverage.

Do we have a high-confidence way to reproduce the issue?

Yes, from current source: export GOG_HOME or a relevant XDG path variable, then run the affected package tests; resolver precedence directs their system-layout fixture writes outside HOME-based test sandboxes.

Is this the best way to solve the issue?

Yes. Clearing ambient overrides once before each affected package runs preserves existing per-test overrides and avoids creating another shared fixture directory.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 45b5d766e137.

Labels

Label changes:

• add P2: Ambient path overrides can make developer test runs write fixtures into persistent local state, but the scope is limited to test environments.
• add proof: sufficient: Contributor real behavior proof is sufficient. The PR provides a self-contained terminal proof matrix that exercises exported XDG and GOG paths, checks both test success and filesystem leakage, and reports the before/after behavior.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR provides a self-contained terminal proof matrix that exercises exported XDG and GOG paths, checks both test success and filesystem leakage, and reports the before/after behavior.

Label justifications:

• P2: Ambient path overrides can make developer test runs write fixtures into persistent local state, but the scope is limited to test environments.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR provides a self-contained terminal proof matrix that exercises exported XDG and GOG paths, checks both test success and filesystem leakage, and reports the before/after behavior.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR provides a self-contained terminal proof matrix that exercises exported XDG and GOG paths, checks both test success and filesystem leakage, and reports the before/after behavior.

Evidence

What I checked:

• Current resolver behavior: Current main reads all five GOG overrides and all four XDG variables from the process environment, and resolves explicit GOG paths before GOG_HOME, XDG, and defaults. (internal/config/layout.go:184, 45b5d766e137)
• Documented precedence: The path documentation confirms that per-kind GOG variables, GOG_HOME, and XDG variables are supported storage-location inputs in that precedence order. (docs/paths.md:12, 45b5d766e137)
• Concrete unsafe test path: On current main, the service-account test sets only HOME and XDG data/config before resolving a data path and writing an ambient fixture; a pre-existing GOG override therefore wins. (internal/googleapi/service_account_test.go:46, 45b5d766e137)
• Patch mitigation: The proposed test now clears all GOG path overrides before resolving and writing the deliberately ambient service-account fixture. (internal/googleapi/service_account_test.go:46, 7d71fa551a4b)
• Feature provenance: The layout resolver was introduced by Peter Steinberger; the existing command-package TestMain isolation harness was also introduced by Peter Steinberger. (internal/config/layout.go:441, c761b28cf8e2)
• Release check: v0.37.0 contains the reviewed main SHA; no tag contains the PR head, so this test isolation is not already released or present on main. (internal/cmd/testmain_test.go:29, 7d71fa551a4b)

Likely related people:

• Peter Steinberger: Introduced the instance layout resolver and authored the existing command-package test isolation harness that this patch extends. (role: layout-resolver introducer and recent area contributor; confidence: high; commits: c761b28cf8e2, 2ca93e9a9f07; files: internal/config/layout.go, internal/cmd/testmain_test.go)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.