From 902c7239c32e5fc7e88f8c4015ab70a02089b0d1 Mon Sep 17 00:00:00 2001 From: Nikshay Date: Fri, 2 Oct 2026 20:06:13 +0530 Subject: [PATCH 1/2] fix: Warned about missing extension IDs when running Firefox --- src/cmd/run.js | 15 +++- tests/functional/test.cli.run.js | 32 +++++++- tests/unit/test-cmd/test.run.js | 137 ++++++++++++++++++++++++++++++- 3 files changed, 180 insertions(+), 4 deletions(-) diff --git a/src/cmd/run.js b/src/cmd/run.js index 08c0033fd2..f24d898f47 100644 --- a/src/cmd/run.js +++ b/src/cmd/run.js @@ -6,7 +6,9 @@ import defaultBuildExtension from './build.js'; import * as defaultFirefoxApp from '../firefox/index.js'; import { connectWithMaxRetries as defaultFirefoxClient } from '../firefox/remote.js'; import { createLogger } from '../util/logger.js'; -import defaultGetValidatedManifest from '../util/manifest.js'; +import defaultGetValidatedManifest, { + getManifestId, +} from '../util/manifest.js'; import { UsageError } from '../errors.js'; import { createExtensionRunner, @@ -109,6 +111,7 @@ export default async function run( } const runners = []; + let hasFirefoxRunner = false; const commonRunnerParams = { // Common options. @@ -140,6 +143,7 @@ export default async function run( params: firefoxDesktopRunnerParams, }); runners.push(firefoxDesktopRunner); + hasFirefoxRunner = true; } if (target && target.includes('firefox-android')) { @@ -186,6 +190,7 @@ export default async function run( params: firefoxAndroidRunnerParams, }); runners.push(firefoxAndroidRunner); + hasFirefoxRunner = true; } if (target && target.includes('chromium')) { @@ -204,6 +209,14 @@ export default async function run( runners.push(chromiumRunner); } + if (hasFirefoxRunner && !getManifestId(manifestData)) { + log.warn( + 'No explicit extension ID found in manifest.json. ' + + 'Add browser_specific_settings.gecko.id: temporary loading can succeed ' + + 'without an ID, but permanent installation or signing may require one.', + ); + } + const extensionRunner = new MultiExtensionRunner({ runners, }); diff --git a/tests/functional/test.cli.run.js b/tests/functional/test.cli.run.js index f2df80fff5..bba66b910f 100644 --- a/tests/functional/test.cli.run.js +++ b/tests/functional/test.cli.run.js @@ -1,5 +1,5 @@ import path from 'path'; -import { writeFileSync } from 'fs'; +import { readFileSync, writeFileSync } from 'fs'; import { describe, it } from 'mocha'; import { assert } from 'chai'; @@ -15,6 +15,36 @@ import { const EXPECTED_MESSAGE = 'Fake Firefox binary executed correctly.'; describe('web-ext run', () => { + it('warns without verbose mode and still temporarily installs an extension without an ID', () => + withTempAddonDir({ addonPath: minimalAddonPath }, async (srcDir) => { + const manifestPath = path.join(srcDir, 'manifest.json'); + const manifest = JSON.parse(readFileSync(manifestPath, 'utf8')); + delete manifest.applications.gecko.id; + writeFileSync(manifestPath, JSON.stringify(manifest)); + + const { exitCode, stdout, stderr } = await execWebExt( + [ + 'run', + '--no-reload', + '--source-dir', + srcDir, + '--firefox', + fakeFirefoxPath, + ], + { env: { EXPECTED_MESSAGE, addonPath: srcDir } }, + ).waitForExit; + + assert.equal(exitCode, 0, stderr); + assert.include(stderr, 'browser_specific_settings.gecko.id'); + assert.include( + stderr, + 'permanent installation or signing may require one', + ); + assert.lengthOf(stderr.match(/No explicit extension ID/g), 1); + assert.notInclude(stdout, 'No explicit extension ID'); + assert.include(stdout, 'Installed'); + })); + it( 'accepts: --no-reload --watch-file --watch-files --source-dir ' + 'SRCDIR --firefox FXPATH --watch-ignored', diff --git a/tests/unit/test-cmd/test.run.js b/tests/unit/test-cmd/test.run.js index 140251ca59..206f02378d 100644 --- a/tests/unit/test-cmd/test.run.js +++ b/tests/unit/test-cmd/test.run.js @@ -7,6 +7,7 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; import { + basicManifest, fixturePath, FakeExtensionRunner, getFakeFirefox, @@ -129,7 +130,7 @@ describe('run', () => { }); it('turns sourceDir into an absolute path', async () => { - const getFakeManifest = sinon.spy(); + const getFakeManifest = sinon.stub().resolves(basicManifest); const cmd = await prepareRun(); await cmd.run( @@ -252,12 +253,144 @@ describe('run', () => { it('allows to replace manifest parser', async () => { const cmd = await prepareRun(); - const getFakeManifest = sinon.spy(); + const getFakeManifest = sinon.stub().resolves(basicManifest); await cmd.run({}, { getValidatedManifest: getFakeManifest }); assert.equal(getFakeManifest.called, true); }); + describe('missing Firefox extension ID warning', () => { + let writeStub; + let runSpy; + + beforeEach(async () => { + const { consoleStream } = await import('../../../src/util/logger.js'); + writeStub = sinon.stub(consoleStream, 'write'); + runSpy = sinon.spy(FakeExtensionRunner.prototype, 'run'); + }); + + afterEach(() => { + writeStub.restore(); + runSpy.restore(); + }); + + function warnings() { + return writeStub + .getCalls() + .map(({ args }) => JSON.parse(args[0])) + .filter(({ level }) => level === 40); + } + + async function runWithManifest(target, manifestProperties = {}) { + const cmd = await prepareRun(); + return cmd.run( + { target }, + { + getValidatedManifest: sinon.stub().resolves({ + name: 'Test extension', + version: '1.0', + manifest_version: 2, + ...manifestProperties, + }), + }, + ); + } + + const firefoxTargets = [ + ['the default target', undefined], + ['an empty target array', []], + ['Firefox Desktop', ['firefox-desktop']], + ['Firefox Android', ['firefox-android']], + ['both Firefox targets', ['firefox-desktop', 'firefox-android']], + ['Firefox Desktop and Chromium', ['firefox-desktop', 'chromium']], + ['Firefox Android and Chromium', ['firefox-android', 'chromium']], + ['all targets', ['firefox-desktop', 'firefox-android', 'chromium']], + ]; + + for (const [description, target] of firefoxTargets) { + it(`warns once and continues running with ${description}`, async () => { + const runner = await runWithManifest(target); + + const messages = warnings(); + assert.lengthOf(messages, 1); + assert.include(messages[0].msg, 'browser_specific_settings.gecko.id'); + assert.include(messages[0].msg, 'manifest.json'); + assert.include(messages[0].msg, 'temporary'); + sinon.assert.calledOnce(runSpy); + const warningCall = writeStub + .getCalls() + .find(({ args }) => JSON.parse(args[0]).level === 40); + assert.isBelow(warningCall.callId, runSpy.firstCall.callId); + assert.instanceOf(runner, FakeExtensionRunner); + }); + } + + it('does not warn for Chromium-only runs without an ID', async () => { + await runWithManifest(['chromium']); + + assert.lengthOf(warnings(), 0); + sinon.assert.calledOnce(runSpy); + }); + + it('also warns for Manifest V3 without an ID', async () => { + await runWithManifest(['firefox-desktop'], { manifest_version: 3 }); + + assert.lengthOf(warnings(), 1); + sinon.assert.calledOnce(runSpy); + }); + + it('preserves manifest validation failures without warning or starting', async () => { + const cmd = await prepareRun(); + const error = new Error('Invalid manifest'); + + const rejection = await assert.isRejected( + cmd.run({}, { getValidatedManifest: sinon.stub().rejects(error) }), + ); + + assert.strictEqual(rejection, error); + assert.lengthOf(warnings(), 0); + sinon.assert.notCalled(runSpy); + }); + + for (const property of ['browser_specific_settings', 'applications']) { + it(`does not warn when ${property}.gecko.id is present`, async () => { + await runWithManifest(['firefox-desktop', 'firefox-android'], { + [property]: { gecko: { id: 'test@example.com' } }, + }); + + assert.lengthOf(warnings(), 0); + sinon.assert.calledOnce(runSpy); + }); + } + + it('warns when browser_specific_settings.gecko overrides a legacy ID', async () => { + await runWithManifest(['firefox-desktop'], { + browser_specific_settings: { gecko: {} }, + applications: { gecko: { id: 'legacy@example.com' } }, + }); + + assert.lengthOf(warnings(), 1); + sinon.assert.calledOnce(runSpy); + }); + + it('does not warn when applications provides the only gecko section', async () => { + await runWithManifest(['firefox-desktop'], { + browser_specific_settings: {}, + applications: { gecko: { id: 'legacy@example.com' } }, + }); + + assert.lengthOf(warnings(), 0); + }); + + it('warns when the explicit ID is empty', async () => { + await runWithManifest(['firefox-desktop'], { + browser_specific_settings: { gecko: { id: '' } }, + }); + + assert.lengthOf(warnings(), 1); + }); + }); + it('returns ExtensonRunner', async () => { const cmd = await prepareRun(); const extensionRunner = await cmd.run(); From 20aa29cb4d1e5cf55113d25966e108f6b99005a1 Mon Sep 17 00:00:00 2001 From: Nikshay Date: Tue, 13 Oct 2026 12:18:43 +0530 Subject: [PATCH 2/2] test: Waited for module mocks before importing the run command --- tests/unit/helpers.js | 4 +++- tests/unit/test-cmd/test.run.js | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/unit/helpers.js b/tests/unit/helpers.js index 708cb9507d..6593233bc5 100644 --- a/tests/unit/helpers.js +++ b/tests/unit/helpers.js @@ -330,7 +330,9 @@ export function mockModule({ path.resolve(path.join(baseDir, moduleURL)), ).href; - td.replaceEsm(fullModuleURL, namedExports, defaultExport); + // The returned promise resolves once the mock is registered: modules + // imported before that are not guaranteed to get the mocked module. + return td.replaceEsm(fullModuleURL, namedExports, defaultExport); } export function resetMockModules() { diff --git a/tests/unit/test-cmd/test.run.js b/tests/unit/test-cmd/test.run.js index 206f02378d..d6d13de2df 100644 --- a/tests/unit/test-cmd/test.run.js +++ b/tests/unit/test-cmd/test.run.js @@ -87,19 +87,19 @@ describe('run', () => { ); chromiumRunnerStub = sinon.stub(chromiumModule, 'ChromiumExtensionRunner'); - mockModule({ + await mockModule({ moduleURL: '../../../src/extension-runners/firefox-android.js', importerModuleURL: import.meta.url, namedExports: firefoxAndroidModule, }); - mockModule({ + await mockModule({ moduleURL: '../../../src/extension-runners/firefox-desktop.js', importerModuleURL: import.meta.url, namedExports: firefoxDesktopModule, }); - mockModule({ + await mockModule({ moduleURL: '../../../src/extension-runners/chromium.js', importerModuleURL: import.meta.url, namedExports: chromiumModule,