Assert secret-bearing paths are not Nix store paths

[kwsantiago]

Summary

Four options document that they need a runtime path and nothing enforced it. Adds the same hasPrefix builtins.storeDir assertion the sibling options already carry, and restages the five test fixtures that were deliberately passing store paths.

Why it matters

keepNode.vaultReplication.rsaKeyFile = "${./secrets/rsa_key.pem}" type-checks. It copies the cluster-wide Vaultwarden JWT signing key into /nix/store at 0444, where every local user can read it. CI's warm job then pushes store paths to the public privkey Cachix cache, so this is not only a local-permissions problem.

Unenforced until now:

option	secret
vaultReplication.rsaKeyFile	cluster-wide Vaultwarden JWT signing key
ingress.tlsKeyFile	TLS private key
ingress.tlsCertFile	TLS certificate
frostGate.keepPasswordEnvFile	systemd EnvironmentFile holding KEEP_PASSWORD

keepWeb.{passwordFile,authTokenFile,stateIdentityFile,storageKeyFile} and mesh.identityDir already assert exactly this for identical wording. These four were the gap.

Test fixtures

Five VM tests passed store paths on purpose, two with comments saying so. They now stage the fixture into /run at boot and point the option at the runtime path, which is the pattern tests/mesh-discovery.nix and tests/wisp-mesh.nix already use for identityDir:

systemd.tmpfiles.rules = [
  "C ${tlsDir}/key.pem 0600 root root - ${cert}/key.pem"
];

Ordering was checked rather than assumed. No consumer runs in initrd or sysinit: the OPRF provision unit is operator-run and explicitly not wantedBy boot, and the gate, nginx and vault-replication are ordinary multi-user.target services, which order after sysinit.target and therefore after systemd-tmpfiles-setup.service. No explicit After= is needed.

Test plan

• All four options assert against builtins.storeDir, null-guarded where the option is nullable
• Zero store-path assignments remain in the five fixtures
• All eight changed files parse under nix-instantiate --parse
• Confirmed systemd-tmpfiles C creates missing parent directories, so the staged files land even though no d rule precedes them. Verified against a scratch root; the copied file keeps its declared 0600 while the auto-created parent is 0755, so the file mode is what protects the secret.
• Full VM matrix, which is the real verification. Labeled full-ci, since the fast lane does not run these tests. The five previously-failing checks are ingress, ha-failover, mesh-replication, oprf-gate and oprf-gate-2of3.

A local nix flake check --no-build cannot complete here: it stops at frost-group-fixture.drv is not valid because the fixture is not realised in this store. That is an environment limitation, not an evaluation error in these changes.

Found by the security review on #104 and split out so the CRNG fix was not held up by fixture rework.

Summary by CodeRabbit

• Bug Fixes

  • Added validation to prevent private keys, TLS certificates, and password environment files from being stored in world-readable Nix store paths.
  • Configuration now reports clear errors when sensitive files use unsafe locations.
  • Improved path checks to distinguish genuine Nix store paths from similarly prefixed paths.

• Tests

  • Updated high-availability, ingress, replication, and OPRF scenarios to use protected runtime locations with restrictive permissions.
  • Verified services securely use runtime-staged credentials and certificates.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5421c275-8990-4f54-8846-ff8cf82dbe7f

📥 Commits

Reviewing files that changed from the base of the PR and between 836625d and 068de0b.

📒 Files selected for processing (10)

• nixos/frost-gate.nix
• nixos/ingress.nix
• nixos/keep-web.nix
• nixos/mesh.nix
• nixos/vault-replication.nix
• tests/ha-failover.nix
• tests/ingress.nix
• tests/mesh-replication.nix
• tests/oprf-gate-2of3.nix
• tests/oprf-gate.nix

🚧 Files skipped from review as they are similar to previous changes (8)

• nixos/ingress.nix
• nixos/frost-gate.nix
• nixos/vault-replication.nix
• tests/ha-failover.nix
• tests/oprf-gate-2of3.nix
• tests/ingress.nix
• tests/oprf-gate.nix
• tests/mesh-replication.nix

---

Walkthrough

The NixOS modules now reject secret-bearing files stored under /nix/store. Integration tests copy TLS files, RSA keys, and password environment files into /run with controlled permissions before using them.

Changes

Runtime secret path validation

Layer / File(s)	Summary
NixOS secret path assertions nixos/frost-gate.nix, nixos/ingress.nix, nixos/vault-replication.nix, nixos/keep-web.nix, nixos/mesh.nix	Assertions reject Nix-store paths for password environment files, TLS certificates and keys, RSA signing keys, identity directories, and storage keys. Store path checks now require a trailing / boundary.
Ingress TLS runtime staging tests/ingress.nix	The test copies the certificate and private key into /run, applies controlled permissions, and configures ingress to use those files.
Replication and password runtime staging tests/ha-failover.nix, tests/mesh-replication.nix, tests/oprf-gate*.nix	Tests stage RSA keys and password environment files under /run with restricted permissions and use the staged paths.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• privkeyio/keep-node#104: Related validation prevents secret-bearing files from referencing Nix store paths, including nixos/frost-gate.nix.

Poem

A rabbit checks each secret path,
Keys leave the store and choose /run.
TLS and passwords stage with care,
Protected files wait safely there.
The burrow builds, and checks pass.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: preventing secret-bearing paths from referencing Nix store paths.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch assert-runtime-secret-paths

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@nixos/frost-gate.nix`:
- Around line 838-840: Update the runtime-path prefix guards in
nixos/frost-gate.nix lines 838-840, nixos/ingress.nix lines 108-114, and
nixos/vault-replication.nix line 206 to check against "${builtins.storeDir}/"
rather than builtins.storeDir, preventing similarly prefixed paths such as
/nix/storehouse from being rejected.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2fc6c62e-2e5d-450a-a233-f65846d80e80

📥 Commits

Reviewing files that changed from the base of the PR and between b766ecd and 836625d.

📒 Files selected for processing (8)

• nixos/frost-gate.nix
• nixos/ingress.nix
• nixos/vault-replication.nix
• tests/ha-failover.nix
• tests/ingress.nix
• tests/mesh-replication.nix
• tests/oprf-gate-2of3.nix
• tests/oprf-gate.nix

[kwsantiago]

Valid, fixed, and applied beyond the four assertions this PR added.

lib.hasPrefix builtins.storeDir matches /nix/storehouse/secrets/key.pem, because that string does begin with /nix/store. A legitimate runtime path would be rejected. Verified rather than reasoned about:

expression	bare storeDir	"${storeDir}/"
/nix/store/abc123-key.pem	true	true
/nix/storehouse/secrets/key.pem	true	false

So the boundary form is strictly better: it still rejects every real store path, which is the security property, and stops rejecting paths that merely share the prefix. The failure was in the fail-closed direction, refusing valid input rather than accepting invalid input, which is why it is a correctness bug rather than a hole.

Fixed all nine sites, not just the four added here. The pre-existing guards on keepWeb.{passwordFile,authTokenFile,stateIdentityFile,storageKeyFile} and mesh.identityDir carried the same pattern, and leaving them would have meant two idioms for the same check with no way for a reader to tell which was intended. vault-replication.nix:54 already used "${dataDir}/" for exactly this reason, so the boundary form is the one the codebase had already settled on elsewhere.

grep for hasPrefix builtins.storeDir now returns nothing; every site uses the anchored form. Files parse, nix fmt is clean.