feat: Add remote Git mirror foundations by pda · Pull Request #4160 · buildkite/agent

pda · 2026-08-01T03:45:02Z

Description

Establish the shared contract for backend-provided remote Git mirrors without enabling a mirror checkout site. PRs 2–4 consume these foundations.

A remote mirror is optional. If its URL is not allowed, the job continues against canonical and emits a redacted warning.

Context

Stack base: #4158. Remote mirrors require Git 2.45.0+ so GIT_NO_LAZY_FETCH=1 keeps presence checks local. Older Git compatibility is outside scope. Initial Windows support is best-effort and rough-edged; canonical checkout support is unchanged.

Changes

Plumb and protect immutable remote-mirror configuration and canonical repository binding.
Mask absent or rejected mirror values against ambient process environment.
Resolve one typed attempt per checkout and emit low-cardinality telemetry.
Preserve Git global arguments as []string.
Use Git’s leading-! helper form with POSIX single-quote escaping, verified through real git credential fill.
Add scoped credentials, protocol v2, debug-proof prompt hiding, and URL redaction.
Classify exact-object misses and provide the shared bounded-fetch helper.
Require Git 2.45+ at the local-presence probe.
Keep recursive-submodule fresh clones canonical, including Git-accepted option abbreviations.

Testing

Focused and race tests cover allowlisting, ambient masking, prompt hiding, debug redaction, real helper execution from metacharacter paths, eligibility and option abbreviations, telemetry, classification, cancellation, and local-only probes.

Disclosures / Credits

Implemented with Cursor under human direction. Independent ship-risk, maintainability, and simplicity reviews pass on the final foundation diff.

buildsworth-bk-app

Two credential-handling issues need addressing before this foundation is safe to build on; details are inline.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10715, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

About buildsworth

Model: gpt-5.6-sol with xhigh thinking.

How to request a review: Comment @buildsworth-bk review on the PR, or request buildsworth-bk as a reviewer.

Risk labels (how buildsworth classifies risk) — buildsworth classifies risk itself from the diff. To let it approve, grant L2 approval by mentioning @buildsworth-bk (see L2 approval grant):

L1 — Low risk (dep bumps, docs/copy, lockfiles, small presentational fixes). buildsworth may approve by default.
L2 — Standard risk (new UI, additive API fields, refactors). Approved only with an L2 grant; otherwise comment-only.
L3 — High risk (auth, migrations, payments, secrets, perf-critical paths). Human review always required.

cursor · 2026-08-01T04:21:50Z

Follow-up review fix in d24ce2b9 moves the shared bulk-mirror transport contract into foundations, where both on-host mirror creation and fresh checkout cloning can use it without stack coupling. It also adds explicit low-speed clone-timeout classification and preserves the canonical clone retry/cleanup behavior. Focused clone/transport tests pass.

buildsworth-bk-app

Both previous credential-handling concerns remain open, and the inherited process environment creates an additional allowlist gap. Details are in the existing threads and the new inline comment.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10716, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

cursor · 2026-08-01T10:06:18Z

Propagated the plan correction from #4158 in merge 544f21ce. This changes documentation only; PR 1 code and its scoped diff are unchanged.

buildsworth-bk-app

The three previous blocking concerns remain open; I’ve re-raised each in its existing thread. I didn’t find any additional issues in this pass.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10795, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

cursor · 2026-08-02T00:14:19Z

Merged the revised plan through dbdb887a. The foundation implementation will now follow its Git 2.45+ support boundary, force-hidden mirror URL commands, ambient-environment masking, and leading-! credential-helper contract.

cursor · 2026-08-02T00:16:00Z

Added focused regression coverage in 74527059 for debug-mode forced prompt hiding, leading-! credential-helper execution shape, mirror URL debug redaction, and ambient process environment masking. This is the red step before the foundation fix.

cursor · 2026-08-02T00:18:10Z

Implemented the foundation review fixes in eb940e57: Git helper values now use the leading-! shell form, mirror fetch prompts stay hidden even in debug mode, debug environment output redacts URL credentials, absent/disallowed mirror values override ambient process state, and the Git 2.45+ support boundary is documented beside GIT_NO_LAZY_FETCH.

cursor · 2026-08-02T00:18:40Z

Adjusted the forced-prompt regression assertion in 4496e693 to ignore unrelated process lifecycle debug lines while still failing if the command argv appears.

cursor · 2026-08-02T00:25:46Z

Added the red-step eligibility coverage in d62dc9fb: fresh checkouts using recursive-submodule clone modes must report skipped/recursive-submodules so submodules remain canonical in this initial stack.

Previous concerns have been addressed.

cursor · 2026-08-02T00:26:24Z

Implemented recursive-submodule eligibility in d6639942. Fresh checkouts using --recursive, --recurse-submodules[=…], or --remote-submodules now stay canonical and emit the dedicated skip reason; on-host and reused-checkout sites remain eligible because clone flags do not affect them.

Previous concerns have been addressed.

buildsworth-bk-app

The three earlier credential/allowlist concerns are resolved. I found two new blocking gaps in the recursive-submodule guard and the debug prompt regression coverage; details are inline.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10827, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

cursor · 2026-08-02T00:38:08Z

Added another red-step review regression in 06dd68df: the credential helper is now exercised through real git credential fill from a path containing spaces and metacharacters, Git-accepted recursive option abbreviations are covered, and prompt tests separate logger/process writers for race safety.

cursor · 2026-08-02T00:38:42Z

Fixed the re-review edge cases in 8df1f5bf: helper executable paths now use POSIX single-quote escaping verified by real Git, and conservative --rec*/--rem* recognition covers Git’s accepted recursive option abbreviations. The associated prompt tests now use separate logger and process writers.

cursor · 2026-08-02T00:39:30Z

Follow-up 1ebdc0a1 replaces shared bytes.Buffer process streams with the repository’s concurrency-safe process.Buffer, directly addressing the race report.

cursor · 2026-08-02T00:43:52Z

Propagated the final plan alignment through 723632cd; foundation code and its scoped diff are otherwise unchanged.

Previous concerns have been addressed.

buildsworth-bk-app

The two previous blockers are fixed; I left one non-blocking note on keeping the new real-Git helper test isolated from host credential configuration. This changes credential handling in the checkout path, so I’m leaving final sign-off to a human.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10847, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

Previous concerns have been addressed.

buildsworth-bk-app

The original blockers are resolved, and the latest merge only updates the base plan; I found no new issues. The existing non-blocking test-isolation note is unchanged. Since this touches checkout credential handling, final sign-off still needs a human.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10861, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

cursor · 2026-08-02T01:30:19Z

Final foundation follow-up 5910b83b keeps --separate-git-dir fresh clones canonical, including Git-accepted option abbreviations, because mirror cleanup cannot safely own an external Git directory.

buildsworth-bk-app

The new --separate-git-dir guard covers Git’s accepted --sep* spellings; I found no new issues. The existing non-blocking real-Git test-isolation note remains open. Since this changes checkout credential handling, final sign-off still needs a human.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10877, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

buildsworth-bk-app

The prior test-isolation note is addressed; I found no new issues. This still changes checkout credential handling, so I’m leaving final sign-off to a human.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10894, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

cursor · 2026-08-02T01:52:08Z

@buildsworth-bk review

@buildsworth-bk approve L2

buildsworth-bk-app

The latest merge only updates the base plan; I found no new issues in the scoped diff. This still changes checkout credential handling, so I’m leaving final sign-off to a human.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 10903, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

buildsworth-bk-app

I found no new issues in the current diff. Because this changes checkout credential handling, final sign-off still needs a human.

Want to dig deeper?

Paste this into your agent to explore the findings from this review's Buildkite build:

Download the buildsworth logs from build 11065, then answer my questions about the findings.

Install the reading-buildsworth-logs skill to run this.

cursor · 2026-08-03T05:13:33Z

Focused review of the credential-helper change requested by @pda

Verdict: approve with residual risk. I found no material correctness or command-injection defect.

One low-severity test gap remains: TestGitCredentialHelperCommandQuotesExecutablePath emits credentials unconditionally, so it does not prove that the real helper receives git-credentials-helper get in the expected positions. It also lacks an apostrophe in the executable path and skips Windows. A stronger fixture should require those arguments, use a path such as Buildkite Agent's (arm64), and run on Windows if practical.

I verified that:

Git strips the leading !, appends get, store, or erase, and executes the result through a shell.
The resulting production invocation is buildkite-agent git-credentials-helper get, so get reaches the CLI's positional Action.
The POSIX '\'' escaping round-trips spaces, apostrophes, backslashes, $, backticks, parentheses, and semicolons without expansion.
The executable path normally comes from os.Executable(), not job input, so it is trusted; the quoting also prevents metacharacter injection.
Generic provider credentials and legacy GitHub App credentials both use this helper.
Canonical global replacement semantics are unchanged, while remote-mirror calls correctly clear inherited helpers before adding Buildkite's helper.

Focused Go tests and real-Git experiments passed. PR CI, including the Windows suite, is green. I also checked Git-for-Windows source: leading-! helpers are run through its bundled sh, so POSIX quoting is the correct model.

Residual risk: I did not execute the helper end-to-end from a native Windows absolute path. The source behavior strongly supports compatibility, but the current Windows skip leaves canonical Windows authentication unverified at runtime.

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

catkins · 2026-08-03T05:23:13Z

+	if r.droppedRemoteMirrorURL != "" {
+		_, _ = fmt.Fprintf(
+			r.jobLogs,
+			"Remote Git mirror is not permitted by --allowed-repositories; using canonical repository: %s\n",


catkins · 2026-08-03T05:26:16Z

+
+type remoteMirrorSkipReason string
+
+const (


Good observability here

catkins · 2026-08-03T05:27:31Z

 type gitFetchArgs struct {
 	Shell         *shell.Shell // The shell to run the command in
-	GitFlags      string       // Global git flags to pass to the command
+	GitFlags      []string     // Global git flags to pass to the command


…-foundations-c275" This reverts commit 979ecc4, reversing changes made to 3ee0b89. Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

pda requested review from a team as code owners August 1, 2026 03:45

buildsworth-bk-app Bot previously requested changes Aug 1, 2026

View reviewed changes

Comment thread internal/job/checkout_ssh.go Outdated

Comment thread internal/job/checkout_remote_mirror.go

buildsworth-bk-app Bot previously requested changes Aug 1, 2026

View reviewed changes

Comment thread agent/job_runner.go Outdated

pda mentioned this pull request Aug 1, 2026

feat: Populate on-host mirrors from remote mirrors #4161

Merged

buildsworth-bk-app Bot previously requested changes Aug 1, 2026

View reviewed changes

buildsworth-bk-app Bot previously requested changes Aug 2, 2026

View reviewed changes

Comment thread internal/job/checkout_remote_mirror.go Outdated

Comment thread internal/shell/shell_test.go Outdated

buildsworth-bk-app Bot reviewed Aug 2, 2026

View reviewed changes

Comment thread internal/job/checkout_remote_mirror_test.go Outdated

buildsworth-bk-app Bot reviewed Aug 2, 2026

View reviewed changes

Base automatically changed from pda/remote-git-mirrors-plan-c275 to main August 3, 2026 03:30

pda force-pushed the pda/remote-git-mirror-foundations-c275 branch from 9783bde to dc340c9 Compare August 3, 2026 03:30

buildsworth-bk-app Bot reviewed Aug 3, 2026

View reviewed changes

pda added feature New user-facing feature! internal Non-user facing, internal change. and removed feature New user-facing feature! labels Aug 3, 2026

cursoragent and others added 14 commits August 3, 2026 15:15

feat: Add remote Git mirror foundations

79bce41

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

fix: Harden remote mirror foundation contracts

aacae5c

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

test: Complete remote mirror foundation coverage

66afdd6

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

fix: Share bulk mirror transport contracts

69e4807

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

test: Cover remote mirror logging boundaries

0864bbb

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

fix: Harden remote mirror logging and environment

715c63a

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

test: Ignore unrelated process debug output

0aa34a7

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

test: Cover recursive submodule mirror eligibility

fb4de84

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

fix: Keep recursive submodule clones canonical

c91e05b

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

test: Exercise Git helper and option edge cases

e259bfc

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

fix: Handle Git shell and option parsing edges

9e8174a

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

test: Use race-safe debug output buffers

3363c2d

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

fix: Keep separate Git directories canonical

cb93240

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

fix: Skip canonical commit fetch on every mirror hit

dc815cc

Co-authored-by: Paul Annesley <pda@users.noreply.github.com>

pda force-pushed the pda/remote-git-mirror-foundations-c275 branch from dc340c9 to dc815cc Compare August 3, 2026 05:15

This was referenced Aug 3, 2026

feat: Refresh reused checkouts from remote mirrors #4162

Merged

feat: Clone fresh checkouts from remote mirrors #4163

Merged

catkins approved these changes Aug 3, 2026

View reviewed changes

pda merged commit 979ecc4 into main Aug 3, 2026
4 checks passed

pda deleted the pda/remote-git-mirror-foundations-c275 branch August 3, 2026 06:12

zhming0 mentioned this pull request Aug 4, 2026

release: v3.136.0 #4170

Merged

pda mentioned this pull request Aug 4, 2026

revert: Remove remote Git mirror changes #4173

Closed

2 tasks

Uh oh!

Conversation

pda commented Aug 1, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Context

Changes

Testing

Disclosures / Credits

Uh oh!

buildsworth-bk-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor Bot commented Aug 1, 2026

Uh oh!

buildsworth-bk-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot commented Aug 1, 2026

Uh oh!

buildsworth-bk-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

buildsworth-bk-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

buildsworth-bk-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

buildsworth-bk-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

buildsworth-bk-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

buildsworth-bk-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot commented Aug 2, 2026

Uh oh!

buildsworth-bk-app Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

buildsworth-bk-app Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot commented Aug 3, 2026

Uh oh!

catkins Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

pda commented Aug 1, 2026 •

edited by cursor Bot

Loading

buildsworth-bk-app Bot left a comment •

edited

Loading