Skip to content

Commit 7eb6a0b

Browse files
kubafloCopilot
andcommitted
Guard every job against reading a pipeline script after it detaches
The sibling guard checked one span of the yml, captured by a lazy regex that began at the first `- pwsh: |` in the file and therefore audited 85 steps across 1017 lines rather than the one step it named. Narrowing it with a tempered `(?!- pwsh: \|)` confines the capture to 90 lines and 0 nested steps -- but the broad form was measured to be doing real work, so it does not ship alone. The replacement is a pipeline-wide invariant: for every `- job:`, no step may read a script from `$(Build.SourcesDirectory)/.github/scripts` after that job runs `git checkout --detach`. All 5 worktree script reads in the pipeline today occur before their job's detach, so the invariant holds; the realistic defect shape (the resolver loaded from the worktree instead of the trusted capture) kills 6 tests. A pipeline-wide zero assertion passes vacuously when the scan is broken, so a proof-of-firing test applies the mutation in memory and requires both CopilotReview and RunUITests to be named. It is the only test that kills the job-segmentation mutant. The separator pattern had two copies, which is a fork of the truth; it now lives once in the file-level BeforeAll. Both forms were measured to agree exactly on today's pipeline, so the wider one is adopted: for a test-suite guard the error modes invert a production gate's -- a false positive is a red test a human resolves in a minute, a false negative is a step reading a script from a detached checkout, which is a lost run nobody is told about. `return , $findings` emits an empty array as one object, so a clean scan reported one nameless finding. Every call site already wraps with `@()`. Suite 4117 -> 4121, both sides measured in the same session by stash; the 35 failures are pre-existing and all in the review half. Mutation: 6 killed, 0 survivors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 735ac9a2-7bec-4baa-ad19-c298e5bc795a
1 parent de1cbec commit 7eb6a0b

1 file changed

Lines changed: 121 additions & 2 deletions

File tree

.github/scripts/Replication-Pipeline.Tests.ps1

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,20 @@
44
BeforeAll {
55
$script:PipelinePath = Join-Path $PSScriptRoot '../../eng/pipelines/ci-copilot.yml'
66
$script:Pipeline = Get-Content -LiteralPath $script:PipelinePath -Raw
7+
8+
# One authority for "this text reads a pipeline script out of the worktree".
9+
# Two consumers ask that question - the Build MSBuild Tasks step guard and
10+
# the pipeline-wide ordering invariant - and a second copy of a pattern is a
11+
# fork of the truth that drifts silently.
12+
#
13+
# The window is deliberately the looser '.{0,4}' rather than a class of
14+
# quotes and separators. Both were measured against today's pipeline and
15+
# agree exactly, so the tighter form is unexercised; and the error modes here
16+
# are asymmetric in the opposite direction from a production gate. A false
17+
# positive is a red suite somebody reads and resolves in a minute. A false
18+
# negative is a step reading a script from a detached checkout, which is a
19+
# lost run nobody is told about.
20+
$script:WorktreeScriptReadPattern = 'Build\.SourcesDirectory.{0,4}[''"]?\.github/scripts'
721
}
822

923
Describe 'MAUI Copilot mode routing' {
@@ -1329,13 +1343,34 @@ Describe 'A build step may not report success for work it never ran' {
13291343
BeforeAll {
13301344
$script:Yaml = Get-Content -LiteralPath (
13311345
Join-Path $PSScriptRoot '../../eng/pipelines/ci-copilot.yml') -Raw
1346+
# The tempered '(?!- pwsh: \|)' is what confines each capture to its own
1347+
# step. Without it the lazy '.*?' still begins at the *first* '- pwsh: |'
1348+
# in the file, because a regex takes the leftmost match, so the two
1349+
# captures spanned 1017 and 2754 lines and held 85 'displayName:' entries
1350+
# between them - this Describe was auditing 85 steps while claiming to
1351+
# audit 2. Narrowed they span 90 and 49 lines and hold none.
1352+
#
1353+
# Narrowing costs real protection: the over-broad span was measured to
1354+
# catch a step reading a pipeline script from the worktree after the
1355+
# checkout detaches, which is a defect that has cost this pipeline two
1356+
# runs. That property is now asserted directly, and pipeline-wide, by
1357+
# 'No job may read a pipeline script from the worktree after it detaches'
1358+
# below. Do not narrow one without the other.
13321359
$script:BuildTaskSteps = @(
13331360
[regex]::Matches(
13341361
$script:Yaml,
1335-
"(?s)- pwsh: \|(.*?)displayName: 'Build MSBuild Tasks'") |
1362+
"(?s)- pwsh: \|((?:(?!- pwsh: \|).)*?)displayName: 'Build MSBuild Tasks'") |
13361363
ForEach-Object { $_.Groups[1].Value })
13371364
}
13381365

1366+
It 'captures each step body rather than everything before it' {
1367+
# The whole point of the narrowing: a capture holding another step's
1368+
# displayName is a capture holding another step.
1369+
foreach ($step in $script:BuildTaskSteps) {
1370+
$step | Should -Not -Match 'displayName:'
1371+
}
1372+
}
1373+
13391374
It 'finds both copies of the step' {
13401375
# One serves the review gate and one serves replicate. A fix applied to
13411376
# only one of them is why Windows kept failing after the first attempt.
@@ -1359,7 +1394,7 @@ Describe 'A build step may not report success for work it never ran' {
13591394
# while the file was present and committed on the pipeline's own branch.
13601395
foreach ($step in $script:BuildTaskSteps) {
13611396
$step | Should -Match "trusted-github/scripts/shared/Resolve-BuildShell\.ps1"
1362-
$step | Should -Not -Match 'Build\.SourcesDirectory.{0,4}''\.github/scripts'
1397+
$step | Should -Not -Match $script:WorktreeScriptReadPattern
13631398
}
13641399
}
13651400

@@ -1423,6 +1458,90 @@ Describe 'A build step may not report success for work it never ran' {
14231458
}
14241459
}
14251460

1461+
Describe 'No job may read a pipeline script from the worktree after it detaches' {
1462+
# This is the property the over-broad 'Build MSBuild Tasks' capture was
1463+
# providing by accident. Every job that detaches its checkout is standing on
1464+
# a different commit from the one that carries these scripts, so a script
1465+
# read from $(Build.SourcesDirectory) after that point is a file that may
1466+
# not exist, or worse, a different file with the same name. Build 15066067
1467+
# reported "the file does not exist" for a resolver that was present and
1468+
# committed on the pipeline's own branch.
1469+
#
1470+
# Reads *before* the detach are correct and deliberate - that is the whole
1471+
# reason the trusted capture is taken while the worktree is still ours - so
1472+
# the assertion is about ordering within a job, not about the path.
1473+
1474+
BeforeAll {
1475+
$script:Yaml = $script:Pipeline
1476+
1477+
function Get-LateWorktreeScriptRead {
1478+
param([Parameter(Mandatory)][AllowEmptyString()][string]$Yaml)
1479+
1480+
$jobs = @([regex]::Matches($Yaml, '(?m)^\s*- job: (\S+)\s*$'))
1481+
$findings = @()
1482+
1483+
for ($i = 0; $i -lt $jobs.Count; $i++) {
1484+
$start = $jobs[$i].Index
1485+
$end = if ($i + 1 -lt $jobs.Count) { $jobs[$i + 1].Index } else { $Yaml.Length }
1486+
$body = $Yaml.Substring($start, $end - $start)
1487+
1488+
$detach = $body.IndexOf('git checkout --detach')
1489+
if ($detach -lt 0) { continue }
1490+
1491+
foreach ($read in [regex]::Matches($body, $script:WorktreeScriptReadPattern)) {
1492+
if ($read.Index -gt $detach) {
1493+
$findings += [pscustomobject]@{
1494+
Job = $jobs[$i].Groups[1].Value
1495+
Line = ($Yaml.Substring(0, $start + $read.Index) -split "`n").Count
1496+
Text = ($body.Substring($read.Index, [Math]::Min(90, $body.Length - $read.Index)) -split "`n")[0]
1497+
}
1498+
}
1499+
}
1500+
}
1501+
1502+
# Deliberately not ', $findings': that idiom preserves a
1503+
# single-element array but emits an *empty* one as a single object,
1504+
# so a clean pipeline would report one nameless finding. Every call
1505+
# site wraps with @(), which handles both ends correctly.
1506+
return $findings
1507+
}
1508+
}
1509+
1510+
It 'holds across every job in the pipeline' {
1511+
$findings = @(Get-LateWorktreeScriptRead -Yaml $script:Yaml)
1512+
$findings.Count | Should -Be 0 -Because (
1513+
"these read a pipeline script from the worktree after their job detached: " +
1514+
(($findings | ForEach-Object { "$($_.Job) line $($_.Line): $($_.Text)" }) -join '; '))
1515+
}
1516+
1517+
It 'fires on a step that loads the resolver from the worktree' {
1518+
# Proving the scan is not vacuous, on the real defect shape rather than a
1519+
# fixture: rewrite the trusted resolver load the way a careless edit
1520+
# would. Both jobs carry that line, so both must be named.
1521+
$trusted = 'Join-Path "$(Build.ArtifactStagingDirectory)" ' +
1522+
"'trusted-github/scripts/shared/Resolve-BuildShell.ps1'"
1523+
$script:Yaml | Should -BeLike "*$trusted*"
1524+
1525+
$mutated = $script:Yaml.Replace(
1526+
$trusted,
1527+
'"$(Build.SourcesDirectory)/.github/scripts/shared/Resolve-BuildShell.ps1"')
1528+
$mutated | Should -Not -Be $script:Yaml
1529+
1530+
$findings = @(Get-LateWorktreeScriptRead -Yaml $mutated)
1531+
@($findings | ForEach-Object { $_.Job }) | Should -Contain 'CopilotReview'
1532+
@($findings | ForEach-Object { $_.Job }) | Should -Contain 'RunUITests'
1533+
}
1534+
1535+
It 'does not object to a repository root passed as an argument' {
1536+
# The upstream-duplicate gate runs after the detach and passes
1537+
# -RepositoryRoot "$(Build.SourcesDirectory)" deliberately: after the
1538+
# detach that *is* the product tree the run will build. Only script
1539+
# loads are the defect.
1540+
$script:Yaml | Should -Match '-RepositoryRoot "\$\(Build\.SourcesDirectory\)"'
1541+
@(Get-LateWorktreeScriptRead -Yaml $script:Yaml).Count | Should -Be 0
1542+
}
1543+
}
1544+
14261545
Describe 'An already-covered issue may be re-run deliberately' {
14271546
# Every certified reproduction opens a pull request, and that pull request
14281547
# then blocks the issue from ever being replicated again. A pipeline change

0 commit comments

Comments
 (0)