From a38149995eeff805c57639b4f2f3102e6ca7600f Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 2 Sep 2026 20:39:40 -0400 Subject: [PATCH] fix(release): handle minified telemetry constants safely --- .github/workflows/release-cut.yml | 10 +++---- .../telemetry-bundle-constant-patterns.mjs | 12 ++++++-- ...elemetry-bundle-constant-patterns.test.mjs | 5 ++++ ...windows-signing-workflow-contract.test.mjs | 30 +++++-------------- 4 files changed, 27 insertions(+), 30 deletions(-) diff --git a/.github/workflows/release-cut.yml b/.github/workflows/release-cut.yml index 9bc7d415d7b..b9fabdb17cb 100644 --- a/.github/workflows/release-cut.yml +++ b/.github/workflows/release-cut.yml @@ -1584,15 +1584,13 @@ jobs: Invoke-RestMethod -Method Post -Uri $env:SLACK_WEBHOOK_URL -ContentType 'application/json' -Body $payload - # Why gate on the notify outcome too: if nobody was told to approve, - # don't hold the release for the approval window — fall through and - # ship like today instead. The 1h wait (vs the installer's 4h) keeps - # both waits plus the build inside the 360-minute job cap; missing it - # falls through to today's unsigned-inner flow rather than blocking. + # Require the inner-binary request to complete before creating the + # installer request. A timeout or notification failure must fail closed: + # submitting a second request for an installer that still contains + # unsigned inner binaries wastes quota and obscures the real blocker. - name: Download signed inner binaries from SignPath id: download-signed-inner if: matrix.platform == 'win' && github.run_attempt == 1 && steps.submit-inner-signing.outcome == 'success' && steps.notify-inner-signing.outcome == 'success' - continue-on-error: true shell: pwsh env: SIGNPATH_API_TOKEN: ${{ secrets.SIGNPATH_API_TOKEN }} diff --git a/config/scripts/telemetry-bundle-constant-patterns.mjs b/config/scripts/telemetry-bundle-constant-patterns.mjs index 04944b7b156..11e08a9a362 100644 --- a/config/scripts/telemetry-bundle-constant-patterns.mjs +++ b/config/scripts/telemetry-bundle-constant-patterns.mjs @@ -1,2 +1,10 @@ -export const BUILD_IDENTITY_RE = /\b(?:const|let|var)\s+BUILD_IDENTITY\s*=\s*"(rc|stable)"/ -export const WRITE_KEY_RE = /\b(?:const|let|var)\s+WRITE_KEY\s*=\s*"(phc_[A-Za-z0-9_-]+)"/ +// The main bundle is minified for release builds, so Rollup may rename these +// locals and Oxc may print string literals with backticks. Match declarations +// by their validated values rather than source-level variable names. +// Oxc can combine adjacent declarations into `var a = ..., b = ...`. +const DECLARATION = String.raw`(?:\b(?:const|let|var)\s+|,\s*)[A-Za-z_$][\w$]*\s*=\s*` +const QUOTE = `["'\x60]` +const END_QUOTE = `["'\x60]` + +export const BUILD_IDENTITY_RE = new RegExp(`${DECLARATION}${QUOTE}(rc|stable)${END_QUOTE}`) +export const WRITE_KEY_RE = new RegExp(`${DECLARATION}${QUOTE}(phc_[A-Za-z0-9_-]+)${END_QUOTE}`) diff --git a/config/scripts/telemetry-bundle-constant-patterns.test.mjs b/config/scripts/telemetry-bundle-constant-patterns.test.mjs index b8df5ec3d0f..8ffd4ef8cc0 100644 --- a/config/scripts/telemetry-bundle-constant-patterns.test.mjs +++ b/config/scripts/telemetry-bundle-constant-patterns.test.mjs @@ -7,6 +7,11 @@ describe('telemetry bundle constant patterns', () => { expect(`${declaration} WRITE_KEY = "phc_example-key_123"`).toMatch(WRITE_KEY_RE) }) + it('accepts minified names and Oxc backtick literals', () => { + expect('var nfe=`stable`,rfe=`phc_test_key`').toMatch(BUILD_IDENTITY_RE) + expect('var nfe=`stable`,rfe=`phc_test_key`').toMatch(WRITE_KEY_RE) + }) + it('rejects assignments and invalid values', () => { expect('BUILD_IDENTITY = "rc"').not.toMatch(BUILD_IDENTITY_RE) expect('const BUILD_IDENTITY = "dev"').not.toMatch(BUILD_IDENTITY_RE) diff --git a/config/scripts/windows-signing-workflow-contract.test.mjs b/config/scripts/windows-signing-workflow-contract.test.mjs index 37edc2196d4..08aa4e4e6d4 100644 --- a/config/scripts/windows-signing-workflow-contract.test.mjs +++ b/config/scripts/windows-signing-workflow-contract.test.mjs @@ -183,7 +183,7 @@ describe('Windows signing workflow contract', () => { ) }) - it('verifies Windows inner binary signatures fail-open before publishing', () => { + it('does not submit installer signing after inner approval times out', () => { const parsedWorkflow = readWorkflow('.github/workflows/release-cut.yml') const steps = parsedWorkflow.jobs.build.steps const stepNames = steps.map((step) => step.name) @@ -191,34 +191,20 @@ describe('Windows signing workflow contract', () => { const innerVerifyIndex = stepNames.indexOf('Verify Windows inner binary signatures') const evidenceIndex = stepNames.indexOf('Upload Windows inner signing evidence') const publishIndex = stepNames.indexOf('Publish signed Windows release artifacts') + const installerRequestIndex = stepNames.indexOf('Submit Windows installer signing request') expect(outerVerifyIndex).toBeGreaterThan(-1) expect(innerVerifyIndex).toBe(outerVerifyIndex + 1) expect(evidenceIndex).toBe(innerVerifyIndex + 1) expect(publishIndex).toBe(evidenceIndex + 1) - // Why fail-open: unsigned inner binaries must warn, not block, until the - // flow is proven on a real release (issue #7785). Flip this to 'true' - // together with the workflow env to make the gate required. + // The evidence report remains warn-only for legacy paths, but an approval + // timeout must fail closed before a second request can consume quota. expect(steps[innerVerifyIndex].env.ORCA_WINDOWS_INNER_SIGNATURE_REQUIRED).toBe('false') - // Why: every step in the inner-signing chain must be unable to fail the - // release — a SignPath outage or timeout falls through to today's - // unsigned-inner flow instead of blocking the cut. - const innerChainStepNames = [ - 'Stage unsigned inner PE files for signing', - 'Upload unsigned inner binaries for SignPath', - 'Submit inner binaries signing request', - 'Notify Slack that inner-binary signing is waiting for approval', - 'Download signed inner binaries from SignPath', - 'Restore signed inner binaries into unpacked app', - 'Replace cached elevate.exe with the signed copy', - 'Rebuild NSIS installer from signed unpacked app' - ] - for (const stepName of innerChainStepNames) { - const step = steps[stepNames.indexOf(stepName)] - expect(step, stepName).toBeDefined() - expect(step['continue-on-error'], stepName).toBe(true) - } + const downloadInnerIndex = stepNames.indexOf('Download signed inner binaries from SignPath') + expect(downloadInnerIndex).toBeGreaterThan(-1) + expect(steps[downloadInnerIndex]['continue-on-error']).toBeUndefined() + expect(installerRequestIndex).toBeGreaterThan(downloadInnerIndex) }) })