[agentic-workflows] Fix scanner fingerprint canonicalization, Skill Validation fork checkouts, and agent tool boundaries - #37722
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GitHub Actions
Inactive
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37722Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37722" |
|
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. |
GitHub Actions
Inactive
Skill Validation Results
❌ Skill Validation Results —
|
|
CI note: the Static validation and Discover skills to evaluate failures are the self-hosting case fixed here. Their annotations show the base-branch pull_request_target workflow rejecting the fork checkout before this PR can supply allow-unsafe-pr-checkout. GitHub intentionally executes pull_request_target from main, so these jobs cannot consume the workflow change until it merges. The targeted invariant verifies all four PR-content checkouts opt in while retaining persist-credentials: false; the independent Pester workflow is unaffected. |
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
github-actions
Bot
added
the
area-ai-agents
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR hardens and unblocks multiple GitHub agentic workflows in dotnet/maui by tightening identity/canonicalization rules for CI-scan issue reuse, restoring skill-validation fork checkout behavior, and removing reliance on integrity-filtered GitHub MCP issue/search reads for bot-authored workflow reports.
Changes:
- Canonicalize CI-scan fingerprints (including apostrophe normalization) and allow controlled reuse of existing tracking issues when stable identity matches but historical category/platform separators differ.
- Restore Skill Validation fork PR-content checkouts (while keeping
persist-credentials: false) and add an invariant test to ensure the opt-in remains present. - Adjust memory leak workflows and Action Pin Refresh to avoid integrity-filtered MCP issue/search flows and to verify trusted gh-aw setup/version instead of installing extensions in the agent sandbox.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/skill-validation.yml | Opts the four PR-content checkouts into fork refs while keeping credentials non-persistent. |
| .github/workflows/leak-fixer.md | Removes MCP issue/search reliance; adds explicit workflow-owned issue gating via gh issue JSON. |
| .github/workflows/leak-fixer.lock.yml | Regenerates lock to reflect toolset and expiry changes. |
| .github/workflows/daily-leak-hunter.md | Removes MCP issue/search reliance; documents bounded gh issue read model. |
| .github/workflows/daily-leak-hunter.lock.yml | Regenerates lock to reflect toolset and expiry changes. |
| .github/workflows/ci-status-main.md | Adds canonical fingerprint extraction + stable-identity reuse rules for existing issues. |
| .github/workflows/ci-status-main.lock.yml | Regenerates lock to reflect publisher logic and expiry changes. |
| .github/workflows/ci-status-net11.md | Same as ci-status-main.md, scoped to net11 workflow. |
| .github/workflows/ci-status-net11.lock.yml | Regenerates lock to reflect publisher logic and expiry changes. |
| .github/workflows/ci-status-fix.md | Documents expected anonymous AzDO access limits (don’t misclassify as missing_tool). |
| .github/workflows/ci-status-fix.lock.yml | Regenerates lock to reflect expiry changes. |
| .github/workflows/ci-status-fix-net11.md | Same as ci-status-fix.md, scoped to net11 workflow. |
| .github/workflows/ci-status-fix-net11.lock.yml | Regenerates lock to reflect expiry changes. |
| .github/workflows/aw-actions-update.md | Adds trusted pre-agent verification of lock-pinned gh-aw version; removes sandbox extension install instructions. |
| .github/workflows/aw-actions-update.lock.yml | Adds setup-cli usage and embeds the trusted verification step; regenerates lock. |
| .github/aw/actions-lock.json | Adds pin entry for github/gh-aw-actions/setup-cli@v0.86.2. |
| .github/scripts/Validate-CiScanManifest.ps1 | Implements apostrophe normalization in the trusted fingerprint validator. |
| .github/scripts/Validate-CiScanManifest.Tests.ps1 | Adds tests for apostrophe canonicalization and collision fail-closed behavior. |
| .github/scripts/Validate-CiScanPublisher.Tests.ps1 | Extends invariants for new coverage proof mode and adds targeted regression tests for issue reuse. |
| .github/scripts/TestPrepareVallyEvaluation.rb | Adds invariant ensuring PR-content checkouts keep persist-credentials: false and explicit opt-in. |
PureWeen
left a comment
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
An independent reviewer found no additional high-confidence correctness, security, or regression issues in this revision.
Methodology: 3 independent reviewers with adversarial consensus. The added coverage directly exercises fingerprint canonicalization/collision handling, bounded historical-marker reuse, fork-checkout invariants, and workflow source/lock parity. Existing inline feedback was not duplicated.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
|
@copilot ready for re-review. The normalized fingerprint length finding is implemented in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/aw-actions-update.md:91
- The version check currently uses a substring match (
grep -F). This can produce false positives (e.g., pinnedv0.86.2matches installedv0.86.20), which defeats the goal of preventing version skew. Parse the semantic version fromgh aw --versionand compare it for exact equality.
installed_version="$(gh aw --version)"
printf '%s\n' "$installed_version"
printf '%s\n' "$installed_version" | grep -F -- "$pinned_version" >/dev/null
.github/workflows/skill-validation.yml:406
- Consider making
allow-unsafe-pr-checkoutconditional (fork PRs only) so same-repo PRs/workflow_dispatch don’t unnecessarily opt into unsafe checkout behavior.
allow-unsafe-pr-checkout: true
.github/workflows/skill-validation.yml:969
- Consider gating
allow-unsafe-pr-checkoutto fork PRs only (head repo != base repo) so this job doesn’t opt into unsafe checkout behavior when it can use the default mode.
allow-unsafe-pr-checkout: true
.github/workflows/skill-validation.yml:535
- This checkout uses
fetch-depth: 0and now unconditionally opts into unsafe PR checkout. Prefer gatingallow-unsafe-pr-checkoutto fork PRs only (head repo != base repo).
allow-unsafe-pr-checkout: true
PureWeen
left a comment
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
An independent reviewer identified one validated workflow-provenance gap.
jq select(...) enforcement is only used while auto-picking. An explicit issue_number dispatch reads title,body without checking open state, bot author, title prefix, or required labels. A write-access dispatcher can therefore target a non-workflow-owned issue whose untrusted body is then processed as a leak report, bypassing the provenance check this change introduces. Enforce the same gate after fetching an explicit target before extracting or acting on its content. (.github/workflows/leak-fixer.md:349)
Flagged by: repo domain specialist + 1/3 reviewers after dispute validation. The remaining reviewer considered the prose gate sufficient; the consensus retained this because the safety property is otherwise not mechanically enforced on the explicit path.
Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist. Existing feedback was not duplicated.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
|
@PureWeen @copilot ready for re-review. |
PureWeen
left a comment
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
An independent reviewer found no additional high-confidence correctness, security, or regression issues at bb583ed4a6fc29045f08c9f4f00f87aafd3dd7ed.
Methodology: 3 independent reviewers with adversarial consensus. Three single-reviewer observations were independently checked and rejected because the stable-identity relaxation remains bounded by trusted evidence and ownership checks, the lock-only expiry value lacks verifiable source semantics, and the narrow apostrophe normalization intentionally fails closed for other code points. Existing exact-head feedback was not duplicated.
Run 32484732312 failed on dotnet#37558 as well as dotnet#37559: the agent re-derived the test identity (`safeareaperedgevalidation`) instead of reusing the one recorded in the issue it was reusing. The publisher's tolerance covers failure-category wording and space/hyphen platform drift, but test identity is the failure's identity and must stay exact, so relaxing it further would let one tracking issue absorb an entire suite. Fix it where the drift is produced instead: - Both scanner prompts now require an `existing` signature to submit the fingerprint recorded in the referenced issue verbatim; a freshly derived fingerprint belongs only to a `filed` payload. - Both scanner prompts document that the trusted validator drops apostrophes when canonicalizing, so the agent searches with the same spelling the publisher recorded and does not file a duplicate on the next run. - Publisher suite pins the boundary with the real dotnet#37558 pair (re-derived identity rejected, recorded fingerprint accepted) and a twin-drift guard over the runtime-imported prompts. - Fold the CI-fixer AzDO note into rule 8 so it stays part of the numbered rule list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: efefd2b9-b01a-4338-9a2b-368a58769e74
kubaflo
changed the title
| const canonicalFingerprint = fingerprintMarkers.length === 1 | ||
| ? fingerprintMarkers[0].slice(markerPrefix.length + 1, -4) |
| run: | | ||
| set -euo pipefail | ||
| lock_file=".github/workflows/aw-actions-update.lock.yml" | ||
| pinned_version="$(sed -nE '1s/^# gh-aw-metadata: .*"compiler_version":"([^"]+)".*$/\1/p' "$lock_file")" |
| .github/agents | ||
| .github/plugin.json | ||
| persist-credentials: false | ||
| allow-unsafe-pr-checkout: ${{ (needs.pr-gate.outputs.head_repo || needs.slash-gate.outputs.head_repo || github.repository) != github.repository }} |
| .github/skills | ||
| .github/plugin.json | ||
| persist-credentials: false | ||
| allow-unsafe-pr-checkout: ${{ (needs.pr-gate.outputs.head_repo || needs.slash-gate.outputs.head_repo || github.repository) != github.repository }} |
| # `git worktree add <SHA>` must be able to resolve them. | ||
| fetch-depth: 0 | ||
| persist-credentials: false | ||
| allow-unsafe-pr-checkout: ${{ (needs.pr-gate.outputs.head_repo || needs.slash-gate.outputs.head_repo || github.repository) != github.repository }} |
| .github/skills | ||
| .github/plugin.json | ||
| persist-credentials: false | ||
| allow-unsafe-pr-checkout: ${{ (needs.pr-gate.outputs.head_repo || needs.slash-gate.outputs.head_repo || github.repository) != github.repository }} |
| if ($RequireCurrentEvidenceInExistingBody) { | ||
| $pattern = '(?m)^(?<indent>\s*)getEvidenceProof\(entry\);\r?\n' + | ||
| '\k<indent>const exactMarker = `<!-- ci-scan-fingerprint: \$\{entry\.fingerprint\} -->`;' | ||
| $pattern = '(?m)^(?<indent>\s*)getEvidenceProof\(entry\);\r?$' |
PureWeen
left a comment
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
An independent reviewer found no additional high-confidence correctness, security, or regression issues at 92e9a4808b0f8025e234bfc51563be6790ef74d8.
Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist. The specialist-only checkout concern was independently disproven against the current actions/checkout@v4 contract. Existing exact-head feedback was not duplicated.
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!
Summary
existingmanifest signatures to submit the referenced issue's recorded fingerprint verbatim, and apply the same apostrophe-drop normalization when searching for duplicates. Run 32484732312 failed on both [ci-scan-net11] Android SafeArea first-navigation visual baseline differs #37559 (category/platform wording) and [ci-scan-net11] Android SafeArea keyboard resize assertions fail in UI tests #37558 (re-derived test identity); publisher tolerance fixes the first, while the prompt contract fixes the second without weakening test identity.persist-credentials: false, with a workflow invariant test.gh issuepaths for bot-authored reports by removing integrity-filtered MCP issue/search tools. The fixer additionally validates workflow-owned author/title/labels before treating issue content as untrusted evidence.gh-awsetup into the compiler-generated trusted setup path instead of asking the sandboxed agent to install an extension.Verified reports
Fixes #37715
Fixes #37716
Fixes #37695
Fixes #37699
Fixes #37696
Fixes #37712
Fixes #37659
Addresses the integrity-filter portion of #37717. Its separate HTTP 422 model-policy interruption is external to the repository and still needs a successful rerun to verify.
Existing PR comparison
Developed independently before searching open PRs. #36664 also changes the memory workflow files, but it focuses on de-duplication, cadence, and mutation-boundary hardening; it does not address integrity filtering of workflow-owned bot reports. This PR keeps that fix orthogonal and may need a straightforward rebase if #36664 merges first. No open PR was found for the scanner, Skill Validation, AzDO-diagnostic, or Action Pin Refresh failures.
Validation
mainand accepted here, and 32484732312 now passes complete coverage validation.Validate-CiScanManifest.Tests.ps1+Validate-CiScanPublisher.Tests.ps1: 335 passed. The new prompt-contract guards fail against the pre-fix prompt.gh aw compile aw-actions-update ci-status-fix ci-status-fix-net11 ci-status-main ci-status-net11 daily-leak-hunter leak-fixer --validate: 7 compiled, 0 warnings; a second compile produced an identical generated diff.GH_AW_ACTION_FAILURE_ISSUE_EXPIRES_HOURSvalues from0to168in the touched lock files.