fix: address PR #15 review feedback — secrets handling, broken links, optional logDirectory

[Copilot]

• Fix firewall example (duplicate allowedTCPPorts)
• Fix accessKeyFile/secretKeyFile descriptions to reflect LoadCredential usage
• Fix examples/flake.nix to not store secrets in Nix store
• Fix broken links in docs/SECURITY.md and docs/MIGRATION.md
• Fix tmpfiles and ReadWritePaths snippets to make logDirectory optional
• Clarify CHANGELOG deprecated accessKey/secretKey entries: old names map to file path options, plain-text strings no longer accepted, file path required when enable = true
• Replace hardcoded credential values in activation script comment with environment-variable-based placeholders
• Update README accessKeyFile/secretKeyFile docs to reflect LoadCredential ownership (root/systemd, not service user) and correct deprecation wording

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Pull request overview

This PR addresses outstanding review feedback from PR #15 by tightening secret-handling guidance, fixing broken documentation links, and aligning documentation snippets with the module’s actual logDirectory = null default behavior.

Changes:

• Updated accessKeyFile/secretKeyFile option descriptions to reflect systemd LoadCredential behavior (systemd/root reads the source files).
• Updated the example flake to avoid demonstrating secrets written into /nix/store, switching to /run/secrets/* paths with guidance for local testing vs production.
• Fixed relative links and updated documentation snippets to make logDirectory conditional where it is nullable.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
nixos/rustfs.nix	Clarifies secret file readability expectations based on LoadCredential.
examples/flake.nix	Removes insecure environment.etc.<name>.text secret pattern; documents /run/secrets usage.
docs/SECURITY.md	Fixes a broken link and updates tmpfiles example to make logDirectory conditional.
docs/MIGRATION.md	Fixes a broken link to the example configuration.
docs/IMPROVEMENTS.md	Updates ReadWritePaths snippet to avoid including a potentially-null logDirectory.
CHANGELOG.md	Rewords prior “BREAKING” entries around secret options and renames.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The changelog entry implies services.rustfs.accessKey/secretKey are fully backward-compatible aliases. In the module, mkRenamedOptionModule only renames the option; it does not preserve the old plain-text secret semantics, and accessKeyFile/secretKeyFile have no defaults (so enabling the service still requires setting valid file paths). Consider rewording these bullets to explicitly state that the old names map to the file path options and that file-based secrets are required when services.rustfs.enable = true.

[houseme]

@copilot apply changes based on this feedback

[Copilot]

Updated in commit 5b0d3a0. The changelog bullets now explicitly state that the old names map to the file path options, that plain-text secret strings are no longer accepted, and that a valid file path is required whenever services.rustfs.enable = true.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The updated option description says secrets are read by root/systemd via LoadCredential and not by the rustfs user directly. This is accurate for the module, but other repo docs still describe the old behavior (e.g. README says the secret files should be readable by the rustfs service user and that deprecated accessKey/secretKey options were removed). Consider updating those docs to match this new LoadCredential-based flow to avoid confusing users about file ownership/permissions and deprecation status.

[Copilot]

The commented activation script example still hardcodes credential values inside the Nix configuration. If a user copies/uncomments it, those secrets will be embedded in the system derivation and end up in the Nix store, which contradicts the goal of keeping secrets out of the store. Consider changing the example to use placeholders and/or show fetching the secret from an external runtime source (or point directly to sops-nix/agenix usage) without ever putting the secret bytes in the Nix expression.

Suggested change

	# real credentials like this in production):
	# system.activationScripts.rustfs-secrets = ''
	# install -d -m 700 /run/secrets
	# echo -n "my-access-key" > /run/secrets/rustfs-access-key
	# echo -n "my-secret-key" > /run/secrets/rustfs-secret-key
	# real credentials like this in Nix – fetch them from an external runtime source):
	# system.activationScripts.rustfs-secrets = ''
	# install -d -m 700 /run/secrets
	# # Example: populate secrets from environment variables or a secrets backend.
	# # The actual secret bytes must NOT appear in this Nix file.
	# # Using environment variables (set outside Nix) for local testing:
	# # test -n "$RUSTFS_ACCESS_KEY" && printf '%s' "$RUSTFS_ACCESS_KEY" > /run/secrets/rustfs-access-key
	# # test -n "$RUSTFS_SECRET_KEY" && printf '%s' "$RUSTFS_SECRET_KEY" > /run/secrets/rustfs-secret-key
	# # Or using a secrets manager CLI (example with aws Secrets Manager):
	# # aws secretsmanager get-secret-value --secret-id rustfs/access-key \
	# # --query SecretString --output text > /run/secrets/rustfs-access-key
	# # aws secretsmanager get-secret-value --secret-id rustfs/secret-key \
	# # --query SecretString --output text > /run/secrets/rustfs-secret-key

[houseme]

@copilot

[Copilot]

@copilot

Addressed both review comments in commit cc9ee25:

• examples/flake.nix: replaced the hardcoded echo -n "my-access-key" activation script with an environment-variable-based placeholder pattern that never embeds secret bytes in the Nix expression.
• README.md: updated accessKeyFile/secretKeyFile option descriptions to say files must be readable by root/systemd (via LoadCredential), not the rustfs service user; also corrected the deprecation note to say the options were renamed (not removed) via mkRenamedOptionModule, requiring a valid file path when enable = true.