Skip to content

Commit 0a6bf70

Browse files
committed
Bug 1974419 - Avoid writing to ExtensionPermissions from install prompt on desktop r=willdurand
Differential Revision: https://phabricator.services.mozilla.com/D255531
1 parent f8f403f commit 0a6bf70

File tree

3 files changed

+89
-8
lines changed

3 files changed

+89
-8
lines changed

browser/base/content/test/webextensions/browser_permissions_dismiss.js

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const INSTALL_PAGE = `${BASE}/file_install_extensions.html`;
44
const INSTALL_XPI = `${BASE}/browser_webext_permissions.xpi`;
5+
const ID = "permissions@test.mozilla.org"; // Add-on ID of INSTALL_XPI.
56

67
// With the new dialog design both wildcards and non-wildcards host
78
// permissions are expected to be shown as a single permission entry
@@ -68,7 +69,7 @@ add_task(async function test_tab_switch_dismiss() {
6869
BrowserTestUtils.removeTab(switchTo);
6970
await installCanceled;
7071

71-
let addon = await AddonManager.getAddonByID("permissions@test.mozilla.org");
72+
let addon = await AddonManager.getAddonByID(ID);
7273
is(addon, null, "Extension is not installed");
7374

7475
BrowserTestUtils.removeTab(tab);
@@ -114,10 +115,70 @@ add_task(async function test_add_tab_by_user_and_switch() {
114115
await listener.canceledPromise;
115116
info("Extension installation is canceled");
116117

117-
let addon = await AddonManager.getAddonByID("permissions@test.mozilla.org");
118+
let addon = await AddonManager.getAddonByID(ID);
118119
is(addon, null, "Extension is not installed");
119120

120121
AddonManager.removeInstallListener(listener);
121122
BrowserTestUtils.removeTab(tab);
122123
BrowserTestUtils.removeTab(newTab);
123124
});
125+
126+
// Regression test for https://bugzilla.mozilla.org/show_bug.cgi?id=1974419
127+
// ExtensionPermissions.get() lazily populates the StartupCache. This method
128+
// should not be used when an extension is not installed, to avoid persisting
129+
// permission data for a non-installed extension.
130+
add_task(async function test_no_permissions_stored_after_dismiss() {
131+
// This part could fail if any of the tests before (which also trigger
132+
// installation of INSTALL_XPI) somehow populate the permissions.
133+
Assert.deepEqual(
134+
await getCachedPermissions(ID),
135+
null,
136+
"ExtensionPermissions should not contain entry before installation"
137+
);
138+
139+
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, INSTALL_PAGE);
140+
141+
let installCanceled = new Promise(resolve => {
142+
let listener = {
143+
onInstallCancelled() {
144+
AddonManager.removeInstallListener(listener);
145+
resolve();
146+
},
147+
};
148+
AddonManager.addInstallListener(listener);
149+
});
150+
151+
SpecialPowers.spawn(gBrowser.selectedBrowser, [INSTALL_XPI], function (url) {
152+
content.wrappedJSObject.installMozAM(url);
153+
});
154+
155+
const panel = await promisePopupNotificationShown("addon-webext-permissions");
156+
157+
let privateBrowsingCheckbox = panel.querySelector(
158+
".webext-perm-privatebrowsing > moz-checkbox"
159+
);
160+
ok(
161+
BrowserTestUtils.isVisible(privateBrowsingCheckbox),
162+
"Private browsing checkbox is present in install prompt"
163+
);
164+
Assert.deepEqual(
165+
await getCachedPermissions(ID),
166+
null,
167+
"ExtensionPermissions should not be written to during installation"
168+
);
169+
170+
// Cancel installation.
171+
panel.secondaryButton.click();
172+
await installCanceled;
173+
174+
let addon = await AddonManager.getAddonByID(ID);
175+
is(addon, null, "Extension is not installed");
176+
177+
Assert.deepEqual(
178+
await getCachedPermissions(ID),
179+
null,
180+
"ExtensionPermissions should not be written to after installation"
181+
);
182+
183+
BrowserTestUtils.removeTab(tab);
184+
});

browser/base/content/test/webextensions/head.js

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
ChromeUtils.defineESModuleGetters(this, {
22
AddonTestUtils: "resource://testing-common/AddonTestUtils.sys.mjs",
3+
ExtensionParent: "resource://gre/modules/ExtensionParent.sys.mjs",
34
ExtensionsUI: "resource:///modules/ExtensionsUI.sys.mjs",
45
});
56

@@ -9,11 +10,7 @@ const BASE = getRootDirectory(gTestPath).replace(
910
);
1011

1112
ChromeUtils.defineLazyGetter(this, "Management", () => {
12-
// eslint-disable-next-line no-shadow
13-
const { Management } = ChromeUtils.importESModule(
14-
"resource://gre/modules/Extension.sys.mjs"
15-
);
16-
return Management;
13+
return ExtensionParent.apiManager;
1714
});
1815

1916
let { CustomizableUITestUtils } = ChromeUtils.importESModule(
@@ -679,6 +676,24 @@ async function interactiveUpdateTest(autoUpdate, checkFn) {
679676
);
680677
}
681678

679+
async function getCachedPermissions(extensionId) {
680+
const NotFound = Symbol("extension ID not found in permissions cache");
681+
try {
682+
return await ExtensionParent.StartupCache.permissions.get(
683+
extensionId,
684+
() => {
685+
// Throw error to prevent the key from being created.
686+
throw NotFound;
687+
}
688+
);
689+
} catch (e) {
690+
if (e === NotFound) {
691+
return null;
692+
}
693+
throw e;
694+
}
695+
}
696+
682697
// The tests in this directory install a bunch of extensions but they
683698
// need to uninstall them before exiting, as a stray leftover extension
684699
// after one test can foul up subsequent tests.

browser/modules/ExtensionsUI.sys.mjs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,12 @@ export var ExtensionsUI = {
414414

415415
const incognitoPermissionName = "internal:privateBrowsingAllowed";
416416
let grantPrivateBrowsingAllowed = false;
417-
if (showIncognitoCheckbox) {
417+
if (
418+
showIncognitoCheckbox &&
419+
// Usually false, unless the user tries to install a XPI file whose ID
420+
// matches an already-installed add-on.
421+
(await lazy.AddonManager.getAddonByID(addon.id))
422+
) {
418423
let { permissions } = await lazy.ExtensionPermissions.get(addon.id);
419424
grantPrivateBrowsingAllowed = permissions.includes(
420425
incognitoPermissionName

0 commit comments

Comments
 (0)