fix: Warned about missing extension IDs when running Firefox

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,
   });

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',

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() {

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,
@@ -86,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,
@@ -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();