1974419 - extensions.webextensions.uuids populated by ExtensionData via loadManifestFromWebManifest (without installing extension)

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P2)

Description

[Rob Wu [:robwu]]

When an extension package is parsed, we write its ID and random UUID to extensions.webextensions.uuids. This is intended for installed extensions, but the logic can also be triggered when an extension is NOT installed, e.g. before installation.

STR:

1. Visit about:config and search for extensions.webextensions.uuids
2. Initiate the installation of any add-on. E.g. from https://addons.mozilla.org/ and cancel the installation.
   • This also works with unsigned extensions, e.g. save https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/test/xpinstall/unsigned.xpi and then enter the file:-URL of the xpi file from the address bar.
3. Look at about:config again and see if its value has changed.

Expected:

• No changes.

Actual:

• The add-on ID (and random uuid) is listed. E.g. in case of the unsigned xpi test, [e-mail] is near the end of extensions.webextensions.uuids

More context on the discovery and affected unit tests:

I added support for --compare-preferences for browser tests in bug 1816549, and ran a try push to see the effect of running every mochitest with this preference set. Among the results, I noticed multiple test failures due to extensions.webextensions.uuids being modified past the end of the test, which I enumerated at https://bugzilla.mozilla.org/show_bug.cgi?id=1363464#c5

Most of these are test-only issues, except for some failures in bc8 and bc29. Here is a small excerpt (but see comment 5 of bug 1363464 for more logs). To reproduce (dependent on bug 1816549), run: ./mach test browser/base/content/test/webextensions/browser_permissions_dismiss.js --compare-preferences

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/webextensions/browser_permissions_dismiss.js | changed preference: extensions.webextensions.uuids

To determine where the entry came from, I logged the stack trace from UUIDMap.get, which reveals the following calls (I added links to the relevant code, even if the line number differs from what is displayed):

Creating uuid 8ebe0b93-e3cb-4091-b4da-c176d7c33d9f for [e-mail] - get@resource://gre/modules/Extension.sys.mjs:445:49
getURL@resource://gre/modules/Extension.sys.mjs:1096:27
parseManifest@resource://gre/modules/Extension.sys.mjs:2115:45
async*loadManifest@resource://gre/modules/Extension.sys.mjs:2340:12
loadManifestFromWebManifest@resource://gre/modules/addons/XPIInstall.sys.mjs:471:34
async*loadManifest@resource://gre/modules/addons/XPIInstall.sys.mjs:682:45
async*loadManifest@resource://gre/modules/addons/XPIInstall.sys.mjs:1584:28
onStopRequest@resource://gre/modules/addons/XPIInstall.sys.mjs:2677:14

NOTE: Although this issue is specifically about preferences, I also note that non-installed extension data is also persisted in other places. E.g. in the permissions key of the startup cache (webext.sc.lz4). This is not ideal, but at least the startup cache file gets wiped on every browser update, which limits the duration of the accumulated data.

In the extensions.webextensions.uuids case, the accumulation of uuids does have a bad impact. The maximum length of a pref is 1024 * 1024, and exceeding that length causes NS_ERROR_ILLEGAL_VALUE to be thrown, which in turn prevents any extension installation attempt from succeeding (which is shown to the user as "Installation aborted because the add-on appears to be corrupt"). Even before this limit is reached, at 4 KB characters the following logspam already starts appearing in the console (source, example: bug 1363464):

Warning: attempting to write 123456 bytes to preference extensions.webextensions.uuids. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file.

Comment 1

[Rob Wu [:robwu]]

This getURL() call in parseManifest is part of a ExtensionData.loadManifest call. It exists because the current logic of computing permissions is located there, but in practice it is only really required for the Extension subclass, not ExtensionData.

All uses of ExtensionData with loadManifest are as follows:

• Parsing extension to prepare for extension install (=this bug), update or metadata lookup: https://searchfox.org/mozilla-central/rev/058836008f131ae5591d04952a1500c9f94bedbc/toolkit/mozapps/extensions/internal/XPIInstall.sys.mjs#454-471
• Parsing manifest for extension update handling: https://searchfox.org/mozilla-central/rev/058836008f131ae5591d04952a1500c9f94bedbc/toolkit/components/extensions/ExtensionParent.sys.mjs#102-103
• Unit test to check permission parsing behavior: https://searchfox.org/mozilla-central/rev/058836008f131ae5591d04952a1500c9f94bedbc/toolkit/components/extensions/test/xpcshell/test_ext_permission_warnings.js#820-833

All of these use cases only care about what is in the manifest, and not what other permissions are stored (some of which may be bogus - see bug 1766915). My recommended resolution for this bug is therefore to move the ExtensionPermissions.get + merge logic in ExtensionData's parseManifest from ExtensionData to Extension. This especially makes sense because the relevant logic that keeps the permission properties in sync is already part of Extension.

One potential step further is to move permissions, allowedOrigins (and dataCollectionPermissions) fields from ExtensionData's loadManifest to Extension. To evaluate the feasibility, I examined all current uses of these fields on ExtensionData:

• activePermissions getter uses all three properties. It is only used from getAll in ext-permissions.js, where extension is an extension instance, not ExtensionData. This method can therefore be moved without issues.
• hasPermissions looks at permissions, which is relied upon by the restrictSchemes getter to check whether the extension has declared mozillaAddons, which is a privileged-only permission cannot be an optional permission. Multiple other fields and methods in ExtensionData depend on restrictSchemes, including those that are used by XPIInstall. This means that we can only move permissions from ExtensionData to Extension if we offer an alternative of ensuring correctness of the restrictSchemes flag in ExtensionData.

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 1974419 - Avoid uuid writing from ExtensionData

Comment 3

[Rob Wu [:robwu]]

Attachment: Bug 1974419 - Avoid writing to ExtensionPermissions from install prompt on desktop

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f8f403f9ee6a
https://hg.mozilla.org/integration/autoland/rev/5ec0b61849d7
Avoid uuid writing from ExtensionData r=willdurand
https://github.com/mozilla-firefox/firefox/commit/0a6bf708e7c3
https://hg.mozilla.org/integration/autoland/rev/db7e41aa7458
Avoid writing to ExtensionPermissions from install prompt on desktop r=willdurand

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5be4aa84732c
https://hg.mozilla.org/integration/autoland/rev/3749c1f762c1
Revert "Bug 1974419 - Avoid writing to ExtensionPermissions from install prompt on desktop r=willdurand" for causing bc failures @ browser_headless_screenshot_1.js

Comment 6

[amarc]

Backed out for causing bc failures @ browser_headless_screenshot_1.js

• Backout link
• Push with failures
• Failure Log
• Failure Log 2

Comment 7

[Rob Wu [:robwu]]

The failing test does something unrealistic that is restricted by the second part of this bug. I'll fix the test at https://searchfox.org/mozilla-central/rev/311230215f69ac675d0fb4d5c0f5108228f17388/toolkit/mozapps/extensions/test/xpinstall/browser_doorhanger_installs.js#1326-1336

Comment 8

[Rob Wu [:robwu]]

I fixed the issue in browser_doorhanger_installs.js in the updated patch. I don't think that the browser_headless_screenshot_1.js failure is related to my patch. That one is more likely related to bug 1959339. For posterity, the relevant part of the log is:

TEST-START | browser/components/shell/test/browser_headless_screenshot_1.js
GECKO(9470) | *** You are running in headless mode.
GECKO(9470) | >>> Screenshot saved to: /tmp/headless_test_screenshot.png
>>> WARNING: At least one completion condition is taking too long to complete. Conditions: [{"name":"XPIProvider shutdown","state":"(none)","filename":"resource://gre/modules/addons/XPIProvider.sys.mjs","lineNumber":2837,"stack":["resource://gre/modules/addons/XPIProvider.sys.mjs:startup:2837","resource://gre/modules/AddonManager.sys.mjs:callProvider:228","resource://gre/modules/AddonManager.sys.mjs:_startProvider:537","resource://gre/modules/AddonManager.sys.mjs:startup:750","resource://gre/modules/AddonManager.sys.mjs:startup:3705","resource://gre/modules/amManager.sys.mjs:observe:73"]}] Barrier: quit-application
TEST-UNEXPECTED-FAIL | browser/components/shell/test/browser_headless_screenshot_1.js | Test timed out -

Comment 10

[Intermittent Failures Robot]

9 failures were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 9

Table

	no_variant
linux1804-x86_64/tsan	1
linux2404-x86_64/debug	1
linux2404-x86_64/opt	1
macosx1470-x86_64/debug	1
macosx1470-x86_64/opt	1
windows11-x86/debug	1
windows11-x86/opt	1
windows11-x86_64/debug	1
windows11-x86_64/opt	1

For more details, see:

https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1974419&startday=2025-06-30&endday=2025-07-06&tree=all

Comment 11

[Rob Wu [:robwu]]

FYI I haven't landed the patches in this bug yet because comment 6 claims that the patch would be backed out for failures in browser_headless_screenshot_1.js. Although this is not related to this bug, I'm fixing that anyway in bug 1959339, which I expect to land soon. After that lands and sticks, I'll land the patches here.

Comment 12

[Rob Wu [:robwu]]

The patch to bug 1959339 is landing. I'll push the patches on this bug to Lando later today or tomorrow.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/fb852155a0a3
https://hg.mozilla.org/integration/autoland/rev/1a9512feb11c
Avoid uuid writing from ExtensionData r=willdurand
https://github.com/mozilla-firefox/firefox/commit/c94d05b1ed44
https://hg.mozilla.org/integration/autoland/rev/eb927771d1d2
Avoid writing to ExtensionPermissions from install prompt on desktop r=willdurand

Comment 14

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/1a9512feb11c
https://hg.mozilla.org/mozilla-central/rev/eb927771d1d2