modules/tools/browsers: finish the browser abstraction - #239
Open
RISK-alt wants to merge 6 commits into
Open
Conversation
This rework the abstraction module for Firefox into a generic abstraction for both major browsers: Chromium and Firefox. This enables developers to let user choose between Firefox and Chromium and configure them uniformly. Obviously, abstracting all options is impossible, so a developer should always consider using the fine-grained API of a browser if needed. Signed-off-by: Ryan Lahfa <ryan.lahfa.ext@numerique.gouv.fr>
The module as introduced did not evaluate:
- homepage-dashboard.nix read securix.homepage-dashboard.{enable,bookmarks}
without declaring either option;
- lockFlags was typed as an enum although it holds a list of flags;
- nullOr was applied to two arguments instead of to (submodule proxyConfig);
- firefox.nix and chromium.nix consumed a homepage option neither declared;
- chromium.nix dereferenced cfg.proxy unconditionally although it defaults to
null, and read autoConfigUrl where option-types.nix declares autoConfigURL;
- the per-browser extensions were typed loosely enough that each browser could
receive the other one's shape.
Type the extensions submodule so that each browser gets what it expects.
The Chromium proxy policy also used mkIf inside programs.chromium.extraOpts.
That option is a plain attribute set, so the module system never discharges the
property, and {_type = "if"; ...} would be serialised as-is into the policy
JSON. Build the conditional parts with optionalAttrs instead.
Signed-off-by: risk-alt <aldu6974@gmail.com>
securix.browser.enable defaulted to false and modules/tools/default.nix only imported the module, so the browser abstraction removed Firefox from the system instead of configuring it. Enable it by default, and default the browser list to Firefox, which is what Sécurix shipped so far. The default lives on the option rather than in modules/tools/default.nix on purpose: list options concatenate on merge, so a definition there would force users to mkForce their way out of Firefox. Restore the two things the refactor dropped along the way: the Tridactyl native messaging host, and the Bitwarden extension. programs.chromium only writes policy files, so install the package explicitly, otherwise picking Chromium configures a browser that is not there. Drop the bare firefox entry from environment.systemPackages: programs.firefox already installs the wrapped package, and shipping both puts two bin/firefox in collision inside the system profile. Enabling the module for real also surfaced one last type error: the browser-wide lockFlags were typed as listOf lockFlagEnum, where lockFlagEnum is the list of allowed flags rather than a type. Signed-off-by: risk-alt <aldu6974@gmail.com>
programs.chromium.initialPrefs feeds /etc/chromium/initial_preferences, whose schema is the initial preferences one, not the Chrome Enterprise policy one. Every policy written there was silently ignored. Move them to extraOpts, which lands in /etc/chromium/policies/managed/extra.json. Three keys do not exist as written, checked against the policy list in components/policy/resources/templates/policies.yaml: - IsolateOrigins is typed as a string holding a comma separated list of origins; the intent here, isolating every origin, is SitePerProcess; - GenAiSettings is GenAiDefaultSettings, an int-enum where 2 means "do not allow GenAI features"; - BuiltInAIAPIIsEnabled is BuiltInAIAPIsEnabled. Wire the home page through programs.chromium.homepageLocation, which was the remaining TODO on the Chromium side, and honour the extension lock flag the way the Firefox module already does. Pass null rather than an empty list when no extension is configured, so that ExtensionInstallForcelist stays out of the generated policy file. Signed-off-by: risk-alt <aldu6974@gmail.com>
The browser modules only produce configuration, so assert on what they emit: the Firefox policy file, both Chromium managed policy files, the presence of each browser in the system profile, and the local homepage dashboard answering on its port. This is what would have caught the policies landing in initial_preferences. Signed-off-by: risk-alt <aldu6974@gmail.com>
securix.firefox is now one browser behind the securix.browser abstraction. Signed-off-by: risk-alt <aldu6974@gmail.com>
RISK-alt
requested review from
jdauphant-dinum,
olebrouster-dinum,
plegrand-dinum and
rlahfa-dinum
as code owners
August 11, 2026 21:51
Contributor
Author
|
For this PR, as usual, I used AI to help me put the problem into context, after which I drafted a plan/implementation strategy to help me think through the solution. I then provided clear instructions so that it could carry out what I wanted to do more quickly. After that, I checked the code for completeness (hence the “temp”). Finally, I made a few corrections. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This picks up #71, which was said not to be moving any time soon, and finishes
it. The first commit is Ryan's original, rebased on main with his authorship and
sign-off intact; everything after it is mine.
The module did not evaluate. Seven distinct issues, fixed in one commit and
listed in its message. The one worth calling out:
programs.chromium.extraOptsis a
types.attrs, whose merge is a shallow//, so the module system neverdischarges a nested
mkIf. The two inProxySettingswould have beenserialised into the policy JSON as
{"_type":"if",...}. They are built withoptionalAttrsnow.The abstraction removed Firefox from the system.
securix.browser.enabledefaulted to false and nothing enabled it. The browser list now defaults to
Firefox, which is what Sécurix shipped so far, and the default lives on the
option rather than in
modules/tools/default.nixbecause list optionsconcatenate on merge — a definition there would force users to
mkForcetheirway out. Chromium stays one option away. The Tridactyl native messaging host and
the Bitwarden extension, both dropped by the refactor, are back.
Chromium was never installed.
programs.chromiumonly writes policy files,so picking Chromium configured a browser that was not there.
The Chromium policies did not apply. They were written to
initialPrefs,which feeds
/etc/chromium/initial_preferencesand follows the initialpreferences schema, not the enterprise policy one. Moved to
extraOpts. Threepolicy names do not exist as written, checked against
components/policy/resources/templates/policies.yaml:IsolateOriginsis typed as a string of comma separated origins; the intenthere is
SitePerProcess;GenAiSettingsisGenAiDefaultSettings, an int-enum where 2 means "do notallow GenAI features";
BuiltInAIAPIIsEnabledisBuiltInAIAPIsEnabled.I only touched placement and invalid names, not the values that were chosen.
A test covers it.
tests/browsersbuilds a terminal with both browsers andasserts on the Firefox policy file, both Chromium managed policy files, the
presence of each browser in the system profile, and the local dashboard
answering on its port. It would have caught the
initialPrefsbug.One behaviour note for reviewers:
securix.firefox.bookmarksstill works, butit no longer feeds the local dashboard —
securix.browser.bookmarksdoes. I didnot add a compatibility shim; say the word if you want one.
This is the groundwork for #208, which I will send as a follow-up on top of this
branch.