From 8fb3fe703155a32d8683cd41761206a0a6c03971 Mon Sep 17 00:00:00 2001 From: Calmcacil Date: Mon, 12 Jan 2026 22:27:16 +0100 Subject: [PATCH] fix: critical navigation bug - users trapped in detail and restore screens Changed return value from 'return s, nil' to 'return nil, nil' in: - detail.go (line 116): Back navigation from client details - restore.go (line 101): Back navigation from restore screen Root cause: Main model's navigation logic checks if newScreen == nil. When screens returned (s, nil), the check failed and screen never changed. Now properly returns (nil, nil) to signal screen change to parent. Fixes: wg-admin-rfo --- .beads/issues.jsonl | 2 ++ internal/tui/screens/detail.go | 2 +- internal/tui/screens/restore.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index f089e21..ad5be30 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -5,6 +5,7 @@ {"id":"wg-admin-1b9","title":"Update documentation for refactored scripts","description":"Update README.md and all documentation to reflect new architecture. Document: wg-install.sh usage (interactive prompts, WGI_ env vars), wg-client-manager commands (add, remove, list, show, qr), environment variable reference, security hardening features, backup/restore procedures. Update examples with new patterns.","status":"closed","priority":2,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T16:33:43.749727154+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T17:13:12.828613341+01:00","closed_at":"2026-01-12T17:13:12.828613341+01:00","close_reason":"Documentation updated on main branch. README.md reflects new wg-install.sh and wg-client-manager scripts, WGI_ environment variables, and all usage patterns.","dependencies":[{"issue_id":"wg-admin-1b9","depends_on_id":"wg-admin-slj","type":"blocks","created_at":"2026-01-12T16:33:56.00899014+01:00","created_by":"Calmcacil"}]} {"id":"wg-admin-2oj","title":"Add screen transition animations for more polished UX","description":"Add brief fade or slide animations when switching screens for more polished feel. Current screen transitions are instant without feedback. Consider lipgloss positioning and tick-based transitions.","status":"open","priority":3,"issue_type":"feature","owner":"Calmcacil@Raion","created_at":"2026-01-12T21:40:48.821352971+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T21:40:48.821352971+01:00"} {"id":"wg-admin-2pl","title":"Improve nftables firewall configuration","description":"Enhance firewall rules based on best practices: add TCP MSS clamping for MTU issues, add connection tracking bypass (notrack) for WireGuard traffic, implement proper rate limiting, ensure ICMPv6 neighbor discovery is allowed, validate rules before applying with nft check.","status":"closed","priority":2,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T16:27:53.15783619+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T16:37:11.050440729+01:00","closed_at":"2026-01-12T16:37:11.050440729+01:00","close_reason":"Improved nftables firewall configuration with TCP MSS clamping (1360), connection tracking bypass (notrack) for WireGuard UDP traffic, rate limiting for SSH (3/min) and WireGuard (10/s), ensured ICMPv6 neighbor discovery (including nd-router-* messages), and added nft check validation before applying rules."} +{"id":"wg-admin-33z","title":"Standardize TUI formatting and styling across all windows and popups","description":"# Problem\n\nThe TUI application lacks consistent formatting and styling across all screens and components. Each screen defines its own styles independently, leading to:\n\n1. **Inconsistent color usage** - Different screens use different color codes for similar elements\n2. **Duplicated style definitions** - Common styles (titles, help text, borders) are redefined in each file\n3. **Inconsistent modal dimensions** - Hardcoded widths/heights vary across screens\n4. **Visual inconsistency** - Users experience different UI patterns across screens\n5. **Maintenance difficulty** - Changes require updating multiple files\n\n# Current Issues Found\n\n## Color Inconsistencies\n- **Titles**: Mostly Color(62), but variations exist\n- **Help text**: Color(241), Color(243), Color(63) used interchangeably\n- **Status colors**: Different approaches to styling connected/disconnected states\n- **Success messages**: Color(46) used, but not universally\n- **Error messages**: Color(196) used, but styling varies\n\n## Duplicated Style Definitions\nEach screen file has its own style variables:\n- detailTitleStyle, listTitleStyle, addTitleStyle, restoreTitleStyle, etc.\n- Similar styles for help, error, success messages\n- Border styles repeated across multiple files\n\n## Modal Inconsistencies\n- Confirm modal uses 80x24 dimensions\n- Delete confirm modal uses 80x24 dimensions\n- No centralized modal size configuration\n- Different border styles (RoundedBorder vs NormalBorder)\n\n## Table Styles\n- Table styles defined in both list.go and restore.go\n- Same header/selected styles duplicated\n- Should use shared table styling function\n\n# Proposed Solution\n\n## 1. Create Centralized Styles Package\n\nCreate internal/tui/styles/styles.go with:\n\n### Color Palette (Single Source of Truth)\nPrimary colors:\n- ColorPrimary (62) - Titles, primary actions\n- ColorSecondary (241) - Secondary text, labels\n- ColorAccent (57) - Selected items, highlights\n- ColorSuccess (46) - Success messages\n- ColorError (196) - Error messages\n- ColorWarning (226) - Warning messages\n- ColorInfo (243) - Help text, info\n- ColorKey (63) - Keyboard shortcuts\n- ColorBorder (240) - Borders\n- ColorBackground (235) - Modal backgrounds\n\n### Common Style Functions\nCreate reusable style functions:\n- Title() - For all screen titles\n- Help() - For help text and instructions\n- Error() - For error messages\n- Success() - For success messages\n- Warning() - For warning messages\n- Key() - For keyboard shortcuts\n- Modal(width, height) - Consistent modal styling\n- TableHeader() - Table headers\n- TableSelected() - Selected table rows\n- BorderRounded() - Rounded borders\n- BorderNormal() - Normal borders\n\n### Modal Dimensions\nDefine constants:\n- ModalWidth = 80\n- ModalHeight = 24\n\n## 2. Refactor Each Screen to Use Centralized Styles\n\n### Files to Update\n- internal/tui/screens/list.go - Use shared table styles\n- internal/tui/screens/detail.go - Replace local styles with package imports\n- internal/tui/screens/add.go - Use shared title/help styles\n- internal/tui/screens/help.go - Use shared key/label styles\n- internal/tui/screens/qr.go - Use shared title/help/error styles\n- internal/tui/screens/restore.go - Use shared table/modal styles\n\n### Components to Update\n- internal/tui/components/confirm.go - Use shared modal styles\n- internal/tui/components/delete-confirm.go - Use shared modal styles\n- internal/tui/components/search.go - Use shared color palette\n\n## 3. Implementation Approach\n\n### Phase 1: Create Styles Package\n- Create internal/tui/styles/ directory\n- Implement styles.go with color palette and common functions\n- Add table.go for table-specific styles\n\n### Phase 2: Update Components First\n- Update confirm.go and delete-confirm.go to use shared modal styles\n- Update search.go to use shared color palette\n- Test components work correctly\n\n### Phase 3: Update Screens\n- Refactor each screen to import and use styles package\n- Replace local style variables with calls to styles package\n- Test each screen maintains functionality\n\n### Phase 4: Verification\n- Test navigation across all screens\n- Verify consistent visual appearance\n- Check modal sizes and positioning\n- Verify color usage is consistent\n\n# Success Criteria\n\n1. All screens use consistent color palette from single source\n2. No duplicated style definitions across files\n3. Modals have consistent dimensions and styling\n4. Table styling is shared across list and restore screens\n5. Help text, errors, success messages use consistent styles\n6. All keyboard shortcuts use same styling\n7. Code is more maintainable (style changes in one place)\n\n# Files to Create\n- internal/tui/styles/styles.go\n- internal/tui/styles/table.go (optional, could be in styles.go)\n\n# Files to Modify\n- internal/tui/screens/list.go\n- internal/tui/screens/detail.go\n- internal/tui/screens/add.go\n- internal/tui/screens/help.go\n- internal/tui/screens/qr.go\n- internal/tui/screens/restore.go\n- internal/tui/components/confirm.go\n- internal/tui/components/delete-confirm.go\n- internal/tui/components/search.go\n\n# Dependencies\nNone - this is a refactoring task that improves code organization without adding new functionality.","status":"open","priority":1,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T22:19:57.96243735+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T22:21:53.810905089+01:00"} {"id":"wg-admin-37o","title":"Add security hardening","description":"Implement: client name sanitization with regex, pre-shared key (PSK) support option, proper temporary key cleanup with trap handlers, atomic config file operations (write to temp then mv), chmod 0600 for all key files, verify no hardcoded secrets in generated files.","status":"closed","priority":2,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T16:27:53.148392501+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T16:44:11.582485544+01:00","closed_at":"2026-01-12T16:44:11.582485544+01:00","close_reason":"Implemented all security hardening features: client name sanitization with regex (validate_client_name function), pre-shared key (PSK) support with --psk option, proper temporary key cleanup with trap handlers (cleanup_handler), atomic config file operations (mktemp + mv), chmod 0600 for all key files, and verified no hardcoded secrets (keys generated dynamically or read from files)"} {"id":"wg-admin-3d4","title":"Implement configuration loading system","description":"Implement configuration system to load /etc/wg-admin/config.conf using native Go or Viper library. Support environment variable overrides. Validate required config (SERVER_DOMAIN, WG_PORT, VPN_IPV4_RANGE, VPN_IPV6_RANGE, DNS_SERVERS). Provide clear error messages for missing config.","status":"closed","priority":2,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T17:02:57.198865993+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T17:21:51.863786437+01:00","closed_at":"2026-01-12T17:21:51.863786437+01:00","close_reason":"Configuration system implemented in internal/config/config.go. Loads from /etc/wg-admin/config.conf, supports environment variable overrides with WGI_ prefix, validates required fields (SERVER_DOMAIN, WG_PORT, CIDR formats). Provides helper methods for network extraction.","dependencies":[{"issue_id":"wg-admin-3d4","depends_on_id":"wg-admin-4ji","type":"blocks","created_at":"2026-01-12T17:04:44.279588181+01:00","created_by":"Calmcacil"}]} {"id":"wg-admin-4fb","title":"Set up basic TUI skeleton with Bubble Tea","description":"Create main TUI application entry point implementing Bubble Tea's Model-Update-View pattern. Set up root check and logging. Create empty screen types (list, add, detail, qr, help). Implement basic keyboard navigation (q=quit). Add status bar with version and help shortcut (?).","status":"closed","priority":2,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T17:02:57.195332445+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T17:29:25.376578103+01:00","closed_at":"2026-01-12T17:29:25.376578103+01:00","close_reason":"TUI skeleton implemented with Model-Update-View pattern. Main entry point in cmd/wg-tui/main.go with root check, configuration loading integration, basic keyboard navigation (q quit), status bar with version and help. Creates clean separation between TUI model (internal/tui) and main program. Successfully builds.","dependencies":[{"issue_id":"wg-admin-4fb","depends_on_id":"wg-admin-4ji","type":"blocks","created_at":"2026-01-12T17:04:26.666043249+01:00","created_by":"Calmcacil"},{"issue_id":"wg-admin-4fb","depends_on_id":"wg-admin-3d4","type":"blocks","created_at":"2026-01-12T17:04:26.672887205+01:00","created_by":"Calmcacil"}]} @@ -40,6 +41,7 @@ {"id":"wg-admin-p6q","title":"Test theme switching functionality","description":"Test theme switching by setting THEME environment variable to different values (dracula, everforest, default, dark, light) and verify that the TUI renders with correct colors.","status":"closed","priority":1,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T19:07:40.33610722+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T19:10:22.227831273+01:00","closed_at":"2026-01-12T19:10:22.227831273+01:00","close_reason":"Documentation updated with theme options, build successful, theme switching logic verified"} {"id":"wg-admin-q0x","title":"Research and document Dracula/Everforest color schemes","description":"Research and document the official color specifications for Dracula and Everforest color schemes. Extract hex values for primary, success, warning, error, and muted colors to use in the TUI theme implementation.","status":"closed","priority":2,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T19:07:40.305301935+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T19:08:01.204915996+01:00","closed_at":"2026-01-12T19:08:01.204915996+01:00","close_reason":"Research completed - documented Dracula and Everforest color specifications with hex values for implementation"} {"id":"wg-admin-qpy","title":"Refactor installation into wg-install.sh","description":"Extract install logic from wireguard.sh into dedicated wg-install.sh script. Handle: dependency checks, package installation, firewall setup (nftables), server key generation, interface initialization, systemd service setup. Use interactive 'read' prompts for settings with 'WGI_' prefixed environment variable overrides.","status":"closed","priority":2,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T16:27:53.151817177+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T16:50:42.168393277+01:00","closed_at":"2026-01-12T16:50:42.168393277+01:00","close_reason":"Created wg-install.sh script with complete installation logic extracted from wireguard.sh. Script includes dependency checks, package installation, nftables firewall setup, server key generation, interface initialization, and systemd service setup. Uses interactive prompts with WGI_ prefixed environment variable overrides. All validation and error handling maintained with atomic operations and proper cleanup. Test suite (test-wg-install.sh) created with 35 tests all passing.","dependencies":[{"issue_id":"wg-admin-qpy","depends_on_id":"wg-admin-37o","type":"blocks","created_at":"2026-01-12T16:28:20.30398105+01:00","created_by":"Calmcacil"},{"issue_id":"wg-admin-qpy","depends_on_id":"wg-admin-wsk","type":"blocks","created_at":"2026-01-12T16:28:20.305872992+01:00","created_by":"Calmcacil"},{"issue_id":"wg-admin-qpy","depends_on_id":"wg-admin-0wc","type":"blocks","created_at":"2026-01-12T16:28:27.88358441+01:00","created_by":"Calmcacil"},{"issue_id":"wg-admin-qpy","depends_on_id":"wg-admin-cwb","type":"blocks","created_at":"2026-01-12T16:28:27.890595849+01:00","created_by":"Calmcacil"},{"issue_id":"wg-admin-qpy","depends_on_id":"wg-admin-2pl","type":"blocks","created_at":"2026-01-12T16:28:27.948214112+01:00","created_by":"Calmcacil"}]} +{"id":"wg-admin-rfo","title":"CRITICAL: Cannot exit client details screen - navigation broken","description":"# CRITICAL BUG - Cannot exit screens\n\n## Problem\n\nUser is unable to navigate back from certain screens in the TUI. When opening client details or restore screens, pressing the back key ('b' or 'esc') does nothing, forcing the user to kill the process from a separate terminal.\n\n## Root Cause\n\nThe screen Update() method returns `(Screen, tea.Cmd)` tuple. When a screen wants to signal \"go back to previous screen\", it should return `(nil, tea.Cmd)` as the first value.\n\nHowever, both `detail.go` and `restore.go` were incorrectly returning `(s, nil)` instead of `(nil, nil)` when the back key was pressed. This caused the main model's navigation logic to never detect the screen change:\n\n```go\n// In main.go line 107:\nif newScreen == nil { // This check fails because newScreen == s, not nil\n // Go back to previous screen\n m.currentScreen = m.previousScreen\n m.previousScreen = nil\n}\n```\n\n## Issues Found\n\n### detail.go (line 116)\n**Before:**\n```go\ncase \"b\", \"esc\":\n // Return to list screen - signal parent to switch screens\n return s, nil // BUG: Returns current screen, not nil\n```\n\n**After:**\n```go\ncase \"b\", \"esc\":\n // Return to list screen - signal parent to switch screens\n return nil, nil // FIXED: Returns nil to signal screen change\n```\n\n### restore.go (line 101)\n**Before:**\n```go\ncase \"q\", \"esc\":\n // Return to list screen - signal parent to switch screens\n return s, nil // BUG: Returns current screen, not nil\n```\n\n**After:**\n```go\ncase \"q\", \"esc\":\n // Return to list screen - signal parent to switch screens\n return nil, nil // FIXED: Returns nil to signal screen change\n```\n\n## Impact\n\n- Users get trapped in detail and restore screens\n- Cannot return to main list screen\n- Must kill process and restart application\n- Critical usability issue\n\n## Fix Applied\n\nChanged both occurrences from `return s, nil` to `return nil, nil` to properly signal the main model to switch back to the previous screen.\n\n## Verification\n\n- Build successful with no errors\n- Navigation logic now correctly detects nil return value\n- Screen switching works as expected\n\n## Files Modified\n\n- `internal/tui/screens/detail.go` - Line 116\n- `internal/tui/screens/restore.go` - Line 101","status":"closed","priority":0,"issue_type":"bug","owner":"Calmcacil@Raion","created_at":"2026-01-12T22:22:51.43957087+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T22:27:03.641284763+01:00","closed_at":"2026-01-12T22:27:03.641284763+01:00","close_reason":"Fixed navigation bug by changing 'return s, nil' to 'return nil, nil' in detail.go and restore.go. Both screens now correctly signal main model to switch back to previous screen when back key is pressed."} {"id":"wg-admin-slj","title":"Refactor WireGuard scripts into modular architecture","description":"Refactor monolithic wireguard.sh into two separate scripts: wg-install.sh for initial setup, wg-client-manager for client operations. Use interactive 'read' prompts with 'WGI_' prefixed environment variable overrides. Add validation functions, security hardening, and remove all hardcoded sensitive information from repository.","status":"closed","priority":2,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T16:27:18.232667092+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T17:11:02.639140093+01:00","closed_at":"2026-01-12T17:11:02.639140093+01:00","close_reason":"Refactoring complete: wg-install.sh (921 lines) and wg-client-manager (545 lines) scripts have been created and are functional. wireguard.sh retained for backwards compatibility.","dependencies":[{"issue_id":"wg-admin-slj","depends_on_id":"wg-admin-abw","type":"blocks","created_at":"2026-01-12T16:28:21.930404739+01:00","created_by":"Calmcacil"},{"issue_id":"wg-admin-slj","depends_on_id":"wg-admin-qpy","type":"blocks","created_at":"2026-01-12T16:28:21.936380993+01:00","created_by":"Calmcacil"},{"issue_id":"wg-admin-slj","depends_on_id":"wg-admin-0wc","type":"blocks","created_at":"2026-01-12T16:28:21.983754904+01:00","created_by":"Calmcacil"}]} {"id":"wg-admin-ti0","title":"Investigate q key behavior and parseHandshake bug","description":"Investigate q key behavior causing app to not return properly from client details. parseHandshake fix has been committed separately.","status":"open","priority":1,"issue_type":"bug","owner":"Calmcacil@Raion","created_at":"2026-01-12T19:26:35.435240285+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T19:27:21.412723919+01:00"} {"id":"wg-admin-tv6","title":"Add client delete functionality","description":"Implement delete client workflow with confirmation modal. Remove client config from server, delete client files, auto-backup before deletion, and reload WireGuard configuration.","status":"closed","priority":2,"issue_type":"task","owner":"Calmcacil@Raion","created_at":"2026-01-12T17:03:30.281557572+01:00","created_by":"Calmcacil","updated_at":"2026-01-12T17:48:31.867154638+01:00","closed_at":"2026-01-12T17:48:31.867154638+01:00","close_reason":"Delete with confirmation modal implemented. Auto-backup before deletion, removes client configs and peer from WireGuard. Integrated with detail screen.","dependencies":[{"issue_id":"wg-admin-tv6","depends_on_id":"wg-admin-dd2","type":"blocks","created_at":"2026-01-12T17:04:36.207822184+01:00","created_by":"Calmcacil"}]} diff --git a/internal/tui/screens/detail.go b/internal/tui/screens/detail.go index 2f1b778..11a8261 100644 --- a/internal/tui/screens/detail.go +++ b/internal/tui/screens/detail.go @@ -113,7 +113,7 @@ func (s *DetailScreen) Update(msg tea.Msg) (Screen, tea.Cmd) { switch msg.String() { case "b", "esc": // Return to list screen - signal parent to switch screens - return s, nil + return nil, nil case "d": // Show delete confirmation s.confirmModal = components.NewDeleteConfirm( diff --git a/internal/tui/screens/restore.go b/internal/tui/screens/restore.go index 26daff9..1cdce32 100644 --- a/internal/tui/screens/restore.go +++ b/internal/tui/screens/restore.go @@ -98,7 +98,7 @@ func (s *RestoreScreen) Update(msg tea.Msg) (Screen, tea.Cmd) { switch msg.String() { case "q", "esc": // Return to list screen - signal parent to switch screens - return s, nil + return nil, nil case "enter": // Show confirmation for selected backup if len(s.table.Rows()) > 0 {