Address post-merge review findings on the PR finalize apply step

.github/scripts/Apply-PRFinalize.Tests.ps1

@@ -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
@@ -317,3 +318,193 @@ 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
+        }
+    }
+
+    # These pin the actual finding-#2 behaviour. The earlier version of this test was
+    # vacuous: it never planted a symlink at a path the helper would try, so a
+    # predictable-name Set-Content write-through implementation still passed it. The
+    # -NameGenerator seam forces known candidates so a symlink can be planted precisely.
+    It 'refuses a pre-planted symlink and leaves its target untouched' {
+        $secret = Join-Path $script:SandboxDir 'secret-existing.txt'
+        Set-Content -LiteralPath $secret -Value 'ORIGINAL' -Encoding UTF8
+
+        $planted = Join-Path $script:SandboxDir 'pr-finalize-body-123-forced0.md'
+        New-Item -ItemType SymbolicLink -Path $planted -Target $secret | Out-Null
+
+        # $script: scope is required — a plain $i++ inside the scriptblock would mutate a
+        # local copy, so every attempt would re-request the planted name.
+        $script:ForcedIndex = 0
+        $path = New-ExclusiveTempFile -Prefix 'pr-finalize-body-123' -NameGenerator {
+            $n = "forced$($script:ForcedIndex)"; $script:ForcedIndex++; $n
+        }
+        try {
+            # It must have skipped the planted path entirely...
+            $path | Should -Not -Be $planted
+            'REPLACEMENT BODY' | Set-Content -LiteralPath $path -Encoding UTF8
+            # ...so the symlink target is untouched, and the link is still a link.
+            (Get-Content -Raw -LiteralPath $secret).Trim() | Should -Be 'ORIGINAL'
+            (Get-Item -LiteralPath $planted).LinkType | Should -Be 'SymbolicLink'
+            (Get-Content -Raw -LiteralPath $path).Trim() | Should -Be 'REPLACEMENT BODY'
+        } finally {
+            Remove-Item -LiteralPath $path -Force -ErrorAction SilentlyContinue
+            Remove-Item -LiteralPath $planted -Force -ErrorAction SilentlyContinue
+        }
+    }
+
+    It 'refuses a dangling pre-planted symlink rather than creating its target' {
+        $missingTarget = Join-Path $script:SandboxDir 'never-created.txt'
+        $planted = Join-Path $script:SandboxDir 'pr-finalize-body-123-dangle0.md'
+        New-Item -ItemType SymbolicLink -Path $planted -Target $missingTarget | Out-Null
+
+        $script:DangleIndex = 0
+        $path = New-ExclusiveTempFile -Prefix 'pr-finalize-body-123' -NameGenerator {
+            $n = "dangle$($script:DangleIndex)"; $script:DangleIndex++; $n
+        }
+        try {
+            $path | Should -Not -Be $planted
+            'REPLACEMENT BODY' | Set-Content -LiteralPath $path -Encoding UTF8
+            # Writing through a dangling link would have created the target.
+            Test-Path -LiteralPath $missingTarget | Should -BeFalse
+        } finally {
+            Remove-Item -LiteralPath $path -Force -ErrorAction SilentlyContinue
+            Remove-Item -LiteralPath $planted -Force -ErrorAction SilentlyContinue
+        }
+    }
+
+    It 'throws instead of falling back to a predictable path when every candidate is occupied' {
+        $secret = Join-Path $script:SandboxDir 'secret-exhaust.txt'
+        Set-Content -LiteralPath $secret -Value 'ORIGINAL' -Encoding UTF8
+
+        $planted = 0..4 | ForEach-Object {
+            $link = Join-Path $script:SandboxDir "pr-finalize-body-123-exhaust$_.md"
+            New-Item -ItemType SymbolicLink -Path $link -Target $secret | Out-Null
+            $link
+        }
+
+        try {
+            $script:ExhaustIndex = 0
+            { New-ExclusiveTempFile -Prefix 'pr-finalize-body-123' -NameGenerator {
+                $n = "exhaust$($script:ExhaustIndex)"; $script:ExhaustIndex++; $n
+            } } | Should -Throw -ExpectedMessage '*after 5 attempts*'
+            # No fallback path was written, so the symlink target is still intact.
+            (Get-Content -Raw -LiteralPath $secret).Trim() | Should -Be 'ORIGINAL'
+        } finally {
+            $planted | ForEach-Object { Remove-Item -LiteralPath $_ -Force -ErrorAction SilentlyContinue }
+        }
+    }
+
+    It 'tries exactly MaxAttempts candidates before giving up' {
+        $secret = Join-Path $script:SandboxDir 'secret-count.txt'
+        Set-Content -LiteralPath $secret -Value 'ORIGINAL' -Encoding UTF8
+        $planted = 0..4 | ForEach-Object {
+            $link = Join-Path $script:SandboxDir "pr-finalize-body-123-count$_.md"
+            New-Item -ItemType SymbolicLink -Path $link -Target $secret | Out-Null
+            $link
+        }
+
+        try {
+            $script:Calls = 0
+            { New-ExclusiveTempFile -Prefix 'pr-finalize-body-123' -NameGenerator { $n = "count$($script:Calls)"; $script:Calls++; $n } } |
+                Should -Throw
+            $script:Calls | Should -Be 5
+        } finally {
+            $planted | ForEach-Object { Remove-Item -LiteralPath $_ -Force -ErrorAction SilentlyContinue }
+        }
+    }
+
+    It 'surfaces a non-collision failure immediately instead of retrying it away' {
+        # A missing base directory can never be resolved by picking another name, so it must
+        # propagate rather than be masked by the generic "after N attempts" message.
+        $saved = $env:AGENT_TEMPDIRECTORY
+        $env:AGENT_TEMPDIRECTORY = $script:SandboxDir
+        try {
+            $script:Calls = 0
+            { New-ExclusiveTempFile -Prefix 'missing-dir/nope/body' -NameGenerator { $script:Calls++; 'x' } } |
+                Should -Throw -ExpectedMessage '*Could not find a part of the path*'
+            $script:Calls | Should -Be 1
+        } finally {
+            $env:AGENT_TEMPDIRECTORY = $saved
+        }
+    }
+
+    It 'ignores AGENT_TEMPDIRECTORY when it points at a file rather than a directory' {
+        $saved = $env:AGENT_TEMPDIRECTORY
+        $asFile = Join-Path $script:SandboxDir 'not-a-directory.txt'
+        Set-Content -LiteralPath $asFile -Value 'x' -Encoding UTF8
+        $env:AGENT_TEMPDIRECTORY = $asFile
+        try {
+            $path = New-ExclusiveTempFile -Prefix 'pr-finalize-body-123'
+            try {
+                Test-Path -LiteralPath $path | Should -BeTrue
+                (Split-Path -Parent $path) | Should -Not -Be $asFile
+            } finally {
+                Remove-Item -LiteralPath $path -Force -ErrorAction SilentlyContinue
+            }
+        } finally {
+            $env:AGENT_TEMPDIRECTORY = $saved
+        }
+    }
+
+    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
+        }
+    }
+}

.github/scripts/Review-PR.ps1

@@ -2413,7 +2413,11 @@ 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
+            # Backstop, not a live path: the child sanitizes its own console output and its
+            # one throw carries no PR-derived text today. Kept because a future throw that
+            # quotes the recommendation would otherwise reach stdout unsanitized, and every
+            # other console sink in this script already goes through the sanitizer.
+            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

.github/scripts/apply-pr-finalize.ps1

@@ -254,6 +254,66 @@ function Merge-PreservedBodyPreamble {
     return "$preamble`n`n$RecommendedBody"
 }
 
+function New-ExclusiveTempFile {
+    <#
+    .SYNOPSIS
+        Creates a temp file at a fresh, unpredictable path, never writing to one that exists.
+    .DESCRIPTION
+        `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) refuses any path that already exists, including a
+        pre-planted or dangling symlink, so the write cannot be redirected. An occupied path
+        is skipped for a fresh random name; after $MaxAttempts the helper throws rather than
+        falling back to a predictable path, so exhaustion can never reopen the vector.
+    .PARAMETER NameGenerator
+        Test seam only. Lets a test force known candidate names so it can pre-plant a symlink
+        at the exact path the helper will try. Production uses random names.
+    .OUTPUTS
+        Full path to the newly created file.
+    #>
+    param(
+        [Parameter(Mandatory = $true)]
+        [string]$Prefix,
+
+        [scriptblock]$NameGenerator = { [System.IO.Path]::GetRandomFileName() },
+
+        [int]$MaxAttempts = 5
+    )
+
+    # -PathType Container so a stale AGENT_TEMPDIRECTORY pointing at a *file* falls back
+    # cleanly instead of failing later inside New-Item.
+    $baseDir = if ($env:AGENT_TEMPDIRECTORY -and (Test-Path -LiteralPath $env:AGENT_TEMPDIRECTORY -PathType Container)) {
+        $env:AGENT_TEMPDIRECTORY
+    } else {
+        [System.IO.Path]::GetTempPath()
+    }
+
+    for ($attempt = 0; $attempt -lt $MaxAttempts; $attempt++) {
+        $candidate = Join-Path $baseDir "$Prefix-$(& $NameGenerator).md"
+        try {
+            $file = New-Item -ItemType File -Path $candidate -ErrorAction Stop
+            return $file.FullName
+        } catch [System.IO.DirectoryNotFoundException] {
+            # Derives from IOException, so it must be caught ahead of the collision case —
+            # a missing base directory will never resolve by picking another name.
+            throw
+        } catch [System.IO.IOException] {
+            # The path is occupied (regular file, or a pre-planted/dangling symlink). Skip it
+            # rather than write through, and try a different name.
+            continue
+        }
+        # Anything else (access denied, invalid path) is a real fault: let it surface
+        # unwrapped instead of being retried into a generic "after N attempts" message.
+    }
+
+    throw "Could not create a temp file under '$baseDir' after $MaxAttempts attempts."
+}
+
 # ─── Main ───────────────────────────────────────────────────────────────────────
 # Dot-sourced by the Pester suite to test the helpers above without executing the flow.
 if ($MyInvocation.InvocationName -eq '.') { return }
@@ -340,8 +400,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)
@@ -359,7 +420,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