Skip to content

feat(greetd): auto-start niri as adminUser on boot#1717

Merged
johnae merged 1 commit into
mainfrom
push-spkqtltmvnzu
Apr 13, 2026
Merged

feat(greetd): auto-start niri as adminUser on boot#1717
johnae merged 1 commit into
mainfrom
push-spkqtltmvnzu

Conversation

@johnae
Copy link
Copy Markdown
Owner

@johnae johnae commented Apr 13, 2026

No description provided.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 13, 2026

Claude finished @johnae's task —— View job


Code Review Completed

I've reviewed this PR that adds auto-start functionality for niri as adminUser on boot via greetd configuration.

Todo List:

  • Gather context by reading the changed file and repo guidelines
  • Analyze code quality and best practices
  • Check for potential bugs or issues
  • Review performance considerations
  • Assess security concerns
  • Evaluate test coverage needs
  • Provide comprehensive review feedback

Review Feedback

Summary

The PR adds an initial_session configuration to greetd that automatically starts the niri Wayland compositor for the admin user on boot. This is a clean, minimal change that follows the existing patterns in the codebase.

Code Quality and Best Practices ✅

  • Good: The change follows NixOS module conventions properly
  • Good: Consistent with the existing code style and patterns
  • Good: Properly adds adminUser to the function parameters
  • Good: Reuses the existing runNiri wrapper which already handles environment setup

Potential Bugs or Issues ⚠️

  1. Missing validation: There's no check to ensure adminUser.name is defined. While this appears to be a standard parameter in this codebase (used in multiple other modules), it could cause evaluation errors if undefined.

    • Consider adding an assertion or making this feature optional with lib.mkIf
  2. Session conflict: If a user is already logged in or if the greeter is still active, having an initial_session might cause conflicts. The current implementation doesn't handle this edge case.

  3. No error recovery: If niri fails to start for any reason, there's no fallback mechanism to show the greeter instead.

Performance Considerations ✅

  • No significant performance concerns. The initial_session starts only once at boot.
  • Uses the same optimized runNiri wrapper that includes systemd-cat integration for proper logging.

Security Concerns ⚠️

  1. Automatic login bypass: This configuration effectively bypasses the login screen for the admin user, which could be a security concern in multi-user environments or systems that require authentication at boot.

    • Consider documenting this behavior clearly
    • May want to make this configurable with an option like services.greetd.autoLogin.enable
  2. Admin privileges: Starting a session automatically as the admin user grants immediate access to administrative capabilities without authentication.

Test Coverage 🔍

  • No tests are included in this PR
  • Consider adding:
    • NixOS VM tests to verify the auto-start behavior
    • Tests to ensure the greeter still works when initial_session fails
    • Documentation or comments explaining the intended use case

Suggestions for Improvement

  1. Make it configurable: Consider wrapping this in an option:
services.greetd.autoLogin = {
  enable = lib.mkEnableOption "automatic login for admin user";
  user = lib.mkOption {
    type = lib.types.str;
    default = adminUser.name;
    description = "User to automatically log in";
  };
  command = lib.mkOption {
    type = lib.types.str;
    default = "${runNiri}/bin/niri";
    description = "Command to run for auto-login session";
  };
};
  1. Add documentation: Include a comment explaining why auto-login is desired and any security implications.

  2. Consider the use case: Is this for a single-user system, kiosk mode, or development environment? The appropriateness of auto-login depends heavily on the context.

Overall Assessment

The implementation is technically correct and follows established patterns. However, the security implications of automatic admin login should be carefully considered. I'd recommend making this behavior optional through a configuration flag rather than always-on.

@johnae johnae merged commit b6e7f33 into main Apr 13, 2026
2 of 4 checks passed
@johnae johnae deleted the push-spkqtltmvnzu branch April 13, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant