Skip to content

fix(selfhost): open Grafana's port for --profile observability, opt-in - #6108

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
luciferlive112116:fix/terraform-grafana-firewall
Jul 15, 2026
Merged

fix(selfhost): open Grafana's port for --profile observability, opt-in#6108
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
luciferlive112116:fix/terraform-grafana-firewall

Conversation

@luciferlive112116

Copy link
Copy Markdown
Contributor

Summary

Closes #5818

terraform/main.tf's hcloud_firewall.gittensory opened exactly four things: TCP/22 (SSH), TCP/80 + TCP/443 + UDP/443 (Caddy), and TCP/8787 (admin_ip_allowlist-scoped). Verified against the file: no rule for port 3000.

docker-compose.yml's grafana service — profiles: ["observability"], ports: ["3000:3000"] — publishes Grafana directly on the host. So an operator following main.tf's own documented flow (provision the VPS, then docker compose --profile observability up -d, a combination the self-hosting docs list as supported) ends up with Grafana bound to the public interface and no path through the cloud firewall to reach it — not even from their own IP. The failure mode is a timed-out connection with no explanation.

Fix — opt-in, never public. The rule is gated behind a new var.expose_grafana (bool, default false) rather than opened unconditionally: the observability profile is itself opt-in, so a default-open port for a service most operators never start would widen the attack surface for nothing. When enabled, the rule is scoped to var.admin_ip_allowlist, matching port 8787's existing pattern — never 0.0.0.0/0/::/0 like the public Caddy ports. This is the config-as-code resolution the issue's first requirement asks for, with the toggle its second requirement asks for.

Left at the default, Grafana stays reachable over an SSH tunnel. The new terraform/README.md (this module had none, unlike packages/loopover-miner/terraform/) documents both access paths — including the runnable ssh -L 3000:localhost:3000 … command — alongside what gets provisioned, the deploy steps main.tf's header already described, and the outputs. It mirrors the miner module README's shape.

Scope

  • Conventional Commit title (fix(selfhost): …).
  • Focused: one firewall rule + its gating variable + the module README the issue asks for + a structural test.
  • Additive and default-inert: with expose_grafana at its false default, the generated plan is byte-identical to today's — no existing rule, resource, or output is touched.
  • Follows CONTRIBUTING.md; no site//CNAME/lovable changes; no changelog edit.
  • Linked open issue (Closes fix(selfhost): Terraform firewall never opens Grafana's port for --profile observability #5818, above).

Validation

  • git diff --check clean.
  • HCL syntax validated with a real parser. terraform is not installed in this environment and — as the issue notes — the repo has no terraform validate/plan CI step for the root module, so a syntax error here would ship silently. I parsed all three .tf files with python-hcl2: main.tf, variables.tf, outputs.tf all parse, and the new block parses to exactly the intended shape: dynamic "rule"for_each = ${var.expose_grafana ? [1] : []}content = {direction "in", protocol "tcp", port "3000", source_ips ${var.admin_ip_allowlist}}.
  • New tests green (5), plus the sibling Terraform/self-host suites — miner-terraform-module, self-host-ops-rename-residue16 tests passed.
  • Proved the tests catch the bug: reverted only the .tf changes and confirmed the three fix-dependent tests fail (rule missing, not allowlist-scoped, variable absent), then pass again with the fix restored.
  • Rebased onto current main.

The test locks the invariants a syntax check cannot see, mirroring test/unit/miner-terraform-module.test.ts:

  • The rule exists and is gated on var.expose_grafana.
  • INVARIANT: it is admin_ip_allowlist-scoped and contains no 0.0.0.0/0/::/0 — it can never be silently made public.
  • INVARIANT: expose_grafana is a bool defaulting to false, with a description (matching this file's self-documenting style).
  • It pins the rule to the compose service it exists for (grafana still publishes 3000:3000 under profiles: ["observability"]), so if that drifts, the rule guarding the wrong port fails CI.
  • The README documents both access paths, including the SSH-tunnel command.

If any required check was skipped, explain why:

  • Full test:ci not run end-to-end locally (Linux-only steps on Windows).
  • No Codecov patch-coverage obligation: the diff touches only terraform/** (HCL + README) and test/**, both outside Codecov's coverage.include. Noting that explicitly per the issue's Test Coverage Requirements rather than omitting the section.

Safety

  • No secrets, wallets, hotkeys, trust scores, rewards, private rankings, or private maintainer evidence. No credential or token is added; hcloud_token/ssh_public_key handling is untouched.
  • Security-relevant, and deliberately conservative: the default (expose_grafana = false) opens nothing, so applying this changes no operator's firewall until they opt in. The port can only ever open to admin_ip_allowlist, and the test enforces that it can never be widened to 0.0.0.0/0 without failing CI. The README warns to restrict admin_ip_allowlist before enabling, since its own default is permissive.
  • No auth/cookie/CORS/GitHub App/session change; no application runtime code touched.
  • No API/OpenAPI/MCP change; no schema change; no generated artifact affected.
  • No UI changes; no changelog edit.

terraform/main.tf's firewall opened 22, 80, 443/tcp, 443/udp and 8787,
but nothing for 3000. docker-compose.yml's grafana service
(--profile observability) publishes 3000:3000 on the host, so an operator
following main.tf's own documented flow -- provision the VPS, then run
docker compose --profile observability up -d -- ended up with Grafana
bound to the public interface and no path through the cloud firewall to
reach it, not even from their own IP. The failure mode is a timed-out
connection with no explanation.

Add the rule gated behind a new var.expose_grafana (bool, default false)
rather than opening it unconditionally: the observability profile is
itself opt-in, so a default-open port for a service most operators never
start would widen the attack surface for nothing. When enabled the rule
is scoped to var.admin_ip_allowlist, matching port 8787's existing
pattern -- never 0.0.0.0/0 like the public Caddy ports.

Left at the default, Grafana stays reachable over an SSH tunnel. The new
terraform/README.md (this module had none, unlike the miner module)
documents both paths, including the ssh -L command, alongside what gets
provisioned and the deploy steps main.tf's header already described.

The test locks the invariants a syntax check can't see -- the port is
opt-in and can never be public -- and pins the rule to the compose
service it exists for, mirroring test/unit/miner-terraform-module.test.ts.

Closes JSONbored#5818
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 15, 2026
@loopover-orb

loopover-orb Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-15 09:51:25 UTC

4 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds an opt-in `var.expose_grafana` (default false) that gates a `dynamic "rule"` block opening TCP/3000 to `admin_ip_allowlist`, matching the existing port-8787 pattern, plus a structural test and a new module README documenting both the SSH-tunnel and open-port access paths. The Terraform is correct: the dynamic rule syntax is valid, the default keeps the port closed, and source_ips is never the public `0.0.0.0/0`/`::/0` ranges. The regex-based structural test is a reasonable static check mirroring the existing miner-module test pattern, and it correctly ties the firewall rule to the actual docker-compose service/port it exists to guard.

Nits — 4 non-blocking
  • The regex in test/unit/root-terraform-grafana-firewall.test.ts:14 (`grafanaRule`) is fairly brittle — reordering fields in the HCL block or changing the closing brace's indentation would silently make it match empty string and fail with a less-clear error; a Terraform-aware parser or a simpler substring check would be more robust long-term.
  • terraform/main.tf's new comment block (lines ~69-73) is quite long for a single rule; consider trimming to the non-obvious rationale (opt-in reasoning) and letting the README carry the detail.
  • Consider whether `expose_grafana=true` combined with the default `admin_ip_allowlist=["0.0.0.0/0","::/0"]` (from terraform/variables.tf) should be more strongly warned against in variables.tf's own description, not just the README, since a user could set expose_grafana without ever reading the README.
  • In terraform/README.md, the `terraform apply` example already narrows `admin_ip_allowlist`, which is good — consider also noting that the same allowlist applies to SSH (port 22) so operators understand the shared blast radius of loosening it.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5818
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 113 registered-repo PR(s), 57 merged, 35 issue(s).
Contributor context ✅ Confirmed Gittensor contributor luciferlive112116; Gittensor profile; 113 PR(s), 35 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ℹ️ Insufficient signal risk: clean · value: insufficient-signal · LLM: moderate
Linked issue satisfaction

Addressed
The diff adds a toggleable, admin-allowlist-scoped firewall rule for port 3000 gated by a new expose_grafana variable (default false) in main.tf and variables.tf, and adds terraform/README.md documenting both the SSH-tunnel and opt-in firewall access paths, satisfying both alternative resolutions and all three deliverables listed in the issue.

Review context
  • Author: luciferlive112116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 113 PR(s), 35 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 0bd50b2 into JSONbored:main Jul 15, 2026
13 checks passed
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.24%. Comparing base (0760e1f) to head (236a3b6).
⚠️ Report is 23 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6108   +/-   ##
=======================================
  Coverage   95.24%   95.24%           
=======================================
  Files         595      595           
  Lines       47035    47046   +11     
  Branches    15015    15015           
=======================================
+ Hits        44797    44808   +11     
  Misses       1493     1493           
  Partials      745      745           
Flag Coverage Δ
shard-1 43.95% <ø> (-0.06%) ⬇️
shard-2 36.64% <ø> (+0.01%) ⬆️
shard-3 31.97% <ø> (-0.09%) ⬇️
shard-4 33.79% <ø> (+0.66%) ⬆️
shard-5 31.46% <ø> (-0.76%) ⬇️
shard-6 44.84% <ø> (+0.34%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(selfhost): Terraform firewall never opens Grafana's port for --profile observability

1 participant