Replace step-cli with certs.generateCertificate for TLS certificates#187
Replace step-cli with certs.generateCertificate for TLS certificates#187attilaolah wants to merge 3 commits intoattilaolah:mainfrom
Conversation
📝 WalkthroughWalkthroughReplaced build-time TLS/CA generation (previous Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hosts/home/configuration.nix`:
- Around line 21-23: The tlsKey is being written into the global Nix store with
pkgs.writeText (tlsKey), making the private key world-readable; change the
approach so the private key is emitted outside the Nix store at activation time
with proper 0600 permissions—remove pkgs.writeText for tlsKey and instead create
the key file from an activation script (system.activationScripts) or manage it
via systemd.tmpfiles.rules so the file is placed under /etc (or another secure
path) with owner root and mode 0600; leave caCert and tlsCert as pkgs.writeText
if public, but ensure tlsKey is generated/copied during activation with secure
ownership and permissions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 609e0a87-9f29-4e13-912e-def07e46994e
📒 Files selected for processing (1)
hosts/home/configuration.nix
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hosts/home/configuration.nix`:
- Around line 13-14: The certificate generation is using a store-side file
(subjectKey = pkgs.lib.fileContents ./tls.key) while activation replaces
/etc/tls/tls.key, causing crt/key mismatch; update the cert generation so it
reads the same activation-time key (the one written to /etc/tls/tls.key) instead
of ./tls.key — either change certs = pkgs.certificates.generateCertificate to
use pkgs.lib.fileContents on /etc/tls/tls.key or move the certificate creation
into the activation phase (so the code that writes /etc/tls/tls.key also runs
pkgs.certificates.generateCertificate against that same file), and make the
corresponding change where hosts/home/services/nginx.nix serves /etc/tls/tls.crt
to ensure the key and cert are produced from the same source.
- Line 23: The current code defines caCrt via pkgs.writeText ("ca.crt")
producing a file path but then uses the incorrect Nix option; replace usage of
security.pki.certificates (which expects PEM strings) with
security.pki.certificateFiles and pass [ caCrt ] instead. Locate the caCrt
binding and any reference where security.pki.certificates is used and change
that reference to security.pki.certificateFiles = [ caCrt ]; leaving the
pkgs.writeText call intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 720f0469-7597-43fa-b17d-fc0805b4ca2c
📒 Files selected for processing (1)
hosts/home/configuration.nix
1. Move cert/key generation to activation time together - Certificate and private key are now generated together in activation script - This ensures they are always paired and the key never enters Nix store 2. Fix security.pki option usage - Changed pki.certificates (for PEM strings) to pki.certificateFiles (for file paths) 3. Generate self-signed cert using openssl in activation script - Uses openssl req -x509 to create both cert and key in one command - Includes proper subjectAltNames for localhost, home, and IP addresses
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hosts/home/configuration.nix`:
- Around line 56-57: The nginx cert being served is self-signed and doesn't
chain to the advertised CA (caCrt) because the activation script uses openssl
req -x509 without signing with the CA; fix by either (A) generating a CSR for
tls.key and signing it with caCrt/caKey (use openssl req to create a CSR and
openssl ca or openssl x509 -req with -CA and -CAkey to produce /etc/tls/tls.crt
so it chains to caCrt), or (B) if you don't intend to provide a CA, remove caCrt
from pki.certificateFiles and from environment.etc so the system and nginx
configuration stop advertising that CA; update the activation script (the
openssl req invocation) and the pki.certificateFiles/caCrt references
accordingly.
- Around line 108-115: The activation snippet invokes the openssl binary by bare
name (the line starting with "openssl req -x509 -newkey ed25519 ..."), but PATH
in activation is limited to core utilities so this will fail; update the command
to call the absolute store path for OpenSSL (use ${pkgs.openssl}/bin/openssl or
builtins.toString pkgs.openssl concatenated with /bin/openssl) wherever the
openssl invocation appears in the activation script so the certificate
generation runs reliably during activation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70b694bc-e5f8-43fe-9477-718ff302873e
📒 Files selected for processing (1)
hosts/home/configuration.nix
| # Use certificateFiles for file paths, certificates for PEM strings | ||
| pki.certificateFiles = [ caCrt ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let me check if the files exist and get their sizes
stat hosts/home/configuration.nix 2>/dev/null && wc -l hosts/home/configuration.nix
stat hosts/home/services/nginx.nix 2>/dev/null && wc -l hosts/home/services/nginx.nixRepository: attilaolah/os
Length of output: 41
🏁 Script executed:
# Read the configuration.nix file to examine the relevant lines
cat -n hosts/home/configuration.nixRepository: attilaolah/os
Length of output: 4144
🏁 Script executed:
# Read the nginx.nix file to see how certificates are configured
cat -n hosts/home/services/nginx.nixRepository: attilaolah/os
Length of output: 633
The certificate nginx serves is self-signed and unrelated to the trusted CA.
Lines 56–57 publish caCrt as a trusted root CA, and line 87 places it at /etc/tls/ca.crt. However, the activation script at lines 108–115 uses openssl req -x509 without -CA or -CAkey, which generates a standalone self-signed certificate with no relationship to caCrt. When nginx (line 8 in services/nginx.nix) presents /etc/tls/tls.crt to clients, those clients will reject it because the certificate is not signed by the advertised CA. Either sign /etc/tls/tls.key with caCrt by generating a CSR and using -CA/-CAkey, or remove caCrt from security.pki.certificateFiles and environment.etc if it is not intended to validate the nginx certificate. (docs.openssl.org)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hosts/home/configuration.nix` around lines 56 - 57, The nginx cert being
served is self-signed and doesn't chain to the advertised CA (caCrt) because the
activation script uses openssl req -x509 without signing with the CA; fix by
either (A) generating a CSR for tls.key and signing it with caCrt/caKey (use
openssl req to create a CSR and openssl ca or openssl x509 -req with -CA and
-CAkey to produce /etc/tls/tls.crt so it chains to caCrt), or (B) if you don't
intend to provide a CA, remove caCrt from pki.certificateFiles and from
environment.etc so the system and nginx configuration stop advertising that CA;
update the activation script (the openssl req invocation) and the
pki.certificateFiles/caCrt references accordingly.
| openssl req -x509 -newkey ed25519 \ | ||
| -keyout /etc/tls/tls.key \ | ||
| -out /etc/tls/tls.crt \ | ||
| -days 3650 \ | ||
| -nodes \ | ||
| -subj "/CN=localhost" \ | ||
| -addext "subjectAltName=DNS:localhost,DNS:home,IP:127.0.0.1,IP:::1,IP:127.0.4.43" \ | ||
| 2>/dev/null |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and locate the file
fd -t f configuration.nix | head -20Repository: attilaolah/os
Length of output: 85
🏁 Script executed:
# Check if the file exists and read the relevant section
if [ -f "hosts/home/configuration.nix" ]; then
echo "=== File found, reading lines 100-120 ==="
sed -n '100,120p' hosts/home/configuration.nix
else
echo "File not found at hosts/home/configuration.nix"
# Try to find it
find . -name "configuration.nix" -type f
fiRepository: attilaolah/os
Length of output: 865
🏁 Script executed:
# Let's understand the broader context - search for where this openssl command is used
rg -B 5 -A 5 "openssl req -x509" hosts/home/Repository: attilaolah/os
Length of output: 763
🏁 Script executed:
# Check the NixOS activation-script handling and PATH initialization
# Let's see if there's any package declaration or imports that might affect this
rg -B 10 "openssl" hosts/home/configuration.nix | head -50Repository: attilaolah/os
Length of output: 795
🏁 Script executed:
# Let's check if there's any context in the file about how packages are made available
# Look for buildInputs, nativeBuildInputs, or package declarations
rg -B 5 "buildInputs|nativeBuildInputs|pkgs\." hosts/home/configuration.nix | head -80Repository: attilaolah/os
Length of output: 454
🏁 Script executed:
# Let's check the NixOS module structure to see if openssl is declared as a dependency
# Look at the entire file structure
wc -l hosts/home/configuration.nixRepository: attilaolah/os
Length of output: 89
🏁 Script executed:
# Read more of the file to understand the overall structure and any package declarations
head -50 hosts/home/configuration.nixRepository: attilaolah/os
Length of output: 1367
🏁 Script executed:
# Search for how install command is available - it's also a bare command in the activation script
# This will help us understand if the reviewer's claim is accurate
rg "install" hosts/home/configuration.nixRepository: attilaolah/os
Length of output: 106
🌐 Web query:
NixOS activation-script PATH environment initialization openssl
💡 Result:
In NixOS, the system activation script (located at /nix/var/nix/profiles/system/activate or similar) initializes the PATH environment variable at the beginning of its execution. It starts by setting PATH=/empty to ensure a clean environment, then appends /bin and /sbin directories from a minimal set of essential packages defined in nixos/modules/system/activation/activation-script.nix. The path list includes (using pkgs.getBin or equivalent): - coreutils - gnugrep - findutils - glibcLocales or stdenv.cc.libc (for getent and nscd) - shadow - nettools (for hostname) - util-linux (for mount, mountpoint) This provides basic utilities needed for activation tasks like creating directories, symlinks, mounting filesystems, and managing users/groups. OpenSSL is NOT included in this minimal PATH by default, as it is not required for core activation operations. OpenSSL binaries would only be available if explicitly added via custom activation scripts or if running in a shell that sources additional environment setups (e.g., source ${config.system.build.setEnvironment}). For custom system.activationScripts, the PATH is inherited from this minimal setup. To use additional tools like openssl in a custom script, explicitly add them: PATH=$PATH:${lib.makeBinPath [ pkgs.openssl ]} or source ${config.system.build.setEnvironment} to include the full system environment (including environment.systemPackages). Sources confirm this behavior across NixOS versions, with minor variations in the exact package list (e.g., glibcLocales vs getent). No evidence of openssl in the default activation PATH.
Citations:
- 1: https://github.com/NixOS/nixos/blob/master/modules/system/activation/activation-script.nix
- 2: https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/system/activation/activation-script.nix
- 3: https://github.com/NixOS/nixpkgs/blob/cd69dcb7dfcf30215e1b0c797bfcdf99ea47af71/nixos/modules/system/activation/activation-script.nix
- 4: https://github.com/NixOS/nixpkgs/blob/master/nixos/doc/manual/development/activation-script.section.md
- 5: https://discourse.nixos.org/t/setting-up-path-for-system-activationscripts/4034
🏁 Script executed:
# Let's search for other uses of openssl in the repository to see patterns
rg "openssl" --type nix | grep -v "Binary" | head -20Repository: attilaolah/os
Length of output: 279
🏁 Script executed:
# Let's look for how activation scripts typically access commands in NixOS examples
# Check if there's any documentation or patterns in the flake or other config files
fd -t f "\.nix$" | xargs rg "system\.activationScripts" -A 10 | head -100Repository: attilaolah/os
Length of output: 763
🏁 Script executed:
# Check if there's any buildInputs or packages declared for the activation script
rg -B 20 "system.activationScripts.generateTlsCerts" hosts/home/configuration.nix | head -50Repository: attilaolah/os
Length of output: 611
🏁 Script executed:
# Check if there's anything in the imports that might provide openssl
# Let's look at the services and programs imports
ls -la hosts/home/services/ hosts/home/programs/ 2>/dev/null | head -20Repository: attilaolah/os
Length of output: 980
🏁 Script executed:
# Look for any environment.systemPackages or similar that might affect activation script PATH
rg "environment\.(systemPackages|variables)" hosts/home/configuration.nixRepository: attilaolah/os
Length of output: 39
Call OpenSSL by store path in the activation snippet.
The activation script's PATH is initialized with only core utilities (coreutils, gnugrep, findutils, shadow, nettools, util-linux); openssl is not included. Using it by bare name will fail at activation time unless some unrelated code mutates PATH first. Use ${pkgs.openssl}/bin/openssl instead.
Suggested change
- openssl req -x509 -newkey ed25519 \
+ ${pkgs.openssl}/bin/openssl req -x509 -newkey ed25519 \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hosts/home/configuration.nix` around lines 108 - 115, The activation snippet
invokes the openssl binary by bare name (the line starting with "openssl req
-x509 -newkey ed25519 ..."), but PATH in activation is limited to core utilities
so this will fail; update the command to call the absolute store path for
OpenSSL (use ${pkgs.openssl}/bin/openssl or builtins.toString pkgs.openssl
concatenated with /bin/openssl) wherever the openssl invocation appears in the
activation script so the certificate generation runs reliably during activation.
1 participant
Summary by CodeRabbit