Skip to content

Add Phoenix LiveDashboard on a separate port#4119

Merged
alco merged 14 commits intomainfrom
alco/phoenix-live-dashboard
Apr 14, 2026
Merged

Add Phoenix LiveDashboard on a separate port#4119
alco merged 14 commits intomainfrom
alco/phoenix-live-dashboard

Conversation

@alco
Copy link
Copy Markdown
Member

@alco alco commented Apr 13, 2026

Summary

  • Adds Phoenix LiveDashboard served on a dedicated Bandit instance, configured via ELECTRIC_LIVE_DASHBOARD_PORT
  • Dashboard is opt-in: not started unless the env var is set
  • Fully isolated from the main Plug-based HTTP API

⚠️ Security note: The dashboard endpoint is completely unauthenticated and exposes internal system state (VM metrics, process info, ETS tables, etc.). In production, operators must ensure the dashboard port is firewalled or otherwise restricted to trusted networks only.

Test plan

  • Set ELECTRIC_LIVE_DASHBOARD_PORT=4000 and verify dashboard loads at http://localhost:4000
  • Verify main API on ELECTRIC_PORT is unaffected
  • Verify dashboard does not start when env var is unset

🤖 Generated with Claude Code

alco and others added 3 commits April 13, 2026 11:57
Serve the dashboard on a dedicated Bandit instance configured via
ELECTRIC_LIVE_DASHBOARD_PORT to avoid interfering with the existing
Plug-based HTTP API. When the env var is unset the dashboard is not
started.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
They tend to not load at all and the graph layout is barely informative
@alco alco added the claude label Apr 13, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 673bd848d0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@claude
Copy link
Copy Markdown

claude bot commented Apr 13, 2026

Claude Code Review

Summary

This PR adds an opt-in Phoenix LiveDashboard on a dedicated Bandit instance. Good progress since the first review — most issues were addressed. Two concerns remain: the default all-interfaces bind address and a config-access pattern inconsistency.

What's Working Well

  • Proper Phoenix controllerFaviconController now uses use Phoenix.Controller as suggested.
  • Changeset file added.changeset/nice-taxis-shake.md is present.
  • IPv6 support — dashboard IP now respects ELECTRIC_LISTEN_ON_IPV6, consistent with the rest of the service. Good catch from the inline review.
  • Security warnings — the README section has a prominent WARNING block and the runtime.exs block has a matching code comment. Adequate operator guidance.
  • Signing salts improved — both salts changed from obviously-predictable strings ("live_dashboard_salt", "live_dashboard") to random-looking values. And since secret_key_base is randomly generated per startup, the HMAC security is sound regardless.
  • Trailing whitespace — fixed.

Issues Found

Critical (Must Fix)

None remaining.

Important (Should Fix)

1. Dashboard still binds to {0, 0, 0, 0} by default

File: packages/sync-service/config/runtime.exs:399-402

dashboard_ip =
  if env!("ELECTRIC_LISTEN_ON_IPV6", :boolean, false),
    do: {0, 0, 0, 0, 0, 0, 0, 0},
    else: {0, 0, 0, 0}

With zero authentication, binding to all interfaces is the most common way operators accidentally expose this endpoint. Container port-forwarding, misconfigured firewall rules, and cloud security group defaults can all leave this reachable from the public internet.

Suggested fix — default to localhost; let operators opt in to a wider bind if needed:

dashboard_ip =
  case env!("ELECTRIC_LIVE_DASHBOARD_IP", :string, "127.0.0.1") do
    "::0" -> {0, 0, 0, 0, 0, 0, 0, 0}
    "0.0.0.0" -> {0, 0, 0, 0}
    _ -> {127, 0, 0, 1}
  end

Or, simpler — just use {127, 0, 0, 1} unconditionally and document that operators must use a reverse proxy or SSH tunnel to access it remotely. The IPv6 option (ELECTRIC_LISTEN_ON_IPV6) affects the main API; sharing that knob for the dashboard too is surprising coupling.


2. Inconsistent config access pattern

File: packages/sync-service/lib/electric/application.ex:57

# New code (line 57):
live_dashboard_endpoint(Application.get_env(:electric, :live_dashboard_port))

# Established pattern one line above (line 56):
prometheus_endpoint(Electric.Config.get_env(:prometheus_port))

This was raised in the first review and hasn't been addressed. Electric.Config.get_env/1 applies centralized default resolution and is the established pattern in this module. Use it for consistency:

live_dashboard_endpoint(Electric.Config.get_env(:live_dashboard_port))

Suggestions (Nice to Have)

Session invalidation on restart — worth a comment

File: packages/sync-service/config/runtime.exs:417

secret_key_base: Base.encode64(:crypto.strong_rand_bytes(48)),

This is the right default for a dev tool, but operators running Electric behind a load balancer with multiple instances will find that dashboard sessions are invalidated across restarts and not portable between nodes. A short comment here would help:

# Random per-startup key: sessions don't survive restarts and aren't
# shared across nodes. Acceptable for a dev/ops tool; if you need
# stable sessions, replace with an operator-supplied env var.
secret_key_base: Base.encode64(:crypto.strong_rand_bytes(48)),

Issue Conformance

No linked issue — unchanged from previous review. The PR description is clear and the README section adequately documents the feature for operators.

Previous Review Status

Issue Status
Critical: Unauthenticated access warning ✅ Resolved — explicit WARNING in README and code comments
Important: Predictable signing salts ✅ Resolved — salts updated; secret_key_base is random per startup
Important: Inconsistent config access (Application.get_env) ❌ Not yet addressed
Important: FaviconController not a Phoenix controller ✅ Resolved — now uses use Phoenix.Controller
Important: Session invalidation comment ⬇️ Downgraded to suggestion
Suggestion: Trailing whitespace in router ✅ Resolved
Suggestion: Missing changeset file ✅ Resolved
IPv6 binding (codex-bot inline) ✅ Resolved

The one remaining Important issue (default bind address) and the unaddressed config pattern inconsistency are both straightforward to fix. Otherwise this is close to merge-ready.


Review iteration: 2 | 2026-04-13

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
2724 1 2723 1
View the top 1 failed test(s) by shortest run time
Elixir.Electric.ShapeCacheTest::test await_snapshot_start/4 does not recursively loop forever if the snapshot fails to start
Stack Traces | 0.129s run time
24) test await_snapshot_start/4 does not recursively loop forever if the snapshot fails to start (Electric.ShapeCacheTest)
     test/electric/shape_cache_test.exs:877
     Expected truthy, got false
     code: assert String.contains?(
             log,
             "[error] No consumer process when waiting on initial snapshot creation for #{shape_handle}"
           )
     arguments:

         # 1
         ""

         # 2
         "[error] No consumer process when waiting on initial snapshot creation for 50617496-1776173665517056"

     stacktrace:
       test/electric/shape_cache_test.exs:925: (test)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

alco and others added 8 commits April 13, 2026 13:03
- Respect ELECTRIC_LISTEN_ON_IPV6 when binding the dashboard socket
- Make FaviconController a proper Phoenix controller
- Add unauthenticated-endpoint warnings in moduledoc, config, and README

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@magnetised magnetised left a comment

Choose a reason for hiding this comment

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

Looks good, this will be very useful so thanks for adding. I don't like all the new top-level modules, especially for an optional component, could we move under the Electric.LiveDashboard namespace?

Addresses review feedback to avoid polluting the top-level namespace
with an optional component. Also uses Electric.Config.get_env for
consistency.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@alco
Copy link
Copy Markdown
Member Author

alco commented Apr 14, 2026

@magnetised Good suggestions! Thanks for taking the time to propose code changes. All have been accepted.

@netlify
Copy link
Copy Markdown

netlify bot commented Apr 14, 2026

Deploy Preview for electric-next ready!

Name Link
🔨 Latest commit de512b2
🔍 Latest deploy log https://app.netlify.com/projects/electric-next/deploys/69de40f272800d0008d7a4da
😎 Deploy Preview https://deploy-preview-4119--electric-next.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Contributor

@magnetised magnetised left a comment

Choose a reason for hiding this comment

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

lovely

@alco alco merged commit 0afe007 into main Apr 14, 2026
45 of 46 checks passed
@alco alco deleted the alco/phoenix-live-dashboard branch April 14, 2026 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants