Harden NixOS config defaults and setup guidance #4

Merged
Copilot merged 13 commits from copilot/review-nixos-setup-issues into main 2026-02-01 21:17:59 +00:00
2 changed files with 3 additions and 2 deletions
Showing only changes of commit 2f4cbd863a - Show all commits

View File

@@ -45,7 +45,8 @@
&& builtins.match "^_+$" username == null
copilot-pull-request-reviewer[bot] commented 2026-02-01 21:21:01 +00:00 (Migrated from github.com)
Review

The username validation regex allows usernames that consist only of underscores followed by other characters (e.g., "___abc"), but the second check builtins.match "^_+$" username == null only rejects usernames that are entirely underscores. According to standard Unix username conventions, usernames starting with underscore are typically reserved for system accounts. Consider strengthening the validation to reject any username starting with underscore unless that's intentionally allowed for system accounts.

      builtins.match "^[a-z][a-z0-9_]*$" username != null
The username validation regex allows usernames that consist only of underscores followed by other characters (e.g., "___abc"), but the second check `builtins.match "^_+$" username == null` only rejects usernames that are entirely underscores. According to standard Unix username conventions, usernames starting with underscore are typically reserved for system accounts. Consider strengthening the validation to reject any username starting with underscore unless that's intentionally allowed for system accounts. ```suggestion builtins.match "^[a-z][a-z0-9_]*$" username != null ```
&& builtins.match "^nix.*" username == null
&& username != "root";
hostConfig = ./hosts + "/${hostname}/hardware-configuration.nix";
hostConfig = assert builtins.pathExists (./hosts + "/${hostname}/hardware-configuration.nix");
./hosts + "/${hostname}/hardware-configuration.nix";
passwordHashPath = "/etc/nixos/secrets/${username}/password.hash";
copilot-pull-request-reviewer[bot] commented 2026-02-01 21:21:01 +00:00 (Migrated from github.com)
Review

The assertion for hostConfig will fail at evaluation time with a cryptic error if the path doesn't exist. The assert statement should provide a helpful error message to guide users. Consider using a let-binding with an abort statement that provides a clear message like: "Hardware configuration not found at ./hosts/${hostname}/hardware-configuration.nix. Please ensure the directory exists and contains the hardware configuration file."

    hostConfig =
      let
        hardwarePath = ./hosts + "/${hostname}/hardware-configuration.nix";
      in
      if builtins.pathExists hardwarePath then
        hardwarePath
      else
        builtins.abort "Hardware configuration not found at ./hosts/${hostname}/hardware-configuration.nix. Please ensure the directory exists and contains the hardware configuration file.";
The assertion for `hostConfig` will fail at evaluation time with a cryptic error if the path doesn't exist. The assert statement should provide a helpful error message to guide users. Consider using a let-binding with an abort statement that provides a clear message like: "Hardware configuration not found at ./hosts/${hostname}/hardware-configuration.nix. Please ensure the directory exists and contains the hardware configuration file." ```suggestion hostConfig = let hardwarePath = ./hosts + "/${hostname}/hardware-configuration.nix"; in if builtins.pathExists hardwarePath then hardwarePath else builtins.abort "Hardware configuration not found at ./hosts/${hostname}/hardware-configuration.nix. Please ensure the directory exists and contains the hardware configuration file."; ```
specialArgs = { inherit inputs system hostname username usernameValid passwordHashPath; };
copilot-pull-request-reviewer[bot] commented 2026-02-01 21:21:01 +00:00 (Migrated from github.com)
Review

The passwordHashPath is hardcoded to a system location that may not exist on first installation, but there's no validation or assertion to check if this file exists. This will cause the system build to fail with an unclear error. Consider adding an assertion in common.nix to check if the password hash file exists, with a helpful error message directing users to the password setup instructions in the README. Alternatively, document that users must create this file before their first build.

    passwordHashPath =
      let
        path = "/etc/nixos/secrets/${username}/password.hash";
      in
      if builtins.pathExists path then
        path
      else
        builtins.throw ''
          Password hash file not found at ${path}.
          
          Please create this file before your first build.
          See the password setup instructions in the project README.
        '';
The `passwordHashPath` is hardcoded to a system location that may not exist on first installation, but there's no validation or assertion to check if this file exists. This will cause the system build to fail with an unclear error. Consider adding an assertion in `common.nix` to check if the password hash file exists, with a helpful error message directing users to the password setup instructions in the README. Alternatively, document that users must create this file before their first build. ```suggestion passwordHashPath = let path = "/etc/nixos/secrets/${username}/password.hash"; in if builtins.pathExists path then path else builtins.throw '' Password hash file not found at ${path}. Please create this file before your first build. See the password setup instructions in the project README. ''; ```

View File

@@ -117,7 +117,7 @@
isNormalUser = true;
extraGroups = [ "wheel" "networkmanager" "video" "seat" ];
# IMPORTANT: Generate a password hash with: mkpasswd -m sha-512
# Save it to the path defined in flake.nix (ensure permissions are 600)
# Save it to /etc/nixos/secrets/${username}/password.hash (ensure permissions are 600)
hashedPasswordFile = passwordHashPath;
packages = with pkgs; [
# -- Noctalia Shell --