fix(gitclone): refresh GitHub App token mid-subprocess via file-based credential helper

[jrobotham-square]

Problem

git subprocesses spawned by cachew (clone, fetch, ls-remote, and
especially git lfs fetch) are configured with a credential.helper
that embeds the GitHub App installation token as a literal inside a
shell-function closure at exec time:

credHelper := "!f() { test \"$1\" = get && echo username=x-access-token && printf 'password=%s\\n' '" + escapedToken + "'; }; f"

GitHub App installation tokens are valid for one hour. So any
subprocess that runs longer than that — which routinely happens for
git lfs fetch on large LFS-heavy repositories — keeps presenting
its now-expired token to GitHub on every retry and loops on
Bad credentials until git-lfs's own retry budget gives up, typically
many hours later. The TokenManager refreshes its in-process cache
fine, but the running subprocess has no way to see the new value.

This was first surfaced as flapping shadow tests in staging but
verified to affect production too. Failure logs look like:

git lfs fetch: Fetching reference refs/heads/main
batch response: Bad credentials
batch response: Bad credentials
...
error: failed to fetch some objects from 'https://github.com/<org>/<repo>.git/info/lfs'
: exit status 2, elapsed=25h58m40s

Self-recovery happens only because the next periodic invocation gets a
fresh token and (if the LFS delta has shrunk enough) finishes inside
the 1 h window. No human action is involved.

Fix

Switch to a file-based credential helper. The token is written to a
0600 temporary file and exposed to git as:

credential.helper=!cat '<path>'

Git re-invokes the helper on every credential query, so it reads the
file fresh each time. A background goroutine bound to the returned
cleanup re-fetches the token on a 30 s ticker and rewrites the file
atomically (write-temp + rename) whenever the value changes. Most
ticks are a cheap cached lookup in TokenManager; only the pre-expiry
refresh actually hits the GitHub API. The atomic rename means a
concurrent cat either sees the old complete content or the new
complete content — never a partial token.

GitCommand now returns a cleanup func() alongside the *exec.Cmd
that callers must defer to stop the goroutine and remove the
credentials file. All in-tree callers (executeClone,
fetchInternal, GetUpstreamRefs, and the LFS snapshot fetch) are
updated. cleanup is always non-nil and safe to call multiple times,
including when GitCommand itself returned an error.

Tests

• Existing TestGitCommandWithCredentialProvider is tightened to
  assert that the token is not embedded in the helper string and
  that the on-disk file contains the full credential response with
  0600 perms.
• TestGitCommand_CleanupRemovesCredentialFile verifies the file is
  removed by cleanup and that cleanup is idempotent.
• TestGitCommand_RefreshGoroutineUpdatesFile drives one tick of the
  refresh loop and asserts the file picks up a rotated token, then
  verifies a same-token tick is a no-op (no file churn).
• TestWriteCredentialFile_Atomic runs concurrent reader/writer
  goroutines for 200 rewrites and asserts the reader never observes a
  partial write.
• TestShellSingleQuote covers the path-quoting helper.

Full just test and just lint pass locally.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b56403ad6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df1479d4fe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jrobotham-square]

Good catch — confirmed locally that with the original !cat '<credfile>' form, a worktree file named get does override the real token (and the same trick works for store/erase).

Fixed in cc097e7 by wrapping the helper in a shell function that both gates on get and absorbs the operation argument:

credential.helper=!f() { test "$1" = get && cat '<credfile>'; }; f

Added TestGitCommand_HelperIgnoresHostileGetFile that drops hostile get/store/erase files into a temp worktree and asserts git credential fill returns the real token. The test would fail against the previous helper string.

[jrobotham-square]

Both valid. Fixed in dda656c.

P1 (symlink attack on path + ".new") — reproduced locally: os.WriteFile follows a pre-planted symlink and writes the rotated token to the symlink target. Switched the intermediate file to os.CreateTemp (O_EXCL + random suffix) in the same directory, then rename into place. Added TestWriteCredentialFile_IgnoresHostileSiblingSymlink that plants the old-style <path>.new symlink to a sentinel and asserts the rotated write lands at path, not at the sentinel.

P2 (cleanup races refresh tick) — cleanup now tracks the refresh goroutine via sync.WaitGroup (using Go 1.25's wg.Go) and waits for it to exit before os.Remove(path). Added TestCleanup_WaitsForInFlightRefresh that shrinks credentialFileRefreshInterval to 1ms, drives the production goroutine into a blocking provider, and asserts cleanup blocks until the goroutine unwinds.

credentialFileRefreshInterval is now a var (with a nolint:gochecknoglobals // test seam annotation) solely to let the cleanup test shrink it.

[alecthomas]

Can you get rid of these incredibly verbose comments please?

[jrobotham-square]

working on it 😆
df1479d

[jrobotham-square]

closed in favour of #322