Skip to content

Fix deterministic CI scan fingerprint publication - #36913

Merged
kubaflo merged 5 commits into
mainfrom
pureween-fix-ci-scan-fingerprints
Jul 30, 2026
Merged

Fix deterministic CI scan fingerprint publication#36913
kubaflo merged 5 commits into
mainfrom
pureween-fix-ci-scan-fingerprints

Conversation

@PureWeen

@PureWeen PureWeen commented Jul 29, 2026

Copy link
Copy Markdown
Member

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description of Change

  • inject canonical CI-scan fingerprint, match-count, and trusted evidence-key markers in the deterministic publisher instead of relying on agent-authored HTML comments
  • apply the same complete-manifest, frozen-evidence, all-or-nothing publisher architecture to both ci-status-main and ci-status-net11
  • separate countable raw failure evidence from synthetic provenance framing and bind canonical recurrence to publisher-derived full raw-evidence lines
  • normalize run-specific AzDO transport timestamps only for trusted azdo-log evidence, while preserving timestamps in non-AzDO failure messages
  • reject markerless issues as authoritative coverage and remove automatic markerless adoption, preventing shared boilerplate from suppressing a distinct failure
  • recognize legacy pipeline lines with no suffix, (ID N), or live (definition N) syntax using the trusted configured pipeline definition
  • reject pre-existing/evasive marker content and marker-like match_pattern variants; revalidate exact post-injection payloads at the GitHub write boundary
  • require complete Helix terminal evidence and bind deadletter placeholders to stable trusted work-item identity
  • align the merged report-only reconciler invariants with publisher-owned marker publication
  • add twin-aware publisher/collector execution tests and named mutation coverage for every security control

Root cause

PR #36848 added fail-closed manifest validation to the net11 scanner and exposed a pre-existing repo-wide publication defect. In run 30413273824, the agent job succeeded, but submit_ci_scan failed before any issue write because the compiled prompt did not contain the authored HTML-comment marker template.

Artifact agent (8709769921) contained 16/16 signatures with zero fingerprint-marker-prefix and zero canonical-marker matches. gh-aw strips literal HTML comments while compiling the authored prompt, so regenerating the lock or strengthening prompt prose cannot make agent-side marker emission reliable. Output-side safe-output stripping is not needed to explain this incident.

Main had the same silent blast radius: sampled issues #36858, #36779, #36709, and #36689 carry no fingerprint marker, but its permissive publisher did not validate the payload. Net11's all-or-nothing gate correctly prevented every write, so the first post-merge run published zero issues.

Architecture

The shared trusted validator resolves a hard-coded scanner configuration for ci-scan|main or ci-scan-net11|net11.0. For each filed manifest entry it:

  1. validates fingerprint provenance, body shape, complete manifest coverage, frozen build/log provenance, and the five-issue mutation cap
  2. rejects pre-existing fingerprint/match-count/evidence-key content and marker-like match_pattern variants, including spacing, case, zero-width, separator, HTML-comment-like, and Unicode-homoglyph evasions
  3. counts matches only in structured .evidence.json raw segments; rendered .log files retain AzDO/Helix provenance for diagnosis, but synthetic headers are not countable evidence
  4. normalizes and hashes each complete raw line containing the match pattern, derives a domain-separated SHA-256 evidence key, and requires the issue body to contain a complete trusted raw-evidence line
  5. injects exactly one fingerprint marker from the validated manifest, one match-count marker from the trusted recount, and one evidence-key marker from the trusted raw-line hashes
  6. validates the exact post-injection body before producing the plan

AzDO's log API prepends a different UTC transport timestamp to each stored line on every build. PowerShell strips that prefix only when structured provenance says the segment is azdo-log; Helix and other message timestamps remain identity-bearing. At the write boundary, publisher body matching computes both raw and AzDO-normalized candidates against the trusted plan hash. The same failure therefore keeps its evidence identity across builds while real non-AzDO timestamps remain distinct.

Both compiled publisher jobs bind the plan to their trusted scanner ID, branch, and label; preflight every issue/reference before any mutation; preserve canonical marker retry/dedup; and revalidate GitHub's stored response. Canonical recurrence requires the exact fingerprint and evidence-key markers plus a current trusted evidence line.

Markerless legacy issues no longer provide authoritative coverage. Their exact pipeline/evidence shape is still recognized for a precise migration error, including no suffix, (ID N), and the live (definition N) suffix with the correct configured definition. An explicit markerless existing reference aborts before any write, and a filed payload never auto-adopts a markerless issue. It instead creates bounded visible canonical coverage. This is intentionally safer than silently merging two same-pipeline failures that share boilerplate such as Build FAILED.

The frozen evidence collector treats a Helix job as complete only when the job has a terminal Finished value, Waiting and Running are zero, and every returned work item is terminal with valid completion evidence. Helix's cumulative Unscheduled counter may remain nonzero after completion and is validated but not treated as active work. AzDO build records with missing or invalid finishTime fail closed. Structured evidence enforces matching producer/consumer caps of 200 segments, 25 MB, and 200 distinct matching lines.

A deadletter placeholder URL contains no run-specific diagnostics and is constant across Helix. The countable evidence line includes the validated stable work-item name plus that URL. This distinguishes unrelated work items while deliberately excluding job/build IDs so recurrence for the same work item remains stable across builds. Deadletters still mark their AzDO submission log as a failed leaf, so absence-only coverage remains forbidden.

The branch is based on current main after PR #36850. Its report-only reconciler asserts that both scanner twins compile trusted validation before publisher-side exact-marker checks, rather than expecting an agent marker template. The reconciler still has no production state-marker writer, so stale-issue closure candidates remain unreachable.

Review findings resolved

  • Universal synthetic evidence header: confirmed; synthetic framing is structurally excluded from countable evidence.
  • Marker-like match replay: confirmed; marker-like patterns fail across exact, spacing, case, zero-width, and homoglyph variants.
  • Constant deadletter identity: confirmed; fixed placeholder content is bound to trusted stable work-item identity.
  • AzDO timestamp-sensitive identity: confirmed; trusted azdo-log transport timestamps are removed symmetrically from PowerShell proof generation and JavaScript body matching.
  • Live legacy (definition N) suffix: confirmed; exact no-suffix, (ID N), and (definition N) forms are recognized for all three configured pipelines and both twins, and a wrong definition is rejected.
  • Generic markerless evidence collision: confirmed; markerless explicit coverage and automatic adoption are disabled rather than relying on fragile length/entropy heuristics.
  • Helix active counts: confirmed defense-in-depth; terminal evidence requires zero Waiting and Running while allowing cumulative Unscheduled.
  • Concurrency overlap note: not reproduced. A fixed GitHub concurrency group permits one running and one pending run; cancel-in-progress: false preserves the active publisher instead of allowing overlap.
  • Benign marker prose over-folding: intentionally unchanged. Its false-positive mode is an all-or-nothing batch abort, not silent issue suppression.

Tests

  • strict gh aw compile for both twins: 0 errors, 0 warnings
  • focused validator/publisher/mutation Pester: 225/225 passed
  • complete .github/scripts Pester: 1489/1489 passed
  • repeated strict compilation produced unchanged lock hashes
  • lock-extracted Node tests execute both compiled publishers and collectors, including raw-vs-synthetic evidence, canonical cross-build recurrence, markerless no-adoption/no-write behavior, exact legacy pipeline formats, unrelated deadletter replay, Helix terminality, no-partial-write batches, retry behavior, evidence caps, and twin symmetry
  • named mutations cover timestamp-sensitive identity, missing (definition N) support, re-enabled markerless explicit coverage, re-enabled markerless auto-adoption, removed injection, untrusted fingerprint/count sourcing, pre-injection-only validation, duplicate rejection removal, synthetic framing, marker-pattern rejection, trusted-state recurrence, evidence-identity removal, constant deadletter identity, omitted twins, and empty discovery
  • independent final code review found no high-confidence defects

There is no scanner-specific gh-aw behavioral eval runner in this repository, so deterministic Pester, lock-extracted Node execution, strict compilation, and static anti-vacuity invariants provide behavioral regression coverage.

Residual risk

Disabling markerless adoption can produce a bounded visible duplicate for a legacy issue until canonical coverage exists. This is intentional: without a publisher-owned historical identity, silently reusing a markerless issue is not a trustworthy dedup decision. Conservative marker-content and evidence-size gates may also fail an entire scan rather than truncate or publish partial evidence. These behaviors fail closed and produce zero partial writes.

No real ci-scan or ci-scan-net11 issue was mutated during development or validation.

Issues Fixed

No scanner tracking issue is closed by this infrastructure correction. Related incident: PR #36848 and Actions run 30413273824.

Copilot AI review requested due to automatic review settings July 29, 2026 16:41
@PureWeen
PureWeen temporarily deployed to copilot-pat-pool July 29, 2026 16:41 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36913

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36913"

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request hardens CI-scan issue publication by making fingerprint + match-count markers publisher-owned and deterministic, removing any reliance on agent prompt output for those markers. It also aligns the ci-status-main and ci-status-net11 workflows onto the same “frozen evidence + trusted validator + all-or-nothing publisher” architecture and adds extensive twin/mutation test coverage to keep the security controls from regressing.

Changes:

  • Move canonical fingerprint and match-count marker injection/verification into the trusted validator + publisher boundary, and fail closed on any marker-like agent body content.
  • Make ci-status-main use the same deterministic publisher flow as net11 (custom safe-output job + trusted validator + preflight + immutable publisher checkout).
  • Add twin-aware Pester suites and mutation tests to enforce symmetry between the two scanner workflows and prove the controls are load-bearing.
Show a summary per file
File Description
.github/workflows/ci-status-main.md Reworks main scanner to use the same trusted “submit-ci-scan” publisher architecture, including dry-run support and frozen evidence flow.
.github/workflows/ci-status-net11.md Wires in trusted scanner identity env + adds publisher-side canonical marker validation and updates agent instructions to be marker-free.
.github/workflows/ci-status-net11.lock.yml Regenerated compiled workflow with updated gh-aw tooling and the publisher-side marker validation logic wired into the lock.
.github/scripts/Validate-CiScanManifest.ps1 Centralizes twin configuration, rejects marker-like content, recomputes match counts from frozen evidence, injects canonical markers, and validates the post-injection payload.
.github/scripts/Validate-CiScanManifest.Tests.ps1 Updates/extends manifest tests for publisher-owned markers, evidence-required match counts, and twin configuration behavior.
.github/scripts/Validate-CiScanPublisher.Tests.ps1 Adds twin discovery + anti-divergence checks and executes extracted compiled publisher code to validate boundary invariants.
.github/scripts/CiScanTwins.Helpers.ps1 Adds helper functions to discover scanner twins and extract the compiled publisher script block for test execution.
.github/scripts/CiScanTrustedInventory.Tests.ps1 Extends trusted inventory collector tests to run across both twins and validate failed-leaf coverage behavior.
.github/scripts/CiScanMutation.Tests.ps1 Adds mutation testing to prove marker injection/rejection and twin discovery invariants fail closed when controls are removed.
.github/aw/actions-lock.json Updates the pinned gh-aw setup action version used by agentic workflow compilation.

Copilot's findings

  • Files reviewed: 10/11 changed files
  • Comments generated: 0

Copilot AI review requested due to automatic review settings July 29, 2026 17:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (1)

.github/workflows/ci-status-main.md:58

  • The safe-outputs comment references gh-aw v0.82.14, but this workflow is now compiled with v0.83.4 (see ci-status-main.lock.yml metadata). Keeping a hard-coded old version here is misleading; either update the version or make the note version-agnostic.
  # gh-aw v0.82.14 does not propagate staged mode into custom safe-output jobs.
  # Keep this expression identical to GH_AW_SAFE_OUTPUTS_STAGED below; tests enforce it.
  staged: ${{ github.event_name == 'workflow_dispatch' && inputs.dry_run == true }}
  • Files reviewed: 10/11 changed files
  • Comments generated: 0 new

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial security review (round 1) — Opus 4.8 + GPT-5.6 Sol, verified against 9636664. This is a solid, well-architected trust-boundary fix: moving marker injection into the deterministic publisher (agent can no longer forge ci-scan-fingerprint/match-count markers) with a complete-manifest, frozen-evidence, all-or-nothing gate is the right design, and marker forgery is genuinely closed. Two residual agent-gaming vectors remain, though — both let an adversarial/prompt-injected agent suppress a real CI failure, which is the exact threat model this PR exists for.

❌ Match-count/coverage bypass via the universal evidence header (Medium)

ci-status-main.md:595 prepends a synthetic prefix to every frozen log: evidence = ['===== AzDO log ${buildId}/${logId} =====', azdoLog]. Validate-CiScanManifest.ps1:400 counts coverage with a substring test ($line.Contains($MatchPattern)) over that evidence — including the synthetic line. So an agent can submit a signature with match_pattern: "===== AzDO log" (14 chars, ≥8) and the required log IDs; it matches every file, granting "coverage" without any real failure-content proof — letting failed leaves be marked covered so the scan files nothing. (The 9636664 terminality hardening doesn't touch this.) Fix (Sol's): exclude the synthetic header from countable evidence and require match_pattern to come from raw failure content.

match_pattern isn't marker-guarded → unrelated-issue replay (Medium)

Test-MarkerLikeContent is applied only to the agent rawBody (Validate-CiScanManifest.ps1:488), not to Get-ValidatedMatchPattern (:361, which checks only length + zero-width). Every ci-scan tracking issue body contains <!-- ci-scan-fingerprint: … -->, so if any scanned log line contains ci-scan-fingerprint, an agent can use it as match_pattern (passes the in-log check at :400) and copy an existing same-pipeline issue's full fingerprint + number (fingerprint binds only parts 0–2 = scanner/branch/pipeline at :338; identity/error/platform are agent-selected). The dedup path then finds the pattern in that issue's body (ci-status-main.md:268) and the exact marker (:271) → the current, unrelated failure is bound to it as canonical-fingerprint and suppressed. Fix (Sol's): apply the marker guard to match_pattern, ignore trusted marker lines during the recurrence body-search, and derive fingerprint identity from trusted evidence rather than agent-selected fields.

ℹ️ Lower-severity (Opus)

  • Test-MarkerLikeContent folds to bare alphanumerics globally, so benign prose like "See ci scan fingerprint below" is rejected (verified) → aborts the all-or-nothing batch. Fail-closed (no spoofing) and real CI excerpts rarely trigger it, but a word-boundary check on the hyphenated token would reduce false batch-aborts.
  • The legacy (marker-less) dedup path adopts an already-ci-scan-labeled issue on agent-chosen fingerprint identity/error tokens — bounded (needs a pre-labeled issue + exact-pipeline evidence) and transitional; the go-forward canonical path is unaffected.

Verdict: NEEDS_CHANGES — the marker-forgery hole is closed, but the two coverage/dedup vectors above still let an untrusted agent hide a real failure. Both have narrow, concrete fixes.

🤖 Adversarial ensemble — Opus 4.8 (READY, LOW-only) + GPT-5.6 Sol (found both Medium agent-gaming bypasses); mechanisms confirmed by reading the publisher + workflow at 9636664.

Copilot AI review requested due to automatic review settings July 29, 2026 20:17
@PureWeen

Copy link
Copy Markdown
Member Author

Addressed the adversarial review in c8d17dfb0a.

  • Synthetic-header bypass — confirmed. On 9636664b30, a header-only fixture with match_pattern: "===== AzDO log" was accepted with match_count: 1 despite no matching raw failure content. The collector now writes separate structured raw segments, and the validator counts only those segments. Synthetic AzDO/Helix provenance remains in the rendered .log for diagnosis but is not countable. The header-only control now fails closed; a real raw-log line passes.
  • Marker-pattern replay — confirmed. On 9636664b30, match_pattern: "ci-scan-fingerprint" was accepted. Match patterns now reject exact, spacing, case, zero-width, and Unicode-homoglyph marker variants. Recurrence excludes fingerprint/match-count/evidence-key and pipeline/build/branch state lines.
  • Agent-selected identity replay — confirmed reachable. Fingerprint identity/error/platform fields are no longer sufficient recurrence proof. The validator hashes normalized complete raw lines containing the match and derives a domain-separated SHA-256 evidence key. Canonical recurrence requires the exact fingerprint and evidence-key markers plus a current trusted raw line; legacy markerless recurrence requires the configured pipeline plus a current trusted raw line. Copied fingerprints and unrelated same-pipeline issues now fail with zero writes.
  • Legacy adoption — constrained, retained. It remains available for the existing markerless backlog, but uses trusted evidence-line recurrence instead of agent-selected identity/error tokens and still fails closed on ambiguity.
  • Benign prose folding — intentionally unchanged. Relaxing the conservative body gate would weaken bare-token spoof detection. Its false-positive mode is an all-or-nothing batch abort, not silent suppression.
  • Stale gh-aw version note — confirmed and fixed in both twins with version-agnostic wording.

Additional producer/consumer caps are aligned at 200 raw segments, 25 MB structured evidence, and 200 distinct matching lines.

Validation: strict compilation passed for both twins; focused scanner suites passed 229/229; full .github/scripts Pester passed 924/924. Named mutations prove the suite fails when synthetic framing becomes countable, marker-pattern rejection is removed, trusted state lines become recurrence evidence, evidence identity binding is removed, either twin is omitted, or discovery is empty. No real ci-scan or ci-scan-net11 issue was mutated.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (2)

.github/workflows/ci-status-main.md:45

  • With cancel-in-progress: false, multiple scanner runs can overlap and race between listing open tracking issues and creating new ones, which can still produce duplicate issues if two runs publish the same fingerprint concurrently. Keeping cancellation enabled preserves the single-writer invariant and reduces the chance of duplicate tracking issues.
  cancel-in-progress: false

.github/workflows/ci-status-main.md:684

  • The Helix terminality check no longer requires Waiting/Running to be zero. If Helix reports a transient inconsistency where details.Finished is present but there are still waiting/running work items (or those work items are omitted from the returned list), this could accept incomplete evidence and publish based on a partial job snapshot. Consider restoring a fail-closed check for waiting/running counts while still allowing cumulative Unscheduled to remain non-zero.
                  terminalJob =
                    validCounts &&
                    Boolean(details?.Finished) &&
                    finishedCount > 0 &&
                    items.length >= finishedCount &&
  • Files reviewed: 10/11 changed files
  • Comments generated: 0 new

Copilot AI added 3 commits July 29, 2026 15:48
Inject canonical scanner fingerprints and trusted match counts in the deterministic publisher for both main and net11 scanners. Add fail-closed twin validation and mutation coverage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8dbefc79-f6c9-422e-90c7-c94bb06d39f9
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8dbefc79-f6c9-422e-90c7-c94bb06d39f9
Exclude synthetic provenance from evidence counts and bind canonical and legacy recurrence to publisher-derived raw evidence keys.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8dbefc79-f6c9-422e-90c7-c94bb06d39f9

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 2 — trusted-evidence binding: 2 prior bypasses closed, 1 new residual

Re-reviewed at c8d17df with an independent adversarial pass (Opus + GPT‑5.6‑sol), then reproduced every claim against the validator myself.

✅ Both round‑1 MEDIUMs are fixed — nice rework

  • Universal synthetic header (===== AzDO log … ===== as match_pattern): the countable/hashable corpus is now the separate *.evidence.json (ci-status-main.md collector :633‑637, :783‑800), and the validator counts/hashes only from it (Validate-CiScanManifest.ps1:560‑574). The synthetic ===== headers live only in the human‑readable .log, so they no longer satisfy coverage. The matchingLineHashes.Count -gt 200 guard (:566‑568) additionally fail‑closes ultra‑generic patterns. Confirmed rejected.
  • match_pattern not marker‑guarded / fingerprint replay: Get-ValidatedMatchPattern now runs Test-MarkerLikeContent (:451‑453), the marker fold covers ciscanevidencekey + homoglyphs (:223‑272), and binding was upgraded from an 8‑char substring to a full normalized evidence‑line hash on both sides (:364‑384). Confirmed rejected (marker / homoglyph / evidence‑key variants all throw).

Also verified fail‑closed on malformed/missing/empty/oversized/null‑segment trusted evidence, the []→$null pipeline trap, ##[…] log‑command injection (ConvertTo-SafeLogValue), and pre‑agent TOCTOU (evidence uploaded at the lock step, publisher is a separate job).

🟡 New residual (MEDIUM) — publisher‑manufactured constant text becomes global dedup identity

The helix-deadletter-uri segment records a fixed placeholder URL as its entire evidence content:

// ci-status-main.md:748-751  (a deadletter's ConsoleOutputUri is the fixed
// https://dotnet.github.io/core-eng/helix-workitem-deadletter.txt placeholder)
rawSegments.push({ kind: 'helix-deadletter-uri',
                   source: `${jobId}/${workItem.Name}`,   // run-specific…
                   content: deadletterUrl.toString() });   // …but only THIS is hashed

Get-TrustedEvidenceMatchProof derives EvidenceKey/EvidenceLineHashes solely from segment.content (Validate-CiScanManifest.ps1:560, :581‑589) — the run‑specific source is never mixed in — and helix-deadletter-uri is an accepted identity kind (:517). So when the current build has any deadletter (which also forces the log into failed_leaf_log_ids, so a presence proof is required), an agent can pick match_pattern = 'helix-workitem-deadletter.txt' and the whole trusted‑evidence identity collapses to a global constant across every build, pipeline and run.

Independently reproduced at c8d17df (two unrelated failures — android‑emulator‑boot in build 900001 vs ios‑device‑lost in build 900002 — sharing only the constant URL):

build900001 key: sha256:eb6fecb501bb9f9a5a6bc7c327a598442300b8d3f8943a43e833719a5c2cb28c
build900002 key: sha256:eb6fecb501bb9f9a5a6bc7c327a598442300b8d3f8943a43e833719a5c2cb28c
IDENTICAL KEY ACROSS UNRELATED BUILDS: True
control (real run-specific line): keys differ = True

That constant key is what every recurrence/dedup consumer is keyed on (hasTrustedEvidenceLine ci-status-main.md:188‑198; existing preflight :304‑319; legacyEvidenceMatcher :270‑277), so the "bind dedup to trusted evidence" invariant degrades on this path to "some deadletter happened at some point":

  • a stale canonical issue from an unrelated old build satisfies the current build's recurrence proof, and
  • a genuinely new device‑test failure that routes through the deadletter path is silently adopted onto that unrelated issue (legacy_dedup:true), so nothing is filed and the run reports complete coverage. This is exactly the mutation Validate-CiScanPublisher.Tests.ps1:385 ("pipeline alone suppresses a distinct failure") is meant to kill — the guard only holds while every evidence line is run‑specific, which helix-deadletter-uri is not.

Suggested fix (either):

  1. Exclude helix-deadletter-uri from the identity computation — still count it for coverage, but drop it from matchingLineHashes/EvidenceKey so a deadletter can't be the sole dedup identity; or
  2. Make the segment content run‑specific, e.g. content: \${jobId}/${workItem.Name} ${deadletterUrl}``, so the hashed identity is unique per build/job/work item.

Add a regression test that a deadletter‑only match_pattern cannot adopt an issue from a different build.

No other exploitable bypass was found in the +1259‑line rework.

Multi‑model adversarial review (Claude Opus 4.8 + GPT‑5.6‑sol); every finding independently reproduced against the validator before posting.

Bind Helix deadletter recurrence to stable work-item identity, reject non-terminal active work items, and align the merged reconciler invariants with publisher-owned fingerprint markers.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8dbefc79-f6c9-422e-90c7-c94bb06d39f9
@PureWeen
PureWeen force-pushed the pureween-fix-ci-scan-fingerprints branch from c8d17df to a535126 Compare July 29, 2026 22:51
Copilot AI review requested due to automatic review settings July 29, 2026 22:51
@PureWeen

Copy link
Copy Markdown
Member Author

Addressed Round 2 and the two suppressed follow-up notes in a535126635e.

  • Constant deadletter identity — confirmed. On the prior head, helix-deadletter-uri
    stored the fixed placeholder URL as its complete segment.content, while the
    run-specific source was excluded from line hashing. Two unrelated work items therefore
    produced the same evidence-line hash and evidence key.
  • Fix: the countable deadletter line now contains the validated stable work-item name
    plus the placeholder URL. The job/build ID remains provenance-only so the same work item
    can recur across builds, while android-emulator-boot and ios-device-lost produce
    distinct proof keys. Missing, oversized, or multiline work-item names fail closed.
    Deadletters still enter failed_leaf_log_ids, so they cannot be covered by an absence
    proof.
  • Recurrence controls: a deadletter-only current proof no longer adopts an unrelated
    same-pipeline legacy issue, while an identical real failure line still recurs across
    builds. The constant-deadletter-content mutation reproduces the old global collision.
  • Helix Waiting/Running note — confirmed as defense-in-depth. Terminal evidence now
    requires both counts to be zero. The live cumulative Unscheduled behavior remains
    accepted and tested.
  • Concurrency overlap note — not reproduced. A fixed GitHub concurrency group allows
    one running and one pending run; cancel-in-progress: false preserves the active
    publisher rather than allowing two publishers to run concurrently. Both twins now
    document and test that single-writer behavior.
  • Latest main integration: the branch was rebased after PR Add report-only ci-scan stale tracking-issue reconciler #36850. Its reconciler
    invariant now verifies trusted validator execution followed by publisher-side exact
    marker checks for both twins, replacing the obsolete agent-emission premise.

Validation on the rebased head:

  • strict gh-aw compilation: 0 errors, 0 warnings for both twins;
  • focused scanner/reconciler suites: 426/426;
  • complete .github/scripts suite: 1470/1470;
  • repeated compilation produced unchanged lock hashes;
  • independent MAUI expert review found no remaining high-confidence defects.

No real ci-scan or ci-scan-net11 issue was mutated.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (1)

.github/scripts/Validate-CiScanPublisher.Tests.ps1:62

  • The failure message still refers to legacyIdentityMatcher, but this helper now extracts legacyEvidenceMatcher + evidence proof helpers. If this path fails, the current message will mislead debugging.
        $helperEnd = $lock.IndexOf('// The plan is produced', $start)
        $matcherStart = $lock.IndexOf('const legacyEvidenceMatcher', $helperEnd)
        $end = $lock.IndexOf('const existingEntries', $matcherStart)
        if ($helperEnd -lt 0 -or $matcherStart -lt 0 -or $end -lt 0) {
            throw 'Could not find the end of the legacyIdentityMatcher block.'
        }
  • Files reviewed: 12/13 changed files
  • Comments generated: 0 new

Normalize trusted AzDO transport timestamps and prevent markerless legacy issues from suppressing distinct failures.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8dbefc79-f6c9-422e-90c7-c94bb06d39f9
Copilot AI review requested due to automatic review settings July 29, 2026 23:55
@PureWeen

Copy link
Copy Markdown
Member Author

Addressed the latest adversarial findings in 133746fde2be8bceb7f39adb762677efc3439730.

  • AzDO timestamp-sensitive evidence identity — confirmed. Two realistic lines differing only in the API transport prefix produced different evidence keys on the prior head. Trusted PowerShell proof generation now strips the UTC prefix only for structured azdo-log segments. Publisher body matching computes both raw and AzDO-normalized candidates, preserving non-AzDO message timestamps while allowing timestamp-free excerpts and canonical recurrence across builds. The timestamp-sensitive-identity mutation restores the failure and is killed.
  • Live (definition N) suffix — confirmed. The backlog uses (definition 302|313|314), while the old parser accepted only (ID N). Both compiled twins now recognize no suffix, (ID N), and (definition N) for all three configured pipelines and reject a mismatched definition. The definition-suffix-unsupported mutation is killed.
  • Generic markerless line collision — confirmed. A same-pipeline issue sharing only Build FAILED. matched the prior legacy path and could suppress a distinct failure. There is no publisher-owned historical identity that can safely disambiguate markerless issues, so automatic markerless adoption is removed and explicit markerless existing references fail before writes. Legacy shape matching remains diagnostic only; filed payloads create bounded visible canonical coverage. Mutations markerless-coverage-enabled and markerless-auto-adoption-enabled reproduce both suppression paths and are killed.

Canonical fingerprint drift continues to fall through to a bounded visible duplicate, contradictory canonical markers still fail closed, and the conservative marker-prose rejection remains unchanged.

Validation on the pushed head:

  • strict gh-aw compilation: 0 errors, 0 warnings for both twins;
  • focused validator/publisher/mutation suites: 225/225;
  • complete .github/scripts Pester suite: 1489/1489;
  • repeated compilation produced unchanged lock hashes;
  • independent final code review found no high-confidence issue.

No real ci-scan or ci-scan-net11 issue was mutated.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

Comments suppressed due to low confidence (2)

.github/workflows/ci-status-main.md:984

  • This section says the publisher injects two hidden markers and only calls out ci-scan-fingerprint / ci-scan-match-count, but the publisher/validator now also injects and enforces ci-scan-evidence-key. The prompt should reflect the third marker, and the “no marker content” rule should include ci-scan-evidence-key so agent guidance matches the enforced contract.
The publisher injects two hidden HTML-comment markers at the top of every issue
it files: one carrying the fingerprint (taken from the validated manifest, not
from your body) and one carrying the match count (recomputed from the frozen
evidence, not from anything you report).

.github/workflows/ci-status-net11.md:984

  • This section says the publisher injects two hidden markers and only calls out ci-scan-fingerprint / ci-scan-match-count, but the publisher/validator now also injects and enforces ci-scan-evidence-key. The prompt should reflect the third marker, and the “no marker content” rule should include ci-scan-evidence-key so agent guidance matches the enforced contract.
The publisher injects two hidden HTML-comment markers at the top of every issue
it files: one carrying the fingerprint (taken from the validated manifest, not
from your body) and one carrying the match count (recomputed from the frozen
evidence, not from anything you report).
  • Files reviewed: 11/13 changed files
  • Comments generated: 0 new

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 4 — LGTM ✅ (approving)

Followed this one across four rounds of adversarial review (Claude Opus 4.8 + GPT‑5.6‑sol), independently reproducing every finding and every fix against the validator. Everything raised is now resolved:

Round Finding Status
r1 Universal synthetic ===== AzDO log header usable as match_pattern (covers any signature) ✅ fixed — countable corpus moved to *.evidence.json, synthetic headers excluded
r1 match_pattern not marker‑guarded → fingerprint replay ✅ fixed — Test-MarkerLikeContent + full‑line‑hash binding
r2 helix-deadletter-uri content was a constant placeholder URL → global‑constant EvidenceKey → stale‑issue replay + silent suppression ✅ fixed at a535126
r3 (both models READY) / r4 Recurrence‑identity hardening ✅ clean

Deadletter fix — independently reproduced at 133746fde

The segment content is now Helix work item ${workItemName} was deadlettered: … and the validator hashes the whole matching line, so identity is work‑item‑specific (empty/CRLF/>1000 names throw — no 'unknown' fallback). My repro:

distinct work items  → DIFFERENT keys  (e8e7499… vs 2f8b054…)   ✅ collision closed
same work item / diff build → SAME key                          ✅ legitimate recurrence preserved

New 133746fde hardening — verified clean

  • StripAzdoTransportTimestamp (azdo‑log only): strips AzDO's run‑specific leading UTC timestamp so the same failure dedupes across builds. Confirmed it does not collapse distinct messages (same‑msg/diff‑ts → same identity; diff‑msg → different identity), retains the timestamp on the deadletter/helix path, and doesn't produce an empty identity. Sol's focused re‑verify (112 tool calls) came back delta_clean: true, no findings.
  • Markerless legacy‑issue rejection tightens the exact adoption path the r2 exploit abused — legacy issues without publisher‑owned identity can no longer be adopted as recurrence evidence.
  • Twin parity (ci-status-net11.md) and lock‑file/JS integrity hold; the reconciler changes are comment‑only.

CI

Pester (.github/scripts) green (408 tests, 0 fail), license/cla green; maui-pr correctly skips for a .github‑only change.

Nicely done working through a genuinely subtle trust‑boundary. Two optional, non‑blocking follow‑ups for later: a dedicated unit test for the empty/CRLF/>1000 work‑item‑name guard, and the collector‑1000 vs validator‑source‑1000 length‑cap alignment (currently fail‑closed, so safe).

LGTM.

Multi‑model adversarial review (Claude Opus 4.8 + GPT‑5.6‑sol) across r1–r4; deadletter key‑collision fix and timestamp‑strip behavior reproduced with pwsh before approving.

@kubaflo
kubaflo merged commit 9e89f76 into main Jul 30, 2026
6 of 7 checks passed
@kubaflo
kubaflo deleted the pureween-fix-ci-scan-fingerprints branch July 30, 2026 13:20
@github-actions github-actions Bot added this to the .NET 10 SR10 milestone Jul 30, 2026
kubaflo pushed a commit that referenced this pull request Aug 5, 2026
<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting
artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

### Description of Change

Fixes two production dry-run failures discovered after #36913:

- [main dry run
30574474829](https://github.com/dotnet/maui/actions/runs/30574474829)
rejected `runoniOS_MauiReleaseTrimFull` because trusted validation
depended on the agent lowercasing every fingerprint component.
- [net11 dry run
30574476689](https://github.com/dotnet/maui/actions/runs/30574476689)
failed while parsing a manifest serialized as JSON inside the
`submit_ci_scan` string argument when multiline issue bodies contained
literal newlines.

The trusted PowerShell boundary now canonicalizes accepted ASCII
fingerprint casing before structural validation, duplicate detection,
marker injection, and plan publication. Unsafe characters still fail
closed, and fingerprints that collide after canonicalization are
rejected.

The scanner twins now use a same-run artifact handoff instead of nested
string transport. The agent writes one manifest to
`/tmp/gh-aw/agent/manifest_final.json`, then calls the argument-free
`submit_ci_scan` tool exactly once to authorize publication. gh-aw
v0.83.4 uploads that fixed directory in the current run's fixed `agent`
artifact; the custom safe job downloads that artifact and reads only
`${{ runner.temp }}/gh-aw/safe-jobs/agent/manifest_final.json`. The file
is treated as untrusted, must be a regular non-symlink file, and is
capped at 500,000 bytes before all existing inventory, evidence, cap,
staged-mode, all-or-nothing preflight, and post-write checks run.

This artifact is only an intra-run handoff. It is not cross-run scanner
state or persistence. The safe-output request cannot select another file
path or carry manifest data. This PR intentionally does not add stale
issue lifecycle or reset behavior.

### Review hardening

Adversarial review found and closed additional gaps while preserving
twin symmetry:

- **Manifest bypassed sanitization and threat detection.** Both twins
now stage the fixed manifest into the threat-detection directory, name
it in the detection prompt, and fail closed if submission was authorized
without it. The staging gate rejects symlinks/non-regular files,
enforces `0 < size ≤ 500000` before copy, uses `cp --no-dereference`,
and revalidates the staged file type and byte size.
- **Exact-once gates ignored `.errors`.** Both the post-steps jq gate
and `Assert-ScannerSubmissionFromAgentOutput` now fail closed on
non-empty collector errors, and `max: 1` rejects duplicate custom
safe-output calls.
- **Hidden content could bypass scalar validation.** The trusted gate
now walks Unicode scalar values, rejects unpaired surrogates, C0/DEL/C1
controls, bidi/invisible format characters, noncharacters, HTML
comments, the full default-ignorable tag/variation-supplement plane, and
reserved default-ignorable Specials. It rejects rather than strips so
hash-verified evidence remains intact.
- **Legitimate emoji presentation was over-blocked.** VS15/VS16 now pass
only after an exact 16-base allowlist of common CI status/callout
symbols. The allowlist includes the production `🛠️ Build
Microsoft.Maui.sln` heading while selectors after ordinary text or
arbitrary symbols still fail closed, so the fix does not reopen a
generic variation-selector steganography channel.
- **Threat detection contradicted the trusted selector rule.** Both
scanner prompts now use that same exact 16-base allowlist and explicitly
retain detection for isolated selectors or selectors after any other
base. Tests parse the validator, both workflow sources, and both
compiled runtime `CUSTOM_PROMPT` values so an omitted compile cannot
silently restore the veto.
- **A manifest test file was silently disabled.** A missing Pester `It`
opener is restored, so `Validate-CiScanManifest.Tests.ps1` is discovered
and executed again.
- **Parameterized hidden-content tests broke NUnit XML export.** Their
names now use the safe ASCII `<Case>` parameter instead of allowing raw
XML-invalid values such as ESC U+001B into the NUnit test-name
attribute.

### Validation

- Focused scanner Pester suites with NUnit XML export and report-total
parity guard: 302 passed
- Full `.github/scripts` Pester 5.9.0 suite with NUnit XML export and
report-total parity guard: 1,566 passed
- `gh aw compile ci-status-main --strict`
- `gh aw compile ci-status-net11 --strict`
- Repeated strict compile produced byte-identical lock hashes
- Replayed the exact main production artifact through full trusted
validation; parsed the exact net11 `manifest_final.json` with all seven
multiline bodies through the fixed-file reader
- Replayed the exact production `🛠️ Build Microsoft.Maui.sln` text
through trusted manifest validation while preserving arbitrary-symbol
selector rejection
- Mutation coverage rejects restoring nested manifest-string transport,
agent-selected paths, unbounded/symlink threat-detection staging,
disabled hidden-content rejection, removed or widened selector
carve-outs, lost disallowed-selector rejection, and stale compiled
selector policy
- Final three-reviewer adversarial confirmation found no remaining
blocking, warning, or actionable findings

No real `ci-scan` or `ci-scan-net11` issue was created, updated, closed,
reopened, or otherwise mutated during development or validation.

### Issues Fixed

Follow-up to #36913; no separate tracking issue.

---------

Co-authored-by: PureWeen <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Vally Fixture <vally-fixture@example.invalid>
Copilot-Session: 41235a9a-79b4-433f-9e0d-7278c916a7c9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants