From eb765c090929e19812e08166d4cae41aa8e99d7e Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Fri, 19 Jun 2026 15:31:33 -0500 Subject: [PATCH 1/6] Harden release-readiness ci-scan title rendering against embedded newlines Some CI Failure Scanner (`ci-scan`) issues have titles that contain a literal newline (observed live: #35957, whose title ends `(maui-pr-uitest\n[Content truncated due to length]`). Both readiness engines copy those titles into the "Recent CI Failure Scanner signals" markdown table; the embedded LF splits the table row across two physical lines and breaks the rendered table in the posted `[Release Readiness]` tracker issues (seen in net10-sr9 #35867 and net11-preview6 #35866). Root cause is upstream (the scanner producing a multi-line title), but the readiness engines should defensively sanitize external content they embed into their tables. Both cell formatters escaped `|` (and the preview one `<`/`>`) but neither stripped newlines. Fix: - Get-ReleaseReadiness.ps1 (`Format-CiScanIssueRows`): collapse `[\r\n]+` -> ' ' before escaping the title cell. - Get-PreviewReadiness.ps1 (`Format-MarkdownCell`): same collapse, applied in the shared cell formatter so every preview table cell is protected. Tests (offline, deterministic): a ci-scan title with an embedded newline now renders as a single table row in the SR engine, and `Format-MarkdownCell` collapses LF/CRLF runs while preserving the existing pipe/angle-bracket escaping contract. Verified these fail on the pre-fix code (4 reds) and pass after (offline suite 537 passed / 0 failed). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../scripts/Get-PreviewReadiness.ps1 | 5 +++- .../scripts/Get-ReleaseReadiness.ps1 | 5 +++- .../tests/Test-ReleaseReadiness.ps1 | 26 +++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/.github/skills/release-readiness/scripts/Get-PreviewReadiness.ps1 b/.github/skills/release-readiness/scripts/Get-PreviewReadiness.ps1 index 086788b95207..0eca27b1c797 100644 --- a/.github/skills/release-readiness/scripts/Get-PreviewReadiness.ps1 +++ b/.github/skills/release-readiness/scripts/Get-PreviewReadiness.ps1 @@ -851,7 +851,10 @@ function Format-MarkdownCell { # `List` that GitHub markdown would otherwise swallow as an HTML tag. The # engine's own markers are emitted via AppendLine, not through this formatter, # so escaping cells never disturbs them. - return (($Value -replace "\|", "\|") -replace "<", "<" -replace ">", ">").Trim() + # Collapse embedded newlines first: a malformed upstream title can contain a + # literal CR/LF (observed: ci-scan issue #35957), which would otherwise split + # the markdown table row across physical lines and break the rendered table. + return ((($Value -replace "[\r\n]+", " ") -replace "\|", "\|") -replace "<", "<" -replace ">", ">").Trim() } function Format-GitHubHandle { diff --git a/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 b/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 index abe8610bb359..364c49b6fe6d 100644 --- a/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 @@ -2842,7 +2842,10 @@ function Format-CiScanIssueRows { } } $issLink = "[#$($iss.number)]($RepoUrl/issues/$($iss.number))" - $title = ($iss.title -replace '\|', '\|').Trim() + # Collapse embedded newlines first: a malformed upstream ci-scan title can + # contain a literal CR/LF (observed: #35957), which would otherwise split this + # markdown table row across physical lines and break the rendered table. + $title = ($iss.title -replace '[\r\n]+', ' ' -replace '\|', '\|').Trim() [void]$sb.AppendLine("| $marker$issLink | $title | $ageDisplay |") } if ($Issues.Count -gt $MaxRows) { diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index 75774a18d0a9..e3721ac2f8a5 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -2263,6 +2263,21 @@ Assert-Eq -Label "Age column shows 'Nd ago' for older issues" -Expected $true ` Assert-Eq -Label "Format-CiScanIssueRows returns null for empty input" -Expected $true ` -Actual ($null -eq (Format-CiScanIssueRows -Issues @() -RepoUrl 'https://github.com/dotnet/maui')) +# Regression: a malformed upstream ci-scan title containing a literal newline +# (observed live: #35957) must NOT split the markdown table row across physical +# lines. The title cell is collapsed to a single line so the rendered table stays +# intact. On the pre-fix code the embedded LF pushed the title tail + age cell onto +# a second line that no longer contained the issue link. +$nlIssue = @([PSCustomObject]@{ number = 35957; url = 'https://github.com/dotnet/maui/issues/35957'; + title = "Recurring long title (maui-pr-uitest`n[Content truncated due to length]"; + createdAt = $nowUtc.AddDays(-3).ToString('o') }) +$nlRows = Format-CiScanIssueRows -Issues $nlIssue -RepoUrl 'https://github.com/dotnet/maui' +$nlRowLines = @($nlRows -split "`r?`n" | Where-Object { $_ -match '#35957' }) +Assert-Eq -Label "Newline ci-scan title: issue row is a single physical line" -Expected 1 ` + -Actual $nlRowLines.Count +Assert-Eq -Label "Newline ci-scan title: title tail + age stay on that row" -Expected $true ` + -Actual ($nlRowLines.Count -eq 1 -and $nlRowLines[0] -match 'truncated due to length.*ago \|') + # Truncation behavior: > MaxRows $manyIssues = 1..20 | ForEach-Object { [PSCustomObject]@{ number = 40000 + $_; url = "https://github.com/dotnet/maui/issues/$(40000+$_)"; @@ -3178,6 +3193,17 @@ try { $null = Get-CategorizedPullRequests -TargetPRs $null -InflightPRs @($null, catch { $explicitNullThrew = $true } Assert-Eq -Label "explicit null target + @(null, maestro) inflight → no throw" -Expected $false -Actual $explicitNullThrew +Write-Host "`n[Unit] Format-MarkdownCell collapses embedded newlines (table-row safety)" -ForegroundColor Cyan +# A malformed upstream title with a literal CR/LF (observed live: ci-scan issue +# #35957) must be collapsed to a single line so it cannot split the markdown table +# row in the rendered Preview tracker body. The existing pipe / angle-bracket +# escaping contract must remain intact. +Assert-Eq -Label "Format-MarkdownCell: LF collapsed to single space" -Expected 'a b' -Actual (Format-MarkdownCell "a`nb") +Assert-Eq -Label "Format-MarkdownCell: CRLF run collapsed to single space" -Expected 'a b' -Actual (Format-MarkdownCell "a`r`n`r`nb") +Assert-Eq -Label "Format-MarkdownCell: no CR/LF survives in the cell" -Expected $false -Actual ((Format-MarkdownCell "x`ny") -match "`r|`n") +Assert-Eq -Label "Format-MarkdownCell: pipe still escaped" -Expected 'a \| b' -Actual (Format-MarkdownCell 'a | b') +Assert-Eq -Label "Format-MarkdownCell: angle brackets still escaped" -Expected 'List<T>' -Actual (Format-MarkdownCell 'List') + Write-Host "`n────────────────────────────────────────" -ForegroundColor Cyan Write-Host "Passed: $script:passed Failed: $script:failed" -ForegroundColor $(if ($script:failed -eq 0) { 'Green' } else { 'Red' }) exit $(if ($script:failed -eq 0) { 0 } else { 1 }) From 03946d08370f36e16a6cd5b1df2eadefc4a8d2f4 Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Sat, 20 Jun 2026 11:34:29 -0500 Subject: [PATCH 2/6] Clarify discriminating vs non-discriminating ci-scan newline assertions Addresses non-blocking review feedback on PR #36031 (kubaflo multi-model review, round 1): note in the test comment + assertion labels that the first ci-scan-newline assertion is a coarse sanity check that also passes pre-fix, while the second (title tail + age stay on the row) is the real regression guard. Comment-only / label-only change; no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../tests/Test-ReleaseReadiness.ps1 | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index e3721ac2f8a5..2bc2f27cf299 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -2268,14 +2268,22 @@ Assert-Eq -Label "Format-CiScanIssueRows returns null for empty input" -Expected # lines. The title cell is collapsed to a single line so the rendered table stays # intact. On the pre-fix code the embedded LF pushed the title tail + age cell onto # a second line that no longer contained the issue link. +# +# Discrimination note: the FIRST assertion below ("issue row is a single physical +# line") is a coarse sanity check and is NON-discriminating — it also passes on the +# pre-fix code, because the split row still leaves '#35957' on exactly one physical +# line (the title tail + age spill onto a SEPARATE line with no issue link). The +# SECOND assertion ("title tail + age stay on that row") is the real regression +# guard: it fails pre-fix and passes post-fix. Do not weaken or remove it assuming +# the first assertion already covers row integrity. $nlIssue = @([PSCustomObject]@{ number = 35957; url = 'https://github.com/dotnet/maui/issues/35957'; title = "Recurring long title (maui-pr-uitest`n[Content truncated due to length]"; createdAt = $nowUtc.AddDays(-3).ToString('o') }) $nlRows = Format-CiScanIssueRows -Issues $nlIssue -RepoUrl 'https://github.com/dotnet/maui' $nlRowLines = @($nlRows -split "`r?`n" | Where-Object { $_ -match '#35957' }) -Assert-Eq -Label "Newline ci-scan title: issue row is a single physical line" -Expected 1 ` +Assert-Eq -Label "Newline ci-scan title: issue row is a single physical line (coarse sanity; non-discriminating)" -Expected 1 ` -Actual $nlRowLines.Count -Assert-Eq -Label "Newline ci-scan title: title tail + age stay on that row" -Expected $true ` +Assert-Eq -Label "Newline ci-scan title: title tail + age stay on that row (discriminating regression guard)" -Expected $true ` -Actual ($nlRowLines.Count -eq 1 -and $nlRowLines[0] -match 'truncated due to length.*ago \|') # Truncation behavior: > MaxRows From f07e14356bb4691b2f2f476c31852e12d968cec3 Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Sat, 20 Jun 2026 11:52:09 -0500 Subject: [PATCH 3/6] Route all SR title cells through shared Format-MarkdownTableCell sanitizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the ci-scan newline fix to the other two SR tables that embed upstream-controlled titles into pipe-delimited rows (review feedback on #36031, kubaflo multi-model review): the 'Open PRs Targeting ' table ($pr.title) and the regression classification table ($it.title). Both previously had NO pipe escaping or newline collapse, so a literal '|' (common in titles) or an embedded newline would corrupt the row today. Introduces Format-MarkdownTableCell — a single null-safe helper that collapses CR/LF runs and escapes pipes — and routes the ci-scan cell plus both sibling sites through it, replacing the ad-hoc inline escaping the reviewers flagged. The SR helper deliberately omits '<'/'>' escaping: unlike the hash-less Preview engine (whose Format-MarkdownCell must escape angle brackets to block an HTML-comment hash-freeze), the SR engine emits its own semantic hash at the top of the body and is structurally immune, so escaping '<>' there would only reduce title fidelity. (The third site the review cited, the 'Open Fix PRs Inbound' table, already escapes pipes at its render site — no change needed.) Tests: direct unit coverage of Format-MarkdownTableCell (pipe/newline/null/empty/trim + the deliberate no-angle-bracket parity), plus end-to-end Format-MarkdownReport assertions for both sibling tables. The end-to-end pipe/trailing-column assertions are discriminating (verified red on pre-fix sibling sites, green after). Offline suite 550/0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../scripts/Get-ReleaseReadiness.ps1 | 34 +++++++++-- .../tests/Test-ReleaseReadiness.ps1 | 57 +++++++++++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 b/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 index 364c49b6fe6d..55e7c2f9ae35 100644 --- a/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 @@ -2814,6 +2814,30 @@ function ConvertTo-LinkedPr { return "[#$PrNumber]($RepoUrl/pull/$PrNumber)" } +function Format-MarkdownTableCell { + <# + .SYNOPSIS + Sanitize an arbitrary (often upstream-controlled) string for safe use inside a + single Markdown table cell. + .DESCRIPTION + Two hazards are neutralized so a hostile/malformed issue or PR title cannot + corrupt the rendered table: + 1. Embedded CR/LF runs are collapsed to a single space, so the value cannot + split the row across physical lines (observed live: ci-scan issue #35957, + whose title contained a literal newline). + 2. Literal `|` is escaped to `\|`, so a pipe in a title cannot open a new + column (common in PR/issue titles such as `[Android] A | B`). + Mirrors the newline+pipe contract of Get-PreviewReadiness.ps1's + Format-MarkdownCell. The SR engine deliberately omits `<`/`>` escaping: it emits + its own semantic hash at the TOP of the body, so — unlike the hash-less Preview + engine — it is not exposed to the HTML-comment hash-freeze vector that makes + angle-bracket escaping load-bearing there. + #> + param([string]$Value) + if ([string]::IsNullOrEmpty($Value)) { return '' } + return (($Value -replace '[\r\n]+', ' ') -replace '\|', '\|').Trim() +} + function Format-CiScanIssueRows { <# .SYNOPSIS @@ -2842,10 +2866,10 @@ function Format-CiScanIssueRows { } } $issLink = "[#$($iss.number)]($RepoUrl/issues/$($iss.number))" - # Collapse embedded newlines first: a malformed upstream ci-scan title can - # contain a literal CR/LF (observed: #35957), which would otherwise split this - # markdown table row across physical lines and break the rendered table. - $title = ($iss.title -replace '[\r\n]+', ' ' -replace '\|', '\|').Trim() + # Sanitize the upstream ci-scan title for a single Markdown table cell: + # collapse embedded CR/LF (observed: #35957) and escape pipes. See + # Format-MarkdownTableCell for the full rationale (and why SR omits `<>`). + $title = Format-MarkdownTableCell $iss.title [void]$sb.AppendLine("| $marker$issLink | $title | $ageDisplay |") } if ($Issues.Count -gt $MaxRows) { @@ -3384,6 +3408,7 @@ function Format-MarkdownReport { [void]$sb.AppendLine('|---|---|---|---|---|---|') foreach ($pr in $Data['openSrPrs']) { $title = if ($pr.title.Length -gt 60) { $pr.title.Substring(0, 60) + '...' } else { $pr.title } + $title = Format-MarkdownTableCell $title $draft = if ($pr.isDraft) { '✏️' } else { '' } $rev = if ($pr.reviewDecision) { $pr.reviewDecision } else { '—' } $prLink = ConvertTo-LinkedPr -PrNumber $pr.number -RepoUrl $RepoUrl @@ -3437,6 +3462,7 @@ function Format-MarkdownReport { # Stable sort: by issue number ascending foreach ($it in ($items | Sort-Object issue)) { $title = if ($it.title.Length -gt 50) { $it.title.Substring(0, 50) + '...' } else { $it.title } + $title = Format-MarkdownTableCell $title $prList = @($it.candidateFixPrs | ForEach-Object { ConvertTo-LinkedPr -PrNumber $_.number -RepoUrl $RepoUrl }) -join ', ' if (-not $prList) { $prList = '—' } $issueLink = if ($RepoUrl) { "[#$($it.issue)]($RepoUrl/issues/$($it.issue))" } else { "#$($it.issue)" } diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index 2bc2f27cf299..dfb78af26ffe 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -1740,6 +1740,46 @@ Assert-Eq -Label "Hostile PR title: @another/user defanged to `another/user`" -E Assert-Eq -Label "Author column also defanged (no bare @jfversluis)" -Expected $true ` -Actual ($mdWithAt -match '`jfversluis`') +# ───── Sibling SR title cells must be sanitized too (Format-MarkdownTableCell) ───── +# Beyond the ci-scan rows, two other SR tables embed upstream titles into pipe-delimited +# rows: the "Open PRs Targeting " table and the regression classification +# table. A literal '|' (common in titles) or an embedded newline in those titles must NOT +# corrupt the row. Each integration test below is DISCRIMINATING on the pre-fix code: +# the trailing-column match fails when an embedded newline splits the row, and the +# escaped-pipe match fails when the pipe is left raw. +Write-Host "`n[Unit] Sibling SR table cells sanitized (Open-PRs + regression tables)" -ForegroundColor Cyan + +# (1) Open PRs Targeting table (shipped mode renders the full table). +$mdDataPipePr = @{} + $mdData +$mdDataPipePr['metadata'] = @{} + $mdData.metadata +$mdDataPipePr['metadata']['mode'] = 'shipped' +$mdDataPipePr['openSrPrs'] = @( + @{ number = 96001; title = "Fix A | B`nand C"; author = @{ login = 'alice' }; + isDraft = $false; reviewDecision = 'APPROVED'; updatedAt = '2026-06-01T00:00:00Z' } +) +$mdPipePr = Format-MarkdownReport -Data $mdDataPipePr -RepoUrl 'https://github.com/dotnet/maui' ` + -TrackerKey 'net10-sr7' -MaxBodyBytes 60000 +$prRowLines = @($mdPipePr -split "`r?`n" | Where-Object { $_ -match '#96001' }) +Assert-Eq -Label "Open-PRs table: piped/newline PR title stays on one physical row" -Expected 1 ` + -Actual $prRowLines.Count +Assert-Eq -Label "Open-PRs table: pipe escaped AND trailing columns intact on the row" -Expected $true ` + -Actual ($prRowLines.Count -eq 1 -and $prRowLines[0] -match 'Fix A \\\| B' -and $prRowLines[0] -match 'APPROVED') + +# (2) Regression classification table (needs-human-review is a Tier-2 class that renders). +$mdDataPipeIss = @{} + $mdData +$mdDataPipeIss['regressions'] = @( + @{ issue = 96002; title = "Crash | NRE`nin layout"; state = 'OPEN'; classification = 'needs-human-review'; + candidateFixPrs = @(); recommendedAction = 'Investigate' } +) +$mdDataPipeIss['summary'] = @{ 'needs-human-review' = 1 } +$mdPipeIss = Format-MarkdownReport -Data $mdDataPipeIss -RepoUrl 'https://github.com/dotnet/maui' ` + -TrackerKey 'net10-sr7' -MaxBodyBytes 60000 +$issRowLines = @($mdPipeIss -split "`r?`n" | Where-Object { $_ -match '#96002' }) +Assert-Eq -Label "Regression table: piped/newline issue title stays on one physical row" -Expected 1 ` + -Actual $issRowLines.Count +Assert-Eq -Label "Regression table: pipe escaped AND trailing action column intact on the row" -Expected $true ` + -Actual ($issRowLines.Count -eq 1 -and $issRowLines[0] -match 'Crash \\\| NRE' -and $issRowLines[0] -match 'Investigate') + # ───── Candidate-mode open-PR collapse: avoid noisy main-PR dump ───── Write-Host "`n[Unit] Candidate-mode open-PR collapse (link to candidate PR only)" -ForegroundColor Cyan @@ -2286,6 +2326,23 @@ Assert-Eq -Label "Newline ci-scan title: issue row is a single physical line (co Assert-Eq -Label "Newline ci-scan title: title tail + age stay on that row (discriminating regression guard)" -Expected $true ` -Actual ($nlRowLines.Count -eq 1 -and $nlRowLines[0] -match 'truncated due to length.*ago \|') +# ───── Format-MarkdownTableCell: shared SR table-cell sanitizer ───── +# This helper backs every SR title cell (ci-scan rows, Open-PRs table, regression +# classification table). It must collapse CR/LF (row-split safety) AND escape pipes +# (column-injection safety), null-safely, while deliberately NOT escaping `<`/`>` +# (the SR engine emits its own hash at the top of the body, so it has no Preview-style +# HTML-comment hash-freeze vector — escaping `<>` here would only reduce fidelity). +Write-Host "`n[Unit] Format-MarkdownTableCell (shared SR table-cell sanitizer)" -ForegroundColor Cyan +Assert-Eq -Label "Format-MarkdownTableCell: pipe escaped" -Expected 'a \| b' -Actual (Format-MarkdownTableCell 'a | b') +Assert-Eq -Label "Format-MarkdownTableCell: LF collapsed to space" -Expected 'a b' -Actual (Format-MarkdownTableCell "a`nb") +Assert-Eq -Label "Format-MarkdownTableCell: CRLF run collapsed" -Expected 'a b' -Actual (Format-MarkdownTableCell "a`r`n`r`nb") +Assert-Eq -Label "Format-MarkdownTableCell: newline + pipe together" -Expected 'a \| b' -Actual (Format-MarkdownTableCell "a`n| b") +Assert-Eq -Label "Format-MarkdownTableCell: no CR/LF survives" -Expected $false -Actual ((Format-MarkdownTableCell "x`ny") -match "`r|`n") +Assert-Eq -Label "Format-MarkdownTableCell: null → empty string" -Expected '' -Actual (Format-MarkdownTableCell $null) +Assert-Eq -Label "Format-MarkdownTableCell: empty → empty string" -Expected '' -Actual (Format-MarkdownTableCell '') +Assert-Eq -Label "Format-MarkdownTableCell: surrounding whitespace trimmed" -Expected 'a b' -Actual (Format-MarkdownTableCell " a`nb ") +Assert-Eq -Label "Format-MarkdownTableCell: angle brackets deliberately NOT escaped (SR parity)" -Expected 'List' -Actual (Format-MarkdownTableCell 'List') + # Truncation behavior: > MaxRows $manyIssues = 1..20 | ForEach-Object { [PSCustomObject]@{ number = 40000 + $_; url = "https://github.com/dotnet/maui/issues/$(40000+$_)"; From 0b7f7555cbdbc6476164af95ede2d5979d4e2635 Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Sat, 20 Jun 2026 12:32:59 -0500 Subject: [PATCH 4/6] Centralize all SR tracker table cells through Format-MarkdownTableCell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review of the ci-scan newline hardening found the centralization was incomplete: the Blocking summary, Cleanup, Open-Fix-PRs, and Ship-readiness checks tables, plus the candidate-PR bulleted list, still escaped pipes inline without collapsing newlines — leaving the row-split vector (live #35957) open on those tables and the candidate list. Route every remaining upstream-controlled cell through the shared Format-MarkdownTableCell sanitizer so the newline-collapse + pipe-escape contract applies uniformly. Only the helper itself now performs the inline escape. Security: an upstream title embedding `...\n\n...` could, pre-fix, isolate that marker on its own physical line and forge a second notes region — corrupting the workflow's full-line-anchored notes-preservation splice and wiping Release Captain Notes. Comprehensive newline collapse defeats this WITHOUT escaping `<>`, preserving deliberate `List` title fidelity (the SR engine is hash-freeze immune via its top-of-body hash, and the anchored marker regex can only fire when a marker lands alone on a line — which requires a newline). Tests: +9 discriminating assertions (Blocking/Open-Fix/Ship-checks-table row integrity + escaped pipes, and two marker-forgery regressions on a table cell and the candidate list asserting exactly one anchored begin-marker survives). 8 of the 9 go red against the pre-fix script; offline suite 559/0. Null-safety of `.title.Length` under StrictMode is pre-existing and deferred (titles are non-null by GitHub API contract). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../scripts/Get-ReleaseReadiness.ps1 | 61 +++++++---- .../tests/Test-ReleaseReadiness.ps1 | 103 ++++++++++++++++++ 2 files changed, 143 insertions(+), 21 deletions(-) diff --git a/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 b/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 index 55e7c2f9ae35..681da30e84e6 100644 --- a/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/scripts/Get-ReleaseReadiness.ps1 @@ -2818,20 +2818,33 @@ function Format-MarkdownTableCell { <# .SYNOPSIS Sanitize an arbitrary (often upstream-controlled) string for safe use inside a - single Markdown table cell. + single Markdown table cell. Also used for the candidate-PR bulleted list, where + the newline collapse matters and `\|` renders as `|`. .DESCRIPTION Two hazards are neutralized so a hostile/malformed issue or PR title cannot - corrupt the rendered table: + corrupt the rendered body: 1. Embedded CR/LF runs are collapsed to a single space, so the value cannot split the row across physical lines (observed live: ci-scan issue #35957, whose title contained a literal newline). 2. Literal `|` is escaped to `\|`, so a pipe in a title cannot open a new column (common in PR/issue titles such as `[Android] A | B`). - Mirrors the newline+pipe contract of Get-PreviewReadiness.ps1's - Format-MarkdownCell. The SR engine deliberately omits `<`/`>` escaping: it emits - its own semantic hash at the TOP of the body, so — unlike the hash-less Preview - engine — it is not exposed to the HTML-comment hash-freeze vector that makes - angle-bracket escaping load-bearing there. + Every SR markdown cell that embeds upstream-controlled text routes through this + single helper: the ci-scan rows, the Open-PRs / regression / Blocking / Cleanup / + ship-readiness-checks / Open-Fix-PRs tables, and the candidate-PR list. + + The SR engine deliberately omits `<`/`>` escaping (unlike Get-PreviewReadiness.ps1's + Format-MarkdownCell, whose newline+pipe contract this otherwise mirrors), preserving + title fidelity like `List`. That omission is safe because: + * Hash-freeze: SR emits its own semantic hash at the TOP of the body, so an + injected `` lower in the body can never win the workflow's + `head -n1` extraction — it is structurally immune (the Preview engine, being + hash-less, is not, which is why `<>` escaping is load-bearing there). + * Human-notes forgery: the workflow matches the `` preservation markers with FULL-LINE-ANCHORED regex (`^\s*\s*$`). + An injected marker can therefore only fire if it lands ALONE on a physical line, + which requires an embedded newline to break out of its surrounding row/list text. + The newline collapse in (1) removes that capability, so a raw `<>` in a title + cannot forge an anchored marker and wipe Release Captain Notes. #> param([string]$Value) if ([string]::IsNullOrEmpty($Value)) { return '' } @@ -3132,9 +3145,9 @@ function Format-MarkdownReport { [void]$sb.AppendLine('| Area | Details | Next action |') [void]$sb.AppendLine('|---|---|---|') foreach ($b in $blockingItems) { - $area = ($b.area -replace '\|', '\|').Trim() - $details = ($b.details -replace '\|', '\|').Trim() - $action = ($b.action -replace '\|', '\|').Trim() + $area = Format-MarkdownTableCell $b.area + $details = Format-MarkdownTableCell $b.details + $action = Format-MarkdownTableCell $b.action [void]$sb.AppendLine("| $area | $details | $action |") } [void]$sb.AppendLine() @@ -3169,9 +3182,9 @@ function Format-MarkdownReport { [void]$sb.AppendLine('| Area | Details | Next action |') [void]$sb.AppendLine('|---|---|---|') foreach ($c in $cleanupItems) { - $area = ($c.area -replace '\|', '\|').Trim() - $details = ($c.details -replace '\|', '\|').Trim() - $action = ($c.action -replace '\|', '\|').Trim() + $area = Format-MarkdownTableCell $c.area + $details = Format-MarkdownTableCell $c.details + $action = Format-MarkdownTableCell $c.action [void]$sb.AppendLine("| $area | $details | $action |") } [void]$sb.AppendLine() @@ -3272,11 +3285,11 @@ function Format-MarkdownReport { [void]$sb.AppendLine('| Fix PR | Base | Regression issue | Status | Next action |') [void]$sb.AppendLine('|---|---|---|---|---|') foreach ($row in $openFixRows) { - $prCell = ($row.prCell -replace '\|', '\|').Trim() - $baseCell = ($row.baseCell -replace '\|', '\|').Trim() - $issCell = ($row.issCell -replace '\|', '\|').Trim() - $statCell = ($row.statusCell -replace '\|', '\|').Trim() - $actCell = ($row.actionCell -replace '\|', '\|').Trim() + $prCell = Format-MarkdownTableCell $row.prCell + $baseCell = Format-MarkdownTableCell $row.baseCell + $issCell = Format-MarkdownTableCell $row.issCell + $statCell = Format-MarkdownTableCell $row.statusCell + $actCell = Format-MarkdownTableCell $row.actionCell [void]$sb.AppendLine("| $prCell | $baseCell | $issCell | $statCell | $actCell |") } [void]$sb.AppendLine() @@ -3297,9 +3310,9 @@ function Format-MarkdownReport { 'CLEANUP' { '🧹 CLEANUP' } default { "⚪ $($sc.Status)" } } - $area = ($sc.Area -replace '\|', '\|').Trim() - $details = ($sc.Details -replace '\|', '\|').Trim() - $action = ($sc.NextAction -replace '\|', '\|').Trim() + $area = Format-MarkdownTableCell $sc.Area + $details = Format-MarkdownTableCell $sc.Details + $action = Format-MarkdownTableCell $sc.NextAction [void]$sb.AppendLine("| $area | $statusEmoji | $details | $action |") } [void]$sb.AppendLine() @@ -3395,6 +3408,12 @@ function Format-MarkdownReport { foreach ($cp in $candidatePrs) { $cpLink = ConvertTo-LinkedPr -PrNumber $cp.number -RepoUrl $RepoUrl $cpTitle = if ($cp.title.Length -gt 80) { $cp.title.Substring(0, 80) + '...' } else { $cp.title } + # Collapse newlines (and escape pipes) even though this is a list, not a + # table: an upstream title with an embedded newline could otherwise push + # injected content (e.g. a forged `` + # marker) onto its own physical line. Markdown renders `\|` as `|` in a + # list, so escaping is harmless here. + $cpTitle = Format-MarkdownTableCell $cpTitle [void]$sb.AppendLine("- $cpLink — $cpTitle (by $(Format-GitHubHandle $cp.author.login), updated $($cp.updatedAt))") } [void]$sb.AppendLine() diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index dfb78af26ffe..d8d392b0bb50 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -1780,6 +1780,88 @@ Assert-Eq -Label "Regression table: piped/newline issue title stays on one physi Assert-Eq -Label "Regression table: pipe escaped AND trailing action column intact on the row" -Expected $true ` -Actual ($issRowLines.Count -eq 1 -and $issRowLines[0] -match 'Crash \\\| NRE' -and $issRowLines[0] -match 'Investigate') +# ───── Blocking summary + Open-Fix-PRs cells sanitized; human-notes marker-forgery defense ───── +# The remaining SR tables that embed upstream titles — the 🔴 Blocking summary +# (Tier-1 regressions + BLOCKED ship-checks) and the 📥 Open Fix PRs Inbound table — +# plus the candidate-PR bulleted list now route every upstream cell through +# Format-MarkdownTableCell. Each assertion below is DISCRIMINATING on pre-fix code: +# an embedded newline splits the row (orphaning the title tail onto its own line), +# and the escaped-pipe match fails when the pipe is left raw. +# +# The marker-forgery assertions are the security centerpiece: the production workflow +# preserves Release-Captain notes by splicing on FULL-LINE-ANCHORED markers +# (^\s*\s*$). A hostile title containing +# `...\n\n...` would, pre-fix, isolate that marker on its own physical +# line and forge a second notes region — letting an attacker's PR/issue title corrupt the +# notes-preservation step. Collapsing newlines defeats this WITHOUT escaping `<>` (so +# legitimate `List` titles stay intact). We assert exactly ONE anchored begin-marker +# survives (the real one the renderer emits) even when an upstream title embeds the marker. +Write-Host "`n[Unit] Blocking/Open-Fix cells sanitized + human-notes marker-forgery defense" -ForegroundColor Cyan + +# (3) Blocking summary table (Tier-1 regression: no-fix-yet + OPEN renders here AND in the tier table). +$mdDataBlock = @{} + $mdData +$mdDataBlock['regressions'] = @( + @{ issue = 96003; title = "Hang | freeze`nat startup"; state = 'OPEN'; classification = 'no-fix-yet'; + candidateFixPrs = @(); recommendedAction = 'Fix before ship' } +) +$mdDataBlock['summary'] = @{ 'no-fix-yet' = 1 } +$mdBlock = Format-MarkdownReport -Data $mdDataBlock -RepoUrl 'https://github.com/dotnet/maui' ` + -TrackerKey 'net10-sr7' -MaxBodyBytes 60000 +$blockOrphans = @($mdBlock -split "`r?`n" | Where-Object { $_ -match '^\s*at startup' }) +Assert-Eq -Label "Blocking table: embedded newline does NOT orphan the title tail onto its own line" -Expected 0 ` + -Actual $blockOrphans.Count +Assert-Eq -Label "Blocking table: title rendered glued + pipe-escaped on one row" -Expected $true ` + -Actual ($mdBlock -match 'Hang \\\| freeze at startup') + +# (4) Open Fix PRs Inbound table (open-on-main regression with an OPEN candidate fix PR). +$mdDataOpenFix = @{} + $mdData +$mdDataOpenFix['regressions'] = @( + @{ issue = 96004; title = "Glitch | bug`nhere"; state = 'OPEN'; classification = 'open-on-main'; + candidateFixPrs = @( @{ number = 96104; state = 'OPEN'; baseRef = 'main' } ); recommendedAction = 'Watch' } +) +$mdDataOpenFix['summary'] = @{ 'open-on-main' = 1 } +$mdOpenFix = Format-MarkdownReport -Data $mdDataOpenFix -RepoUrl 'https://github.com/dotnet/maui' ` + -TrackerKey 'net10-sr7' -MaxBodyBytes 60000 +$ofOrphans = @($mdOpenFix -split "`r?`n" | Where-Object { $_ -match '^\s*here' }) +Assert-Eq -Label "Open-Fix-PRs table: embedded newline does NOT orphan the title tail onto its own line" -Expected 0 ` + -Actual $ofOrphans.Count +$ofRow = @($mdOpenFix -split "`r?`n" | Where-Object { $_ -match '🔵 OPEN — awaiting main merge' }) +Assert-Eq -Label "Open-Fix-PRs table: regression-issue cell glued + pipe-escaped, status column intact" -Expected $true ` + -Actual ($ofRow.Count -eq 1 -and $ofRow[0] -match 'Glitch \\\| bug here') + +# (5) Marker-forgery via a TABLE cell: a Tier-1 title embedding the begin-marker between +# newlines must NOT forge a second anchored marker line. +$mdDataForgeTbl = @{} + $mdData +$mdDataForgeTbl['regressions'] = @( + @{ issue = 96006; title = "Spoof`n`ntail"; state = 'OPEN'; + classification = 'no-fix-yet'; candidateFixPrs = @(); recommendedAction = 'Investigate' } +) +$mdDataForgeTbl['summary'] = @{ 'no-fix-yet' = 1 } +$mdForgeTbl = Format-MarkdownReport -Data $mdDataForgeTbl -RepoUrl 'https://github.com/dotnet/maui' ` + -TrackerKey 'net10-sr7' -MaxBodyBytes 60000 +$forgeTblMarkers = @($mdForgeTbl -split "`r?`n" | Where-Object { $_ -match '^\s*\s*$' }) +Assert-Eq -Label "Marker-forgery (table cell): exactly ONE anchored begin-marker survives (the legit one)" -Expected 1 ` + -Actual $forgeTblMarkers.Count + +# (6) Marker-forgery via the candidate-PR LIST (the bulleted site, not a table). The title +# must match \bcandidate\b to be selected, and embeds the marker between newlines. +$mdDataForgeList = @{} + $mdData +$mdDataForgeList['metadata'] = @{} + $mdData.metadata +$mdDataForgeList['metadata']['mode'] = 'candidate' +$mdDataForgeList['metadata']['priorSrBranch'] = 'release/10.0.1xx-sr7' +$mdDataForgeList['metadata']['srBranch'] = 'main' +$mdDataForgeList['openSrPrs'] = @( + @{ number = 96005; title = "Candidate`n`ntail"; + author = @{ login = 'mallory' }; isDraft = $false; reviewDecision = 'APPROVED'; updatedAt = '2026-06-01T00:00:00Z' } +) +$mdForgeList = Format-MarkdownReport -Data $mdDataForgeList -RepoUrl 'https://github.com/dotnet/maui' ` + -TrackerKey 'net10-sr8' -MaxBodyBytes 60000 +$forgeListMarkers = @($mdForgeList -split "`r?`n" | Where-Object { $_ -match '^\s*\s*$' }) +Assert-Eq -Label "Marker-forgery (candidate list): exactly ONE anchored begin-marker survives (the legit one)" -Expected 1 ` + -Actual $forgeListMarkers.Count +Assert-Eq -Label "Candidate list: hostile title collapsed onto the bullet line (no isolated tail)" -Expected 0 ` + -Actual (@($mdForgeList -split "`r?`n" | Where-Object { $_ -match '^\s*tail\b' }).Count) + # ───── Candidate-mode open-PR collapse: avoid noisy main-PR dump ───── Write-Host "`n[Unit] Candidate-mode open-PR collapse (link to candidate PR only)" -ForegroundColor Cyan @@ -1904,6 +1986,27 @@ $mdReady = Format-MarkdownReport -Data $mdDataReady -RepoUrl 'https://github.com Assert-Eq -Label "All ship checks READY (no Tier 1 regressions): '🟢 No blocking items'" -Expected $true ` -Actual ($mdReady -match '🟢 No blocking items') +# Ship-readiness checks TABLE: a WATCH check (renders only in the table, not the blocking +# summary) whose Details carry a literal pipe + embedded newline must stay on one row, +# pipe-escaped — proving the table's Details/NextAction cells route through the sanitizer. +$mdDataWatchCell = @{} + $mdData +$mdDataWatchCell['shipChecks'] = @( + [PSCustomObject]@{ + Area = 'Maestro channel' + Status = 'WATCH' + Details = "Default channel A | B`nnot yet mapped" + NextAction = 'Verify mapping' + } +) +$mdWatchCell = Format-MarkdownReport -Data $mdDataWatchCell -RepoUrl 'https://github.com/dotnet/maui' ` + -TrackerKey 'net10-sr7' -MaxBodyBytes 60000 +$watchOrphans = @($mdWatchCell -split "`r?`n" | Where-Object { $_ -match '^\s*not yet mapped' }) +Assert-Eq -Label "Ship-checks table: embedded newline in Details does NOT orphan a line" -Expected 0 ` + -Actual $watchOrphans.Count +$watchRow = @($mdWatchCell -split "`r?`n" | Where-Object { $_ -match '🟡 WATCH' }) +Assert-Eq -Label "Ship-checks table: Details glued + pipe-escaped, NextAction column intact" -Expected $true ` + -Actual ($watchRow.Count -eq 1 -and $watchRow[0] -match 'Default channel A \\\| B not yet mapped' -and $watchRow[0] -match 'Verify mapping') + # Hash includes shipChecks state (changing a ship check status flips the hash) $h1 = if ($mdReady -match '') { $Matches[1] } else { $null } $h2 = if ($mdBlocked -match '') { $Matches[1] } else { $null } From 769d79b6f30e4ea7a64f7fb7bf0b2b43748483c9 Mon Sep 17 00:00:00 2001 From: Shane Neuville <5375137+PureWeen@users.noreply.github.com> Date: Sat, 20 Jun 2026 15:28:17 -0500 Subject: [PATCH 5/6] Harden table-cell escaping: backslash-first pipe escape + SR <> entity escaping Two correctness/robustness fixes surfaced by adversarial review of the cell sanitizers this PR centralizes: 1. Escape-the-escaper table breakout (both engines). A GitHub issue/PR title may legally contain a literal \| (backslash + pipe). Escaping only the pipe turned that into \\| which GFM renders as a literal backslash followed by an ACTIVE column delimiter, breaking the row out into extra columns. Double pre-existing backslashes BEFORE escaping pipes so \| -> \\\| (renders literal \|). No-backslash titles are unchanged (a | b -> a \| b), so existing assertions hold. 2. SR <>/HTML escaping (Get-ReleaseReadiness.ps1). Format-MarkdownTableCell now escapes < and > to </>, matching Get-PreviewReadiness.ps1's Format-MarkdownCell. Zero visual cost (List<T> renders as List) and defense-in-depth: an injected ` comment) into the body. This matches the Preview + engine's Format-MarkdownCell, has zero visual cost (`<T>` renders as + ``, preserving titles like `List`), and is defense-in-depth: even + though SR is structurally hash-freeze-immune (it emits its own semantic hash + at the TOP of the body, so an injected lower `` can never + win the workflow's `head -n1` extraction) and its human-notes markers are + matched FULL-LINE-ANCHORED (a forged marker only fires if it lands alone on a + physical line, which hazard 1 already prevents), escaping `<>` keeps SR and + Preview consistent and removes any reliance on those backend invariants. Every SR markdown cell that embeds upstream-controlled text routes through this single helper: the ci-scan rows, the Open-PRs / regression / Blocking / Cleanup / ship-readiness-checks / Open-Fix-PRs tables, and the candidate-PR list. - - The SR engine deliberately omits `<`/`>` escaping (unlike Get-PreviewReadiness.ps1's - Format-MarkdownCell, whose newline+pipe contract this otherwise mirrors), preserving - title fidelity like `List`. That omission is safe because: - * Hash-freeze: SR emits its own semantic hash at the TOP of the body, so an - injected `` lower in the body can never win the workflow's - `head -n1` extraction — it is structurally immune (the Preview engine, being - hash-less, is not, which is why `<>` escaping is load-bearing there). - * Human-notes forgery: the workflow matches the `` preservation markers with FULL-LINE-ANCHORED regex (`^\s*\s*$`). - An injected marker can therefore only fire if it lands ALONE on a physical line, - which requires an embedded newline to break out of its surrounding row/list text. - The newline collapse in (1) removes that capability, so a raw `<>` in a title - cannot forge an anchored marker and wipe Release Captain Notes. #> param([string]$Value) if ([string]::IsNullOrEmpty($Value)) { return '' } - return (($Value -replace '[\r\n]+', ' ') -replace '\|', '\|').Trim() + return (((($Value -replace '[\r\n]+', ' ') -replace '\\', '\\') -replace '\|', '\|') -replace '<', '<' -replace '>', '>').Trim() } function Format-CiScanIssueRows { diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index d8d392b0bb50..2c9b9020d01b 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -1793,9 +1793,11 @@ Assert-Eq -Label "Regression table: pipe escaped AND trailing action column inta # (^\s*\s*$). A hostile title containing # `...\n\n...` would, pre-fix, isolate that marker on its own physical # line and forge a second notes region — letting an attacker's PR/issue title corrupt the -# notes-preservation step. Collapsing newlines defeats this WITHOUT escaping `<>` (so -# legitimate `List` titles stay intact). We assert exactly ONE anchored begin-marker -# survives (the real one the renderer emits) even when an upstream title embeds the marker. +# notes-preservation step. Collapsing newlines defeats this (the marker can no longer land +# alone on a line), and escaping `<>` to entities is belt-and-suspenders (the injected +# `` region). +Assert-Eq -Label "Format-MarkdownTableCell: HTML-comment opener neutralized" -Expected 'Crash <!--' -Actual (Format-MarkdownTableCell 'Crash ` comment) into the body. This matches the Preview engine's Format-MarkdownCell, has zero visual cost (`<T>` renders as ``, preserving titles like `List`), and is defense-in-depth: even @@ -2852,7 +2855,12 @@ function Format-MarkdownTableCell { #> param([string]$Value) if ([string]::IsNullOrEmpty($Value)) { return '' } - return (((($Value -replace '[\r\n]+', ' ') -replace '\\', '\\') -replace '\|', '\|') -replace '<', '<' -replace '>', '>').Trim() + $v = $Value -replace '[\r\n]+', ' ' + # Escape each pipe AND double only the backslash run immediately before it, so a + # pre-existing `\|` cannot survive as `\\|` (literal `\` + active delimiter). Non- + # pipe backslash escapes elsewhere in the title are left intact. + $v = [regex]::Replace($v, '(\\*)\|', { param($m) ($m.Groups[1].Value * 2) + '\|' }) + return ($v -replace '<', '<' -replace '>', '>').Trim() } function Format-CiScanIssueRows { diff --git a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 index 2c9b9020d01b..e6767e10b721 100644 --- a/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 +++ b/.github/skills/release-readiness/tests/Test-ReleaseReadiness.ps1 @@ -2451,7 +2451,8 @@ Assert-Eq -Label "Format-MarkdownTableCell: angle brackets escaped to entities ( # already contains a literal `\|` must NOT collapse to `\\|` (literal `\` + ACTIVE pipe). # Pre-fix (pipe-only escape) returns 'A \\| B' and these go red. Assert-Eq -Label "Format-MarkdownTableCell: literal backslash-pipe does NOT break out (doubled backslash)" -Expected 'A \\\| B' -Actual (Format-MarkdownTableCell 'A \| B') -Assert-Eq -Label "Format-MarkdownTableCell: pre-existing backslash doubled" -Expected 'C:\\dir' -Actual (Format-MarkdownTableCell 'C:\dir') +Assert-Eq -Label "Format-MarkdownTableCell: pre-existing NON-pipe backslash preserved (doubling is scoped to pipe-adjacent runs)" -Expected 'C:\dir' -Actual (Format-MarkdownTableCell 'C:\dir') +Assert-Eq -Label "Format-MarkdownTableCell: author-escaped non-pipe Markdown NOT de-escaped" -Expected '\[link\](url)' -Actual (Format-MarkdownTableCell '\[link\](url)') # Injected HTML comment opener is rendered inert (cannot start an `` region). Assert-Eq -Label "Format-MarkdownTableCell: HTML-comment opener neutralized" -Expected 'Crash <!--' -Actual (Format-MarkdownTableCell 'Crash