Skip to content

Redesign UI, harden WireGuard/networking, and make Docker production-… - #1

Merged
iamfarhad merged 3 commits into
mainfrom
feat/ui-redesign-production-docker
Jul 17, 2026
Merged

iamfarhad merged 3 commits into
mainfrom
feat/ui-redesign-production-docker

Conversation

@iamfarhad

Copy link
Copy Markdown
Owner

…ready

Frontend

  • Full UI/UX redesign: semantic design-token system (light/dark) with an Inter + JetBrains Mono type stack, a per-user light/dark/system theme switcher, refreshed component kit (Button/Card/Badge/Dialog/Table/Field/ Progress/…), grouped sidebar + mobile drawer, and restyled pages.
  • Accounts: searchable list with quota bars and a redesigned tabbed detail modal (Overview/Devices/Usage/Edit) that finally exposes account editing (label, quota, device limit, bandwidth, expiry).
  • Nodes: subnet picker dropdown of curated RFC1918/RFC6598 ranges that avoid home-LAN clashes; already-used ranges are disabled, Custom stays.

WireGuard / networking

  • Fix IPv6 leak: client configs now set AllowedIPs = 0.0.0.0/0, ::/0 so dual-stack clients can't leak IPv6 outside the tunnel.
  • Replace drop-based upload policing with proper IFB HTB shaping (falls back to policing when the ifb module is unavailable).
  • install-node.sh: real agent source (repo/override/local), egress-iface override; node-entrypoint: idempotent wg-quick up.

Production Docker

  • Split compose into production docker-compose.yml (5 services: pinned image digests, log rotation, memory limits, no-new-privileges, cap_drop on api, healthchecks) and a dev-only docker-compose.override.yml (builds
    • in-Docker test node).
  • redis pinned to noeviction/maxmemory (it stores refresh tokens).
  • Both installers now run WireGuard nodes as production Docker containers (bridge networking; NAT handled inside the container + by the Docker daemon), with a build-from-source fallback; add build-node.yml CI to publish the node image, plus backend .dockerignore and digest-pinned runtime bases.

Docs

  • README: screenshots plus an expanded developer guide (repo layout, local dev workflow, dev/prod compose split, tests, image/CI overview).

…ready

Frontend
- Full UI/UX redesign: semantic design-token system (light/dark) with an
  Inter + JetBrains Mono type stack, a per-user light/dark/system theme
  switcher, refreshed component kit (Button/Card/Badge/Dialog/Table/Field/
  Progress/…), grouped sidebar + mobile drawer, and restyled pages.
- Accounts: searchable list with quota bars and a redesigned tabbed detail
  modal (Overview/Devices/Usage/Edit) that finally exposes account editing
  (label, quota, device limit, bandwidth, expiry).
- Nodes: subnet picker dropdown of curated RFC1918/RFC6598 ranges that
  avoid home-LAN clashes; already-used ranges are disabled, Custom stays.

WireGuard / networking
- Fix IPv6 leak: client configs now set AllowedIPs = 0.0.0.0/0, ::/0 so
  dual-stack clients can't leak IPv6 outside the tunnel.
- Replace drop-based upload policing with proper IFB HTB shaping (falls
  back to policing when the ifb module is unavailable).
- install-node.sh: real agent source (repo/override/local), egress-iface
  override; node-entrypoint: idempotent wg-quick up.

Production Docker
- Split compose into production docker-compose.yml (5 services: pinned
  image digests, log rotation, memory limits, no-new-privileges, cap_drop
  on api, healthchecks) and a dev-only docker-compose.override.yml (builds
  + in-Docker test node).
- redis pinned to noeviction/maxmemory (it stores refresh tokens).
- Both installers now run WireGuard nodes as production Docker containers
  (bridge networking; NAT handled inside the container + by the Docker
  daemon), with a build-from-source fallback; add build-node.yml CI to
  publish the node image, plus backend .dockerignore and digest-pinned
  runtime bases.

Docs
- README: screenshots plus an expanded developer guide (repo layout, local
  dev workflow, dev/prod compose split, tests, image/CI overview).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request implements key VPN product features, including per-account bandwidth limits enforced via tc, device-limit tracking and enforcement, unauthenticated subscription URLs for client provisioning, and a smart node steering engine. It also transitions the WireGuard node agent to a containerized model and updates the frontend UI with theme support and improved form fields. The review feedback highlights several critical issues to address: a potential database transaction rollback during heartbeat ingestion due to duplicate device observations, a lack of cleanup for half-configured qdisc trees on setup failure, timezone parsing inconsistencies in the frontend, a truncation bug in subscription filename generation, and a scroll-locking bug when handling multiple open dialogs.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread backend/internal/store/devices.go Outdated
Comment on lines +45 to +54
accountIDs := make([]string, len(observations))
endpoints := make([]string, len(observations))
seenAts := make([]time.Time, len(observations))
touched := make(map[string]bool, len(observations))
for i, o := range observations {
accountIDs[i] = o.accountID
endpoints[i] = o.endpoint
seenAts[i] = o.seenAt
touched[o.accountID] = true
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

If multiple peers under the same account connect from the same NAT'd endpoint (sharing the same public IP and port), or if there are duplicate reports in the heartbeat, the observations slice will contain duplicate (account_id, source_endpoint) pairs. When Postgres executes INSERT ... ON CONFLICT DO UPDATE, it will fail with ERROR: ON CONFLICT DO UPDATE command cannot affect row a second time because a single SQL statement cannot update the same row twice. This will cause the entire heartbeat ingestion transaction to roll back, breaking telemetry and device tracking. We should de-duplicate the observations by (accountID, endpoint) before building the arrays for unnest.

    type key struct {
        accountID string
        endpoint  string
    }
    unique := make(map[key]time.Time)
    for _, o := range observations {
        k := key{accountID: o.accountID, endpoint: o.endpoint}
        if existing, ok := unique[k]; !ok || o.seenAt.After(existing) {
            unique[k] = o.seenAt
        }
    }

    accountIDs := make([]string, 0, len(unique))
    endpoints := make([]string, 0, len(unique))
    seenAts := make([]time.Time, 0, len(unique))
    touched := make(map[string]bool)
    for k, seenAt := range unique {
        accountIDs = append(accountIDs, k.accountID)
        endpoints = append(endpoints, k.endpoint)
        seenAts = append(seenAts, seenAt)
        touched[k.accountID] = true
    }

Comment thread backend/cmd/agent/tc.go
Comment on lines +193 to +197
for _, cmd := range setup {
if out, err := exec.Command(cmd[0], cmd[1:]...).CombinedOutput(); err != nil {
return fmt.Errorf("%s: %w (%s)", strings.Join(cmd, " "), err, strings.TrimSpace(string(out)))
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If any command in the setup phase fails (for example, due to a transient network configuration issue or invalid arguments), the interface is left in a partially-configured, half-built state. To prevent this, we should run the teardown commands on failure to clean up any half-built qdisc trees.

Suggested change
for _, cmd := range setup {
if out, err := exec.Command(cmd[0], cmd[1:]...).CombinedOutput(); err != nil {
return fmt.Errorf("%s: %w (%s)", strings.Join(cmd, " "), err, strings.TrimSpace(string(out)))
}
}
for _, cmd := range setup {
if out, err := exec.Command(cmd[0], cmd[1:]...).CombinedOutput(); err != nil {
for _, tdCmd := range teardown {
_ = exec.Command(tdCmd[0], tdCmd[1:]...).Run()
}
return fmt.Errorf("%s: %w (%s)", strings.Join(cmd, " "), err, strings.TrimSpace(string(out)))
}
}

if (quotaGb !== initial.quotaGb && quotaGb !== '' && Number(quotaGb) > 0) changes.data_quota_gb = Number(quotaGb)
if (deviceLimit !== initial.deviceLimit && deviceLimit !== '' && Number(deviceLimit) >= 1) changes.device_limit = Number(deviceLimit)
if (bandwidth !== initial.bandwidth) changes.bandwidth_limit_mbps = bandwidth === '' ? 0 : Number(bandwidth)
if (expiry !== initial.expiry && expiry !== '') changes.expiry_at = new Date(`${expiry}T23:59:59`).toISOString()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Parsing a date string like YYYY-MM-DDT23:59:59 without an explicit timezone offset is inconsistent across browsers and will be parsed as local time in most environments. This causes the resulting UTC timestamp to vary depending on the client's local timezone. To ensure consistent behavior, append Z to make it an explicit UTC timestamp.

Suggested change
if (expiry !== initial.expiry && expiry !== '') changes.expiry_at = new Date(`${expiry}T23:59:59`).toISOString()
if (expiry !== initial.expiry && expiry !== '') changes.expiry_at = new Date(expiry + "T23:59:59Z").toISOString()

Comment on lines +193 to +203
// confFilename derives a safe attachment filename from the account label. WireGuard
// clients derive the tunnel name from this, and wg-quick tunnel names max out at 15
// chars - keep the stem within that so imports don't fail or get truncated oddly.
func confFilename(label string) string {
stem := confFilenameUnsafe.ReplaceAllString(strings.ToLower(label), "-")
stem = strings.Trim(stem, "-.")
if stem == "" {
stem = "wgpanel"
}
if len(stem) > 15 {
stem = stem[:15]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If the generated stem is truncated to 15 characters, it might end with a trailing hyphen or dot (e.g., very-long-label-.conf). To prevent this, the strings.Trim(stem, "-.") operation should be performed after the truncation.

func confFilename(label string) string {
    stem := confFilenameUnsafe.ReplaceAllString(strings.ToLower(label), "-")
    if len(stem) > 15 {
        stem = stem[:15]
    }
    stem = strings.Trim(stem, "-.")
    if stem == "" {
        stem = "wgpanel"
    }
    return stem + ".conf"
}

Comment on lines +17 to +29
useEffect(() => {
if (!open) return
const onKey = (e: KeyboardEvent) => {
if (e.key === 'Escape') onClose()
}
document.addEventListener('keydown', onKey)
const prevOverflow = document.body.style.overflow
document.body.style.overflow = 'hidden'
return () => {
document.removeEventListener('keydown', onKey)
document.body.style.overflow = prevOverflow
}
}, [open, onClose])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Managing document.body.style.overflow directly inside a useEffect hook can lead to bugs when multiple dialogs are open simultaneously or when they are closed out of order. For example, if Dialog A and Dialog B are both open, closing Dialog A first will restore the overflow to "" (allowing scrolling while Dialog B is still open), and closing Dialog B will then set the overflow to "hidden" permanently. Consider using a global open-dialog counter or a dedicated library like react-remove-scroll to manage body scroll locking safely.

farhadzand and others added 2 commits July 17, 2026 19:45
The gofmt -l check in build-api.yml failed on struct-field alignment in
accounts.go (pre-existing STORY-11 code that predated the format check).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…scroll lock

- store/devices.go (critical): de-duplicate (account_id, source_endpoint)
  observations before the ON CONFLICT DO UPDATE upsert. A duplicated peer in a
  kernel snapshot would otherwise make one statement touch the same row twice,
  which Postgres rejects and rolls back the whole heartbeat transaction.
- cmd/agent/tc.go: on a shaping setup failure, run the teardown commands so the
  interface is left cleanly unshaped instead of in a half-built qdisc state.
- httpapi/subscription.go: trim the conf filename AFTER truncating to 15 chars so
  a cut landing on a separator can't leave a trailing "-"/"." before .conf; add a
  test case.
- ui/Dialog.tsx: ref-count the body-scroll lock so stacked dialogs (delete confirm
  over an account detail) can't re-enable scrolling early or leave the body locked.

Declined: the expiry-date UTC ("Z") suggestion - the edit form intentionally
matches CreateAccountDialog's existing local-time end-of-day parsing; changing
one alone would make the two dialogs inconsistent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@iamfarhad
iamfarhad merged commit 498a4cb into main Jul 17, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant