fix(security-scan): name Git's initial branch for every exact-head checkout - #2154
fix(security-scan): name Git's initial branch for every exact-head checkout#2154seonghobae wants to merge 10 commits into
Conversation
…eckout Closes #2101. Each actions/checkout step initialises a fresh repository before fetching the exact SHA, and Git 2.28+ prints the "Using 'master' as the name for the initial branch" hint plus the Git 3.0 rename warning on every hosted job. Workflow-level GIT_CONFIG_COUNT/KEY_0/VALUE_0 sets init.defaultBranch=main process-locally -- no global gitconfig write, no stderr suppression -- which the issue's hosted probe verified removes both lines. A dedicated contract test pins the three variables at workflow level and rejects per-job overrides or global writes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
추가 contract finding을 확인해 current head에서 수리했습니다. 기존 Regression-only exact Current-head PR-triggered Security Scan, SAST Semgrep, CodeQL PR, Python Security runs are currently queued, so hosted GREEN으로는 아직 승격하지 않습니다. 이 finding은 #2101의 exact acceptance를 강화하는 test-contract repair이며 production @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='af7a068772204d9bde0518510d80733bd5a5206d'
git cat-file -e "${target}^{commit}"
printf '%s\n' '=== exact commit summary ==='
git show --no-ext-diff --format=fuller --stat "$target"
printf '%s\n' '=== exact commit patch ==='
git diff --no-ext-diff "${target}^" "$target" -- \
.github/workflows/security-scan.yml \
tests/test_security_scan_checkout_default_branch_contract.py
printf '%s\n' '=== relevant test outline ==='
ast-grep outline tests/test_security_scan_checkout_default_branch_contract.py --items all || true
printf '%s\n' '=== relevant test content ==='
cat -n tests/test_security_scan_checkout_default_branch_contract.py
printf '%s\n' '=== production workflow header and checkout locations ==='
sed -n '1,100p' .github/workflows/security-scan.yml
rg -n -C 5 'uses:\s*actions/checkout@|GIT_CONFIG_(COUNT|KEY_0|VALUE_0)|^\s*jobs:' .github/workflows/security-scan.yml
printf '%s\n' '=== target-parent versions of the test (for exact-head comparison) ==='
git show "${target}^:tests/test_security_scan_checkout_default_branch_contract.py" | cat -n
printf '%s\n' '=== all tracked test references to initial branch contract ==='
rg -n -C 3 'GIT_CONFIG_(COUNT|KEY_0|VALUE_0)|init\.defaultBranch' tests/test_security_scan_checkout_default_branch_contract.py .github/workflows/security-scan.ymlLength of output: 23523
이제 validator는 세 정확한 환경 변수 이름을 모두 거부합니다.
새 hostile fixture는 이전에 누락된 workflow-level repair도 유지됩니다. Hosted check은 아직 queued 상태이므로, 이 결론은 exact head의 정적 계약 검토에 한정됩니다. ✏️ Learnings added
|
|
Protected Current exact #2154 head is
So the contextual-orchestrator sidecar advance and current-head coalescer changes from protected main are preserved, while #2154's production/test delta is unchanged. No force push, destructive rebase, gate weakening, or predecessor-check transfer occurred. Fresh PR-triggered runs on |
|
Fresh protected-main adoption completed without rebase or force-push. I constructed an ordinary two-parent descendant using protected |
![cwl-noema-review[bot]](https://avatars.githubusercontent.com/u/295022177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Noema LLM review
The PR introduces workflow-level process-local Git configuration (GIT_CONFIG_COUNT/KEY_0/VALUE_0) to suppress Git 2.28+ initial-branch hints during exact-head checkouts, paired with contract tests that enforce presence, prevent shadowing, and reject hostile overrides. Verified probes confirm the configuration suppresses the hint without leaking into downstream jobs, the contract tests pass, and the checkout count assertion is accurate.
Reviewed changed lines
.github/workflows/security-scan.yml:63 (RIGHT): GIT_CONFIG_COUNT=1 scopes the process-local Git configuration correctly, suppressing initial-branch hints during actions/checkout..github/workflows/security-scan.yml:64 (RIGHT): GIT_CONFIG_KEY_0 defines init.defaultBranch; simulation confirms downstream jobs are not adversely affected..github/workflows/security-scan.yml:65 (RIGHT): GIT_CONFIG_VALUE_0=main names the initial branch, matching the intended behavior.tests/test_security_scan_checkout_default_branch_contract.py:37 (RIGHT): The helper correctly extracts the workflow-level env block; executable pytest run passes.tests/test_security_scan_checkout_default_branch_contract.py:51 (RIGHT): The checkout count assertion matches exactly six actual checkout steps, providing an accurate lower bound.
Adversarial validation
.github/workflows/security-scan.yml:63 (RIGHT)falsified: GIT_CONFIG_COUNT=1 plus KEY_0/VALUE_0 does not suppress Git 2.28+ hints during actions/checkout. — Checkout succeeded with initial branch 'main' and no hint in stderr..github/workflows/security-scan.yml:64 (RIGHT)falsified: Process-local Git config leaks into downstream jobs, changing expected init.defaultBranch behavior. — No downstream job performs a fresh git init expecting a non-main default branch; the setting is benign.- Residual risk: No concrete regression identified; future changes to the workflow env block or checkout steps could require updating the contract tests.
Findings
- No blocking findings.
- Result: APPROVE
- Head SHA:
c393e3af1d73cafd87002eb8037b7854eac1173a - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
|
Fresh protected-base repair finding: this PR is now stale against protected Current protected |
1 participant
Closes #2101.
Production change
.github/workflows/security-scan.ymladds one workflow-level process-local Git configuration block betweenpermissions:andjobs::All six
actions/checkoutsteps in the central Security Scan initialise a fresh repository before fetching their exact SHA. Git 2.28+ otherwise emits the implicit-master/ Git-3.0 default-branch warning. The three process-local variables reach checkout's internalgit initwithout writing global user configuration and without suppressing stderr. No job, step, checkout identity,ref/sha, or security-gate semantics changed.Permanent contract
tests/test_security_scan_checkout_default_branch_contract.pynow requires the three reviewed settings as unique direct workflow-levelenv:scalar entries, rejects job/step shadowing ofGIT_CONFIG_COUNT,GIT_CONFIG_KEY_0, orGIT_CONFIG_VALUE_0, rejectsgit config --global/ downstreaminit.defaultBranchwrites, rejects indented block-scalar payload text impersonating workflow env authority, and requires the six exact-head checkouts to remain present.The test remains separate from #2143's
tests/test_required_workflow_queue_contract.pylane; the original branch verified those branches have no merge-tree conflict.RED → GREEN evidence
The initial implementation head
fa06536652edf707e961f3e6ff950340b2c41315had PR-triggered Security Scan, SAST Semgrep, CodeQL PR, and Python Security terminal SUCCESS. Its local full-suite evidence wascoverage run -m pytest tests: 3058 passed, 1 skipped, 7 failed; coverage 100%;interrogate100%; YAML load clean. The seven failures were timing/process-smoke cases reported as unrelated to this diff and reran 31/31 green in isolation under a heavily loaded host. They are predecessor evidence, not current-head authority.A first contract review found
GIT_CONFIG_VALUE_0shadowing was not rejected. Regression-only exactdf7b8564fc6042858cc232371f823cf9d5440bcdadded a hostile job-levelGIT_CONFIG_VALUE_0: master; repair exactaf7a068772204d9bde0518510d80733bd5a5206drejects all three COUNT/KEY/VALUE shadowing paths. That exact later reacquired Security Scan34753620111, SAST Semgrep34753620117, CodeQL PR34753620110, and Python Security34753620169terminal SUCCESS. Productionsecurity-scan.ymldid not change in this contract-only repair.A second fresh review found the workflow-level helper still used substring search inside the captured
env:text, so valid YAML could place the reviewed strings insideDECOY: |rather than as direct environment keys. The first hostile-fixture attempt at exact184d348e7ad65ce4258781c4d4d72deff8c307a4was ineffective because its final scalar line met the regex capture boundary; its 4/4 GREEN verifier is therefore a harness-fixture defect, not RED evidence.Corrected regression-only exact
7d8fe3b250bf8edac654ae5e7e18b826badcbb8ekeeps all three impersonating strings inside the captured scalar payload. Purpose run34756214540 / 103720904858completed FAILURE in the focused test, proving the old validator accepted non-authoritative scalar text.Minimal repair exact
287b0c1c5cf4031ca091f5000c01580ad8766db3requires exactly one direct two-space scalar entry for each reviewed workflow-level Git config key/value. Purpose run34756286194 / 103721090370completed SUCCESS on Ubuntu 24.04: exact checkout, pinned Python dependencies, all four focused cases including the hostile block-scalar regression, and clean working-tree verification passed. The temporary verifier was removed immediately by ordinary descendant77a4eacb64014650d2818ced739c30e334185ddc. The second hardening slice is test-contract-only; productionsecurity-scan.ymlremains unchanged.Current exact-head gates
Current exact head is
77a4eacb64014650d2818ced739c30e334185ddc, basemain@64f483db9d052322c65bcdf1675d66138156f306.Normal PR-triggered lanes on this exact cleanup head are now terminal:
34756326070: SUCCESS.34756326039: SUCCESS.34756325935: SUCCESS.34756325904: initial attempt FAILURE only because current-head dispatch verdicts were stillpending; language detection and dispatch itself succeeded. The compatibility jobs failed atRelease runner or enforce current-head CodeQL verdictwith the explicit message that the dispatch workflow must publish the terminal verdict and rerun the exact failed job. This is the still-open canonical.github#1929terminal-status-publication lane, not a fix(security-scan): name Git's initial branch for every exact-head checkout #2154 source failure to suppress or bypass.Commit statuses currently show CodeRabbit and Devin Review success, but there is still no independent submitted current-head
APPROVED. Status checks or bot quota state are not substituted for review authority.Promotion boundary
Keep the normal protected merge path. #2154 may merge only when current-head applicable required checks satisfy repository rules and an independent approval exists. Do not self-approve, bypass, weaken CodeQL, force-push, or reuse predecessor checks as current-head authority.
After normal protected integration, run an unchanged LifeOS consumer canary against the central Security Scan from
.github/main; require the implicit-master/ Git-3.0 warning to be absent while the same security jobs retain their prior semantics. That downstream proof is the completion boundary for #2101.No warning suppression, global gitconfig mutation, force-push, destructive rebase, self-approval, administrator bypass, or gate weakening.