Skip to content

refactor: remove flake-utils, various other nit picks #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LordGrimmauld merged 10 commits into from
Mar 24, 2026
Merged

refactor: remove flake-utils, various other nit picks #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
677b33b
refactor: remove upstreamed config options
Gerg-L Feb 11, 2026
8204182
refactor: remove flake-utils
Gerg-L Feb 11, 2026
669dd27
refactor: drop rust-overlay
Gerg-L Feb 11, 2026
50a9475
refactor: use callPackage on package
Gerg-L Feb 11, 2026
168a6fc
refactor: use lib.importTOML
Gerg-L Feb 11, 2026
f3b8ebf
fix: use final not prev from overlay
Gerg-L Feb 11, 2026
319f4fc
lint: don't use function when inherit works
Gerg-L Feb 11, 2026
6403239
lint: condense polkit rules
Gerg-L Feb 11, 2026
dee032b
fix: make docs make sense with mkEnableOption prepending 'Whether to …
Gerg-L Feb 11, 2026
39a7688
fix: set dbus implementation to allow checks to pass
Gerg-L Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 6 additions & 61 deletions flake.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

172 changes: 88 additions & 84 deletions flake.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@

inputs = {
nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable-small";
flake-utils.url = "github:numtide/flake-utils";
rust-overlay = {
url = "github:oxalica/rust-overlay";
inputs.nixpkgs.follows = "nixpkgs";
};
nix-github-actions = {
url = "github:nix-community/nix-github-actions";
inputs.nixpkgs.follows = "nixpkgs";
Expand All @@ -22,22 +17,40 @@
{
self,
nixpkgs,
flake-utils,
rust-overlay,
nix-github-actions,
treefmt-nix,
...
}:
let
cargo-toml = (builtins.fromTOML (builtins.readFile ./Cargo.toml)).package;
inherit (nixpkgs) lib;
cargo-toml = (lib.importTOML ./Cargo.toml).package;
Comment on lines +25 to +26
Copy link
Copy Markdown
Owner

@LordGrimmauld LordGrimmauld Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the use of lib here cause eval slowdown? We are later loading nixpkgs.legacyPackages.${system}.lib, and importing lib twice may have some costs. If it is the same lib, then eval may be cached though, i am not quite familiar enough with evaluator internals to judge this correctly.

inherit (cargo-toml) name;

build-pkg =
pkgs:
let
inherit (pkgs) lib;
in
pkgs.rustPlatform.buildRustPackage {
forEachSystem =
f:
builtins.listToAttrs (
map
(system: {
name = system;
value = f {
inherit system;
pkgs = nixpkgs.legacyPackages.${system};
};
})
[
"x86_64-linux"
"x86_64-darwin"
"aarch64-linux"
"aarch64-darwin"
Comment on lines +40 to +43
Copy link
Copy Markdown
Owner

@LordGrimmauld LordGrimmauld Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo darwin is silly here. run0 as part of systemd will never run on darwin. I am aware this is a 1:1 replacement of what flakeutils did. For dev shell and formatting and such, darwin makes sense. For the package, not so much. Not a big issue, you don't have to fix everything that was bad before all at once, i won't block on this. Just noticed because it is now spelled out.

]
);

package =
{
lib,
rustPlatform,
...
Copy link
Copy Markdown
Owner

@LordGrimmauld LordGrimmauld Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
...

Not a functional change, so won't block, but it is common for nixpkgs packages to not allow wildcard arguments and i like to follow those standards at least a little.

Copy link
Copy Markdown
Contributor Author

@Gerg-L Gerg-L Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're 100% correct, just a habit from the module system to add , ...

}:
rustPlatform.buildRustPackage {
inherit name;
inherit (cargo-toml) version;
src = lib.cleanSource ./.;
Expand All @@ -55,66 +68,70 @@
};
};

outputs = flake-utils.lib.eachDefaultSystem (
system:
let
overlays = [ (import rust-overlay) ];
pkgs = import nixpkgs {
inherit system overlays;
};
rustToolchain = pkgs.pkgsBuildHost.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml;
treefmtEval = treefmt-nix.lib.evalModule pkgs ./treefmt.nix;
in
treefmtEval = (lib.flip treefmt-nix.lib.evalModule) ./treefmt.nix;
in
{
packages = forEachSystem (
{ pkgs, system }:
{
packages.${name} = build-pkg pkgs;
packages.default = self.packages.${system}.${name};
${name} = pkgs.callPackage package { };
default = self.packages.${system}.${name};
}
);

devShells.default = pkgs.mkShell {
buildInputs = [
rustToolchain
devShells = forEachSystem (
{ pkgs, system }:
{
default = pkgs.mkShell {
inputsFrom = [ self.packages.${system}.default ];
packages = [
pkgs.clippy
pkgs.rust-analyzer
pkgs.rustfmt
];
};
}
);

formatter = treefmtEval.config.build.wrapper;

checks = {
formatting = treefmtEval.config.build.check self;
vm = pkgs.testers.runNixOSTest {
name = "run0-sudo-shim-vm-test";
nodes.machine = {
imports = [ self.nixosModules.default ];
security.polkit.persistentAuthentication = true;
security.run0-sudo-shim.enable = true;

users.users = {
admin = {
isNormalUser = true;
extraGroups = [ "wheel" ];
};
noadmin = {
isNormalUser = true;
};
formatter = forEachSystem ({ pkgs, ... }: (treefmtEval pkgs).config.build.wrapper);

checks = forEachSystem (
{ pkgs, system }:
{
formatting = (treefmtEval pkgs).config.build.check self;
vm = pkgs.testers.runNixOSTest {
name = "run0-sudo-shim-vm-test";
nodes.machine = {
imports = [ self.nixosModules.default ];
services.dbus.implementation = "broker";
security = {
polkit.persistentAuthentication = true;
run0-sudo-shim.enable = true;
};

users.users = {
admin = {
isNormalUser = true;
extraGroups = [ "wheel" ];
};
noadmin = {
isNormalUser = true;
};
};
testScript = ''
# machine.succeed('su - admin -c "sudo -v"') # can't yet give password, needs hacks to never ask for password in the test or enter the password
machine.fail('su - noadmin -c "sudo -v"')
'';
};
}
// self.packages.${system};
testScript = ''
# machine.succeed('su - admin -c "sudo -v"') # can't yet give password, needs hacks to never ask for password in the test or enter the password
machine.fail('su - noadmin -c "sudo -v"')
'';
};
}
// self.packages.${system}
);
in
outputs
// {

githubActions = nix-github-actions.lib.mkGithubMatrix {
checks = nixpkgs.lib.getAttrs [ "x86_64-linux" ] outputs.checks;
checks = { inherit (self.checks) x86_64-linux; };
};

overlays.default = final: prev: { ${name} = build-pkg prev; };
overlays.default = final: _: { ${name} = final.callPackage package { }; };

nixosModules.default =
{
Expand All @@ -128,12 +145,12 @@
in
{
options.security = {
polkit.persistentAuthentication = lib.mkEnableOption "patch polkit to allow persistent authentication and add rules";
polkit.persistentAuthentication = lib.mkEnableOption "patching polkit to allow persistent authentication and adding rules";
run0-sudo-shim = {
enable = lib.mkEnableOption "enable run0-sudo-shim instead of sudo";
enable = lib.mkEnableOption "run0-sudo-shim instead of sudo";
package = lib.mkPackageOption pkgs "run0-sudo-shim" { } // {
# should be removed when upstreaming to nixpkgs
default = pkgs.run0-sudo-shim or build-pkg pkgs;
default = pkgs.run0-sudo-shim or self.packages.${pkgs.stdenv.system}.default;
};
};
};
Expand All @@ -143,23 +160,15 @@
environment.systemPackages = [ cfg.package ];
security.sudo.enable = false;
security.polkit.enable = true;

# https://github.com/NixOS/nixpkgs/pull/419588
security.pam.services.systemd-run0 = {
setLoginUid = true;
pamMount = false;
};
})
(lib.mkIf config.security.polkit.persistentAuthentication {
assertions =
let
mkMessage = (
package: minVer: ''
To provide persistent authentication, Polkit requires `pidfd` support when fetching process details from D-Bus, which is only available in `${package}` version ${minVer} or later.
mkMessage = package: minVer: ''
To provide persistent authentication, Polkit requires `pidfd` support when fetching process details from D-Bus, which is only available in `${package}` version ${minVer} or later.

Please update the package or switch `services.dbus.implementation` in the configuration.
''
);
Please update the package or switch `services.dbus.implementation` in the configuration.
'';
in
[
(lib.mkIf (config.services.dbus.implementation == "dbus") {
Expand All @@ -174,14 +183,9 @@

security.polkit.extraConfig = ''
polkit.addRule(function(action, subject) {
if (action.id == "org.freedesktop.policykit.exec") {
return polkit.Result.AUTH_ADMIN_KEEP;
}
});

polkit.addRule(function(action, subject) {
if (action.id.indexOf("org.freedesktop.systemd1.") == 0) {
return polkit.Result.AUTH_ADMIN_KEEP;
if (action.id == "org.freedesktop.policykit.exec" ||
action.id.indexOf("org.freedesktop.systemd1.") {
return polkit.Result.AUTH_ADMIN_KEEP;
Comment on lines +186 to +188
Copy link
Copy Markdown
Owner

@LordGrimmauld LordGrimmauld Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this during review, i really need to put polkit stuff on the test path.

Suggested change
if (action.id == "org.freedesktop.policykit.exec" ||
action.id.indexOf("org.freedesktop.systemd1.") {
return polkit.Result.AUTH_ADMIN_KEEP;
if (action.id == "org.freedesktop.policykit.exec" ||
action.id.indexOf("org.freedesktop.systemd1.") == 0) {
return polkit.Result.AUTH_ADMIN_KEEP;

We are lucky the ) was missing, crashing the entire rule and triggering polkits fail-closed. This was noticed and responsibly reported by @zimward, who has been amazing, even providing a reproduction in a VM (which should have been part of the test suite to begin with).

Had ) been here, but just == 0 missing, chances are it'd have taken significantly longer to spot this issue, and would have been an active security risk (early return of AUTH_ADMIN_KEEP even for non-admin stuff and things that shouldn't be kept - particularly, anything except org.freedesktop.systemd1.*.

This has been fixed as of ef368ac. Considering this was a fail-closed issue, i do not believe we need to file for a github security advisory - i do want to document what went wrong though.

Copy link
Copy Markdown
Contributor Author

@Gerg-L Gerg-L Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry. major skill issue on my part.

I don't even know how it happened because I copy-pasted that bit from my config. which I just checked and it's correct there

}
});
'';
Expand Down