Add SSH config and env var

[mcncl]

Description

This allows for folks to set the location of an SSH key, with a view to later support that setting via YAML.

Context

Part of the agent checkout improvements project.

Changes

• add bootstrap/env wiring for BUILDKITE_GIT_SSH_KEY
• add GitSSHKey to internal/job.ExecutorConfig
• update default checkout to:
• create a private temp directory adjacent to the checkout path
• write the SSH key to id with 0600 permissions
• normalize the key to a trailing newline
• set GIT_SSH_COMMAND=ssh -i -o IdentitiesOnly=yes
• restore any previous GIT_SSH_COMMAND
• remove the temp key directory on success/failure
• add *_SSH_KEY to default redaction patterns
• add unit/integration coverage for key creation, permissions, cleanup, and checkout behavior

Testing

• Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
• Code is formatted (with go tool gofumpt -extra -w .)

Ran locally:

go test ./...

Disclosures / Credits

Used AI to highlight any shortcomings or lacking test coverage; it fixed the initial issue with the tests running on Windows where explicitly closing the checkoutRoot was required.

I also used it to help write this description; scanning the files for changes to remind me what I'd done.

[chatgpt-codex-connector]

💡 Codex Review

agent/internal/job/checkout.go

Line 231 in cfb160e

return fmt.Errorf("checkout attempt timed out after %s: %w", timeout, err)

Preserve timeout sentinel so checkout attempts actually retry

When BUILDKITE_GIT_CHECKOUT_TIMEOUT is hit, this wraps the checkout error with %w, which preserves underlying exec.ExitError values (typically exit code -1 after signal-based cancellation). In checkout(), the retrier treats shell.IsExitError(err) && shell.ExitCode(err) == -1 as an interrupt and calls r.Break(), so timed-out attempts stop after the first failure instead of being retried according to checkout-attempt settings.

---

agent/internal/job/checkout.go

Lines 849 to 852 in cfb160e

	if err == nil {
	err = cleanupErr
	} else {
	err = errors.Join(err, cleanupErr)

Propagate SSH key cleanup failures from default checkout

The deferred cleanup writes cleanup errors into the local err variable from prepareGitSSHKey, but defaultCheckoutPhase returns the named retErr. As a result, a failed key-directory removal is silently dropped and the phase can return success even when SSH key cleanup failed, potentially leaving key material on disk.

ℹ️ 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".

[mcncl]

This isn't really a change, it's just a result of using gofumpt for my formatting rather than gofmt, I'm happy to default to the latter, though

[DrJosh9000]

err has (meanwhile) become retErr, and errors.Join does the right thing with nil (i.e. drops it), so:

Suggested change

	if err == nil {
	err = cleanupErr
	} else {
	err = errors.Join(err, cleanupErr)
	}
	retErr = errors.Join(retErr, cleanupErr)

[DrJosh9000]

Sibling builds would have to be running under a different uid to be prevented from accessing inside a dir with mode 0o700, right?

[DrJosh9000]

Naming the return args has clear readability and documentation upsides over using _:

Suggested change

	func (e *Executor) prepareGitSSHKey() (_ string, _ func() error, retErr error) {
	func (e *Executor) prepareGitSSHKey() (sshKeyPath string, cleanup func() error, retErr error) {