Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 20 minutes and 53 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis pull request replaces the Cloudflare tunnel service with Let's Encrypt ACME-based certificate provisioning, adds a NixOS ACME module and local nginx challenge host, updates HAProxy to use ACME certificates and route ACME challenges, removes gcw/n8n backends, adjusts Gitea SSH port and firewall, and removes Cloudflare references from editor/validation lists. Changes
Sequence DiagramsequenceDiagram
participant ACME as ACME Server
participant HAProxy as HAProxy
participant nginx as nginx (127.0.0.1:8402)
participant systemd as systemd
ACME->>HAProxy: HTTP GET /.well-known/acme-challenge/<token>
HAProxy->>nginx: Route to acme_challenge backend (127.0.0.1:8402)
nginx->>ACME: Serve challenge token from /var/lib/acme/.challenges
ACME->>ACME: Validate domain ownership
ACME->>systemd: Notify certificate issuance/renewal
systemd->>HAProxy: Reload haproxy.service
HAProxy->>HAProxy: Load new certificates from /var/lib/acme/*/full.pem
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
systems/jeeves/services/gitea.nix (1)
5-27:⚠️ Potential issue | 🟡 MinorRun
treefmton this file before merge.CI is already failing on
systems/jeeves/services/gitea.nix, so this will block the PR even if the config is otherwise correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@systems/jeeves/services/gitea.nix` around lines 5 - 27, The CI failure is due to formatting; run the project's treefmt formatter on systems/jeeves/services/gitea.nix to fix style issues (e.g., the networking.firewall.allowedTCPPorts and services.gitea blocks including settings.server entries like HTTP_PORT/SSH_PORT), then add and commit the reformatted file so the CI passes—ensure you run the same treefmt command the repo uses locally (or via pre-commit) before pushing.
🧹 Nitpick comments (1)
systems/jeeves/services/acme.nix (1)
8-36: Generate the cert definitions from a shared domain list.These five blocks are identical apart from the hostname, and the same hostnames are duplicated again in
systems/jeeves/services/haproxy.cfg:26. The next domain change is easy to miss in one place. Buildingsecurity.acme.certsfrom a single list would keep issuance and HAProxy bindings from drifting.Refactor sketch
+let + acmeDomains = [ + "gitea.tmmworkshop.com" + "audiobookshelf.tmmworkshop.com" + "cache.tmmworkshop.com" + "jellyfin.tmmworkshop.com" + "share.tmmworkshop.com" + ]; +in { users.users.haproxy.extraGroups = [ "acme" ]; security.acme = { acceptTerms = true; defaults.email = "Richie@tmmworkshop.com"; - certs."gitea.tmmworkshop.com" = { - webroot = "/var/lib/acme/.challenges"; - group = "acme"; - reloadServices = [ "haproxy.service" ]; - }; - - certs."audiobookshelf.tmmworkshop.com" = { - webroot = "/var/lib/acme/.challenges"; - group = "acme"; - reloadServices = [ "haproxy.service" ]; - }; - - certs."cache.tmmworkshop.com" = { - webroot = "/var/lib/acme/.challenges"; - group = "acme"; - reloadServices = [ "haproxy.service" ]; - }; - - certs."jellyfin.tmmworkshop.com" = { - webroot = "/var/lib/acme/.challenges"; - group = "acme"; - reloadServices = [ "haproxy.service" ]; - }; - - certs."share.tmmworkshop.com" = { - webroot = "/var/lib/acme/.challenges"; - group = "acme"; - reloadServices = [ "haproxy.service" ]; - }; + certs = builtins.listToAttrs (map (domain: { + name = domain; + value = { + webroot = "/var/lib/acme/.challenges"; + group = "acme"; + reloadServices = [ "haproxy.service" ]; + }; + }) acmeDomains); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@systems/jeeves/services/acme.nix` around lines 8 - 36, The repeated certs entries (certs."gitea.tmmworkshop.com", certs."audiobookshelf.tmmworkshop.com", certs."cache.tmmworkshop.com", certs."jellyfin.tmmworkshop.com", certs."share.tmmworkshop.com") should be generated from a single shared domain list so issuance and HAProxy bindings can't drift; replace the five nearly identical certs.<* entries with a map/list (e.g., security.acme.domains or a local let-bound list) and programmatically build security.acme.certs by iterating that list to produce keys and the same webroot/group/reloadServices for each host, and update systems/jeeves/services/haproxy.cfg to consume that same domain list rather than duplicating hostnames so both cert generation (security.acme.certs) and HAProxy config reference one canonical source of domain names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@systems/jeeves/services/gitea.nix`:
- Line 5: The Nix service currently exposes port 6443 via
networking.firewall.allowedTCPPorts which allows direct access bypassing
HAProxy's TLS; either remove 6443 from the allowedTCPPorts array in the
gitea.nix service or change Gitea's HTTP listener to bind to loopback by setting
HTTP_ADDR = "127.0.0.1" in the Gitea configuration (so HAProxy can proxy via
127.0.0.1:6443 as defined in haproxy.cfg). Locate
networking.firewall.allowedTCPPorts in systems/jeeves/services/gitea.nix and
remove 6443, or update the Gitea config block that controls HTTP_ADDR (the Gitea
service config) to explicitly set HTTP_ADDR to 127.0.0.1 and restart the
service.
In `@systems/jeeves/services/haproxy.cfg`:
- Around line 26-31: The TLS bind on bind *:443 currently requires
/var/lib/acme/*/full.pem to exist which prevents HAProxy from starting before
ACME completes; change the bootstrap so HAProxy can start without real certs by
adding a fallback self-signed PEM (e.g. /var/lib/acme/seed.pem) or use a
crt-list fallback and reference it in the same bind line (bind *:443 ssl crt
/var/lib/acme/seed.pem crt /var/lib/acme/cache.tmmworkshop.com/full.pem ...),
and ensure your init/bootstrap creates the seed PEM if missing; keep the ACME
routing ACLs (acl is_acme, use_backend acme_challenge) as-is so HTTP-01 requests
forwarded to nginx can provision real certs which will replace the seed.
---
Outside diff comments:
In `@systems/jeeves/services/gitea.nix`:
- Around line 5-27: The CI failure is due to formatting; run the project's
treefmt formatter on systems/jeeves/services/gitea.nix to fix style issues
(e.g., the networking.firewall.allowedTCPPorts and services.gitea blocks
including settings.server entries like HTTP_PORT/SSH_PORT), then add and commit
the reformatted file so the CI passes—ensure you run the same treefmt command
the repo uses locally (or via pre-commit) before pushing.
---
Nitpick comments:
In `@systems/jeeves/services/acme.nix`:
- Around line 8-36: The repeated certs entries (certs."gitea.tmmworkshop.com",
certs."audiobookshelf.tmmworkshop.com", certs."cache.tmmworkshop.com",
certs."jellyfin.tmmworkshop.com", certs."share.tmmworkshop.com") should be
generated from a single shared domain list so issuance and HAProxy bindings
can't drift; replace the five nearly identical certs.<* entries with a map/list
(e.g., security.acme.domains or a local let-bound list) and programmatically
build security.acme.certs by iterating that list to produce keys and the same
webroot/group/reloadServices for each host, and update
systems/jeeves/services/haproxy.cfg to consume that same domain list rather than
duplicating hostnames so both cert generation (security.acme.certs) and HAProxy
config reference one canonical source of domain names.
🪄 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: 5edbdc62-8a5e-4ba6-a017-6b93968cf0ce
📒 Files selected for processing (6)
.vscode/settings.jsonsystems/jeeves/services/acme.nixsystems/jeeves/services/cloud_flare_tunnel.nixsystems/jeeves/services/gitea.nixsystems/jeeves/services/haproxy.cfgsystems/jeeves/services/validate_system.toml
💤 Files with no reviewable changes (3)
- .vscode/settings.json
- systems/jeeves/services/validate_system.toml
- systems/jeeves/services/cloud_flare_tunnel.nix
systems/jeeves/services/gitea.nix
Outdated
| in | ||
| { | ||
| networking.firewall.allowedTCPPorts = [ 6443 ]; | ||
| networking.firewall.allowedTCPPorts = [ 6443 2223 ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspecting Gitea bind/firewall configuration for public exposure of 6443..."
rg -n -C2 'services\.gitea|HTTP_ADDR|HTTP_PORT|allowedTCPPorts|6443|2223' systemsRepository: RichieCahill/dotfiles
Length of output: 4835
Remove port 6443 from the host firewall or bind Gitea's HTTP listener to loopback.
Line 5 exposes port 6443 on the host firewall while HAProxy proxies Gitea through 127.0.0.1:6443 in systems/jeeves/services/haproxy.cfg:76-78. However, there is no HTTP_ADDR = "127.0.0.1" in the Gitea configuration, meaning Gitea listens on all interfaces by default. This creates a direct, unencrypted path around HAProxy's TLS/redirect layer. Either remove 6443 from allowedTCPPorts or explicitly bind the HTTP listener to 127.0.0.1.
Suggested fix
- networking.firewall.allowedTCPPorts = [ 6443 2223 ];
+ networking.firewall.allowedTCPPorts = [ 2223 ]; DOMAIN = "tmmworkshop.com";
ROOT_URL = "https://gitea.tmmworkshop.com/";
+ HTTP_ADDR = "127.0.0.1";
HTTP_PORT = 6443;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| networking.firewall.allowedTCPPorts = [ 6443 2223 ]; | |
| networking.firewall.allowedTCPPorts = [ 2223 ]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@systems/jeeves/services/gitea.nix` at line 5, The Nix service currently
exposes port 6443 via networking.firewall.allowedTCPPorts which allows direct
access bypassing HAProxy's TLS; either remove 6443 from the allowedTCPPorts
array in the gitea.nix service or change Gitea's HTTP listener to bind to
loopback by setting HTTP_ADDR = "127.0.0.1" in the Gitea configuration (so
HAProxy can proxy via 127.0.0.1:6443 as defined in haproxy.cfg). Locate
networking.firewall.allowedTCPPorts in systems/jeeves/services/gitea.nix and
remove 6443, or update the Gitea config block that controls HTTP_ADDR (the Gitea
service config) to explicitly set HTTP_ADDR to 127.0.0.1 and restart the
service.
| bind *:443 v4v6 ssl crt /var/lib/acme/audiobookshelf.tmmworkshop.com/full.pem crt /var/lib/acme/cache.tmmworkshop.com/full.pem crt /var/lib/acme/jellyfin.tmmworkshop.com/full.pem crt /var/lib/acme/share.tmmworkshop.com/full.pem crt /var/lib/acme/gitea.tmmworkshop.com/full.pem | ||
| mode http | ||
|
|
||
| # ACME challenge routing (must be first) | ||
| acl is_acme path_beg /.well-known/acme-challenge/ | ||
| use_backend acme_challenge if is_acme |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for any bootstrap certificate or fallback TLS path..."
rg -n -C2 '/var/lib/acme/.*/full\.pem|full\.pem|cloudflare\.pem|self-?signed|mkcert|openssl|bind \*:443|security\.acme' systemsRepository: RichieCahill/dotfiles
Length of output: 977
🏁 Script executed:
cat -n systems/jeeves/services/acme.nix | head -60Repository: RichieCahill/dotfiles
Length of output: 2100
🏁 Script executed:
# Check for any bootstrap scripts, initialization, or pre-seeded certificates
find systems -type f \( -name "*.sh" -o -name "*.nix" \) | xargs rg -l "bootstrap|init|pre.*cert|seeded|temporary.*pem" | head -20Repository: RichieCahill/dotfiles
Length of output: 214
🏁 Script executed:
# Check if there are any HAProxy-specific bootstrap mechanisms or optional cert handling
rg -n -A5 -B5 "bind.*ssl|crt|certFile|sslCertificates" systems/jeeves/services/haproxy.cfgRepository: RichieCahill/dotfiles
Length of output: 589
This breaks first-time ACME bootstrap.
Line 26 makes HAProxy require /var/lib/acme/*/full.pem to start on :443, but those files do not exist on a fresh host until ACME succeeds. In this setup, ACME's HTTP-01 validation depends on HAProxy already serving :80 and forwarding /.well-known/acme-challenge/ to nginx at 127.0.0.1:8402 (lines 29-31, configured in systems/jeeves/services/acme.nix). This creates a circular dependency: HAProxy cannot start without the certs, and ACME cannot provision the certs without HAProxy running.
A bootstrap path is needed: e.g. seed a temporary self-signed PEM, use an optional certificate binding, or provision the HTTP-01 responder on a separate listener that starts before TLS binding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@systems/jeeves/services/haproxy.cfg` around lines 26 - 31, The TLS bind on
bind *:443 currently requires /var/lib/acme/*/full.pem to exist which prevents
HAProxy from starting before ACME completes; change the bootstrap so HAProxy can
start without real certs by adding a fallback self-signed PEM (e.g.
/var/lib/acme/seed.pem) or use a crt-list fallback and reference it in the same
bind line (bind *:443 ssl crt /var/lib/acme/seed.pem crt
/var/lib/acme/cache.tmmworkshop.com/full.pem ...), and ensure your
init/bootstrap creates the seed PEM if missing; keep the ACME routing ACLs (acl
is_acme, use_backend acme_challenge) as-is so HTTP-01 requests forwarded to
nginx can provision real certs which will replace the seed.
RichieCahill
force-pushed
the
feature/moving-off-cloudflare
branch
from
2a0a5e2 to
8d4ba8e
Compare
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores