1958723 - Fail with a better "invalid web extension" error message when trying to install an unpacked web extension permanently

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, enhancement, P3)

Description

[aaronklinker1]

Steps to reproduce:

Reproduction: https://github.com/aklinker1/webdriver-bidi-firefox-bug

Quick summary:

Browser version: Firefox Developer Edition, 138.0b3

Here's my project structure:

demo-extension/
  manifest.json
  background.js
index.ts

A very simple MV2 extension:

// mainfest.json
{
  "name": "Test",
  "version": "1.0.0",
  "manifest_version": 2,
  "background": {
    "scripts": ["background.js"]
  }
}

// background.js
console.log("Hello Background");

And here's the script I'm running to start firefox and connect to bidi/the debugger:

import { spawn } from "node:child_process";
import { resolve } from "node:path";

const extensionDir = resolve("demo-extension");

const browserProcess = spawn(
  process.env.FIREFOX_BIN ??
    "/Applications/Firefox Developer Edition.app/Contents/MacOS/firefox",
  ["--remote-debugging-port=9222"],
  { stdio: "inherit" },
);

// Wait for the web socket to be ready...
await Bun.sleep(5000);

const socket = new WebSocket("ws://localhost:9222/session");
await new Promise<void>((resolve, reject) => {
  socket.onopen = () => resolve();
  socket.onclose = () => reject(Error("Connection closed"));
  socket.onerror = (err) => reject(Error("Connection error", { cause: err }));
});

const session = await sendSocketRequest(socket, "session.new", {
  capabilities: {},
});
console.log({ session });
const extension = await sendSocketRequest(socket, "webExtension.install", {
  extensionData: {
    type: "path",
    path: extensionDir,
  },
});
console.log({ extension });

function sendSocketRequest<T>(
  socket: WebSocket,
  method: string,
  params: Record<string, any>,
): Promise<T> {
  // See linked repo for full implementation
}

Actual results:

$ bun index.ts
WebDriver BiDi listening on ws://127.0.0.1:9222
...
SENDING {
  id: 0,
  method: "session.new",
  params: {
    capabilities: {},
  },
}
{
  session: {
    sessionId: "a04cf48c-0c8e-4743-8963-ef66085de4bf",
    capabilities: {
      acceptInsecureCerts: false,
      browserName: "firefox",
      browserVersion: "138.0",
      platformName: "mac",
      userAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:138.0) Gecko/20100101 Firefox/138.0",
      "moz:buildID": "20250404092111",
      "moz:headless": false,
      "moz:platformVersion": "24.3.0",
      "moz:processID": 1882,
      "moz:profile": "/Users/<user>/Library/Application Support/Firefox/Profiles/s7ma1ryh.dev-edition-default-1692818333672",
      "moz:shutdownTimeout": 60000,
      proxy: [Object ...],
    },
  },
}
SENDING {
  id: 1,
  method: "webExtension.install",
  params: {
    extensionData: {
      type: "path",
      path: "/Users/aklinker1/Development/github.com/wxt-dev/runner/demo-extension",
    },
  },
}
error: Could not install Add-on: ERROR_CORRUPT_FILE: The file appears to be corrupt.
 cause: {
  "type": "error",
  "id": 1,
  "error": "invalid web extension",
  "message": "Could not install Add-on: ERROR_CORRUPT_FILE: The file appears to be corrupt.",
  "stacktrace": "RemoteError@chrome://remote/content/shared/RemoteError.sys.mjs:8:8\nWebDriverError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:199:5\nInvalidWebExtensionError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:485:5\ninstallAddon@chrome://remote/content/shared/Addon.sys.mjs:56:11\n"
}

Bun v1.2.8 (macOS arm64)

Expected results:

The extension should have been installed successfully. I have verified that the extension is valid, it can be installed manually from about:debugging#/runtime/this-firefox and runs successfully.

Comment 1

[aaronklinker1]

I will also add that the same error happens if I use type: archivePath and type: base64.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Could you please check with a recent Firefox Nightly build? At the moment the install command will install the extension permanently in Firefox 138 but I changed that to temporary on bug 1947678, because it would fail for unsigned and unpacked extensions.

Comment 3

[aaronklinker1]

Yup, that worked!

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Great to hear! I'm going to request uplift to beta for the temporary install by default. So if it is accepted we can have the fix already in version 138.

Luca, I'm still puzzled about the ERROR_CORRUPT_FILE failure that is raised by the user when the unpacked extension is tried to get installed permanently. I would rather expect a ERROR_SIGNEDSTATE_REQUIRED error here as well - similar to bug 1957620. The extension as used can be found as part of https://github.com/aklinker1/webdriver-bidi-firefox-bug. Thanks!

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #4)

Great to hear! I'm going to request uplift to beta for the temporary install by default. So if it is accepted we can have the fix already in version 138.

Luca, I'm still puzzled about the ERROR_CORRUPT_FILE failure that is raised by the user when the unpacked extension is tried to get installed permanently. I would rather expect a ERROR_SIGNEDSTATE_REQUIRED error here as well - similar to bug 1957620. The extension as used can be found as part of https://github.com/aklinker1/webdriver-bidi-firefox-bug. Thanks!

That ERROR_CORRUPT_FILE error is being originated from AddonInstall's loadManifest method here, the underlying reason is that the verifySignedStateForRoot method provided by the DirPackage class (which is the one used for an unpacked extension) is returning signedState set to AddonManager.SIGNEDSTATE_UNKNOWN here.

The XPIProvider is currently using the ERROR_CORRUPT_FILE error as a generic error with the general user population in mind, for which this kind of scenario (trying to install an unpacked extension as permently installed) it is harder to be hit in practice (e.g. the UI should already be disallowing to the user to select a directory for permanent installation).

On the contrary, in the context of WebDriver BiDi installing an extension, the user is not unlikely to be an extension developer for which a more precise would be more likely actionable then for a more general user population.

One option could be to consider changing DirPackage class's verifySignedStateForRoot method to return a signedState set to AddonManager.SIGNEDSTATE_MISSING so that AddonInstall's loadManifest method will reject with the AddonManager.ERROR_SIGNEDSTATE_REQUIRED error under this corner case, it would slightly change the behavior of that method under this corner case but a regular users shouldn't be able to hit that through the Firefox UI and so it may not be a noticeable difference in practice (and it wouldn't change much given that in both case we are raising an error and the installation is going to be cancelled anyway).

Alternatively, I wonder if considering to introduce some additional validation on the WebDriver BiDi's WebExtensionModule's install method side, right before calling the lazy.Addon.installWithPath call, may be a reasonable alternative option, one that could also allow us to produce more precise error messages specifically for WebDriver BiDi than the ones that the XPIProvider would be raising.

Henrik, what do you think? are there motivations that may make introducing some additional validation on the WebDriver BiDi's WebExtensionModule's install method side for this kind of corner cae not a viable or reasonable option?

Separately (but kind of related) I was also wondering about the different ERROR_FILE_ACCESS error mentioned in Bug 1957620, do we know what I could change in the STR I used to reproduce this one locally to be able to trigger the ERROR_FILE_ACCESS error mentioned in Bug 1957620 instead of the ERROR_CORRUPT_FILE?

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #5)

Alternatively, I wonder if considering to introduce some additional validation on the WebDriver BiDi's WebExtensionModule's install method side, right before calling the lazy.Addon.installWithPath call, may be a reasonable alternative option, one that could also allow us to produce more precise error messages specifically for WebDriver BiDi than the ones that the XPIProvider would be raising.

Yes, extending support for this scenario in both WebDriver BiDi and classic should be fine. Since we now install web extensions temporarily by default, this will mainly affect users explicitly setting the moz:permanent parameter to install an unpacked extension permanently - which is Firefox only. In that case, we can return an unsupported operation error. An unsigned XPI is already correctly handled.

If you'd like to propose additional changes to the Add-on Manager, feel free to file a separate bug so that other consumers of the APIs can benefit as well.

Separately (but kind of related) I was also wondering about the different ERROR_FILE_ACCESS error mentioned in Bug 1957620, do we know what I could change in the STR I used to reproduce this one locally to be able to trigger the ERROR_FILE_ACCESS error mentioned in Bug 1957620 instead of the ERROR_CORRUPT_FILE?

Mind requesting more information from me on that other bug? I'll try to have a look at it soon. Thanks!

Comment 7

[Ben Chatterton [:benchatt]]

So, to clarify just a little, the check inside of the ExtensionDataType.Path case should be that moz:permanent is set to false. If it's set to true we want to raise Unsupported Operation. Is that right?

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Sure, here's a clearer and more concise rephrasing:

The main case we need to handle here is when moz:permanent=true is set. In this scenario, a signed web extension is required, but signing isn’t possible for unpacked extensions. Since the Addon Manager returns a somewhat unclear error in this case, we want to proactively block this code path with a clear explanation stating that installation isn’t possible. Getting the same wording as for the XPI case would be good.

We also need a new test to cover the various scenarios: an unsigned unpacked extension, an unsigned XPI, and a base64-encoded version of that unsigned XPI. This can be added to:

https://searchfox.org/mozilla-central/source/testing/web-platform/mozilla/tests/webdriver/bidi/web_extension/moz_permanent.py

Let me know if you need any additional details.

Comment 9

[Ben Chatterton [:benchatt]]

So I've added what I believe is the correct error message, but when I added the test to permanently install unsigned XPIs and base64 versions of the same, results seem to indicate that those XPIs are installed permanently and no error is thrown. I have a try run in progress here, and you can see the test code. I'm wondering if I missed something about what happens when you attempt to install an unsigned XPI... (I expected it to throw UnsupportedOperation)

https://hg-edge.mozilla.org/try/rev/dfb2af62d837d3c6e7f8c0380d81282e923ddc24

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Ah, so the common profile actually sets xpinstall.signatures.required to false. That means that we would allow the installation of unsigned add-ons. Given that the new test is in the Mozilla-only category you could use clear_pref similar to what this test is doing it and reset the preference for just this test, and reset it afterward.

Comment 11

[Ben Chatterton [:benchatt]]

Attachment: Bug 1958723: [webdriver] Improve error for attempts to install unpacked extension permanently. r=whimboo

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

To be in sync with the unsigned XPI case we are going to fail with an invalid web extension error for now.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cc5f900836da
https://hg.mozilla.org/integration/autoland/rev/046b49e8c58e
[webdriver] Improve error for attempts to install unpacked extension permanently. r=webdriver-reviewers,whimboo

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/bb1115572fd5
https://hg.mozilla.org/integration/autoland/rev/883ac0a4be8a
Revert "Bug 1958723: [webdriver] Improve error for attempts to install unpacked extension permanently. r=webdriver-reviewers,whimboo" for causing webdriver failures in moz_permanent.py.

Comment 15

[Serban Stanca [:SerbanS]]

Reverted this because it was causing webdriver failures in moz_permanent.py.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | /_mozilla/webdriver/bidi/web_extension/moz_permanent.py | test_install_with_permanent[signed-archivePath-default] - webdriver.bidi.error.UnknownErrorException: unknown error (No such file or directory: /builds/worker/workspace/build/tests/web-platform/tests/webdriver/tests/support/webextensions/firefox/signed.xpi)

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

As discussed on Matrix/Slack the issue is only manifesting on Android. That happens because we do not push test files to the device yet (see bug 1762066). Just mark them as expected fail for this platform with a reference to this bug.

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/53098 for changes under testing/web-platform/tests

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 19

[Ben Chatterton [:benchatt]]

(In reply to Serban Stanca [:SerbanS] from comment #15)

Reverted this because it was causing webdriver failures in moz_permanent.py.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | /_mozilla/webdriver/bidi/web_extension/moz_permanent.py | test_install_with_permanent[signed-archivePath-default] - webdriver.bidi.error.UnknownErrorException: unknown error (No such file or directory: /builds/worker/workspace/build/tests/web-platform/tests/webdriver/tests/support/webextensions/firefox/signed.xpi)

A fix is ready, waiting for GCP outage issues to resolve before we can determine whether it's good to go

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1ce8148bfee4
https://hg.mozilla.org/integration/autoland/rev/6a26f249f772
[webdriver] Improve error for attempts to install unpacked extension permanently. r=webdriver-reviewers,whimboo

Comment 21

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/6a26f249f772

Comment 22

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/53209 for changes under testing/web-platform/tests

Comment 23

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot