feat(weller): add hardware diagnostics and MemTest86+#295
feat(weller): add hardware diagnostics and MemTest86+#295
Conversation
- Refactor 'hosts/weller/default.nix' to extract hardware config to 'hosts/weller/hardware.nix'. - Create 'modules/bootstrap.nix' for initial mutable install with SSH enabled and no secrets. - Add 'weller-bootstrap' nixosConfiguration to 'flake.nix'. Fixes #263
- Add smartmontools for S.M.A.R.T. monitoring and testing. - Add nvme-cli for detailed NVMe status and error reporting. - These tools are needed to debug recent Btrfs corruption issues on 'weller'.
Add MemTest86+ to systemd-boot menu for proper overnight memory testing. Add lm_sensors and memtester to system packages for hardware diagnostics.
Summary of ChangesHello @patflynn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances system stability diagnostics by integrating advanced memory testing and hardware monitoring tools into the NixOS configuration. It introduces MemTest86+ as a bootable option for in-depth RAM analysis and adds Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully adds hardware diagnostic tools like MemTest86+, lm_sensors, and memtester, improving system stability analysis, and refactors the weller host configuration into a separate hardware.nix file for better modularity. However, the modules/bootstrap.nix file introduces critical security vulnerabilities by enabling SSH root login with password authentication, setting a weak, hardcoded password (nixos) for the default user, and allowing passwordless sudo. These insecure defaults create a high risk of an attacker gaining full root control during the initial installation phase. It is strongly recommended to rely exclusively on SSH key authentication and disable password-based authentication and passwordless sudo. Additionally, there's a minor suggestion to improve separation of concerns by moving a user-space package out of the hardware configuration.
| PermitRootLogin = "yes"; | ||
| PasswordAuthentication = true; |
There was a problem hiding this comment.
Enabling password authentication and permitting root login over SSH is highly insecure, especially when combined with a hardcoded initial password. Since SSH keys are already configured, password authentication should be disabled to prevent unauthorized access during the bootstrap phase. Permitting root login is also unnecessary when a user with sudo access is available, and it's better to rely solely on SSH key authentication.
PermitRootLogin = "no";
PasswordAuthentication = false;
| "networkmanager" | ||
| "video" | ||
| ]; | ||
| initialPassword = "nixos"; |
There was a problem hiding this comment.
Hardcoding an initial password (nixos) is a significant security risk. If password authentication is enabled, an attacker could easily gain access to this user account. Since security.sudo.wheelNeedsPassword is false, this could lead to immediate root access. This hardcoded password should be removed, relying instead on SSH key authentication for initial access. If a password is required for local console access, consider using hashedPassword with a pre-computed hash.
| }; | ||
|
|
||
| # Make it easy to assume root during bootstrap | ||
| security.sudo.wheelNeedsPassword = false; |
There was a problem hiding this comment.
Allowing members of the 'wheel' group to use sudo without a password reduces the security of the system. In combination with the hardcoded password and enabled SSH password authentication, this allows an attacker to gain root access immediately upon logging in as the default user. It is safer to require a password for sudo even during the bootstrap phase.
security.sudo.wheelNeedsPassword = true;
| }; | ||
| }; | ||
| }; | ||
| environment.systemPackages = with pkgs; [ bluetuith ]; |
There was a problem hiding this comment.
The bluetuith package is a user-space application. While related to Bluetooth, defining environment.systemPackages here mixes hardware configuration with software installation. For better separation of concerns, it's recommended to manage host-specific packages in a different module, like hosts/weller/default.nix. This keeps hardware.nix focused purely on hardware-level settings. Please consider moving this line to hosts/weller/default.nix.
1 participant
Summary
memtest=5kernel parameter (basic kernel memtest that runs silently with no UI)lm_sensorsandmemtesterto system packages for temperature monitoring and userspace memory testingContext
Investigating system instability (game data corruption, Chrome crashes). NVMe SMART data looks clean — RAM is the primary suspect. MemTest86+ provides a thorough, standalone memory test accessible from the boot menu.
Test plan