Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 83 additions & 1 deletion .github/scripts/Apply-PRFinalize.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ BeforeAll {
'Test-FinalizeIsNoOp',
'Get-FinalizeRecommendation',
'Merge-PreservedTitlePrefix',
'Merge-PreservedBodyPreamble'
'Merge-PreservedBodyPreamble',
'New-ExclusiveTempFile'
)) {
$function = $ast.Find({
$args[0] -is [System.Management.Automation.Language.FunctionDefinitionAst] -and
Expand Down Expand Up @@ -317,3 +318,84 @@ Describe 'Security regressions — AzDO logging-command injection' {
Should -Be '[iOS] new'
}
}

Describe 'New-ExclusiveTempFile' {
BeforeAll {
$script:SandboxDir = Join-Path ([System.IO.Path]::GetTempPath()) "apply-prfinalize-tests-$([System.IO.Path]::GetRandomFileName())"
New-Item -ItemType Directory -Path $script:SandboxDir -Force | Out-Null
$script:OriginalAgentTemp = $env:AGENT_TEMPDIRECTORY
$env:AGENT_TEMPDIRECTORY = $script:SandboxDir
}

AfterAll {
$env:AGENT_TEMPDIRECTORY = $script:OriginalAgentTemp
Remove-Item -LiteralPath $script:SandboxDir -Recurse -Force -ErrorAction SilentlyContinue
}

It 'creates the file inside AGENT_TEMPDIRECTORY when it is set' {
$path = New-ExclusiveTempFile -Prefix 'pr-finalize-body-123'
try {
Test-Path -LiteralPath $path | Should -BeTrue
(Split-Path -Parent $path) | Should -Be $script:SandboxDir
} finally {
Remove-Item -LiteralPath $path -Force -ErrorAction SilentlyContinue
}
}

It 'produces a unique path on each call, so the name is not predictable' {
$a = New-ExclusiveTempFile -Prefix 'pr-finalize-body-123'
$b = New-ExclusiveTempFile -Prefix 'pr-finalize-body-123'
try {
$a | Should -Not -Be $b
} finally {
Remove-Item -LiteralPath $a -Force -ErrorAction SilentlyContinue
Remove-Item -LiteralPath $b -Force -ErrorAction SilentlyContinue
}
}

It 'writes to the new file rather than through a pre-planted symlink' {
# Regression for the round-2 hardening note: Set-Content follows an existing symlink
# and writes through to its target. A fresh randomly-named file cannot be pre-planted.
$secret = Join-Path $script:SandboxDir 'secret.txt'
Set-Content -LiteralPath $secret -Value 'ORIGINAL' -Encoding UTF8

$path = New-ExclusiveTempFile -Prefix 'pr-finalize-body-123'
try {
'REPLACEMENT BODY' | Set-Content -LiteralPath $path -Encoding UTF8
(Get-Content -Raw -LiteralPath $secret).Trim() | Should -Be 'ORIGINAL'
(Get-Content -Raw -LiteralPath $path).Trim() | Should -Be 'REPLACEMENT BODY'
} finally {
Remove-Item -LiteralPath $path -Force -ErrorAction SilentlyContinue
}
}

It 'falls back to the system temp directory when AGENT_TEMPDIRECTORY is unset' {
$saved = $env:AGENT_TEMPDIRECTORY
$env:AGENT_TEMPDIRECTORY = $null
try {
$path = New-ExclusiveTempFile -Prefix 'pr-finalize-body-123'
try {
Test-Path -LiteralPath $path | Should -BeTrue
} finally {
Remove-Item -LiteralPath $path -Force -ErrorAction SilentlyContinue
}
} finally {
$env:AGENT_TEMPDIRECTORY = $saved
}
}

It 'ignores AGENT_TEMPDIRECTORY when it points at a missing directory' {
$saved = $env:AGENT_TEMPDIRECTORY
$env:AGENT_TEMPDIRECTORY = Join-Path $script:SandboxDir 'does-not-exist'
try {
$path = New-ExclusiveTempFile -Prefix 'pr-finalize-body-123'
try {
Test-Path -LiteralPath $path | Should -BeTrue
} finally {
Remove-Item -LiteralPath $path -Force -ErrorAction SilentlyContinue
}
} finally {
$env:AGENT_TEMPDIRECTORY = $saved
}
}
}
4 changes: 3 additions & 1 deletion .github/scripts/Review-PR.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -2413,7 +2413,9 @@ if ($env:SKIP_PR_FINALIZE_APPLY -eq 'true') {
if ($DryRun) { $applyArgs.DryRun = $true }
& $applyFinalizeScript @applyArgs
} catch {
Write-Host " ⚠️ Failed to apply PR title/description (non-fatal): $_" -ForegroundColor Yellow
# The exception can carry agent-authored text from content.md, so it goes through
# the sanitizer like every other PR-derived console value in this script.
Write-Host " ⚠️ Failed to apply PR title/description (non-fatal): $(ConvertTo-AzdoSafeConsole "$_")" -ForegroundColor Yellow
}
} else {
Write-Host " ⚠️ apply-pr-finalize.ps1 not found — skipping" -ForegroundColor Yellow
Expand Down
47 changes: 45 additions & 2 deletions .github/scripts/apply-pr-finalize.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,48 @@ function Merge-PreservedBodyPreamble {
return "$preamble`n`n$RecommendedBody"
}

function New-ExclusiveTempFile {
<#
.SYNOPSIS
Creates a new, uniquely-named temp file, failing if the path already exists.
.DESCRIPTION
Comment on lines +258 to +261
`Set-Content` follows a pre-existing symlink and writes through to its target, so a
predictable temp path (pr-finalize-body-<PR>.md) is a write-through primitive if
anything can pre-create it. Reaching that requires arbitrary filesystem write as the
agent user, which already grants strictly more capability — but the fix is cheap, so
close it anyway rather than relying on that argument holding.

Prefers the AzDO agent temp directory over the shared system temp when available.
Creating with New-Item (no -Force) fails when the path exists, including when it is a
dangling or pre-planted symlink, so the write cannot be redirected.
.OUTPUTS
Full path to the newly created file.
#>
param(
[Parameter(Mandatory = $true)]
[string]$Prefix
)

$baseDir = if ($env:AGENT_TEMPDIRECTORY -and (Test-Path -LiteralPath $env:AGENT_TEMPDIRECTORY)) {
$env:AGENT_TEMPDIRECTORY
} else {
[System.IO.Path]::GetTempPath()
}

# Retry only guards against an astronomically unlikely name collision.
for ($attempt = 0; $attempt -lt 5; $attempt++) {
$candidate = Join-Path $baseDir "$Prefix-$([System.IO.Path]::GetRandomFileName()).md"
try {
$file = New-Item -ItemType File -Path $candidate -ErrorAction Stop
return $file.FullName
} catch {
continue
}
Comment on lines +299 to +309
}

throw "Could not create a temp file under '$baseDir' after 5 attempts."
}

# ─── Main ───────────────────────────────────────────────────────────────────────
# Dot-sourced by the Pester suite to test the helpers above without executing the flow.
if ($MyInvocation.InvocationName -eq '.') { return }
Expand Down Expand Up @@ -340,8 +382,9 @@ if ($DryRun) {
exit 0
}

$bodyFile = Join-Path ([System.IO.Path]::GetTempPath()) "pr-finalize-body-$PRNumber.md"
$bodyFile = $null
try {
$bodyFile = New-ExclusiveTempFile -Prefix "pr-finalize-body-$PRNumber"
$newBody | Set-Content -LiteralPath $bodyFile -Encoding UTF8

$ghArgs = @('pr', 'edit', "$PRNumber", '--repo', $Repo)
Expand All @@ -359,7 +402,7 @@ try {
} catch {
Write-Host " ⚠️ Failed to apply the PR finalize recommendation (non-fatal): $(ConvertTo-AzdoSafeConsole "$_")" -ForegroundColor Yellow
} finally {
Remove-Item -LiteralPath $bodyFile -Force -ErrorAction SilentlyContinue
if ($bodyFile) { Remove-Item -LiteralPath $bodyFile -Force -ErrorAction SilentlyContinue }
}

exit 0
Loading