Skip to content

fix(ci): use GraphQL variables for fork-sourced report content#40973

Merged
yury-s merged 1 commit into
microsoft:mainfrom
yury-s:fix-msrc-117222
May 22, 2026
Merged

fix(ci): use GraphQL variables for fork-sourced report content#40973
yury-s merged 1 commit into
microsoft:mainfrom
yury-s:fix-msrc-117222

Conversation

@yury-s
Copy link
Copy Markdown
Member

@yury-s yury-s commented May 22, 2026

Summary

  • ghaMarkdownReporter posts the merged blob-report markdown back to the PR via a GraphQL mutation. The report body was interpolated into a """...""" block string, so untrusted content (the report from a fork PR) could close the block and inject arbitrary GraphQL.
  • Switch the three call sites (collapse query, minimize mutations, addComment) to parameterized $variables so octokit routes user input through the typed variable channel.

…erpolation

The Publish Test Results workflow runs on workflow_run with checks: write,
pull-requests: write, statuses: write. It merges blob reports produced by
fork PR runs (untrusted) and posts a markdown comment back via the GraphQL
API. The report text was interpolated directly into a """..."""  block string
inside the addComment mutation, so a fork could craft report content
containing """ to escape the block string and inject arbitrary mutations
in the privileged context.

Switch the three call sites in ghaMarkdownReporter.ts (collapse query,
minimize mutations, addComment) to parameterized GraphQL with $variables,
which routes user input through octokit's typed variable channel and
removes the injection surface entirely.

Related: MSRC 117222
@github-actions
Copy link
Copy Markdown
Contributor

Test results for "MCP"

7181 passed, 1113 skipped


Merge workflow run.

@yury-s yury-s merged commit f31b8fe into microsoft:main May 22, 2026
43 of 46 checks passed
@yury-s yury-s deleted the fix-msrc-117222 branch May 22, 2026 21:57
@github-actions
Copy link
Copy Markdown
Contributor

Test results for "tests 1"

129 failed
❌ [playwright-test] › list-mode.spec.ts:200 › should list tests once @ubuntu-latest-node26
❌ [playwright-test] › reporter-dot.spec.ts:50 › created › render unexpected after retry @ubuntu-latest-node26
❌ [playwright-test] › reporter-dot.spec.ts:65 › created › render flaky @ubuntu-latest-node26
❌ [playwright-test] › reporter-dot.spec.ts:50 › merged › render unexpected after retry @ubuntu-latest-node26
❌ [playwright-test] › reporter-dot.spec.ts:65 › merged › render flaky @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:26 › created › should render expected @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:53 › created › should render unexpected @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:73 › created › should render thrown error as element @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:95 › created › should render TypeError as element with correct type @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:144 › created › should render stdout @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:169 › created › should render stdout without ansi escapes @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:191 › created › should render, by default, character data as CDATA sections @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:213 › created › should render skipped @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:233 › created › should report skipped due to sharding @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:262 › created › should not render projects if they dont exist @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:288 › created › should render projects @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:323 › created › should includeProjectInTestName @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:350 › created › should render existing attachments, but not missing ones @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:415 › created › should render annotations to custom testcase properties @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:434 › created › should render built-in annotations to testcase properties @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:453 › created › should render all annotations to testcase value based properties, if requested @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:488 › created › should not embed attachments to a custom testcase property, if not explicitly requested @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:621 › created › testsuites time is test run wall time @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:637 › created › should emit flakyFailure for flaky tests when includeRetries is enabled @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:669 › created › should not include retries by default @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:688 › created › should emit rerunFailure for permanent failures when includeRetries is enabled @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:26 › merged › should render expected @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:53 › merged › should render unexpected @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:73 › merged › should render thrown error as element @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:95 › merged › should render TypeError as element with correct type @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:144 › merged › should render stdout @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:169 › merged › should render stdout without ansi escapes @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:191 › merged › should render, by default, character data as CDATA sections @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:213 › merged › should render skipped @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:233 › merged › should report skipped due to sharding @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:262 › merged › should not render projects if they dont exist @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:288 › merged › should render projects @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:323 › merged › should includeProjectInTestName @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:415 › merged › should render annotations to custom testcase properties @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:434 › merged › should render built-in annotations to testcase properties @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:453 › merged › should render all annotations to testcase value based properties, if requested @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:488 › merged › should not embed attachments to a custom testcase property, if not explicitly requested @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:621 › merged › testsuites time is test run wall time @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:637 › merged › should emit flakyFailure for flaky tests when includeRetries is enabled @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:669 › merged › should not include retries by default @ubuntu-latest-node26
❌ [playwright-test] › reporter-junit.spec.ts:688 › merged › should emit rerunFailure for permanent failures when includeRetries is enabled @ubuntu-latest-node26
❌ [playwright-test] › reporter-line.spec.ts:252 › onTestPaused › pause at end @ubuntu-latest-node26
❌ [playwright-test] › reporter-line.spec.ts:291 › onTestPaused › pause at end - error in teardown @ubuntu-latest-node26
❌ [playwright-test] › reporter-line.spec.ts:339 › onTestPaused › pause on error @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:58 › created › render steps @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:94 › created › render steps inline @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:178 › created › render retries @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:198 › created › should truncate long test names @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:262 › created › print stdio @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:307 › created › should update test status row only when TTY has not scrolled @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:367 › created › should update test status row only within configured TTY height @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:58 › merged › render steps @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:94 › merged › render steps inline @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:178 › merged › render retries @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:198 › merged › should truncate long test names @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:262 › merged › print stdio @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:307 › merged › should update test status row only when TTY has not scrolled @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:367 › merged › should update test status row only within configured TTY height @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:422 › onTestPaused › pause at end @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:452 › onTestPaused › pause at end - error in teardown @ubuntu-latest-node26
❌ [playwright-test] › reporter-list.spec.ts:498 › onTestPaused › pause on error @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:128 › created › should work with custom reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:167 › created › should work without a file extension @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:192 › created › should report onEnd after global teardown @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:226 › created › should load reporter from node_modules @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:274 › created › should report forbid-only error to reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:297 › created › should report no-tests error to reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:316 › created › should report require error to reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:339 › created › should report global setup error to reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:466 › created › test and step error should have code snippet @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:523 › created › onError should have code snippet @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:128 › merged › should work with custom reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:167 › merged › should work without a file extension @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:226 › merged › should load reporter from node_modules @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:274 › merged › should report forbid-only error to reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:297 › merged › should report no-tests error to reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:316 › merged › should report require error to reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:339 › merged › should report global setup error to reporter @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:466 › merged › test and step error should have code snippet @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:523 › merged › onError should have code snippet @ubuntu-latest-node26
❌ [playwright-test] › reporter.spec.ts:732 › tests skipped in serial mode receive onTestBegin/onTestEnd @ubuntu-latest-node26
❌ [playwright-test] › retry.spec.ts:118 › should retry timeout @ubuntu-latest-node26
❌ [playwright-test] › retry.spec.ts:148 › should retry unexpected pass @ubuntu-latest-node26
❌ [playwright-test] › retry.spec.ts:164 › should not retry expected failure @ubuntu-latest-node26
❌ [playwright-test] › retry.spec.ts:184 › should retry unhandled rejection @ubuntu-latest-node26
❌ [playwright-test] › retry.spec.ts:203 › should retry beforeAll failure @ubuntu-latest-node26
❌ [playwright-test] › retry.spec.ts:224 › should retry worker fixture setup failure @ubuntu-latest-node26
❌ [playwright-test] › reporter-blob.spec.ts:89 › should call methods in right order @ubuntu-latest-node26
❌ [playwright-test] › reporter-blob.spec.ts:426 › merge into list report by default @ubuntu-latest-node26
❌ [playwright-test] › reporter-blob.spec.ts:1058 › preserve stdout and stderr @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:115 › should report api step hierarchy @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:160 › should report before hooks step error @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:190 › should not report nested after hooks @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:288 › should report expect step locations @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:312 › should report custom expect steps @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:483 › should mark step as failed when soft expect fails @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:520 › should nest steps based on zones @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:596 › should not mark page.close as failed when page.click fails @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:644 › should not propagate errors from within toPass @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:672 › should show final toPass error @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:699 › should propagate nested soft errors @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:743 › should not propagate nested hard errors @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:787 › should step w/o box @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:820 › should step w/ box @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:853 › should soft step w/ box @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:886 › should not generate dupes for named expects @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:922 › step inside toPass @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:970 › library API call inside toPass @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1019 › library API call inside expect.poll @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1072 › web assertion inside expect.poll @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1125 › should report expect steps @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1175 › should report api steps @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1264 › should report api step failure @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1303 › should show nice stacks for locators @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1337 › should allow passing location to test.step @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1386 › should show tracing.group nested inside test.step @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1427 › calls from waitForEvent callback should be under its parent step @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1482 › reading network request / response should not be listed as step @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1521 › calls from page.route callback should be under its parent step @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1566 › test.step.skip should work @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1604 › skip test.step.skip body @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1633 › step.skip should work at runtime @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1728 › show api calls inside expects @ubuntu-latest-node26
❌ [playwright-test] › test-step.spec.ts:1792 › should box fixtures @ubuntu-latest-node26

4 flaky ⚠️ [chromium-library] › library/popup.spec.ts:261 › should not throw when click closes popup `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [chromium-library] › library/video.spec.ts:719 › screencast › should work with video+trace `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:1088 › cli codegen › should clear when recording is disabled `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-emulate-media.spec.ts:144 › should keep reduced motion and color emulation after reload `@firefox-ubuntu-22.04-node20`

43778 passed, 861 skipped


Merge workflow run.

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.

2 participants