Skip to content

Commit 278d47d

Browse files
committed
Bug 2061439 - Install ExtensionSettings add-ons from update_url when install_url is absent r=joporter,extension-reviewers,rpl
Entries with only update_url fell through to the AMO lookup, which fails for a self-hosted add-on. Resolve the XPI from the update manifest instead, and drop any update_url naming Firefox's own update service, which also overrides the manifest and default URLs for later update checks. Differential Revision: https://phabricator.services.mozilla.com/D322423
1 parent bee0963 commit 278d47d

3 files changed

Lines changed: 455 additions & 2 deletions

File tree

‎browser/components/enterprisepolicies/Policies.sys.mjs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ ChromeUtils.defineESModuleGetters(lazy, {
4646
clearBlockedAboutPages: "resource://gre/modules/PoliciesHelpers.sys.mjs",
4747
clearRunOnceModification: "resource://gre/modules/PoliciesHelpers.sys.mjs",
4848
describePreferenceFailure: "resource://gre/modules/PoliciesHelpers.sys.mjs",
49+
discardAMOUpdateURLs: "resource://gre/modules/PoliciesHelpers.sys.mjs",
4950
installAddonFromURL: "resource://gre/modules/PoliciesHelpers.sys.mjs",
5051
installAddonFromRepository: "resource://gre/modules/PoliciesHelpers.sys.mjs",
52+
installAddonFromUpdateURL: "resource://gre/modules/PoliciesHelpers.sys.mjs",
5153
pemToBase64: "resource://gre/modules/PoliciesHelpers.sys.mjs",
5254
processMIMEInfo: "resource://gre/modules/PoliciesHelpers.sys.mjs",
5355
replacePathVariables: "resource://gre/modules/PoliciesHelpers.sys.mjs",
@@ -1788,6 +1790,7 @@ export var Policies = {
17881790

17891791
ExtensionSettings: {
17901792
onBeforeAddons(manager, param) {
1793+
lazy.discardAMOUpdateURLs(param, "ExtensionSettings");
17911794
try {
17921795
manager.setExtensionSettings(param);
17931796
} catch (e) {
@@ -1865,7 +1868,20 @@ export var Policies = {
18651868
"ExtensionSettings"
18661869
);
18671870
} else if (!existingAddon) {
1868-
lazy.installAddonFromRepository(extensionID, "ExtensionSettings");
1871+
// An unusable update_url is an error, not a reason to install a
1872+
// different build of the add-on from AMO.
1873+
if (extensionSettings[extensionID].update_url) {
1874+
lazy.installAddonFromUpdateURL(
1875+
extensionSettings[extensionID].update_url,
1876+
extensionID,
1877+
"ExtensionSettings"
1878+
);
1879+
} else {
1880+
lazy.installAddonFromRepository(
1881+
extensionID,
1882+
"ExtensionSettings"
1883+
);
1884+
}
18691885
}
18701886
manager.disallowFeature(`uninstall-extension:${extensionID}`);
18711887
if (

‎browser/components/enterprisepolicies/tests/xpcshell/test_extensionsettings_install_without_url.js‎

Lines changed: 322 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,325 @@ add_task(
184184
await AddonTestUtils.promiseShutdownManager();
185185
}
186186
);
187+
188+
// With only update_url, the newest compatible version in the update manifest is
189+
// installed; a newer but incompatible entry is skipped. An entry for an
190+
// unrelated add-on listed first in the manifest is ignored, confirming the
191+
// lookup is keyed by extension ID rather than by response order.
192+
add_task(
193+
{
194+
pref_set: [["extensions.checkUpdateSecurity", false]],
195+
},
196+
async function test_install_from_update_url() {
197+
await AddonTestUtils.promiseStartupManager();
198+
199+
const id = "update-url-only@test";
200+
const unrelatedId = "unrelated-extension@test";
201+
202+
for (const version of ["1.0", "2.0"]) {
203+
server.registerFile(
204+
`/data/update-url-only-${version}.xpi`,
205+
AddonTestUtils.createTempWebExtensionFile({
206+
manifest: {
207+
version,
208+
browser_specific_settings: { gecko: { id } },
209+
},
210+
})
211+
);
212+
}
213+
214+
server.registerFile(
215+
"/data/unrelated-extension-5.0.xpi",
216+
AddonTestUtils.createTempWebExtensionFile({
217+
manifest: {
218+
version: "5.0",
219+
browser_specific_settings: { gecko: { id: unrelatedId } },
220+
},
221+
})
222+
);
223+
224+
server.registerPathHandler("/data/update.json", (request, response) => {
225+
response.setHeader("Content-Type", "application/json");
226+
response.write(
227+
JSON.stringify({
228+
addons: {
229+
[unrelatedId]: {
230+
updates: [
231+
{
232+
version: "5.0",
233+
update_link: `${BASE_URL}/unrelated-extension-5.0.xpi`,
234+
},
235+
],
236+
},
237+
[id]: {
238+
updates: [
239+
{
240+
version: "1.0",
241+
update_link: `${BASE_URL}/update-url-only-1.0.xpi`,
242+
},
243+
{
244+
version: "2.0",
245+
update_link: `${BASE_URL}/update-url-only-2.0.xpi`,
246+
},
247+
{
248+
version: "3.0",
249+
update_link: `${BASE_URL}/update-url-only-3.0.xpi`,
250+
applications: { gecko: { strict_min_version: "9999" } },
251+
},
252+
],
253+
},
254+
},
255+
})
256+
);
257+
});
258+
259+
let extension = ExtensionTestUtils.expectExtension(id);
260+
await Promise.all([
261+
extension.awaitStartup(),
262+
setupPolicyEngineWithJson({
263+
policies: {
264+
ExtensionSettings: {
265+
[id]: {
266+
installation_mode: "force_installed",
267+
update_url: `${BASE_URL}/update.json`,
268+
},
269+
},
270+
},
271+
}),
272+
]);
273+
274+
const addon = await AddonManager.getAddonByID(id);
275+
notEqual(addon, null, "Addon should be installed from the update manifest");
276+
equal(addon.version, "2.0", "Newest compatible version was installed");
277+
278+
await addon.uninstall();
279+
await AddonTestUtils.promiseShutdownManager();
280+
}
281+
);
282+
283+
// update_hash from the manifest is enforced against the downloaded XPI. No prefs
284+
// needed: a strong update_hash keeps the http update_link past
285+
// checkUpdateSecurity.
286+
add_task(async function test_update_hash_mismatch_does_not_install() {
287+
await AddonTestUtils.promiseStartupManager();
288+
289+
const id = "update-url-badhash@test";
290+
291+
server.registerFile(
292+
"/data/badhash.xpi",
293+
AddonTestUtils.createTempWebExtensionFile({
294+
manifest: {
295+
version: "1.0",
296+
browser_specific_settings: { gecko: { id } },
297+
},
298+
})
299+
);
300+
301+
server.registerPathHandler(
302+
"/data/update-badhash.json",
303+
(request, response) => {
304+
response.setHeader("Content-Type", "application/json");
305+
response.write(
306+
JSON.stringify({
307+
addons: {
308+
[id]: {
309+
updates: [
310+
{
311+
version: "1.0",
312+
update_link: `${BASE_URL}/badhash.xpi`,
313+
update_hash: `sha256:${"0".repeat(64)}`,
314+
},
315+
],
316+
},
317+
},
318+
})
319+
);
320+
}
321+
);
322+
323+
const downloadFailed = AddonTestUtils.promiseInstallEvent("onDownloadFailed");
324+
325+
await setupPolicyEngineWithJson({
326+
policies: {
327+
ExtensionSettings: {
328+
[id]: {
329+
installation_mode: "force_installed",
330+
update_url: `${BASE_URL}/update-badhash.json`,
331+
},
332+
},
333+
},
334+
});
335+
await downloadFailed;
336+
337+
equal(
338+
await AddonManager.getAddonByID(id),
339+
null,
340+
"Addon should not be installed when the update_hash does not match"
341+
);
342+
343+
await AddonTestUtils.promiseShutdownManager();
344+
});
345+
346+
// An update_url the URL parser rejects is an error, not a fallback to AMO.
347+
add_task(
348+
{
349+
pref_set: [
350+
["extensions.getAddons.get.url", `${BASE_URL}/amo.json?guid=%IDS%`],
351+
],
352+
},
353+
async function test_unparseable_update_url_does_not_fall_back_to_amo() {
354+
await AddonTestUtils.promiseStartupManager();
355+
356+
const id = "update-url-unparseable@test";
357+
358+
const errorLogged = TestUtils.consoleMessageObserved(msg =>
359+
msg.wrappedJSObject.arguments[0]?.includes(
360+
`update_url for ${id} is not a valid URL`
361+
)
362+
);
363+
364+
await setupPolicyEngineWithJson({
365+
policies: {
366+
ExtensionSettings: {
367+
[id]: {
368+
installation_mode: "force_installed",
369+
update_url: "http://",
370+
},
371+
},
372+
},
373+
});
374+
await errorLogged;
375+
376+
equal(
377+
await AddonManager.getAddonByID(id),
378+
null,
379+
"Addon should not be installed from AMO when update_url is unusable"
380+
);
381+
382+
await AddonTestUtils.promiseShutdownManager();
383+
}
384+
);
385+
386+
// An update_url naming the default add-on update service is discarded outright,
387+
// so neither the install nor a later update check can reuse it, and the add-on
388+
// still installs through the AMO fallback.
389+
add_task(
390+
{
391+
pref_set: [
392+
["extensions.getAddons.get.url", `${BASE_URL}/amo.json?guid=%IDS%`],
393+
],
394+
},
395+
async function test_update_url_matching_amo_default_is_discarded() {
396+
await AddonTestUtils.promiseStartupManager();
397+
398+
const id = "update-url-amo-default@test";
399+
const backgroundId = "update-url-amo-background@test";
400+
const defaults = Services.prefs.getDefaultBranch(null);
401+
const defaultUpdateHostname = new URL(
402+
defaults.getCharPref("extensions.update.url")
403+
).hostname;
404+
const backgroundUpdateHostname = new URL(
405+
defaults.getCharPref("extensions.update.background.url")
406+
).hostname;
407+
const xpiURL = `${BASE_URL}/update-url-amo-default.xpi`;
408+
409+
server.registerFile(
410+
"/data/update-url-amo-default.xpi",
411+
AddonTestUtils.createTempWebExtensionFile({
412+
manifest: {
413+
version: "1.0",
414+
browser_specific_settings: { gecko: { id } },
415+
},
416+
})
417+
);
418+
419+
server.registerPathHandler("/data/amo.json", (request, response) => {
420+
response.setHeader("Content-Type", "application/json");
421+
response.write(JSON.stringify(makeAMOResponse(id, xpiURL)));
422+
});
423+
424+
const warningLogged = TestUtils.consoleMessageObserved(msg =>
425+
msg.wrappedJSObject.arguments[0]?.includes(`Invalid update_url for ${id}`)
426+
);
427+
428+
let extension = ExtensionTestUtils.expectExtension(id);
429+
await Promise.all([
430+
extension.awaitStartup(),
431+
setupPolicyEngineWithJson({
432+
policies: {
433+
ExtensionSettings: {
434+
[id]: {
435+
installation_mode: "force_installed",
436+
update_url: `http://${defaultUpdateHostname}:8080/update/VersionCheck.php`,
437+
},
438+
[backgroundId]: {
439+
installation_mode: "allowed",
440+
update_url: `http://${backgroundUpdateHostname}:8080/update/VersionCheck.php`,
441+
},
442+
},
443+
},
444+
}),
445+
]);
446+
await warningLogged;
447+
448+
const addon = await AddonManager.getAddonByID(id);
449+
notEqual(
450+
addon,
451+
null,
452+
"Addon should still be installed via AMO when update_url matches the default service"
453+
);
454+
equal(addon.version, "1.0", "Addon installed from the AMO fallback");
455+
456+
equal(
457+
Services.policies.getExtensionSettings(id)?.update_url,
458+
undefined,
459+
"update_url should be discarded, not just skipped, so later update checks don't reuse it"
460+
);
461+
462+
equal(
463+
Services.policies.getExtensionSettings(backgroundId)?.update_url,
464+
undefined,
465+
"The background update service hostname should be discarded too"
466+
);
467+
468+
await addon.uninstall();
469+
await AddonTestUtils.promiseShutdownManager();
470+
}
471+
);
472+
473+
// The "%...%" placeholders normal update checks substitute are an add-on manager
474+
// implementation detail, not policy syntax: "%" must be followed by two hex
475+
// digits to pass the schema's "format": "uri" check. The whole ExtensionSettings
476+
// entry is dropped when one property fails validation, so installation_mode goes
477+
// with it and the add-on is never installed by any path.
478+
add_task(async function test_update_url_with_placeholders_fails_validation() {
479+
await AddonTestUtils.promiseStartupManager();
480+
481+
const id = "update-url-placeholders@test";
482+
483+
const schemaErrorLogged = TestUtils.consoleMessageObserved(msg =>
484+
msg.wrappedJSObject.arguments[0]?.includes(
485+
'update_url: String does not match format "uri"'
486+
)
487+
);
488+
489+
await setupPolicyEngineWithJson({
490+
policies: {
491+
ExtensionSettings: {
492+
[id]: {
493+
installation_mode: "force_installed",
494+
update_url: `${BASE_URL}/update-placeholders.json?item=%ITEM_ID%`,
495+
},
496+
},
497+
},
498+
});
499+
await schemaErrorLogged;
500+
501+
equal(
502+
await AddonManager.getAddonByID(id),
503+
null,
504+
"Addon should not be installed when update_url fails schema validation"
505+
);
506+
507+
await AddonTestUtils.promiseShutdownManager();
508+
});

0 commit comments

Comments
 (0)