1602543 - Add a mochitest to check that hidden addons are properly handled in about:debugging

Status: NEW

(DevTools :: about:debugging, task, P3)

Description

[Julian Descottes [:jdescottes]]

This bug is to verify if hidden extensions are properly hidden in about:debugging. :bdanforth reported inconsistent behaviors between about:addons and about:debugging.

about:debugging checks both addon.isSystem and addon.hidden. If any of those is true, the addon is only shown in about:debugging if devtools.aboutdebugging.showHiddenAddons is true.

(also non-debuggable addons are all hidden in about:debugging)

Testing locally the behavior of about:debugging seems fine on Nightly. I have addons with only hidden=true, addons with only isSystem=true, addons with both set to true. Any of those addons are hidden unless I flip the preference devtools.aboutdebugging.showHiddenAddons to true.

Comment 1

[Julian Descottes [:jdescottes]]

Bianca: Hi I filed this bug to start investigating your issue. As I mention in the summary, I couldn't reproduce the issue.
Can you share your steps to reproduce, and possibly the extension you are using for your tests?

Comment 2

[Julian Descottes [:jdescottes]]

As discussed on Zoom, it seems that extensions with hidden=true installed from about:addons are still visible in both about:debugging and about:addons. We should understand what are the criteria to get the hidden flag to true on an addon returned by the addon manager. Maybe this only works with real signed addons?

Comment 3

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

Hey Julian, I just noticed this issue and I wanted to point you which are the criteria used internally:

• the addon is hidden if:
  • the location where it is installed is an hidden location (e.g. the system addons location and the builtin location, this last one means that the extension is packaged into the application)
  • or if it has the hidden property in the manifest AND the addon is recognized as privileged

(See AddonInternal's hidden getter defined in XPIDatabase.jsm)

Comment 4

[Bianca Danforth [:bdanforth]]

:rpl: Thanks for sharing the source code; that is extremely helpful.

or if it has the hidden property in the manifest AND the addon is recognized as privileged

Looking at the isPrivileged method in XPIDatabase.jsm, how could we verify this behavior with an unsigned extension in Nightly? In a local Nightly build? It doesn't look like we could hide the extension without the extension being signed or in a built-in location.

Also I noticed from the comment above that method that there is a different isPrivileged method in Extension.jsm as well. Why is that, and in what situations are they used? For that one it looks like the extension would be hidden if it were temporarily installed and the extensions.legacy.enabled pref set to true?

Comment 5

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

(In reply to Bianca Danforth [:bdanforth] from comment #4)

Looking at the isPrivileged method in XPIDatabase.jsm, how could we verify this behavior with an unsigned extension in Nightly? In a local Nightly build? It doesn't look like we could hide the extension without the extension being signed or in a built-in location.

From what I recall there isn't any other way at the moment.
Signing the extension with the privileged signature is definitely going to make the extension to be detected as privileged,
but if I recall correctly for the shield study extensions we do also use a "development key" to let QA to verify it as a privileged extension
before it is fully approved (iirc they also have to flip a pref to make the dev key to be considered valid) and ready to be signed with the
production key.

The "QA verification of a shield study extensions" usecase seems pretty similar to what we may need in this case, could the same strategy being used for your usecases?

Also I noticed from the comment above that method that there is a different isPrivileged method in Extension.jsm as well. Why is that, and in what situations are they used? For that one it looks like the extension would be hidden if it were temporarily installed and the extensions.legacy.enabled pref set to true?

The isPrivileged method defined on the Extension class is a bit more "relaxed" on non-release builds (in particular Nightly and DevEdition), because we use it internally to detect if an extension can use a privileged API or embed experimental APIs, and we want to let the developers to be able to develop and test their privileged extensions by just installing it temporarily on non-release channels.

Comment 6

[Julian Descottes [:jdescottes]]

Thanks for all the detailed information Luca!

Currently this is only tested using mocks in about:debugging. Do you know if there is a privileged hidden extension that we could use reliably in a mochitest? Any prior art there?

Comment 7

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

There is some prior art, but I'm pretty sure that we can't use it for the about:debugging mochitest:

• In xpcshell test we are able to mock the certDB and make the test extension to be detected as privileged when we need to, without having to sign them or install them into a specific location, by using AddonTestUtils.overrideCertDB but we can't use that in mochitest.
• In mochitest we do have an isPrivileged property that can be used in the properties passed to ExtensionTestUtils.loadExtension, but only if the test extension doesn't also use the useAddonManager property (see https://searchfox.org/mozilla-central/rev/c61720a7d0c094d772059f9d6a7844eb7619f107/toolkit/components/extensions/ExtensionTestCommon.jsm#430-456)

Nevertheless, we do have some privileged hidden extensions installed by default that should be available, though.
[e-mail] could be one that we may use to test the expected behavior on a non-mock extension.

Running the following snippet in the webconsole shows that it is detected as privileged and hidden:

const tmpaddon = await AddonManager.getAddonByID("[e-mail]");
console.log("is privileged and hidden?", tmpaddon.isPrivileged, tmpaddon.hidden);

Let me know if this helps (or if it doesn't and we would need to look for another testing strategy).

Comment 8

[Julian Descottes [:jdescottes]]

Thanks for the feedback!

I wanted to avoid a dependency on a built-in extension (in case they want to remove it/rename it, then the test will fail).
I guess the ideal would be an empty privileged hidden xpi file, that we could install during the test. But using [e-mail] to get started with a real test seems ok.

And I can add a comment in the test for next steps, so whenever this dependency becomes an issue, we can try to create a dedicated xpi.

Comment 9

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

(In reply to Julian Descottes [:jdescottes] from comment #8)

Thanks for the feedback!

I wanted to avoid a dependency on a built-in extension (in case they want to remove it/rename it, then the test will fail).

yeah, I definitely agree that it would be better to don't depend from a specific built-in extension.

I took a look on searchfox for signed .xpi files that we do have in tree and it looks that we have some that may be signed as privileged ([1])
https://searchfox.org/mozilla-central/search?q=&case=false&regexp=false&path=.xpi

e.g. in looks that toolkit/mozapps/extensions/test/xpcshell/data/signing_checks may already have one signed as privileged that we could use
(and also toolkit/components/corroborator/test/xpcshell/data seems to have some).

I haven't tried them to double-check if the signature is actually valid and recognized as privileged, but it may be worth a look.

[1]: I haven't checked if they do also have an hidden property in the manifest, that manifest property is needed to make the extension to be recognized as hidden if it isn't installed in a location that is already hidden on its own.