From a23a380e1348871e2bb19575d943369251b120ee Mon Sep 17 00:00:00 2001 From: Peter Dave Hello <3691490+PeterDaveHello@users.noreply.github.com> Date: Wed, 1 Jul 2026 03:34:57 +0800 Subject: [PATCH 1/3] Harden GitHub Actions manual store submission with secret-free preflight Manual dispatches should validate release artifacts without invoking publish-browser-extension's authenticated dry-run path. Run workflow_dispatch as a read-only GitHub Actions preflight job with no persisted checkout credentials. Keep real store submission on tag pushes only. Cover artifact, manifest shape, process environment, and credential boundaries with targeted tests. --- .github/workflows/tagged-release.yml | 160 ++++---- package.json | 1 + scripts/submit-stores.mjs | 86 +++-- tests/unit/release/submit-stores.test.mjs | 434 +++++++++++++++++++++- 4 files changed, 577 insertions(+), 104 deletions(-) diff --git a/.github/workflows/tagged-release.yml b/.github/workflows/tagged-release.yml index e36931d65..a547b6cc9 100644 --- a/.github/workflows/tagged-release.yml +++ b/.github/workflows/tagged-release.yml @@ -3,62 +3,64 @@ on: workflow_dispatch: inputs: submit_stores: - description: "Run Chrome, Firefox, and Edge store submission preflight" + description: 'Run store submission artifact preflight without store credentials' required: false - default: "false" + default: 'false' type: choice options: - - "false" - - "true" - dry_run: - description: "Validate store submission without uploading artifacts" - required: false - default: "true" - type: choice - options: - - "true" - - "false" + - 'false' + - 'true' push: tags: - - "v*" - -permissions: - id-token: "write" - contents: "write" -env: - GH_TOKEN: ${{ github.token }} + - 'v*' jobs: - build_and_release: + manual_preflight: + if: github.event_name == 'workflow_dispatch' runs-on: macos-14 + permissions: + contents: read steps: - uses: actions/checkout@v7 with: - ref: ${{ github.event_name == 'push' && 'master' || github.ref_name }} + ref: ${{ github.ref }} + persist-credentials: false - uses: actions/setup-node@v7 with: node-version: 22 - - uses: actions/setup-python@v7 + - run: npm ci + + - run: npm run build + + - run: npm run release:firefox-sources + + - name: Submit stores preflight + if: inputs.submit_stores == 'true' + run: npm run release:submit:preflight + + release: + if: github.event_name == 'push' + runs-on: macos-14 + permissions: + contents: write + + steps: + - uses: actions/checkout@v7 with: - python-version: '3.10' # for appdmg - - uses: maxim-lobanov/setup-xcode@v1 + ref: master + fetch-depth: 0 + persist-credentials: true + + - uses: actions/setup-node@v7 with: - xcode-version: 16.2 - - run: npm ci + node-version: 22 - name: Resolve release version - run: | - if [ "${{ github.event_name }}" = "push" ]; then - echo "VERSION=${GITHUB_REF_NAME#v}" >> $GITHUB_ENV - else - version="$(node -p "require('./src/manifest.json').version")" - echo "VERSION=${version}" >> $GITHUB_ENV - fi + run: printf 'VERSION=%s\n' "${GITHUB_REF_NAME#v}" >> "$GITHUB_ENV" - name: Update manifest.json version - if: github.event_name == 'push' uses: jossef/action-set-json-field@v2.2 with: file: src/manifest.json @@ -66,7 +68,6 @@ jobs: value: ${{ env.VERSION }} - name: Update manifest.v2.json version - if: github.event_name == 'push' uses: jossef/action-set-json-field@v2.2 with: file: src/manifest.v2.json @@ -74,46 +75,69 @@ jobs: value: ${{ env.VERSION }} - name: Push files - if: github.event_name == 'push' - continue-on-error: true run: | git config --global user.email "github-actions[bot]@users.noreply.github.com" git config --global user.name "github-actions[bot]" - git commit -am "release v${{ env.VERSION }}" - git push + git add src/manifest.json src/manifest.v2.json + if git diff --cached --quiet; then + echo "No release version changes to commit" + else + git commit -m "release v${VERSION}" + git fetch origin master + git rebase FETCH_HEAD + git push origin HEAD:master + fi - - if: github.event_name == 'push' - run: | - gh release create ${{github.ref_name}} -d -F CURRENT_CHANGE.md -t ${{github.ref_name}} + - name: Checkout release tag for artifacts + uses: actions/checkout@v7 + with: + ref: ${{ github.ref }} + persist-credentials: false + + - uses: actions/setup-python@v7 + with: + python-version: '3.10' # for appdmg + - uses: maxim-lobanov/setup-xcode@v1 + with: + xcode-version: 16.2 + - run: npm ci + + - name: Update release artifact manifest.json version + uses: jossef/action-set-json-field@v2.2 + with: + file: src/manifest.json + field: version + value: ${{ env.VERSION }} + + - name: Update release artifact manifest.v2.json version + uses: jossef/action-set-json-field@v2.2 + with: + file: src/manifest.v2.json + field: version + value: ${{ env.VERSION }} - - if: github.event_name == 'push' - run: npm run build:safari + - run: | + gh release create "$RELEASE_TAG" -d -F CURRENT_CHANGE.md -t "$RELEASE_TAG" + env: + GH_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ github.ref_name }} - - if: github.event_name != 'push' - run: npm run build + - run: npm run build:safari - run: npm run release:firefox-sources - - if: github.event_name == 'push' - run: | - gh release upload ${{github.ref_name}} build/chromium.zip - gh release upload ${{github.ref_name}} build/firefox.zip - gh release upload ${{github.ref_name}} build/safari.dmg - gh release upload ${{github.ref_name}} build/chromium-without-katex-and-tiktoken.zip - gh release upload ${{github.ref_name}} build/firefox-without-katex-and-tiktoken.zip + - run: | + gh release upload "$RELEASE_TAG" build/chromium.zip + gh release upload "$RELEASE_TAG" build/firefox.zip + gh release upload "$RELEASE_TAG" build/safari.dmg + gh release upload "$RELEASE_TAG" build/chromium-without-katex-and-tiktoken.zip + gh release upload "$RELEASE_TAG" build/firefox-without-katex-and-tiktoken.zip + env: + GH_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ github.ref_name }} - name: Submit stores - if: github.event_name == 'push' || inputs.submit_stores == 'true' - run: | - args=() - if [ "${{ github.event_name }}" != "push" ]; then - if [ "${{ inputs.dry_run }}" != "true" ]; then - echo "::error::Manual store submission only supports dry_run=true. Push a v* tag for a real submission." - exit 1 - fi - args+=(--dry-run) - fi - npm run release:submit -- "${args[@]}" + run: npm run release:submit env: CHROME_EXTENSION_ID: ${{ secrets.CHROME_EXTENSION_ID }} CHROME_CLIENT_ID: ${{ secrets.CHROME_CLIENT_ID }} @@ -131,6 +155,8 @@ jobs: EDGE_CLIENT_ID: ${{ secrets.EDGE_CLIENT_ID }} EDGE_API_KEY: ${{ secrets.EDGE_API_KEY }} - - if: github.event_name == 'push' - run: | - gh release edit ${{github.ref_name}} --draft=false + - run: | + gh release edit "$RELEASE_TAG" --draft=false + env: + GH_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ github.ref_name }} diff --git a/package.json b/package.json index 10b55ed9d..43d49b3d4 100644 --- a/package.json +++ b/package.json @@ -20,6 +20,7 @@ "release:firefox-sources": "node scripts/create-firefox-sources-zip.mjs", "release:submit": "node scripts/submit-stores.mjs", "release:submit:dry-run": "node scripts/submit-stores.mjs --dry-run", + "release:submit:preflight": "node scripts/submit-stores.mjs --preflight-only", "release:update-firefox-metadata": "node scripts/update-firefox-metadata.mjs", "release:check-edge-api-key": "node scripts/check-edge-api-key-expiry.mjs" }, diff --git a/scripts/submit-stores.mjs b/scripts/submit-stores.mjs index 6712344ed..5c417ff4f 100644 --- a/scripts/submit-stores.mjs +++ b/scripts/submit-stores.mjs @@ -3,12 +3,14 @@ import fs from 'fs-extra' import jwt from 'jsonwebtoken' import { spawn } from 'node:child_process' -import path from 'node:path' import { randomUUID } from 'node:crypto' +import { createRequire } from 'node:module' +import path from 'node:path' import { fileURLToPath } from 'node:url' const REQUIRED_ARTIFACTS = ['build/chromium.zip', 'build/firefox.zip', 'build/firefox-sources.zip'] const AMO_BASE_URL = 'https://addons.mozilla.org' +const require = createRequire(import.meta.url) export const FIREFOX_COMPATIBILITY = { firefox: { min: '58.0', @@ -36,11 +38,12 @@ const REQUIRED_ENV = [ export function parseArgs(args) { return { dryRun: args.includes('--dry-run'), + preflightOnly: args.includes('--preflight-only'), } } export function findMissingEnv(env = process.env) { - return REQUIRED_ENV.filter((name) => !env[name]) + return REQUIRED_ENV.filter((name) => String(env[name] ?? '').trim().length === 0) } export async function findMissingArtifacts({ exists = fs.pathExists } = {}) { @@ -156,18 +159,29 @@ export async function updateFirefoxVersionNotes({ } function resolvePublishExtensionBin() { - const command = process.platform === 'win32' ? 'publish-extension.cmd' : 'publish-extension' - return path.join(process.cwd(), 'node_modules', '.bin', command) + return require.resolve('publish-browser-extension/cli') +} + +function buildPublishExtensionEnv(env, baseEnv = process.env) { + const merged = { ...baseEnv, ...(env ?? {}) } + return Object.fromEntries( + Object.entries(merged) + .filter(([, value]) => value !== undefined && value !== null) + .map(([name, value]) => [name, String(value)]), + ) } -async function runPublishExtension(args) { - const command = resolvePublishExtensionBin() +export async function runPublishExtension( + args, + { env, baseEnv = process.env, spawnImpl = spawn } = {}, +) { + const childArgs = [resolvePublishExtensionBin(), ...args] await new Promise((resolve, reject) => { - const child = spawn(command, args, { + const child = spawnImpl(process.execPath, childArgs, { stdio: 'inherit', shell: false, - env: process.env, + env: buildPublishExtensionEnv(env, baseEnv), }) child.once('error', reject) @@ -181,34 +195,62 @@ async function runPublishExtension(args) { }) } -export async function submitStores({ argv = process.argv.slice(2), env = process.env } = {}) { - const { dryRun } = parseArgs(argv) - const missingArtifacts = await findMissingArtifacts() - const missingEnv = findMissingEnv(env) +export async function submitStores({ + argv = process.argv.slice(2), + env: envInput, + exists = fs.pathExists, + readJson = fs.readJson, + runPublishExtensionImpl = runPublishExtension, + updateFirefoxVersionNotesImpl = updateFirefoxVersionNotes, + logger = console.log, + errorLogger = console.error, +} = {}) { + const { dryRun, preflightOnly } = parseArgs(argv) + const env = envInput ?? process.env + const missingArtifacts = await findMissingArtifacts({ exists }) + const missingEnv = preflightOnly ? [] : findMissingEnv(env) if (missingArtifacts.length > 0 || missingEnv.length > 0) { if (missingArtifacts.length > 0) { - console.error(`Missing release artifacts: ${missingArtifacts.join(', ')}`) + errorLogger(`Missing release artifacts: ${missingArtifacts.join(', ')}`) } if (missingEnv.length > 0) { - console.error(`Missing store submission environment variables: ${missingEnv.join(', ')}`) + errorLogger(`Missing store submission environment variables: ${missingEnv.join(', ')}`) } throw new Error('Store submission preflight failed') } - const manifest = await fs.readJson('build/firefox/manifest.json') - const args = buildPublishExtensionArgs({ dryRun }) + let manifest + try { + manifest = await readJson('build/firefox/manifest.json') + } catch (error) { + errorLogger('Missing or invalid Firefox manifest: build/firefox/manifest.json') + throw new Error('Store submission preflight failed', { cause: error }) + } + + if (!manifest || typeof manifest.version !== 'string' || manifest.version.trim().length === 0) { + errorLogger('Missing Firefox manifest version: build/firefox/manifest.json') + throw new Error('Store submission preflight failed') + } + const firefoxReleaseNotes = buildFirefoxReleaseNotes(manifest.version) + const mode = preflightOnly ? 'preflight' : dryRun ? 'dry-run' : 'submit' + + logger(`${preflightOnly ? 'Checking' : 'Submitting'} ChatGPTBox ${manifest.version}`) + logger(`Mode: ${mode}`) + logger(`Artifacts: ${REQUIRED_ARTIFACTS.join(', ')}`) + logger(`Firefox version notes: ${firefoxReleaseNotes}`) - console.log(`Submitting ChatGPTBox ${manifest.version} to Chrome, Firefox, and Edge`) - console.log(`Mode: ${dryRun ? 'dry-run' : 'submit'}`) - console.log(`Artifacts: ${REQUIRED_ARTIFACTS.join(', ')}`) - console.log(`Firefox version notes: ${firefoxReleaseNotes}`) + if (preflightOnly) { + logger('Store authentication, upload, and submission are skipped in preflight mode') + return + } - await runPublishExtension(args) + const args = buildPublishExtensionArgs({ dryRun }) + await runPublishExtensionImpl(args, { env }) if (!dryRun) { - await updateFirefoxVersionNotes({ + await updateFirefoxVersionNotesImpl({ extensionId: env.FIREFOX_EXTENSION_ID, version: manifest.version, jwtIssuer: env.FIREFOX_JWT_ISSUER, diff --git a/tests/unit/release/submit-stores.test.mjs b/tests/unit/release/submit-stores.test.mjs index 209122776..f41ff1fdb 100644 --- a/tests/unit/release/submit-stores.test.mjs +++ b/tests/unit/release/submit-stores.test.mjs @@ -1,21 +1,49 @@ import assert from 'node:assert/strict' +import { EventEmitter } from 'node:events' +import { createRequire } from 'node:module' +import process from 'node:process' import test from 'node:test' import { buildFirefoxReleaseNotes, buildPublishExtensionArgs, + runPublishExtension, FIREFOX_COMPATIBILITY, findMissingArtifacts, findMissingEnv, parseArgs, stripFirefoxExtensionId, + submitStores, updateFirefoxVersionNotes, } from '../../../scripts/submit-stores.mjs' +const require = createRequire(import.meta.url) +const publishExtensionCli = require.resolve('publish-browser-extension/cli') + test('parseArgs detects dry run', () => { - assert.deepEqual(parseArgs(['--dry-run']), { dryRun: true }) - assert.deepEqual(parseArgs([]), { dryRun: false }) + assert.deepEqual(parseArgs(['--dry-run']), { dryRun: true, preflightOnly: false }) + assert.deepEqual(parseArgs(['--preflight-only']), { dryRun: false, preflightOnly: true }) + assert.deepEqual(parseArgs(['--dry-run', '--preflight-only']), { + dryRun: true, + preflightOnly: true, + }) + assert.deepEqual(parseArgs([]), { dryRun: false, preflightOnly: false }) }) +function createStoreEnv() { + return { + CHROME_EXTENSION_ID: 'chrome-id', + CHROME_CLIENT_ID: 'chrome-client', + CHROME_CLIENT_SECRET: 'chrome-secret', + CHROME_REFRESH_TOKEN: 'chrome-refresh', + FIREFOX_EXTENSION_ID: 'chatgptbox', + FIREFOX_JWT_ISSUER: 'firefox-issuer', + FIREFOX_JWT_SECRET: 'firefox-secret', + EDGE_PRODUCT_ID: 'edge-product', + EDGE_CLIENT_ID: 'edge-client', + EDGE_API_KEY: 'edge-key', + } +} + test('findMissingEnv reports all required secrets', () => { const missing = findMissingEnv({}) assert.deepEqual(missing, [ @@ -33,20 +61,14 @@ test('findMissingEnv reports all required secrets', () => { }) test('findMissingEnv accepts required secrets', () => { - const env = { - CHROME_EXTENSION_ID: 'chrome-id', - CHROME_CLIENT_ID: 'chrome-client', - CHROME_CLIENT_SECRET: 'chrome-secret', - CHROME_REFRESH_TOKEN: 'chrome-refresh', - FIREFOX_EXTENSION_ID: 'chatgptbox', - FIREFOX_JWT_ISSUER: 'firefox-issuer', - FIREFOX_JWT_SECRET: 'firefox-secret', - EDGE_PRODUCT_ID: 'edge-product', - EDGE_CLIENT_ID: 'edge-client', - EDGE_API_KEY: 'edge-key', - } + assert.deepEqual(findMissingEnv(createStoreEnv()), []) +}) + +test('findMissingEnv rejects blank required secrets', () => { + const env = createStoreEnv() + env.FIREFOX_JWT_SECRET = ' ' - assert.deepEqual(findMissingEnv(env), []) + assert.deepEqual(findMissingEnv(env), ['FIREFOX_JWT_SECRET']) }) test('findMissingArtifacts reports missing artifacts', async () => { @@ -72,6 +94,108 @@ test('buildPublishExtensionArgs includes all stores and dry run', () => { ]) }) +test('runPublishExtension merges env overrides before spawning publish-extension', async () => { + const child = new EventEmitter() + const spawnCalls = [] + + await runPublishExtension(['--dry-run'], { + baseEnv: { PATH: 'parent-path', CHROME_EXTENSION_ID: 'parent-chrome-id' }, + env: { CHROME_EXTENSION_ID: 'override-chrome-id' }, + spawnImpl: (command, args, options) => { + spawnCalls.push({ command, args, options }) + queueMicrotask(() => child.emit('exit', 0)) + return child + }, + }) + + assert.equal(spawnCalls.length, 1) + assert.equal(spawnCalls[0].command, process.execPath) + assert.equal(spawnCalls[0].args[0], publishExtensionCli) + assert.deepEqual(spawnCalls[0].args.slice(1), ['--dry-run']) + assert.equal(spawnCalls[0].options.shell, false) + assert.equal(spawnCalls[0].options.env.PATH, 'parent-path') + assert.equal(spawnCalls[0].options.env.CHROME_EXTENSION_ID, 'override-chrome-id') +}) + +test('runPublishExtension omits nullish env values before spawning publish-extension', async () => { + const child = new EventEmitter() + const spawnCalls = [] + + await runPublishExtension([], { + baseEnv: { + PATH: 'parent-path', + CHROME_EXTENSION_ID: 'parent-chrome-id', + EMPTY_VALUE: 'parent-empty', + }, + env: { + CHROME_EXTENSION_ID: undefined, + FIREFOX_JWT_SECRET: null, + EMPTY_VALUE: '', + NUMERIC_VALUE: 123, + BOOLEAN_VALUE: false, + }, + spawnImpl: (command, args, options) => { + spawnCalls.push({ command, args, options }) + queueMicrotask(() => child.emit('exit', 0)) + return child + }, + }) + + assert.equal(spawnCalls[0].options.env.PATH, 'parent-path') + assert.equal(spawnCalls[0].options.env.EMPTY_VALUE, '') + assert.equal(spawnCalls[0].options.env.NUMERIC_VALUE, '123') + assert.equal(spawnCalls[0].options.env.BOOLEAN_VALUE, 'false') + assert.equal('CHROME_EXTENSION_ID' in spawnCalls[0].options.env, false) + assert.equal('FIREFOX_JWT_SECRET' in spawnCalls[0].options.env, false) +}) + +test('runPublishExtension invokes publish-extension through node', async () => { + const child = new EventEmitter() + const spawnCalls = [] + + await runPublishExtension(['--dry-run'], { + spawnImpl: (command, args, options) => { + spawnCalls.push({ command, args, options }) + queueMicrotask(() => child.emit('exit', 0)) + return child + }, + }) + + assert.equal(spawnCalls[0].command, process.execPath) + assert.equal(spawnCalls[0].args[0], publishExtensionCli) + assert.deepEqual(spawnCalls[0].args.slice(1), ['--dry-run']) + assert.equal(spawnCalls[0].options.shell, false) +}) + +test('runPublishExtension rejects when publish-extension exits with non-zero code', async () => { + const child = new EventEmitter() + + await assert.rejects( + runPublishExtension(['--dry-run'], { + spawnImpl: () => { + queueMicrotask(() => child.emit('exit', 1)) + return child + }, + }), + /publish-extension exited with code 1/, + ) +}) + +test('runPublishExtension rejects when publish-extension cannot start', async () => { + const child = new EventEmitter() + const error = new Error('spawn failed') + + await assert.rejects( + runPublishExtension(['--dry-run'], { + spawnImpl: () => { + queueMicrotask(() => child.emit('error', error)) + return child + }, + }), + (actual) => actual === error, + ) +}) + test('buildFirefoxReleaseNotes returns the fixed GitHub release URL', () => { assert.equal( buildFirefoxReleaseNotes('2.6.1'), @@ -79,6 +203,286 @@ test('buildFirefoxReleaseNotes returns the fixed GitHub release URL', () => { ) }) +test('submitStores preflight skips store env and publish-extension', async () => { + const publishCalls = [] + + await submitStores({ + argv: ['--preflight-only'], + env: {}, + exists: async () => true, + readJson: async () => ({ version: '2.6.1' }), + runPublishExtensionImpl: async (args) => publishCalls.push(args), + logger: () => {}, + errorLogger: () => {}, + }) + + assert.deepEqual(publishCalls, []) +}) + +test('submitStores preflight takes precedence over dry run', async () => { + const publishCalls = [] + const metadataCalls = [] + + await submitStores({ + argv: ['--dry-run', '--preflight-only'], + env: {}, + exists: async () => true, + readJson: async () => ({ version: '2.6.1' }), + runPublishExtensionImpl: async (args) => publishCalls.push(args), + updateFirefoxVersionNotesImpl: async (options) => metadataCalls.push(options), + logger: () => {}, + errorLogger: () => {}, + }) + + assert.deepEqual(publishCalls, []) + assert.deepEqual(metadataCalls, []) +}) + +test('submitStores preflight fails on missing artifacts before publishing', async () => { + const publishCalls = [] + let manifestRead = false + + await assert.rejects( + submitStores({ + argv: ['--preflight-only'], + env: {}, + exists: async (file) => file !== 'build/firefox-sources.zip', + readJson: async () => { + manifestRead = true + return { version: '2.6.1' } + }, + runPublishExtensionImpl: async (args) => publishCalls.push(args), + logger: () => {}, + errorLogger: () => {}, + }), + /Store submission preflight failed/, + ) + + assert.equal(manifestRead, false) + assert.deepEqual(publishCalls, []) +}) + +test('submitStores preflight fails when Firefox manifest cannot be read', async () => { + const publishCalls = [] + + await assert.rejects( + submitStores({ + argv: ['--preflight-only'], + env: {}, + exists: async () => true, + readJson: async () => { + throw new Error('ENOENT') + }, + runPublishExtensionImpl: async (args) => publishCalls.push(args), + logger: () => {}, + errorLogger: () => {}, + }), + /Store submission preflight failed/, + ) + + assert.deepEqual(publishCalls, []) +}) + +test('submitStores preflight fails when Firefox manifest version is missing or invalid', async () => { + for (const manifest of [ + null, + {}, + { version: '' }, + { version: ' ' }, + { version: 123 }, + { version: null }, + ]) { + const publishCalls = [] + + await assert.rejects( + submitStores({ + argv: ['--preflight-only'], + env: {}, + exists: async () => true, + readJson: async () => manifest, + runPublishExtensionImpl: async (args) => publishCalls.push(args), + logger: () => {}, + errorLogger: () => {}, + }), + /Store submission preflight failed/, + ) + + assert.deepEqual(publishCalls, []) + } +}) + +test('submitStores dry run and submit fail when Firefox manifest is invalid before publishing', async () => { + for (const argv of [['--dry-run'], []]) { + for (const readJson of [ + async () => { + throw new Error('ENOENT') + }, + async () => null, + async () => ({ version: ' ' }), + ]) { + const publishCalls = [] + const metadataCalls = [] + + await assert.rejects( + submitStores({ + argv, + env: createStoreEnv(), + exists: async () => true, + readJson, + runPublishExtensionImpl: async (args) => publishCalls.push(args), + updateFirefoxVersionNotesImpl: async (options) => metadataCalls.push(options), + logger: () => {}, + errorLogger: () => {}, + }), + /Store submission preflight failed/, + ) + + assert.deepEqual(publishCalls, []) + assert.deepEqual(metadataCalls, []) + } + } +}) + +test('submitStores falls back to process env when env is null', async () => { + const storeEnv = createStoreEnv() + const previousEnv = Object.fromEntries( + Object.keys(storeEnv).map((name) => [name, process.env[name]]), + ) + + for (const [name, value] of Object.entries(storeEnv)) { + process.env[name] = value + } + + try { + const publishCalls = [] + const metadataCalls = [] + + await submitStores({ + argv: [], + env: null, + exists: async () => true, + readJson: async () => ({ version: '2.6.1' }), + runPublishExtensionImpl: async (args, options) => publishCalls.push({ args, options }), + updateFirefoxVersionNotesImpl: async (options) => metadataCalls.push(options), + logger: () => {}, + errorLogger: () => {}, + }) + + assert.deepEqual(publishCalls[0].args, buildPublishExtensionArgs({ dryRun: false })) + assert.equal(publishCalls[0].options.env, process.env) + assert.equal(metadataCalls[0].extensionId, storeEnv.FIREFOX_EXTENSION_ID) + assert.equal(metadataCalls[0].jwtIssuer, storeEnv.FIREFOX_JWT_ISSUER) + assert.equal(metadataCalls[0].jwtSecret, storeEnv.FIREFOX_JWT_SECRET) + } finally { + for (const [name, value] of Object.entries(previousEnv)) { + if (value === undefined) { + delete process.env[name] + } else { + process.env[name] = value + } + } + } +}) + +test('submitStores dry run still invokes publish-extension with dry-run args', async () => { + const publishCalls = [] + const metadataCalls = [] + + await submitStores({ + argv: ['--dry-run'], + env: createStoreEnv(), + exists: async () => true, + readJson: async () => ({ version: '2.6.1' }), + runPublishExtensionImpl: async (args) => publishCalls.push(args), + updateFirefoxVersionNotesImpl: async (options) => metadataCalls.push(options), + logger: () => {}, + errorLogger: () => {}, + }) + + assert.deepEqual(publishCalls, [buildPublishExtensionArgs({ dryRun: true })]) + assert.deepEqual(metadataCalls, []) +}) + +test('submitStores dry run fails without store env before publishing', async () => { + const publishCalls = [] + let manifestRead = false + + await assert.rejects( + submitStores({ + argv: ['--dry-run'], + env: {}, + exists: async () => true, + readJson: async () => { + manifestRead = true + return { version: '2.6.1' } + }, + runPublishExtensionImpl: async (args) => publishCalls.push(args), + logger: () => {}, + errorLogger: () => {}, + }), + /Store submission preflight failed/, + ) + + assert.equal(manifestRead, false) + assert.deepEqual(publishCalls, []) +}) + +test('submitStores submit fails without store env before publishing', async () => { + const publishCalls = [] + let manifestRead = false + + await assert.rejects( + submitStores({ + argv: [], + env: {}, + exists: async () => true, + readJson: async () => { + manifestRead = true + return { version: '2.6.1' } + }, + runPublishExtensionImpl: async (args) => publishCalls.push(args), + logger: () => {}, + errorLogger: () => {}, + }), + /Store submission preflight failed/, + ) + + assert.equal(manifestRead, false) + assert.deepEqual(publishCalls, []) +}) + +test('submitStores submit invokes publish-extension and updates Firefox metadata', async () => { + const env = createStoreEnv() + const publishCalls = [] + const metadataCalls = [] + + await submitStores({ + argv: [], + env, + exists: async () => true, + readJson: async () => ({ version: '2.6.1' }), + runPublishExtensionImpl: async (args, options) => publishCalls.push({ args, env: options.env }), + updateFirefoxVersionNotesImpl: async (options) => metadataCalls.push(options), + logger: () => {}, + errorLogger: () => {}, + }) + + assert.deepEqual(publishCalls, [ + { + args: buildPublishExtensionArgs({ dryRun: false }), + env, + }, + ]) + assert.deepEqual(metadataCalls, [ + { + extensionId: 'chatgptbox', + version: '2.6.1', + jwtIssuer: 'firefox-issuer', + jwtSecret: 'firefox-secret', + }, + ]) +}) + test('stripFirefoxExtensionId removes AMO GUID braces', () => { assert.equal(stripFirefoxExtensionId('{chatgptbox@example.com}'), 'chatgptbox@example.com') assert.equal(stripFirefoxExtensionId('chatgptbox'), 'chatgptbox') From 9aa2a511fdc4e1fa10f62b721abcfc2d63efc4c0 Mon Sep 17 00:00:00 2001 From: Peter Dave Hello Date: Sun, 23 Aug 2026 05:43:27 +0800 Subject: [PATCH 2/3] Harden release version sync retries Retry transient fetch and push races, abort conflicted rebases, and surface failures in the step summary without blocking publication. --- .github/workflows/tagged-release.yml | 33 ++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tagged-release.yml b/.github/workflows/tagged-release.yml index a547b6cc9..59653a5bb 100644 --- a/.github/workflows/tagged-release.yml +++ b/.github/workflows/tagged-release.yml @@ -79,13 +79,38 @@ jobs: git config --global user.email "github-actions[bot]@users.noreply.github.com" git config --global user.name "github-actions[bot]" git add src/manifest.json src/manifest.v2.json + report_sync_warning() { + echo "::warning::$1" + echo "Release version sync warning: $1" >> "${GITHUB_STEP_SUMMARY:-/dev/null}" || true + } if git diff --cached --quiet; then echo "No release version changes to commit" + elif ! git commit -m "release v${VERSION}"; then + report_sync_warning "Failed to commit the release version sync; continuing with the release" else - git commit -m "release v${VERSION}" - git fetch origin master - git rebase FETCH_HEAD - git push origin HEAD:master + max_attempts=3 + for attempt in $(seq 1 "${max_attempts}"); do + if ! git fetch origin master; then + if [ "${attempt}" -eq "${max_attempts}" ]; then + report_sync_warning "Failed to fetch origin/master; skipping the release version sync" + else + echo "Release version sync fetch failed; retrying (${attempt}/${max_attempts})" + sleep $((attempt * 2)) + fi + elif ! git rebase FETCH_HEAD; then + git rebase --abort || true + report_sync_warning "Release version sync conflicted with origin/master; skipping the master push" + break + elif git push origin HEAD:master; then + echo "Release version sync succeeded" >> "${GITHUB_STEP_SUMMARY:-/dev/null}" || true + break + elif [ "${attempt}" -eq "${max_attempts}" ]; then + report_sync_warning "Failed to push the release version sync; continuing with the release" + else + echo "Release version sync push rejected; retrying (${attempt}/${max_attempts})" + sleep $((attempt * 2)) + fi + done fi - name: Checkout release tag for artifacts From 466db4b7b7d8808499685a09e4519f487258193d Mon Sep 17 00:00:00 2001 From: Peter Dave Hello Date: Sun, 23 Aug 2026 16:10:33 +0800 Subject: [PATCH 3/3] Harden tagged release retries and version sync Reuse existing releases and clobber partial uploads so retries can finish. Skip master sync when it already contains a newer manifest version. Reject malformed store credentials before they reach publisher code. --- .github/workflows/tagged-release.yml | 32 ++++++++++++++++++----- scripts/submit-stores.mjs | 4 ++- tests/unit/release/submit-stores.test.mjs | 10 ++++++- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/.github/workflows/tagged-release.yml b/.github/workflows/tagged-release.yml index 59653a5bb..bc76014fb 100644 --- a/.github/workflows/tagged-release.yml +++ b/.github/workflows/tagged-release.yml @@ -83,6 +83,19 @@ jobs: echo "::warning::$1" echo "Release version sync warning: $1" >> "${GITHUB_STEP_SUMMARY:-/dev/null}" || true } + release_version_is_newer() { + node -e ' + const [remote, requested] = process.argv.slice(1) + const parse = (version) => version.split(".").map(Number) + const compare = (left, right) => { + for (let index = 0; index < 3; index += 1) { + if (left[index] !== right[index]) return left[index] - right[index] + } + return 0 + } + process.exit(compare(parse(remote), parse(requested)) > 0 ? 0 : 1) + ' "$1" "$2" + } if git diff --cached --quiet; then echo "No release version changes to commit" elif ! git commit -m "release v${VERSION}"; then @@ -97,6 +110,11 @@ jobs: echo "Release version sync fetch failed; retrying (${attempt}/${max_attempts})" sleep $((attempt * 2)) fi + elif release_version_is_newer \ + "$(git show FETCH_HEAD:src/manifest.json | node -p "JSON.parse(require('fs').readFileSync(0, 'utf8')).version")" \ + "$VERSION"; then + report_sync_warning "Skipping older release version sync; origin/master is already newer than v${VERSION}" + break elif ! git rebase FETCH_HEAD; then git rebase --abort || true report_sync_warning "Release version sync conflicted with origin/master; skipping the master push" @@ -142,7 +160,9 @@ jobs: value: ${{ env.VERSION }} - run: | - gh release create "$RELEASE_TAG" -d -F CURRENT_CHANGE.md -t "$RELEASE_TAG" + if ! gh release view "$RELEASE_TAG" >/dev/null 2>&1; then + gh release create "$RELEASE_TAG" -d -F CURRENT_CHANGE.md -t "$RELEASE_TAG" + fi env: GH_TOKEN: ${{ github.token }} RELEASE_TAG: ${{ github.ref_name }} @@ -152,11 +172,11 @@ jobs: - run: npm run release:firefox-sources - run: | - gh release upload "$RELEASE_TAG" build/chromium.zip - gh release upload "$RELEASE_TAG" build/firefox.zip - gh release upload "$RELEASE_TAG" build/safari.dmg - gh release upload "$RELEASE_TAG" build/chromium-without-katex-and-tiktoken.zip - gh release upload "$RELEASE_TAG" build/firefox-without-katex-and-tiktoken.zip + gh release upload "$RELEASE_TAG" --clobber build/chromium.zip + gh release upload "$RELEASE_TAG" --clobber build/firefox.zip + gh release upload "$RELEASE_TAG" --clobber build/safari.dmg + gh release upload "$RELEASE_TAG" --clobber build/chromium-without-katex-and-tiktoken.zip + gh release upload "$RELEASE_TAG" --clobber build/firefox-without-katex-and-tiktoken.zip env: GH_TOKEN: ${{ github.token }} RELEASE_TAG: ${{ github.ref_name }} diff --git a/scripts/submit-stores.mjs b/scripts/submit-stores.mjs index 5c417ff4f..28604a06c 100644 --- a/scripts/submit-stores.mjs +++ b/scripts/submit-stores.mjs @@ -43,7 +43,9 @@ export function parseArgs(args) { } export function findMissingEnv(env = process.env) { - return REQUIRED_ENV.filter((name) => String(env[name] ?? '').trim().length === 0) + return REQUIRED_ENV.filter( + (name) => typeof env[name] !== 'string' || env[name].trim().length === 0, + ) } export async function findMissingArtifacts({ exists = fs.pathExists } = {}) { diff --git a/tests/unit/release/submit-stores.test.mjs b/tests/unit/release/submit-stores.test.mjs index f41ff1fdb..7abcb9f52 100644 --- a/tests/unit/release/submit-stores.test.mjs +++ b/tests/unit/release/submit-stores.test.mjs @@ -71,6 +71,14 @@ test('findMissingEnv rejects blank required secrets', () => { assert.deepEqual(findMissingEnv(env), ['FIREFOX_JWT_SECRET']) }) +test('findMissingEnv rejects non-string required secrets', () => { + const env = createStoreEnv() + env.FIREFOX_EXTENSION_ID = 123 + env.FIREFOX_JWT_SECRET = false + + assert.deepEqual(findMissingEnv(env), ['FIREFOX_EXTENSION_ID', 'FIREFOX_JWT_SECRET']) +}) + test('findMissingArtifacts reports missing artifacts', async () => { const exists = async (file) => file.endsWith('firefox.zip') const missing = await findMissingArtifacts({ exists }) @@ -311,7 +319,7 @@ test('submitStores preflight fails when Firefox manifest version is missing or i } }) -test('submitStores dry run and submit fail when Firefox manifest is invalid before publishing', async () => { +test('submitStores rejects invalid Firefox manifests before publishing', async () => { for (const argv of [['--dry-run'], []]) { for (const readJson of [ async () => {