Hi Alexandre, I'm a Flatpak developer and helped Mozilla get the Flatpak set up and publishing to Flathub - thanks for working on this issue and #1785278. I've put some comments on the related ticket https://bugzilla.mozilla.org/show_bug.cgi?id=1682462#c31 but the tl;dr is that in the Flatpak world, you can't map /etc paths from the host system outside the sandbox - the whole of /etc is provided by the runtime environment inside the sandbox, apart from specific files which Flatpak itself maps from outside (stuff like /etc/resolv.conf).

At the moment I think we have two problems - the Flatpak itself provides a policies.json to disable the non-functional in-app updater (see https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/docker/firefox-flatpak/policies.json) which we need to handle a different way, and then we can provide an extension point that the system administrator can use to add a config folder which does show up inside the Flatpak - eg /app/etc/firefox and have Firefox check there. Does the patch let us have the "/etc/firefox" path be influenced at runtime with an env var? If so we can set that and I can send a patch to get the Flatpak path in place.

Outside the sandbox, the sysadmin or an OS vendor that relies on Flatpak for Firefox can provide a symlink from the extension path back to /etc, which is how Endless OS arranges Chromium extensions:
/var/lib/flatpak/extension/org.chromium.Chromium.Policy.system-policies/x86_64/1 -> /etc/chromium-browser

Does the patch let us have the "/etc/firefox" path be influenced at runtime with an env var?

Ah, is this MOZ_SYSTEM_CONFIG_DIR ? 👍

(In reply to Robert McQueen from comment #2)

Does the patch let us have the "/etc/firefox" path be influenced at runtime with an env var?

Ah, is this MOZ_SYSTEM_CONFIG_DIR ? 👍

Yes. So for context, we fixed bug 1785278 for Snap and when finalizing the patch I thought it would be a good idea to makae it workable for Flatpak as well. That variable was mostly introduced at first to be able to write tests, but also in case people needed to override, which seems to be the case.

So it seems you could use MOZ_SYSTEM_CONFIG_DIR for that matter? Feel free to steal this bug if you can fix it faster than me :)

I can throw a patch together for the Flatpak bits but and then we can try and work out how to test it! Which branch of Firefox do I need to be able to use this env var?

In parallel, can you tell me whether setting MOZ_SYSTEM_CONFIG_DIR will prevent either of these being read correctly which the Flatpak relies on? Are policies and preferences added together from multiple files? Preferences I think yes but policies possibly not? Or can we get rid of any of this in favour of code patches that better adapt to Firefox being used inside a Flatpak?

ramcq@xi:~$ flatpak run --command=/bin/bash org.mozilla.firefox
[📦 org.mozilla.firefox ~]$ cd /app/lib/firefox/
[📦 org.mozilla.firefox firefox]$ cat distribution/policies.json 
{
  "policies": {
    "DisableAppUpdate": true,
    "DontCheckDefaultBrowser": true
  }
}
[📦 org.mozilla.firefox firefox]$ cat browser/defaults/preferences/default-preferences.js 
/*global pref*/
/*eslint no-undef: "error"*/
/* This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
pref("intl.locale.requested", "");
pref("app.update.auto", false);
pref("app.update.enabled", false);
pref("app.update.autoInstallEnabled", false);
pref("browser.shell.checkDefaultBrowser", false);
pref("spellchecker.dictionary_path", "/usr/share/hunspell");
[📦 org.mozilla.firefox firefox]$ 

(Also these seem a bit repetitive/redundant anyway...?)

This landed yesterday, so it's only on mozilla/central for the moment. We use MOZ_SYSTEM_CONFIG_DIR to:

• source extra default preferences ($MOZ_SYSTEM_CONFIG_DIR/defaults/pref)
• source AutoConfig file

I dont know about policies.

I've attached a patch which creates a Flatpak extension point named org.mozilla.firefox.system-config which if provided (which can be done by the system administrator or OS vendor in the extensions path) is mapped into /app/etc/firefox inside the sandbox, and Mozilla is directed to look there with the new env var. This is I think all you need for the "Flatpak bit" for a config to be provided from outside the sandbox.

The other half of the task is to understand whether this has any consequences with how policies and preferences are already used in the Flatpak - it seems like preferences are merged but maybe Mike Kaply can help clarify the situation for the policies. Some of the preferences + policies set by the Flatpak are probably garbage and it would be better for Firefox to detect when run in a Flatpak environment and disable the updater by itself.

ramcq@xi:~/.local/share/flatpak$ ls extension/org.mozilla.firefox.system-config/x86_64/1/hello 
extension/org.mozilla.firefox.system-config/x86_64/1/hello
ramcq@xi:~/.local/share/flatpak$ flatpak run --user --command=/bin/bash org.mozilla.firefox
[📦 org.mozilla.firefox flatpak]$ echo $MOZ_SYSTEM_CONFIG_DIR
/app/etc/firefox
[📦 org.mozilla.firefox flatpak]$ ls /app/etc/firefox
hello

I would really like to get to a point where Flatpak doesn't need to use policies to disable updating. I opened this bug a while ago:

https://bugzilla.mozilla.org/show_bug.cgi?id=1769265

but it hasn't got any traction.

I'm starting to think I should just detect Flatpak and disable the updater internally, then we won't have the policy merging problem.

If I understand the above patch (from comment #8) correctly, it would allow to pass set policy for sysadmins using the Firefox Flatpak. This would be tremendously valuable alone.

(In reply to Mike Kaply [:mkaply] from comment #9)

I'm starting to think I should just detect Flatpak and disable the updater internally, then we won't have the policy merging problem.

👍 That's probably a lot more simple than what's being discussed in #1769265 and is sufficient for the Flatpak and Snap cases so that we can step out of needing to touch policy at all, which would be much cleaner.

(In reply to behrmann from comment #10)

If I understand the above patch (from comment #8) correctly, it would allow to pass set policy for sysadmins using the Firefox Flatpak. This would be tremendously valuable alone.

It should do, if my understanding of #1785278 is correct that it also changes system policy to be read from the path we can set with MOZ_SYSTEM_CONFIG_DIR.

What else needs to be done now ? Do you want to submit this change on phabricator?

I've never done that before, I can have a dig and try to figure out. I'd be equally happy if someone else wanted to submit it. It's already useful and I believe what will happen is that if a local admin provides a policies.json it will be used in preference to the one built in to the Flatpak.

However in light of bug 1769265 we can strip out one half of our Flatpak policies.json - the other is disabling the default browser check because at present Flatpak doesn't offer a "blessed" API to modify the user's default app settings. Is there a light touch way for us to simply turn this check off in Flatpak mode?

(I'm keen to get rid of the baked in policies.json not least because it opens the way for site-specific policies with no conflicts/regressions, but also very recently an end-user was seeking help because they thought computer had been hacked because Firefox was showing "controlled by your organisation"... 😬)

However in light of bug 1769265 we can strip out one half of our Flatpak policies.json - the other is disabling the default browser check because at present Flatpak doesn't offer a "blessed" API to modify the user's default app settings. Is there a light touch way for us to simply turn this check off in Flatpak mode?

Let's get a bug opened and I'll add a Flatpak check for the that.

(In reply to Mike Kaply [:mkaply] from comment #14)

However in light of bug 1769265 we can strip out one half of our Flatpak policies.json - the other is disabling the default browser check because at present Flatpak doesn't offer a "blessed" API to modify the user's default app settings. Is there a light touch way for us to simply turn this check off in Flatpak mode?

Let's get a bug opened and I'll add a Flatpak check for the that.

Looking a little harder at this; maybe it's not necessary to do in code? Looking at making an additional patch which removes the policies.json and the update stuff from default-preferences.js - we seem to disable the browser check in two other places, once in default-preferences.js (https://github.com/mozilla/gecko-dev/blob/master/taskcluster/docker/firefox-flatpak/default-preferences.js#L10) and once in distribution.ini (https://github.com/mozilla-partners/flatpak/blob/master/desktop/flatpak/distribution/distribution.ini#L8). What is the difference between the distributions.ini and setting default preferences in JavaScript? Should we be consolidating on one or the other?

Looking at the other preferences, as far as I can tell the spellchecker.dictionary_path is necessary, ie we can't define it anywhere else very easily (unless we set DICTDIR but this seems "worse") but I can't figure out what intl.locale.requested is doing - is that necessary?

This defines a Flatpak extension point which allows the system config
dir (ie /etc/flatpak) to be defined from outside the Flatpak sandbox.
A Linux system administrator can use what Flatpak calls an unmanaged
extension - ie a folder in the right place - to provide their own
policies.json, autoconf, etc.

For a system-wide Flatpak install in /var/lib/flatpak the path would
be /var/lib/flatpak/extension/org.mozilla.firefox.system-config/x86_64/1
and can even be a symlink to the host /etc/firefox if desired for
consistency with unsandboxed Firefox instances.

The patch in bug 1769265 means that Firefox disables its own updater when
running under a Linux package such as Flatpak. We can stop shipping a
policies.json now so that the system administrator is free to provide
their own site policies as they wish.

Depends on D161021

Submitted WIP patches; not sure who should be marked to review - let me know.

Did you verify that my Snap/Flatpak update check definitely disables the updater?

You can mark me as the reviewer. You should be able to do a "Request review" in the phabricator UI and set it to me and it will move out of WIP.

Looking at the other preferences, as far as I can tell the spellchecker.dictionary_path is necessary, ie we can't define it anywhere else very easily (unless we set DICTDIR but this seems "worse") but I can't figure out what intl.locale.requested is doing - is that necessary?

intl.locale.requested set to blank tells Firefox to use the language pack that corresponds to the operating system. So for Flatpak which I think bundles all languages, we make sure we use the right one.

Speaking of bundling all languages, assuming we do that, there's a better way to do it that is more performant.

(In reply to Robert McQueen [:ramcq] from comment #15)

we seem to disable the browser check in two other places, once in default-preferences.js (https://github.com/mozilla/gecko-dev/blob/master/taskcluster/docker/firefox-flatpak/default-preferences.js#L10) and once in distribution.ini (https://github.com/mozilla-partners/flatpak/blob/master/desktop/flatpak/distribution/distribution.ini#L8). What is the difference between the distributions.ini and setting default preferences in JavaScript? Should we be consolidating on one or the other?

Was this question answered anywhere?

(In reply to Mike Kaply [:mkaply] from comment #19)

Did you verify that my Snap/Flatpak update check definitely disables the updater?

No, I suppose it would be more responsible to test this end to end before moving out of WIP. :) I'll have to work out how to fake out enough of the Taskcluster env to make this work. Are there binary tarballs that contain that patch now, which I can just test the repackaging with?

(In reply to Mike Kaply [:mkaply] from comment #21)

Looking at the other preferences, as far as I can tell the spellchecker.dictionary_path is necessary, ie we can't define it anywhere else very easily (unless we set DICTDIR but this seems "worse") but I can't figure out what intl.locale.requested is doing - is that necessary?

intl.locale.requested set to blank tells Firefox to use the language pack that corresponds to the operating system. So for Flatpak which I think bundles all languages, we make sure we use the right one.

OK, cool. Sounds like a keeper then. :)

Speaking of bundling all languages, assuming we do that, there's a better way to do it that is more performant.

Flatpak has special handling for Locales extensions which are structured correctly, so we move the langpacks into the paths Flatpak understands, and then symlink them back to where Firefox can find them. So it appears like all languages are bundled, but in practice the Flatpak client will only download the subdirectory matching the user or system locale. https://bugzilla.mozilla.org/show_bug.cgi?id=1621074#c21 has a bit more detail.

(In reply to Emerson Bernier from comment #22)

(In reply to Robert McQueen [:ramcq] from comment #15)

What is the difference between the distributions.ini and setting default preferences in JavaScript? Should we be consolidating on one or the other?

Was this question answered anywhere?

Seconded! :)

Any nightly should have that fix in it now.

we seem to disable the browser check in two other places, once in default-preferences.js (https://github.com/mozilla/gecko-dev/blob/master/taskcluster/docker/firefox-flatpak/default-preferences.js#L10) and once in distribution.ini (https://github.com/mozilla-partners/flatpak/blob/master/desktop/flatpak/distribution/distribution.ini#L8). What is the difference between the distributions.ini and setting default preferences in JavaScript? Should we be consolidating on one or the other?

There's no difference. Since we're going to have a distribution.ini anyway (for the distribution ID), we can move everything from default-preferences to distribution.ini and get rid of it.

The system config extension was added in https://phabricator.services.mozilla.com/D168803

I don't know if there is anything left to do here?

Has policies.json been removed for the flatpak now?