Skip to content

Protect GIT_ALLOW_PROTOCOL and document the Git environment preset - #4527

Merged
moskyb merged 1 commit into
mainfrom
document-git-env-protection
Oct 16, 2026
Merged

moskyb merged 1 commit into
mainfrom
document-git-env-protection

Conversation

@moskyb

@moskyb moskyb commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Description

GIT_ALLOW_PROTOCOL can enable Git's command-running ext:: transport, including for submodule URLs. Add it to the preset enabled by --protect-dangerous-git-environment-variables, so a job can't use it to override the operator's transport choices when protection is enabled.

Also show the exact preset in the flag help, generated from the same list used for enforcement. Explain the inclusion criteria: variables that can select executables, inject Git configuration or enable command-running transports. The help makes clear that the preset isn't exhaustive and points to --protected-environment-variables for additional names or prefixes.

Protection remains opt-in. Agent-inherited values remain available; matching job values are ignored with a warning, and secret mappings to protected names fail the job.

Context

Follow-up to #4503.

Public documentation

Status: publish

The CLI and configuration reference generators consume this help text, so the list and rubric will appear there through the release-docs update without maintaining another copy of the preset.

Testing

  • Tests have run locally (with go test ./...).
  • Code is formatted (with go tool gofumpt -extra -w on changed Go files).

Disclosures / Credits

Maybe he's born with it, maybe it's ampelline

Include the command-running transport override in the opt-in preset, and generate flag help from that same list with its inclusion criteria and extension guidance.

Amp-Thread-ID: https://ampcode.com/threads/T-01a13385-7597-7212-b382-e01d8cdff743
Co-authored-by: Amp <amp@ampcode.com>
@moskyb
moskyb requested review from a team as code owners October 16, 2026 04:41
@moskyb moskyb added bug Fixes something that was broken security labels Oct 16, 2026

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No findings to flag. I’m leaving approval to a human because this expands the checkout-time Git environment protection policy (L2), above the repository’s L1 approval ceiling.

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 41760, then answer my questions about the findings.

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

About buildsworth

Model: gpt-6.1-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. Unless repository policy already allows L2 approval, grant it by mentioning @buildsworth-bk (see approval ceiling and 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 when repository policy or a verified grant allows it; otherwise comment-only.
  • L3 — High risk (auth, migrations, payments, secrets, perf-critical pipeline paths). Human approval required unless repository policy explicitly allows L3 approval; per-PR grants cannot allow it.

@moskyb
moskyb merged commit afe4a90 into main Oct 16, 2026
6 checks passed
@moskyb
moskyb deleted the document-git-env-protection branch October 16, 2026 05:02
@bk-docsbot

Copy link
Copy Markdown

🤖 Docs Draft Bot: I've created a draft documentation PR based on this change:

📝 Docs PR: https://github.com/buildkite/docs-private/pull/2478

Please review the draft and make any necessary adjustments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Fixes something that was broken security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants