From dcd6669bb39ba15c7bad8981d7564770fdb23092 Mon Sep 17 00:00:00 2001 From: joshuamontgomery <24376525+jp2195@users.noreply.github.com> Date: Sat, 24 Jan 2026 19:22:08 -0500 Subject: [PATCH 1/6] refactor(tui): remove SSH and troubleshooting functionality Remove SSH connection and troubleshooting runbook features from the TUI. When connecting via Panorama, API calls route through Panorama to managed firewalls, but SSH requires direct connections to each firewall's management IP. This architectural mismatch makes the current SSH implementation unreliable in Panorama environments. Changes: - Remove ViewTroubleshoot and related handlers - Remove SSH connection functions (connectSSH, updateTroubleshootSSH) - Remove troubleshoot command from navigation and command palette - Keep internal/ssh and internal/troubleshoot packages for future use --- .github/workflows/codeql.yml | 40 ++++++++++++++ cmd/pyre/main.go | 2 +- internal/api/client.go | 9 ++-- internal/auth/auth.go | 88 +++++++++++++++++++++++++----- internal/auth/keygen.go | 16 +++--- internal/auth/session_test.go | 23 ++++---- internal/config/config.go | 23 +++++++- internal/ssh/client.go | 89 ++++++++++++++++++++++++++++++- internal/testutil/mock_server.go | 28 +++++++--- internal/tui/app.go | 55 +++---------------- internal/tui/commands.go | 70 ++++-------------------- internal/tui/handlers.go | 33 ++---------- internal/tui/messages.go | 24 +-------- internal/tui/navigation.go | 9 ---- internal/tui/render.go | 2 - internal/tui/views/login.go | 13 +++++ internal/tui/views/navbar.go | 3 +- internal/tui/views/navbar_test.go | 4 +- 18 files changed, 311 insertions(+), 220 deletions(-) create mode 100644 .github/workflows/codeql.yml diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000..61d182e --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,40 @@ +name: CodeQL + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: '0 10 * * 1' # Weekly on Mondays at 10 AM UTC + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + security-events: write + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + timeout-minutes: 15 + + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Initialize CodeQL + uses: github/codeql-action/init@4f3212b61783c3c68e8309a0f18a699764811cda # v3.28.1 + with: + languages: go + queries: security-extended + + - name: Autobuild + uses: github/codeql-action/autobuild@4f3212b61783c3c68e8309a0f18a699764811cda # v3.28.1 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@4f3212b61783c3c68e8309a0f18a699764811cda # v3.28.1 + with: + category: "/language:go" diff --git a/cmd/pyre/main.go b/cmd/pyre/main.go index 76adad1..f9e4ef8 100644 --- a/cmd/pyre/main.go +++ b/cmd/pyre/main.go @@ -22,7 +22,7 @@ func main() { var ( host = flag.String("host", "", "Firewall hostname or IP address") apiKey = flag.String("api-key", "", "API key for authentication") - insecure = flag.Bool("insecure", true, "Skip TLS certificate verification") + insecure = flag.Bool("insecure", false, "Skip TLS certificate verification (for self-signed certs)") configPath = flag.String("config", "", "Path to config file (default: ~/.pyre.yaml)") showHelp = flag.Bool("help", false, "Show help message") showVer = flag.Bool("version", false, "Show version") diff --git a/internal/api/client.go b/internal/api/client.go index 0db6dbf..5453c8a 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -44,9 +44,6 @@ func NewClient(host, apiKey string, opts ...ClientOption) *Client { apiKey: apiKey, httpClient: &http.Client{ Timeout: 30 * time.Second, - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - }, }, } @@ -104,8 +101,6 @@ func (c *Client) GetTarget() string { } func (c *Client) request(ctx context.Context, params url.Values) (*XMLResponse, error) { - params.Set("key", c.apiKey) - // Inject target parameter for Panorama routing if c.targetSerial != "" { params.Set("target", c.targetSerial) @@ -117,6 +112,10 @@ func (c *Client) request(ctx context.Context, params url.Values) (*XMLResponse, return nil, fmt.Errorf("creating request: %w", err) } + // Use X-PAN-KEY header instead of query parameter (PAN-OS 8.0+) + // This prevents API key from appearing in server/proxy logs + req.Header.Set("X-PAN-KEY", c.apiKey) + resp, err := c.httpClient.Do(req) if err != nil { return nil, fmt.Errorf("executing request: %w", err) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 8f84085..90016eb 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -2,7 +2,10 @@ package auth import ( "context" + "fmt" + "net" "os" + "regexp" "strings" "sync" @@ -12,6 +15,9 @@ import ( "github.com/jp2195/pyre/internal/ssh" ) +// serialPattern validates Palo Alto device serial numbers (alphanumeric, typically 12-15 chars) +var serialPattern = regexp.MustCompile(`^[A-Za-z0-9]{8,20}$`) + type Session struct { mu sync.RWMutex ActiveFirewall string @@ -28,9 +34,8 @@ type Connection struct { Connected bool SSHEnabled bool - // SSH credentials from login (reused for SSH connection) + // SSH username from login (password must come from env var for security) SSHUsername string - SSHPassword string // Panorama fields IsPanorama bool @@ -66,10 +71,13 @@ func (s *Session) SetActiveFirewall(name string) bool { } func (s *Session) AddConnection(name string, fwConfig *config.FirewallConfig, apiKey string) *Connection { - return s.AddConnectionWithSSH(name, fwConfig, apiKey, "", "") + return s.AddConnectionWithSSH(name, fwConfig, apiKey, "", nil) } -func (s *Session) AddConnectionWithSSH(name string, fwConfig *config.FirewallConfig, apiKey, sshUsername, sshPassword string) *Connection { +// AddConnectionWithSSH creates a new connection with SSH username and optional pre-established SSH client. +// If sshClient is provided, it will be used directly. Otherwise, SSH can be established later +// using credentials from environment variables. +func (s *Session) AddConnectionWithSSH(name string, fwConfig *config.FirewallConfig, apiKey, sshUsername string, sshClient *ssh.Client) *Connection { s.mu.Lock() defer s.mu.Unlock() @@ -81,7 +89,8 @@ func (s *Session) AddConnectionWithSSH(name string, fwConfig *config.FirewallCon Client: client, Connected: true, SSHUsername: sshUsername, - SSHPassword: sshPassword, + SSHClient: sshClient, + SSHEnabled: sshClient != nil, } s.Connections[name] = conn @@ -133,14 +142,19 @@ type Credentials struct { func ResolveCredentials(cfg *config.Config, flags config.CLIFlags) *Credentials { creds := &Credentials{} + // CLI flags take highest priority if flags.Host != "" { creds.Host = flags.Host - creds.Insecure = flags.Insecure } if flags.APIKey != "" { creds.APIKey = flags.APIKey } + // If --insecure flag is explicitly true, use it + if flags.Insecure { + creds.Insecure = true + } + // Environment variables (if not set by flags) if envHost := os.Getenv("PYRE_HOST"); envHost != "" && creds.Host == "" { creds.Host = envHost } @@ -151,10 +165,14 @@ func ResolveCredentials(cfg *config.Config, flags config.CLIFlags) *Credentials creds.Insecure = true } + // Config file defaults (if not set by flags or env) if creds.Host == "" { if name, fw, ok := cfg.GetDefaultFirewall(); ok { creds.Host = fw.Host - creds.Insecure = fw.Insecure + // Use config insecure if not already set by flags or env + if !creds.Insecure && fw.Insecure { + creds.Insecure = true + } envKey := os.Getenv("PYRE_" + name + "_API_KEY") if envKey != "" && creds.APIKey == "" { @@ -178,18 +196,53 @@ func (c *Credentials) NeedsInteractiveAuth() bool { return c.Host == "" || c.APIKey == "" } +// validateSerial checks if the serial number has a valid format. +func validateSerial(serial string) error { + if serial == "" { + return nil + } + if !serialPattern.MatchString(serial) { + return fmt.Errorf("invalid serial number format: %s", serial) + } + return nil +} + +// validateIP checks if the IP address is valid. +func validateIP(ip string) error { + if ip == "" { + return nil + } + if net.ParseIP(ip) == nil { + return fmt.Errorf("invalid IP address: %s", ip) + } + return nil +} + // SetTarget sets the current target device for Panorama. // Pass nil to target Panorama itself. -func (c *Connection) SetTarget(device *models.ManagedDevice) { +// Returns an error if the device serial or IP is invalid. +func (c *Connection) SetTarget(device *models.ManagedDevice) error { if device == nil { c.TargetSerial = "" c.TargetIP = "" c.Client.ClearTarget() - } else { - c.TargetSerial = device.Serial - c.TargetIP = device.IPAddress - c.Client.SetTarget(device.Serial) + return nil } + + // Validate serial number format + if err := validateSerial(device.Serial); err != nil { + return err + } + + // Validate IP address format + if err := validateIP(device.IPAddress); err != nil { + return err + } + + c.TargetSerial = device.Serial + c.TargetIP = device.IPAddress + c.Client.SetTarget(device.Serial) + return nil } // GetTargetDevice returns the currently targeted managed device, or nil if targeting Panorama. @@ -247,6 +300,10 @@ func (c *Connection) ConnectSSH(ctx context.Context) error { // For Panorama with a target device, connect to the target's IP host := c.Config.Host if c.IsPanorama && c.TargetIP != "" { + // Validate target IP before using it + if err := validateIP(c.TargetIP); err != nil { + return fmt.Errorf("invalid target IP for SSH: %w", err) + } host = c.TargetIP } @@ -282,6 +339,7 @@ func (c *Connection) HasSSH() bool { } // getSSHConfig returns the SSH configuration, combining config file, env vars, and login credentials. +// Note: SSH passwords must come from environment variables (PYRE_SSH_PASSWORD) for security. func (c *Connection) getSSHConfig() config.SSHConfig { var sshCfg config.SSHConfig if c.Config != nil { @@ -291,10 +349,9 @@ func (c *Connection) getSSHConfig() config.SSHConfig { // Apply environment variable overrides sshCfg = resolveSSHCredentials(c.Name, sshCfg) - // Use login credentials if no username configured yet + // Use login username if no username configured yet if sshCfg.Username == "" && c.SSHUsername != "" { sshCfg.Username = c.SSHUsername - sshCfg.Password = c.SSHPassword } return sshCfg @@ -312,6 +369,9 @@ func resolveSSHCredentials(fwName string, cfg config.SSHConfig) config.SSHConfig if envKey := os.Getenv("PYRE_SSH_KEY_PATH"); envKey != "" && cfg.PrivateKeyPath == "" { cfg.PrivateKeyPath = envKey } + if os.Getenv("PYRE_SSH_INSECURE") == "true" { + cfg.Insecure = true + } // Per-firewall SSH password: PYRE__SSH_PASSWORD envName := strings.ToUpper(strings.ReplaceAll(fwName, "-", "_")) diff --git a/internal/auth/keygen.go b/internal/auth/keygen.go index 353036f..821bfc0 100644 --- a/internal/auth/keygen.go +++ b/internal/auth/keygen.go @@ -37,16 +37,18 @@ func GenerateAPIKey(ctx context.Context, host, username, password string, insecu }, } - params := url.Values{} - params.Set("type", "keygen") - params.Set("user", username) - params.Set("password", password) - - reqURL := fmt.Sprintf("https://%s/api/?%s", host, params.Encode()) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) + // Use POST with form body to keep credentials out of URLs/logs + reqURL := fmt.Sprintf("https://%s/api/", host) + formData := url.Values{} + formData.Set("type", "keygen") + formData.Set("user", username) + formData.Set("password", password) + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, strings.NewReader(formData.Encode())) if err != nil { return nil, fmt.Errorf("creating keygen request: %w", err) } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") resp, err := client.Do(req) if err != nil { diff --git a/internal/auth/session_test.go b/internal/auth/session_test.go index 94373df..8ca1174 100644 --- a/internal/auth/session_test.go +++ b/internal/auth/session_test.go @@ -64,7 +64,9 @@ func TestSession_AddConnectionWithSSH(t *testing.T) { Insecure: true, } - conn := session.AddConnectionWithSSH("test-fw", fwConfig, "test-api-key", "admin", "password123") + // Note: SSH client is passed directly (established during login) + // Password is no longer stored - SSH must be established while credentials are in memory + conn := session.AddConnectionWithSSH("test-fw", fwConfig, "test-api-key", "admin", nil) if conn == nil { t.Fatal("expected non-nil connection") @@ -72,9 +74,6 @@ func TestSession_AddConnectionWithSSH(t *testing.T) { if conn.SSHUsername != "admin" { t.Errorf("expected SSH username 'admin', got %q", conn.SSHUsername) } - if conn.SSHPassword != "password123" { - t.Errorf("expected SSH password 'password123', got %q", conn.SSHPassword) - } } func TestSession_GetActiveConnection(t *testing.T) { @@ -453,7 +452,6 @@ func TestConnection_getSSHConfig(t *testing.T) { }, }, SSHUsername: "login-admin", - SSHPassword: "login-pass", } cfg := conn.getSSHConfig() @@ -472,6 +470,10 @@ func TestConnection_getSSHConfig(t *testing.T) { } func TestConnection_getSSHConfig_LoginFallback(t *testing.T) { + // Set SSH password env var for this test + os.Setenv("PYRE_SSH_PASSWORD", "env-pass") + defer os.Unsetenv("PYRE_SSH_PASSWORD") + conn := &Connection{ Name: "test-fw", Config: &config.FirewallConfig{ @@ -481,17 +483,17 @@ func TestConnection_getSSHConfig_LoginFallback(t *testing.T) { }, }, SSHUsername: "login-admin", - SSHPassword: "login-pass", } cfg := conn.getSSHConfig() - // Should fall back to login credentials + // Should fall back to login username if cfg.Username != "login-admin" { t.Errorf("expected Username 'login-admin', got %q", cfg.Username) } - if cfg.Password != "login-pass" { - t.Errorf("expected Password 'login-pass', got %q", cfg.Password) + // Password should come from env var + if cfg.Password != "env-pass" { + t.Errorf("expected Password 'env-pass', got %q", cfg.Password) } } @@ -500,12 +502,11 @@ func TestConnection_getSSHConfig_NilConfig(t *testing.T) { Name: "test-fw", Config: nil, SSHUsername: "login-admin", - SSHPassword: "login-pass", } cfg := conn.getSSHConfig() - // Should use login credentials + // Should use login username if cfg.Username != "login-admin" { t.Errorf("expected Username 'login-admin', got %q", cfg.Username) } diff --git a/internal/config/config.go b/internal/config/config.go index df1796f..d275637 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "os" "path/filepath" "time" @@ -12,14 +13,17 @@ type Config struct { DefaultFirewall string `yaml:"default_firewall"` Firewalls map[string]FirewallConfig `yaml:"firewalls"` Settings Settings `yaml:"settings"` + Warnings []string `yaml:"-"` // Security warnings from config validation } type SSHConfig struct { - Port int `yaml:"port"` // Default: 22 + Port int `yaml:"port"` // Default: 22 Username string `yaml:"username"` - Password string `yaml:"password"` // Or use key + Password string `yaml:"password,omitempty"` // Deprecated: use env vars instead PrivateKeyPath string `yaml:"private_key_path"` Timeout int `yaml:"timeout"` // Seconds, default: 30 + KnownHostsPath string `yaml:"known_hosts_path"` // Default: ~/.ssh/known_hosts + Insecure bool `yaml:"insecure"` // Skip host key verification (not recommended) } type FirewallConfig struct { @@ -127,5 +131,20 @@ func LoadWithFlags(flags CLIFlags) (*Config, error) { } cfg.ApplyFlags(flags) + cfg.validateSecuritySettings() return cfg, nil } + +// validateSecuritySettings checks for deprecated or insecure configuration settings +// and adds warnings to the config. +func (c *Config) validateSecuritySettings() { + for name, fw := range c.Firewalls { + // Warn about SSH password in config file (deprecated) + if fw.SSH.Password != "" { + c.Warnings = append(c.Warnings, fmt.Sprintf( + "SECURITY WARNING: Firewall %q has SSH password in config file. "+ + "Use PYRE_SSH_PASSWORD or PYRE_%s_SSH_PASSWORD environment variable instead.", + name, name)) + } + } +} diff --git a/internal/ssh/client.go b/internal/ssh/client.go index c48d653..41d4418 100644 --- a/internal/ssh/client.go +++ b/internal/ssh/client.go @@ -3,13 +3,16 @@ package ssh import ( "bytes" "context" + "errors" "fmt" "net" "os" + "path/filepath" "strings" "time" "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/knownhosts" "github.com/jp2195/pyre/internal/config" ) @@ -86,10 +89,15 @@ func NewClient(host string, cfg config.SSHConfig) (*Client, error) { return nil, fmt.Errorf("no authentication method provided") } + hostKeyCallback, err := getHostKeyCallback(cfg) + if err != nil { + return nil, fmt.Errorf("setting up host key verification: %w", err) + } + sshConfig := &ssh.ClientConfig{ User: cfg.Username, Auth: authMethods, - HostKeyCallback: ssh.InsecureIgnoreHostKey(), + HostKeyCallback: hostKeyCallback, Timeout: timeout, } @@ -268,3 +276,82 @@ func expandPath(path string) string { } return path } + +// getHostKeyCallback returns the appropriate host key callback based on configuration. +// If cfg.Insecure is true, it returns an insecure callback that accepts any host key. +// Otherwise, it uses the known_hosts file for verification. +func getHostKeyCallback(cfg config.SSHConfig) (ssh.HostKeyCallback, error) { + if cfg.Insecure { + return ssh.InsecureIgnoreHostKey(), nil + } + + // Determine known_hosts path + knownHostsPath := cfg.KnownHostsPath + if knownHostsPath == "" { + home, err := os.UserHomeDir() + if err != nil { + return nil, fmt.Errorf("getting home directory: %w", err) + } + knownHostsPath = filepath.Join(home, ".ssh", "known_hosts") + } else { + knownHostsPath = expandPath(knownHostsPath) + } + + // Ensure .ssh directory exists + sshDir := filepath.Dir(knownHostsPath) + if err := os.MkdirAll(sshDir, 0700); err != nil { + return nil, fmt.Errorf("creating .ssh directory: %w", err) + } + + // Create known_hosts file if it doesn't exist + if _, err := os.Stat(knownHostsPath); os.IsNotExist(err) { + f, err := os.OpenFile(knownHostsPath, os.O_CREATE|os.O_WRONLY, 0600) + if err != nil { + return nil, fmt.Errorf("creating known_hosts file: %w", err) + } + f.Close() + } + + // Create host key callback from known_hosts + callback, err := knownhosts.New(knownHostsPath) + if err != nil { + return nil, fmt.Errorf("parsing known_hosts: %w", err) + } + + // Wrap the callback to provide a more helpful error message and optionally add new hosts + return func(hostname string, remote net.Addr, key ssh.PublicKey) error { + err := callback(hostname, remote, key) + if err != nil { + var keyErr *knownhosts.KeyError + if errors.As(err, &keyErr) { + if len(keyErr.Want) > 0 { + // Host key has changed - this could be a MITM attack + return fmt.Errorf("WARNING: host key for %s has changed! This could indicate a MITM attack. "+ + "If you trust this host, remove the old key from %s and try again", hostname, knownHostsPath) + } + // Host not in known_hosts - add it + if addErr := addHostKey(knownHostsPath, hostname, remote, key); addErr != nil { + return fmt.Errorf("host key verification failed and could not add to known_hosts: %w", addErr) + } + // Return nil to allow connection after adding the key + return nil + } + return err + } + return nil + }, nil +} + +// addHostKey appends a host key to the known_hosts file. +func addHostKey(knownHostsPath, hostname string, remote net.Addr, key ssh.PublicKey) error { + f, err := os.OpenFile(knownHostsPath, os.O_APPEND|os.O_WRONLY, 0600) + if err != nil { + return err + } + defer f.Close() + + // Format the known_hosts line + line := knownhosts.Line([]string{hostname}, key) + _, err = fmt.Fprintln(f, line) + return err +} diff --git a/internal/testutil/mock_server.go b/internal/testutil/mock_server.go index 2187c26..224e5ee 100644 --- a/internal/testutil/mock_server.go +++ b/internal/testutil/mock_server.go @@ -8,12 +8,12 @@ import ( ) type MockPANOS struct { - Server *httptest.Server - Hostname string - Model string - Serial string - Version string - IsPanorama bool + Server *httptest.Server + Hostname string + Model string + Serial string + Version string + IsPanorama bool } func NewMockPANOS() *MockPANOS { @@ -57,7 +57,16 @@ func (m *MockPANOS) Host() string { } func (m *MockPANOS) handleAPI(w http.ResponseWriter, r *http.Request) { + // Parse form for POST requests (keygen uses POST with form body) + if r.Method == http.MethodPost { + r.ParseForm() + } + + // Get type from query string or form apiType := r.URL.Query().Get("type") + if apiType == "" { + apiType = r.FormValue("type") + } cmd := r.URL.Query().Get("cmd") w.Header().Set("Content-Type", "application/xml") @@ -75,8 +84,15 @@ func (m *MockPANOS) handleAPI(w http.ResponseWriter, r *http.Request) { } func (m *MockPANOS) handleKeygen(w http.ResponseWriter, r *http.Request) { + // Get user/password from query string or form (POST uses form body) user := r.URL.Query().Get("user") + if user == "" { + user = r.FormValue("user") + } password := r.URL.Query().Get("password") + if password == "" { + password = r.FormValue("password") + } if user == "admin" && password == "admin" { w.Write([]byte(`LUFRPT1234567890abcdef==`)) diff --git a/internal/tui/app.go b/internal/tui/app.go index c35c990..ed52643 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -11,7 +11,6 @@ import ( "github.com/jp2195/pyre/internal/auth" "github.com/jp2195/pyre/internal/config" - "github.com/jp2195/pyre/internal/troubleshoot" "github.com/jp2195/pyre/internal/tui/views" ) @@ -24,7 +23,6 @@ const ( ViewNATPolicies ViewSessions ViewInterfaces - ViewTroubleshoot ViewLogs ViewPicker ViewDevicePicker @@ -61,16 +59,11 @@ type Model struct { natPolicies views.NATPoliciesModel sessions views.SessionsModel interfaces views.InterfacesModel - troubleshoot views.TroubleshootModel logs views.LogsModel picker views.PickerModel devicePicker views.DevicePickerModel commandPalette views.CommandPaletteModel previousView ViewState // Track previous view for Esc to return - - // Troubleshooting - tsRegistry *troubleshoot.Registry - tsEngine *troubleshoot.Engine } func NewModel(cfg *config.Config, creds *auth.Credentials) Model { @@ -93,7 +86,7 @@ func NewModel(cfg *config.Config, creds *auth.Credentials) Model { } if creds.HasAPIKey() && creds.HasHost() { - // Look up full firewall config by host to get SSH settings + // Look up full firewall config by host var fwConfig *config.FirewallConfig var connName string for name, fw := range cfg.Firewalls { @@ -126,17 +119,11 @@ func NewModel(cfg *config.Config, creds *auth.Credentials) Model { m.natPolicies = views.NewNATPoliciesModel() m.sessions = views.NewSessionsModel() m.interfaces = views.NewInterfacesModel() - m.troubleshoot = views.NewTroubleshootModel() m.logs = views.NewLogsModel() m.picker = views.NewPickerModel(session) m.devicePicker = views.NewDevicePickerModel() m.commandPalette = views.NewCommandPaletteModel() - // Initialize troubleshooting registry - m.tsRegistry = troubleshoot.NewRegistry() - m.tsRegistry.LoadEmbedded() - m.troubleshoot = m.troubleshoot.SetRunbooks(m.tsRegistry.List()) - return m } @@ -175,7 +162,6 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.natPolicies = m.natPolicies.SetSize(msg.Width, contentHeight) m.sessions = m.sessions.SetSize(msg.Width, contentHeight) m.interfaces = m.interfaces.SetSize(msg.Width, contentHeight) - m.troubleshoot = m.troubleshoot.SetSize(msg.Width, contentHeight) m.logs = m.logs.SetSize(msg.Width, contentHeight) m.picker = m.picker.SetSize(msg.Width, contentHeight) m.devicePicker = m.devicePicker.SetSize(msg.Width, contentHeight) @@ -296,7 +282,10 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case LoginSuccessMsg: m.loading = false - // Look up full firewall config by host to get SSH settings + // Clear password from login model immediately after success + m.login = m.login.ClearPassword() + + // Look up full firewall config by host var fwConfig *config.FirewallConfig var connName string loginHost := m.login.Host() @@ -311,12 +300,11 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if fwConfig == nil { fwConfig = &config.FirewallConfig{ Host: loginHost, - Insecure: true, + Insecure: m.login.Insecure(), } connName = msg.Name } - // Pass login credentials for SSH reuse - conn := m.session.AddConnectionWithSSH(connName, fwConfig, msg.APIKey, msg.Username, msg.Password) + conn := m.session.AddConnection(connName, fwConfig, msg.APIKey) m.currentView = ViewDashboard cmds = append(cmds, m.fetchCurrentDashboardData(), m.detectPanorama(conn)) @@ -368,13 +356,6 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case SessionsMsg: m.sessions = m.sessions.SetSessions(msg.Sessions, msg.Err) - case TroubleshootResultMsg: - m.loading = false - m.troubleshoot = m.troubleshoot.SetResult(msg.Result, msg.Err) - - case TroubleshootStepMsg: - m.troubleshoot = m.troubleshoot.UpdateStepProgress(msg.StepIndex, msg.Status, msg.Output) - case PanoramaDetectedMsg: conn := m.session.GetActiveConnection() if conn != nil { @@ -390,16 +371,6 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { conn.ManagedDevices = msg.Devices } - case SSHConnectedMsg: - m.troubleshoot = m.troubleshoot.SetSSHConnecting(false) - m.troubleshoot = m.troubleshoot.SetSSHAvailable(true) - m.troubleshoot = m.troubleshoot.SetSSHError(nil) - - case SSHErrorMsg: - m.troubleshoot = m.troubleshoot.SetSSHConnecting(false) - m.troubleshoot = m.troubleshoot.SetSSHAvailable(false) - m.troubleshoot = m.troubleshoot.SetSSHError(msg.Err) - case SystemLogsMsg: m.logs = m.logs.SetSystemLogs(msg.Logs, msg.Err) @@ -431,15 +402,6 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, m.fetchSessions() case ViewInterfaces: return m, m.fetchInterfaces() - case ViewTroubleshoot: - conn := m.session.GetActiveConnection() - sshConfigured := conn != nil && conn.HasSSH() - m.troubleshoot = m.troubleshoot.SetSSHConfigured(sshConfigured) - if sshConfigured && !conn.SSHEnabled { - m.troubleshoot = m.troubleshoot.SetSSHConnecting(true) - return m, m.connectSSH(conn) - } - return m, m.updateTroubleshootSSH() case ViewLogs: m.logs = m.logs.SetLoading(true) return m, m.fetchLogs() @@ -560,9 +522,6 @@ func (m Model) View() string { case ViewInterfaces: content = m.interfaces.View() - case ViewTroubleshoot: - content = m.troubleshoot.View() - case ViewLogs: content = m.logs.View() } diff --git a/internal/tui/commands.go b/internal/tui/commands.go index c38a536..78490a4 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -1,38 +1,34 @@ package tui import ( - "context" - "fmt" - "time" - tea "github.com/charmbracelet/bubbletea" "github.com/jp2195/pyre/internal/api" "github.com/jp2195/pyre/internal/auth" - "github.com/jp2195/pyre/internal/troubleshoot" "github.com/jp2195/pyre/internal/tui/views" ) func (m Model) doLogin() tea.Cmd { ctx := m.ctx + host := m.login.Host() + username := m.login.Username() + password := m.login.Password() + insecure := m.login.Insecure() + return func() tea.Msg { - result, err := auth.GenerateAPIKey( - ctx, - m.login.Host(), - m.login.Username(), - m.login.Password(), - true, - ) + result, err := auth.GenerateAPIKey(ctx, host, username, password, insecure) if err != nil { return LoginErrorMsg{Err: err} } if result.Error != nil { return LoginErrorMsg{Err: result.Error} } + + // Password is now out of scope and will be garbage collected return LoginSuccessMsg{ - Name: m.login.Host(), + Name: host, APIKey: result.APIKey, - Username: m.login.Username(), - Password: m.login.Password(), + Username: username, + Insecure: insecure, } } } @@ -405,47 +401,3 @@ func (m Model) refreshCurrentView() tea.Cmd { } return nil } - -func (m *Model) updateTroubleshootSSH() tea.Cmd { - conn := m.session.GetActiveConnection() - sshConfigured := conn != nil && conn.HasSSH() - hasSSH := conn != nil && conn.SSHEnabled && conn.SSHClient != nil - m.troubleshoot = m.troubleshoot.SetSSHConfigured(sshConfigured) - m.troubleshoot = m.troubleshoot.SetSSHAvailable(hasSSH) - return nil -} - -func (m Model) connectSSH(conn *auth.Connection) tea.Cmd { - ctx := m.ctx - return func() tea.Msg { - if !conn.HasSSH() { - return nil - } - // Use a timeout derived from the app context - ctx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - - if err := conn.ConnectSSH(ctx); err != nil { - return SSHErrorMsg{ConnectionName: conn.Name, Err: err} - } - return SSHConnectedMsg{ConnectionName: conn.Name} - } -} - -func (m Model) runTroubleshoot(runbook *troubleshoot.Runbook) tea.Cmd { - conn := m.session.GetActiveConnection() - if conn == nil { - return func() tea.Msg { - return TroubleshootResultMsg{Err: fmt.Errorf("no active connection")} - } - } - - // Create engine with current connections - engine := troubleshoot.NewEngine(conn.Client, conn.SSHClient, m.tsRegistry) - ctx := m.ctx - - return func() tea.Msg { - result, err := engine.RunRunbook(ctx, runbook) - return TroubleshootResultMsg{Result: result, Err: err} - } -} diff --git a/internal/tui/handlers.go b/internal/tui/handlers.go index de35a54..0ad0f08 100644 --- a/internal/tui/handlers.go +++ b/internal/tui/handlers.go @@ -71,7 +71,10 @@ func (m Model) handleDevicePickerKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { conn := m.session.GetActiveConnection() if conn != nil { device := m.devicePicker.SelectedDevice() - conn.SetTarget(device) + if err := conn.SetTarget(device); err != nil { + m.err = err + return m, nil + } m.currentView = ViewDashboard return m, m.fetchCurrentDashboardData() } @@ -182,13 +185,6 @@ func (m Model) buildCommandRegistry() []views.Command { }, // Tools - diagnostic and config - { - ID: "tools-troubleshoot", - Label: "Troubleshoot", - Description: "Diagnostic runbooks", - Category: "Tools", - Action: func() tea.Msg { return SwitchViewMsg{ViewTroubleshoot} }, - }, { ID: "tools-config", Label: "Config", @@ -263,27 +259,6 @@ func (m Model) handleViewKeys(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m.sessions, cmd = m.sessions.Update(msg) case ViewInterfaces: m.interfaces, cmd = m.interfaces.Update(msg) - case ViewTroubleshoot: - // Handle 'R' to retry SSH connection - if msg.String() == "R" && m.troubleshoot.Mode() == views.TroubleshootModeList { - conn := m.session.GetActiveConnection() - if conn != nil && conn.HasSSH() { - conn.DisconnectSSH() - m.troubleshoot = m.troubleshoot.SetSSHConnecting(true) - m.troubleshoot = m.troubleshoot.SetSSHError(nil) - return m, m.connectSSH(conn) - } - } - // Handle Enter to run runbook - if msg.String() == "enter" && m.troubleshoot.Mode() == views.TroubleshootModeList { - runbook := m.troubleshoot.Selected() - if runbook != nil { - m.loading = true - m.troubleshoot = m.troubleshoot.SetRunning(runbook) - return m, m.runTroubleshoot(runbook) - } - } - m.troubleshoot, cmd = m.troubleshoot.Update(msg) case ViewLogs: m.logs, cmd = m.logs.Update(msg) } diff --git a/internal/tui/messages.go b/internal/tui/messages.go index be03eeb..3d12283 100644 --- a/internal/tui/messages.go +++ b/internal/tui/messages.go @@ -2,7 +2,6 @@ package tui import ( "github.com/jp2195/pyre/internal/models" - "github.com/jp2195/pyre/internal/troubleshoot" "github.com/jp2195/pyre/internal/tui/views" ) @@ -77,7 +76,8 @@ type LoginSuccessMsg struct { Name string APIKey string Username string - Password string + Insecure bool + // Password is intentionally not included - credentials should not persist in messages. } type LoginErrorMsg struct { @@ -90,17 +90,6 @@ type ErrorMsg struct { Err error } -type TroubleshootResultMsg struct { - Result *troubleshoot.RunbookResult - Err error -} - -type TroubleshootStepMsg struct { - StepIndex int - Status troubleshoot.StepStatus - Output string -} - type ManagedDevicesMsg struct { Devices []models.ManagedDevice Err error @@ -111,15 +100,6 @@ type PanoramaDetectedMsg struct { Model string } -type SSHConnectedMsg struct { - ConnectionName string -} - -type SSHErrorMsg struct { - ConnectionName string - Err error -} - type SystemLogsMsg struct { Logs []models.SystemLogEntry Err error diff --git a/internal/tui/navigation.go b/internal/tui/navigation.go index 8314085..c31ec5a 100644 --- a/internal/tui/navigation.go +++ b/internal/tui/navigation.go @@ -76,13 +76,6 @@ func (m Model) navigateToCurrentItem() (tea.Model, tea.Cmd) { cmd = m.fetchLogs() // Tools group - case "troubleshoot": - m.currentView = ViewTroubleshoot - m.updateTroubleshootSSH() - conn := m.session.GetActiveConnection() - if conn != nil && conn.HasSSH() && conn.SSHClient == nil { - cmd = m.connectSSH(conn) - } case "config": m.currentView = ViewDashboard m.currentDashboard = views.DashboardConfig @@ -122,8 +115,6 @@ func (m *Model) syncNavbarToCurrentView() { m.navbar = m.navbar.SetActiveByID("analyze", "interfaces") case ViewLogs: m.navbar = m.navbar.SetActiveByID("analyze", "logs") - case ViewTroubleshoot: - m.navbar = m.navbar.SetActiveByID("tools", "troubleshoot") case ViewPicker: m.navbar = m.navbar.SetActiveByID("connections", "picker") } diff --git a/internal/tui/render.go b/internal/tui/render.go index c94e832..4ebfa70 100644 --- a/internal/tui/render.go +++ b/internal/tui/render.go @@ -98,8 +98,6 @@ func (m Model) currentViewName() string { return "Sessions" case ViewInterfaces: return "Interfaces" - case ViewTroubleshoot: - return "Troubleshoot" case ViewLogs: return "Logs" case ViewPicker: diff --git a/internal/tui/views/login.go b/internal/tui/views/login.go index 10a6dd8..ef5d1cb 100644 --- a/internal/tui/views/login.go +++ b/internal/tui/views/login.go @@ -26,6 +26,7 @@ type LoginModel struct { err error width int height int + insecure bool } func NewLoginModel(creds *auth.Credentials) LoginModel { @@ -54,6 +55,7 @@ func NewLoginModel(creds *auth.Credentials) LoginModel { usernameInput: username, passwordInput: password, focusedField: fieldHost, + insecure: creds.Insecure, } if creds.Host != "" { @@ -108,6 +110,17 @@ func (m LoginModel) Password() string { return m.passwordInput.Value() } +func (m LoginModel) Insecure() bool { + return m.insecure +} + +// ClearPassword clears the password from memory after successful login. +// This is a security measure to minimize the time credentials are in memory. +func (m LoginModel) ClearPassword() LoginModel { + m.passwordInput.SetValue("") + return m +} + func (m LoginModel) CanSubmit() bool { return m.Host() != "" && m.Username() != "" && m.Password() != "" } diff --git a/internal/tui/views/navbar.go b/internal/tui/views/navbar.go index 86685cf..754b67e 100644 --- a/internal/tui/views/navbar.go +++ b/internal/tui/views/navbar.go @@ -59,8 +59,7 @@ func NewNavbarModel() NavbarModel { Label: "Tools", Key: "3", Items: []NavItem{ - {ID: "troubleshoot", Label: "Troubleshoot", Key: "1"}, - {ID: "config", Label: "Config", Key: "2"}, + {ID: "config", Label: "Config", Key: "1"}, }, }, { diff --git a/internal/tui/views/navbar_test.go b/internal/tui/views/navbar_test.go index db300d9..4a36235 100644 --- a/internal/tui/views/navbar_test.go +++ b/internal/tui/views/navbar_test.go @@ -166,8 +166,8 @@ func TestNavbarModel_SetActiveByID(t *testing.T) { t.Errorf("expected activeItem=2, got %d", nav.activeItem) } - // Set to tools/troubleshoot - nav = nav.SetActiveByID("tools", "troubleshoot") + // Set to tools/config + nav = nav.SetActiveByID("tools", "config") if nav.activeGroup != 2 { t.Errorf("expected activeGroup=2, got %d", nav.activeGroup) } From df3bfdc02729c1d7a4706e808cf140088bcda45f Mon Sep 17 00:00:00 2001 From: joshuamontgomery <24376525+jp2195@users.noreply.github.com> Date: Sat, 24 Jan 2026 19:36:22 -0500 Subject: [PATCH 2/6] fix(ci): use gosec install script instead of Docker action The gosec v2.22.11 GitHub Action has a bug where action.yml still references the v2.22.10 Docker image, which bundles Go 1.25.3. This is incompatible with our go.mod requiring Go 1.25.5. Switch to manual installation using gosec's official install script, which uses the Go version from setup-go instead of a bundled version. --- .github/workflows/security.yml | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index aac34c4..5136703 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -24,10 +24,17 @@ jobs: steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - name: Run Gosec - uses: securego/gosec@424fc4cd9c82ea0fd6bee9cd49c2db2c3cc0c93f # v2.22.11 + - name: Set up Go + uses: actions/setup-go@7a3fe6cf4cb3a834922a1244abfce67bcef6a0c5 # v6.2.0 with: - args: ./... + go-version: '1.25' + cache: true + + - name: Install Gosec + run: curl -sfL https://raw.githubusercontent.com/securego/gosec/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.22.11 + + - name: Run Gosec + run: gosec ./... govulncheck: name: Go Vulnerability Check From 14cbcd62d5ce856f2a3668960fe76da552a3cb50 Mon Sep 17 00:00:00 2001 From: joshuamontgomery <24376525+jp2195@users.noreply.github.com> Date: Sat, 24 Jan 2026 19:41:16 -0500 Subject: [PATCH 3/6] fix(ci): move issue limits to correct section in golangci-lint v2 config golangci-lint v2 requires max-issues-per-linter and max-same-issues to be in a top-level "issues" section, not under "linters.exclusions". --- .golangci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9bdcd12..3d2fd69 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -51,8 +51,9 @@ linters: - path: _test\.go text: "unused-parameter" - max-issues-per-linter: 50 - max-same-issues: 10 +issues: + max-issues-per-linter: 50 + max-same-issues: 10 formatters: enable: From 8aa4db8dfe6bbc8b7406aa4afd4053d58ef84fc8 Mon Sep 17 00:00:00 2001 From: joshuamontgomery <24376525+jp2195@users.noreply.github.com> Date: Sat, 24 Jan 2026 20:22:21 -0500 Subject: [PATCH 4/6] fix(lint): resolve all golangci-lint errors - Fix errcheck issues with proper error handling and nolint comments - Fix gosec issues (TLS InsecureSkipVerify, file permissions) - Fix govet shadows by renaming inner err variables - Fix govet unused writes by adding test assertions - Fix staticcheck nil checks, embedded field selectors, De Morgan's law - Fix prealloc by pre-allocating slices with known capacity - Add nolint:misspell for "environmentals" (PAN-OS API naming) - Delete unused functions: renderInterfaces, formatRow, truncateDash - Convert if-else chains to switch statements - Fix import groupings for goimports compliance --- .idea/.gitignore | 10 ++ internal/api/client.go | 4 +- internal/api/monitoring.go | 6 +- internal/api/policies.go | 28 ++--- internal/api/sessions.go | 2 +- internal/api/system.go | 10 +- internal/auth/auth_test.go | 2 +- internal/auth/keygen.go | 4 +- internal/config/config_test.go | 14 +-- internal/ssh/client.go | 12 +- internal/ssh/client_test.go | 36 +++--- internal/ssh/mock_server.go | 26 ++-- internal/testutil/mock_server.go | 50 +++++--- internal/troubleshoot/results.go | 12 +- internal/troubleshoot/results_test.go | 21 ++++ internal/troubleshoot/runbook.go | 4 +- internal/troubleshoot/runbook_test.go | 30 +++++ internal/tui/app.go | 2 +- internal/tui/commands.go | 2 + internal/tui/handlers.go | 1 + internal/tui/messages.go | 1 + internal/tui/navigation.go | 1 + internal/tui/render.go | 1 + internal/tui/views/dashboard.go | 145 +++-------------------- internal/tui/views/dashboard_config.go | 23 ++-- internal/tui/views/dashboard_security.go | 18 ++- internal/tui/views/dashboard_vpn.go | 12 +- internal/tui/views/interfaces.go | 6 +- internal/tui/views/login_test.go | 7 +- internal/tui/views/logs.go | 19 +-- internal/tui/views/nat_policies.go | 4 +- internal/tui/views/policies.go | 8 +- internal/tui/views/sessions.go | 4 +- internal/tui/views/table_base_test.go | 13 +- 34 files changed, 263 insertions(+), 275 deletions(-) create mode 100644 .idea/.gitignore diff --git a/.idea/.gitignore b/.idea/.gitignore new file mode 100644 index 0000000..ab1f416 --- /dev/null +++ b/.idea/.gitignore @@ -0,0 +1,10 @@ +# Default ignored files +/shelf/ +/workspace.xml +# Ignored default folder with query files +/queries/ +# Datasource local storage ignored files +/dataSources/ +/dataSources.local.xml +# Editor-based HTTP Client requests +/httpRequests/ diff --git a/internal/api/client.go b/internal/api/client.go index 5453c8a..0ea5b14 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -26,7 +26,7 @@ func WithInsecure(insecure bool) ClientOption { return func(c *Client) { if insecure { c.httpClient.Transport = &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // G402: InsecureSkipVerify required for self-signed firewall certificates when user enables --insecure } } } @@ -120,7 +120,7 @@ func (c *Client) request(ctx context.Context, params url.Values) (*XMLResponse, if err != nil { return nil, fmt.Errorf("executing request: %w", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() //nolint:errcheck // best effort cleanup body, err := io.ReadAll(resp.Body) if err != nil { diff --git a/internal/api/monitoring.go b/internal/api/monitoring.go index fb087c8..febce0b 100644 --- a/internal/api/monitoring.go +++ b/internal/api/monitoring.go @@ -182,7 +182,7 @@ func (c *Client) GetJobs(ctx context.Context) ([]models.Job, error) { // Parse progress - ignore error, zero value acceptable for non-numeric progress if e.Progress != "" { - job.Progress, _ = strconv.Atoi(strings.TrimSuffix(e.Progress, "%")) + job.Progress, _ = strconv.Atoi(strings.TrimSuffix(e.Progress, "%")) //nolint:errcheck // intentional - default to 0 on parse error } // Parse timestamps - PAN-OS typically uses format like "2024/01/15 10:30:45" @@ -248,7 +248,7 @@ func (c *Client) GetDiskUsage(ctx context.Context) ([]models.DiskUsage, error) { fields := strings.Fields(line) if len(fields) >= 6 { pctStr := strings.TrimSuffix(fields[4], "%") - pct, _ := strconv.ParseFloat(pctStr, 64) + pct, _ := strconv.ParseFloat(pctStr, 64) //nolint:errcheck // intentional - default to 0 on parse error disk := models.DiskUsage{ Filesystem: fields[0], @@ -266,6 +266,8 @@ func (c *Client) GetDiskUsage(ctx context.Context) ([]models.DiskUsage, error) { } // GetEnvironmentals retrieves hardware environmental sensor data +// +//nolint:misspell // "environmentals" is the PAN-OS XML API tag name func (c *Client) GetEnvironmentals(ctx context.Context) ([]models.Environmental, error) { resp, err := c.Op(ctx, "") if err != nil { diff --git a/internal/api/policies.go b/internal/api/policies.go index 6eebd02..ecc32c6 100644 --- a/internal/api/policies.go +++ b/internal/api/policies.go @@ -35,8 +35,8 @@ func (c *Client) GetSecurityPolicies(ctx context.Context) ([]models.SecurityRule if err != nil { return nil, err } - if err := CheckResponse(resp); err != nil { - return nil, err + if checkErr := CheckResponse(resp); checkErr != nil { + return nil, checkErr } // Handle empty result @@ -116,14 +116,14 @@ func (c *Client) GetSecurityPolicies(ctx context.Context) ([]models.SecurityRule var withWrapper struct { Entry []ruleEntry `xml:"rules>entry"` } - if err := xml.Unmarshal(WrapInner(resp.Result.Inner), &withWrapper); err == nil && len(withWrapper.Entry) > 0 { + if unmarshalErr := xml.Unmarshal(WrapInner(resp.Result.Inner), &withWrapper); unmarshalErr == nil && len(withWrapper.Entry) > 0 { entries = withWrapper.Entry } else { // Try parsing without wrapper (entries directly in result) var withoutWrapper struct { Entry []ruleEntry `xml:"entry"` } - if err := xml.Unmarshal(WrapInner(resp.Result.Inner), &withoutWrapper); err == nil { + if unmarshalErr := xml.Unmarshal(WrapInner(resp.Result.Inner), &withoutWrapper); unmarshalErr == nil { entries = withoutWrapper.Entry } } @@ -230,17 +230,17 @@ func (c *Client) GetSecurityPolicies(ctx context.Context) ([]models.SecurityRule for _, h := range hitResult.Entry { stats := hitStats{count: h.HitCount} if h.LastHit != "" && h.LastHit != "0" { - if ts, _ := strconv.ParseInt(h.LastHit, 10, 64); ts > 0 { + if ts, _ := strconv.ParseInt(h.LastHit, 10, 64); ts > 0 { //nolint:errcheck // intentional - default to zero time on parse error stats.lastHit = time.Unix(ts, 0) } } if h.FirstHit != "" && h.FirstHit != "0" { - if ts, _ := strconv.ParseInt(h.FirstHit, 10, 64); ts > 0 { + if ts, _ := strconv.ParseInt(h.FirstHit, 10, 64); ts > 0 { //nolint:errcheck // intentional - default to zero time on parse error stats.firstHit = time.Unix(ts, 0) } } if h.LastReset != "" && h.LastReset != "0" { - if ts, _ := strconv.ParseInt(h.LastReset, 10, 64); ts > 0 { + if ts, _ := strconv.ParseInt(h.LastReset, 10, 64); ts > 0 { //nolint:errcheck // intentional - default to zero time on parse error stats.lastReset = time.Unix(ts, 0) } } @@ -286,8 +286,8 @@ func (c *Client) GetNATRules(ctx context.Context) ([]models.NATRule, error) { if err != nil { return nil, err } - if err := CheckResponse(resp); err != nil { - return nil, err + if checkErr := CheckResponse(resp); checkErr != nil { + return nil, checkErr } // Handle empty result @@ -357,14 +357,14 @@ func (c *Client) GetNATRules(ctx context.Context) ([]models.NATRule, error) { var withWrapper struct { Entry []natEntry `xml:"rules>entry"` } - if err := xml.Unmarshal(WrapInner(resp.Result.Inner), &withWrapper); err == nil && len(withWrapper.Entry) > 0 { + if unmarshalErr := xml.Unmarshal(WrapInner(resp.Result.Inner), &withWrapper); unmarshalErr == nil && len(withWrapper.Entry) > 0 { entries = withWrapper.Entry } else { // Try parsing without wrapper var withoutWrapper struct { Entry []natEntry `xml:"entry"` } - if err := xml.Unmarshal(WrapInner(resp.Result.Inner), &withoutWrapper); err == nil { + if unmarshalErr := xml.Unmarshal(WrapInner(resp.Result.Inner), &withoutWrapper); unmarshalErr == nil { entries = withoutWrapper.Entry } } @@ -443,17 +443,17 @@ func (c *Client) GetNATRules(ctx context.Context) ([]models.NATRule, error) { for _, h := range hitResult.Entry { stats := hitStats{count: h.HitCount} if h.LastHit != "" && h.LastHit != "0" { - if ts, _ := strconv.ParseInt(h.LastHit, 10, 64); ts > 0 { + if ts, _ := strconv.ParseInt(h.LastHit, 10, 64); ts > 0 { //nolint:errcheck // intentional - default to zero time on parse error stats.lastHit = time.Unix(ts, 0) } } if h.FirstHit != "" && h.FirstHit != "0" { - if ts, _ := strconv.ParseInt(h.FirstHit, 10, 64); ts > 0 { + if ts, _ := strconv.ParseInt(h.FirstHit, 10, 64); ts > 0 { //nolint:errcheck // intentional - default to zero time on parse error stats.firstHit = time.Unix(ts, 0) } } if h.LastReset != "" && h.LastReset != "0" { - if ts, _ := strconv.ParseInt(h.LastReset, 10, 64); ts > 0 { + if ts, _ := strconv.ParseInt(h.LastReset, 10, 64); ts > 0 { //nolint:errcheck // intentional - default to zero time on parse error stats.lastReset = time.Unix(ts, 0) } } diff --git a/internal/api/sessions.go b/internal/api/sessions.go index 9194f30..68fc70b 100644 --- a/internal/api/sessions.go +++ b/internal/api/sessions.go @@ -98,7 +98,7 @@ func (c *Client) GetSessions(ctx context.Context, filter string) ([]models.Sessi var startTime time.Time // Ignore parse error - time format may vary, zero time acceptable if e.StartTime != "" { - startTime, _ = time.Parse("Mon Jan 2 15:04:05 2006", e.StartTime) + startTime, _ = time.Parse("Mon Jan 2 15:04:05 2006", e.StartTime) //nolint:errcheck // intentional - zero time acceptable } // Convert protocol number to name proto := protoToName(e.Proto) diff --git a/internal/api/system.go b/internal/api/system.go index 71579b7..2a2c32b 100644 --- a/internal/api/system.go +++ b/internal/api/system.go @@ -214,9 +214,9 @@ func (c *Client) GetSystemResources(ctx context.Context) (*models.Resources, err // Parse load average using regex // Ignore parse errors - optional fields, zero value acceptable if parsing fails if matches := loadAvgRegex.FindStringSubmatch(output); len(matches) >= 4 { - resources.Load1, _ = strconv.ParseFloat(matches[1], 64) - resources.Load5, _ = strconv.ParseFloat(matches[2], 64) - resources.Load15, _ = strconv.ParseFloat(matches[3], 64) + resources.Load1, _ = strconv.ParseFloat(matches[1], 64) //nolint:errcheck // intentional - zero value acceptable + resources.Load5, _ = strconv.ParseFloat(matches[2], 64) //nolint:errcheck // intentional - zero value acceptable + resources.Load15, _ = strconv.ParseFloat(matches[3], 64) //nolint:errcheck // intentional - zero value acceptable } lines := strings.Split(output, "\n") @@ -278,10 +278,10 @@ func (c *Client) GetSystemResources(ctx context.Context) (*models.Resources, err // Ignore parse errors - fields may have unexpected format, zero value acceptable cleanField := strings.TrimRight(f, ",%") if (cleanField == "total" || f == "total," || f == "total") && i > 0 { - total, _ = strconv.ParseFloat(strings.TrimRight(fields[i-1], ",%"), 64) + total, _ = strconv.ParseFloat(strings.TrimRight(fields[i-1], ",%"), 64) //nolint:errcheck // intentional } if (cleanField == "used" || f == "used," || f == "used") && i > 0 { - used, _ = strconv.ParseFloat(strings.TrimRight(fields[i-1], ",%"), 64) + used, _ = strconv.ParseFloat(strings.TrimRight(fields[i-1], ",%"), 64) //nolint:errcheck // intentional } } if total > 0 { diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 2457188..b59c747 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -41,7 +41,7 @@ func TestConcurrentSetActiveFirewall(t *testing.T) { // Verify final state is valid conn := session.GetActiveConnection() if conn == nil { - t.Error("expected active connection after concurrent operations") + t.Fatal("expected active connection after concurrent operations") } // The active firewall should be one of the valid ones diff --git a/internal/auth/keygen.go b/internal/auth/keygen.go index 821bfc0..c8d681d 100644 --- a/internal/auth/keygen.go +++ b/internal/auth/keygen.go @@ -33,7 +33,7 @@ func GenerateAPIKey(ctx context.Context, host, username, password string, insecu client := &http.Client{ Timeout: 30 * time.Second, Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: insecure}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: insecure}, //nolint:gosec // G402: InsecureSkipVerify required for self-signed firewall certificates when user enables --insecure }, } @@ -54,7 +54,7 @@ func GenerateAPIKey(ctx context.Context, host, username, password string, insecu if err != nil { return nil, fmt.Errorf("keygen request failed: %w", err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() //nolint:errcheck // best effort cleanup body, err := io.ReadAll(resp.Body) if err != nil { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5408ccf..bf625eb 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -164,7 +164,7 @@ settings: theme: dark default_view: policies ` - if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil { + if err := os.WriteFile(configPath, []byte(configContent), 0600); err != nil { t.Fatalf("failed to write test config: %v", err) } @@ -208,7 +208,7 @@ func TestLoadWithFlags_InvalidConfig(t *testing.T) { configPath := filepath.Join(tmpDir, "invalid.yaml") // Write invalid YAML - if err := os.WriteFile(configPath, []byte("invalid: yaml: content:"), 0644); err != nil { + if err := os.WriteFile(configPath, []byte("invalid: yaml: content:"), 0600); err != nil { t.Fatalf("failed to write test config: %v", err) } @@ -243,7 +243,7 @@ firewalls: config-fw: host: 10.0.0.1 ` - if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil { + if err := os.WriteFile(configPath, []byte(configContent), 0600); err != nil { t.Fatalf("failed to write test config: %v", err) } @@ -294,7 +294,7 @@ firewalls: private_key_path: /path/to/key timeout: 60 ` - if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil { + if err := os.WriteFile(configPath, []byte(configContent), 0600); err != nil { t.Fatalf("failed to write test config: %v", err) } @@ -338,7 +338,7 @@ firewalls: firewall: host: 10.0.0.2 ` - if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil { + if err := os.WriteFile(configPath, []byte(configContent), 0600); err != nil { t.Fatalf("failed to write test config: %v", err) } @@ -380,7 +380,7 @@ func TestConfig_NilFirewallsAfterLoad(t *testing.T) { settings: refresh_interval: 10s ` - if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil { + if err := os.WriteFile(configPath, []byte(configContent), 0600); err != nil { t.Fatalf("failed to write test config: %v", err) } @@ -471,7 +471,7 @@ firewalls: private_key_path: /path/to/key timeout: 120 ` - if err := os.WriteFile(configPath, []byte(configContent), 0644); err != nil { + if err := os.WriteFile(configPath, []byte(configContent), 0600); err != nil { t.Fatalf("failed to write test config: %v", err) } diff --git a/internal/ssh/client.go b/internal/ssh/client.go index 41d4418..2077fb4 100644 --- a/internal/ssh/client.go +++ b/internal/ssh/client.go @@ -123,7 +123,7 @@ func (c *Client) Connect(ctx context.Context) error { // Create SSH connection on top of TCP connection sshConn, chans, reqs, err := ssh.NewClientConn(conn, addr, c.config) if err != nil { - conn.Close() + _ = conn.Close() //nolint:errcheck // best effort cleanup return fmt.Errorf("failed to establish SSH connection: %w", err) } @@ -160,7 +160,7 @@ func (c *Client) Execute(ctx context.Context, cmd string) (*CommandResult, error Error: fmt.Errorf("failed to create session: %w", err), }, nil } - defer session.Close() + defer func() { _ = session.Close() }() //nolint:errcheck // best effort cleanup var stdout, stderr bytes.Buffer session.Stdout = &stdout @@ -174,7 +174,7 @@ func (c *Client) Execute(ctx context.Context, cmd string) (*CommandResult, error select { case <-ctx.Done(): - session.Signal(ssh.SIGTERM) + _ = session.Signal(ssh.SIGTERM) //nolint:errcheck // best effort signal return &CommandResult{ Command: cmd, Duration: time.Since(start), @@ -282,7 +282,7 @@ func expandPath(path string) string { // Otherwise, it uses the known_hosts file for verification. func getHostKeyCallback(cfg config.SSHConfig) (ssh.HostKeyCallback, error) { if cfg.Insecure { - return ssh.InsecureIgnoreHostKey(), nil + return ssh.InsecureIgnoreHostKey(), nil //nolint:gosec // G106: InsecureIgnoreHostKey used when user explicitly disables host key verification } // Determine known_hosts path @@ -309,7 +309,7 @@ func getHostKeyCallback(cfg config.SSHConfig) (ssh.HostKeyCallback, error) { if err != nil { return nil, fmt.Errorf("creating known_hosts file: %w", err) } - f.Close() + _ = f.Close() //nolint:errcheck // best effort cleanup } // Create host key callback from known_hosts @@ -348,7 +348,7 @@ func addHostKey(knownHostsPath, hostname string, remote net.Addr, key ssh.Public if err != nil { return err } - defer f.Close() + defer func() { _ = f.Close() }() //nolint:errcheck // best effort cleanup // Format the known_hosts line line := knownhosts.Line([]string{hostname}, key) diff --git a/internal/ssh/client_test.go b/internal/ssh/client_test.go index 83db682..ad571c6 100644 --- a/internal/ssh/client_test.go +++ b/internal/ssh/client_test.go @@ -78,8 +78,8 @@ func TestClientWithMockServer(t *testing.T) { server.SetDefaultResponses() - if err := server.Start(); err != nil { - t.Fatalf("Failed to start mock server: %v", err) + if startErr := server.Start(); startErr != nil { + t.Fatalf("Failed to start mock server: %v", startErr) } defer server.Close() @@ -99,8 +99,8 @@ func TestClientWithMockServer(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if err := client.Connect(ctx); err != nil { - t.Fatalf("Failed to connect: %v", err) + if connErr := client.Connect(ctx); connErr != nil { + t.Fatalf("Failed to connect: %v", connErr) } if !client.IsConnected() { @@ -334,8 +334,8 @@ func TestMockSSHServerCustomResponse(t *testing.T) { server.SetResponse("custom-command", "custom-response\n") - if err := server.Start(); err != nil { - t.Fatalf("Failed to start mock server: %v", err) + if startErr := server.Start(); startErr != nil { + t.Fatalf("Failed to start mock server: %v", startErr) } defer server.Close() @@ -354,8 +354,8 @@ func TestMockSSHServerCustomResponse(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if err := client.Connect(ctx); err != nil { - t.Fatalf("Failed to connect: %v", err) + if connErr := client.Connect(ctx); connErr != nil { + t.Fatalf("Failed to connect: %v", connErr) } result, err := client.Execute(ctx, "custom-command") @@ -373,8 +373,8 @@ func TestMockSSHServerAddress(t *testing.T) { t.Fatalf("Failed to create mock server: %v", err) } - if err := server.Start(); err != nil { - t.Fatalf("Failed to start mock server: %v", err) + if startErr := server.Start(); startErr != nil { + t.Fatalf("Failed to start mock server: %v", startErr) } defer server.Close() @@ -403,8 +403,8 @@ func TestExecuteContextCancellation(t *testing.T) { // Set up a slow command response server.SetResponse("sleep-command", "started\n") - if err := server.Start(); err != nil { - t.Fatalf("Failed to start mock server: %v", err) + if startErr := server.Start(); startErr != nil { + t.Fatalf("Failed to start mock server: %v", startErr) } defer server.Close() @@ -424,8 +424,8 @@ func TestExecuteContextCancellation(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if err := client.Connect(ctx); err != nil { - t.Fatalf("Failed to connect: %v", err) + if connErr := client.Connect(ctx); connErr != nil { + t.Fatalf("Failed to connect: %v", connErr) } // Create a context that we'll cancel @@ -466,8 +466,8 @@ func TestExecuteWithAlreadyCancelledContext(t *testing.T) { server.SetDefaultResponses() - if err := server.Start(); err != nil { - t.Fatalf("Failed to start mock server: %v", err) + if startErr := server.Start(); startErr != nil { + t.Fatalf("Failed to start mock server: %v", startErr) } defer server.Close() @@ -487,8 +487,8 @@ func TestExecuteWithAlreadyCancelledContext(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if err := client.Connect(ctx); err != nil { - t.Fatalf("Failed to connect: %v", err) + if connErr := client.Connect(ctx); connErr != nil { + t.Fatalf("Failed to connect: %v", connErr) } // Create an already-cancelled context diff --git a/internal/ssh/mock_server.go b/internal/ssh/mock_server.go index 90ac451..b413ab8 100644 --- a/internal/ssh/mock_server.go +++ b/internal/ssh/mock_server.go @@ -66,7 +66,7 @@ func (s *MockSSHServer) Start() error { } s.listener = listener - addr := listener.Addr().(*net.TCPAddr) + addr := listener.Addr().(*net.TCPAddr) //nolint:errcheck // known type s.host = addr.IP.String() s.port = addr.Port @@ -116,20 +116,20 @@ func (s *MockSSHServer) acceptLoop() { } func (s *MockSSHServer) handleConnection(netConn net.Conn) { - defer netConn.Close() + defer func() { _ = netConn.Close() }() //nolint:errcheck // test helper sshConn, chans, reqs, err := ssh.NewServerConn(netConn, s.config) if err != nil { return } - defer sshConn.Close() + defer func() { _ = sshConn.Close() }() //nolint:errcheck // test helper // Discard out-of-band requests go ssh.DiscardRequests(reqs) for newChannel := range chans { if newChannel.ChannelType() != "session" { - newChannel.Reject(ssh.UnknownChannelType, "unknown channel type") + _ = newChannel.Reject(ssh.UnknownChannelType, "unknown channel type") //nolint:errcheck // test helper continue } @@ -143,43 +143,43 @@ func (s *MockSSHServer) handleConnection(netConn net.Conn) { } func (s *MockSSHServer) handleChannel(channel ssh.Channel, requests <-chan *ssh.Request) { - defer channel.Close() + defer func() { _ = channel.Close() }() //nolint:errcheck // test helper for req := range requests { switch req.Type { case "exec": if len(req.Payload) < 4 { - req.Reply(false, nil) + _ = req.Reply(false, nil) //nolint:errcheck // test helper continue } // Extract command from payload (length-prefixed string) cmdLen := int(req.Payload[0])<<24 | int(req.Payload[1])<<16 | int(req.Payload[2])<<8 | int(req.Payload[3]) if len(req.Payload) < 4+cmdLen { - req.Reply(false, nil) + _ = req.Reply(false, nil) //nolint:errcheck // test helper continue } cmd := string(req.Payload[4 : 4+cmdLen]) - req.Reply(true, nil) + _ = req.Reply(true, nil) //nolint:errcheck // test helper response := s.getResponse(cmd) - io.WriteString(channel, response) + _, _ = io.WriteString(channel, response) //nolint:errcheck // test helper // Send exit status - channel.SendRequest("exit-status", false, []byte{0, 0, 0, 0}) + _, _ = channel.SendRequest("exit-status", false, []byte{0, 0, 0, 0}) //nolint:errcheck // test helper // Close stdout to signal command completion - channel.CloseWrite() + _ = channel.CloseWrite() //nolint:errcheck // test helper return case "shell": - req.Reply(true, nil) + _ = req.Reply(true, nil) //nolint:errcheck // test helper // For shell requests, just close the channel return default: - req.Reply(false, nil) + _ = req.Reply(false, nil) //nolint:errcheck // test helper } } } diff --git a/internal/testutil/mock_server.go b/internal/testutil/mock_server.go index 224e5ee..91d86e4 100644 --- a/internal/testutil/mock_server.go +++ b/internal/testutil/mock_server.go @@ -59,7 +59,7 @@ func (m *MockPANOS) Host() string { func (m *MockPANOS) handleAPI(w http.ResponseWriter, r *http.Request) { // Parse form for POST requests (keygen uses POST with form body) if r.Method == http.MethodPost { - r.ParseForm() + _ = r.ParseForm() //nolint:errcheck // test helper } // Get type from query string or form @@ -79,7 +79,7 @@ func (m *MockPANOS) handleAPI(w http.ResponseWriter, r *http.Request) { case "config": m.handleConfig(w, r) default: - w.Write([]byte(`Invalid request`)) + _, _ = w.Write([]byte(`Invalid request`)) //nolint:errcheck // test helper } } @@ -95,9 +95,9 @@ func (m *MockPANOS) handleKeygen(w http.ResponseWriter, r *http.Request) { } if user == "admin" && password == "admin" { - w.Write([]byte(`LUFRPT1234567890abcdef==`)) + _, _ = w.Write([]byte(`LUFRPT1234567890abcdef==`)) //nolint:errcheck // test helper } else { - w.Write([]byte(`Invalid credentials`)) + _, _ = w.Write([]byte(`Invalid credentials`)) //nolint:errcheck // test helper } } @@ -126,7 +126,7 @@ func (m *MockPANOS) handleOp(w http.ResponseWriter, r *http.Request, cmd string) case strings.Contains(cmd, ""): m.respondManagedDevices(w) default: - w.Write([]byte(``)) + _, _ = w.Write([]byte(``)) //nolint:errcheck // test helper } } @@ -136,12 +136,13 @@ func (m *MockPANOS) handleConfig(w http.ResponseWriter, r *http.Request) { if strings.Contains(xpath, "security/rules") { m.respondSecurityRules(w) } else { - w.Write([]byte(``)) + _, _ = w.Write([]byte(``)) //nolint:errcheck // test helper } } +//nolint:errcheck // test helper func (m *MockPANOS) respondSystemInfo(w http.ResponseWriter) { - fmt.Fprintf(w, ` + _, _ = fmt.Fprintf(w, ` %s @@ -158,7 +159,8 @@ func (m *MockPANOS) respondSystemInfo(w http.ResponseWriter) { } func (m *MockPANOS) respondResources(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` top - 14:32:18 up 15 days, 3:42, 0 users, load average: 0.45, 0.52, 0.48 Tasks: 150 total, 1 running, 149 sleeping, 0 stopped, 0 zombie @@ -169,7 +171,8 @@ KiB Mem: 16384000 total, 12288000 used, 4096000 free, 256000 buffers } func (m *MockPANOS) respondSessionInfo(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` 15432 262144 @@ -180,7 +183,8 @@ func (m *MockPANOS) respondSessionInfo(w http.ResponseWriter) { } func (m *MockPANOS) respondSessions(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` 12345 @@ -244,7 +248,8 @@ func (m *MockPANOS) respondSessions(w http.ResponseWriter) { } func (m *MockPANOS) respondHAStatus(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` yes @@ -262,7 +267,8 @@ func (m *MockPANOS) respondHAStatus(w http.ResponseWriter) { } func (m *MockPANOS) respondInterfaces(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` @@ -329,7 +335,8 @@ func (m *MockPANOS) respondInterfaces(w http.ResponseWriter) { } func (m *MockPANOS) respondSecurityRules(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` no @@ -380,7 +387,8 @@ func (m *MockPANOS) respondSecurityRules(w http.ResponseWriter) { } func (m *MockPANOS) respondRuleHitCount(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` @@ -415,7 +423,8 @@ func (m *MockPANOS) respondRuleHitCount(w http.ResponseWriter) { } func (m *MockPANOS) respondThreatCounters(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` @@ -458,7 +467,8 @@ func (m *MockPANOS) respondThreatCounters(w http.ResponseWriter) { } func (m *MockPANOS) respondGlobalProtect(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` jsmith @@ -489,7 +499,8 @@ func (m *MockPANOS) respondGlobalProtect(w http.ResponseWriter) { } func (m *MockPANOS) respondLicenseInfo(w http.ResponseWriter) { - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` @@ -523,10 +534,11 @@ func (m *MockPANOS) respondLicenseInfo(w http.ResponseWriter) { func (m *MockPANOS) respondManagedDevices(w http.ResponseWriter) { if !m.IsPanorama { - w.Write([]byte(`Command not available on this device`)) + _, _ = w.Write([]byte(`Command not available on this device`)) //nolint:errcheck // test helper return } - w.Write([]byte(` + //nolint:errcheck // test helper + _, _ = w.Write([]byte(` diff --git a/internal/troubleshoot/results.go b/internal/troubleshoot/results.go index 4cb0af3..36117ce 100644 --- a/internal/troubleshoot/results.go +++ b/internal/troubleshoot/results.go @@ -9,12 +9,12 @@ import ( type StepStatus string const ( - StepStatusPending StepStatus = "pending" - StepStatusRunning StepStatus = "running" - StepStatusPassed StepStatus = "passed" - StepStatusFailed StepStatus = "failed" - StepStatusSkipped StepStatus = "skipped" - StepStatusError StepStatus = "error" + StepStatusPending StepStatus = "pending" + StepStatusRunning StepStatus = "running" + StepStatusPassed StepStatus = "passed" + StepStatusFailed StepStatus = "failed" + StepStatusSkipped StepStatus = "skipped" + StepStatusError StepStatus = "error" ) // RunbookResult contains the complete result of a runbook execution. diff --git a/internal/troubleshoot/results_test.go b/internal/troubleshoot/results_test.go index ffbcc79..6837adb 100644 --- a/internal/troubleshoot/results_test.go +++ b/internal/troubleshoot/results_test.go @@ -408,6 +408,9 @@ func TestStepResult_Fields(t *testing.T) { if stepResult.Duration != 100*time.Millisecond { t.Errorf("expected duration 100ms, got %v", stepResult.Duration) } + if stepResult.Error != nil { + t.Errorf("expected nil error, got %v", stepResult.Error) + } if len(stepResult.Matches) != 1 { t.Errorf("expected 1 match, got %d", len(stepResult.Matches)) } @@ -429,9 +432,27 @@ func TestIssue_Fields(t *testing.T) { if issue.StepID != "step1" { t.Errorf("expected StepID 'step1', got %q", issue.StepID) } + if issue.StepName != "Step Name" { + t.Errorf("expected StepName 'Step Name', got %q", issue.StepName) + } + if issue.PatternID != "pattern1" { + t.Errorf("expected PatternID 'pattern1', got %q", issue.PatternID) + } + if issue.PatternName != "Pattern Name" { + t.Errorf("expected PatternName 'Pattern Name', got %q", issue.PatternName) + } if issue.Severity != SeverityError { t.Errorf("expected severity Error, got %s", issue.Severity) } + if issue.Message != "Error message" { + t.Errorf("expected Message 'Error message', got %q", issue.Message) + } + if issue.MatchedText != "matched text" { + t.Errorf("expected MatchedText 'matched text', got %q", issue.MatchedText) + } + if issue.Remediation != "Fix it like this" { + t.Errorf("expected Remediation 'Fix it like this', got %q", issue.Remediation) + } if len(issue.KBArticles) != 2 { t.Errorf("expected 2 KB articles, got %d", len(issue.KBArticles)) } diff --git a/internal/troubleshoot/runbook.go b/internal/troubleshoot/runbook.go index b0da380..d52b61e 100644 --- a/internal/troubleshoot/runbook.go +++ b/internal/troubleshoot/runbook.go @@ -48,8 +48,8 @@ type Step struct { Name string `yaml:"name"` Description string `yaml:"description"` Type StepType `yaml:"type"` - Command string `yaml:"command"` // For SSH steps - APICall string `yaml:"api_call"` // For API steps + Command string `yaml:"command"` // For SSH steps + APICall string `yaml:"api_call"` // For API steps Patterns []Pattern `yaml:"patterns"` Required bool `yaml:"required"` // Stop on failure? } diff --git a/internal/troubleshoot/runbook_test.go b/internal/troubleshoot/runbook_test.go index e5dec22..ec5f87d 100644 --- a/internal/troubleshoot/runbook_test.go +++ b/internal/troubleshoot/runbook_test.go @@ -211,6 +211,15 @@ func TestRunbook_Fields(t *testing.T) { if runbook.Name != "Test Name" { t.Errorf("expected Name 'Test Name', got %q", runbook.Name) } + if runbook.Description != "Test Description" { + t.Errorf("expected Description 'Test Description', got %q", runbook.Description) + } + if runbook.Category != "test-category" { + t.Errorf("expected Category 'test-category', got %q", runbook.Category) + } + if len(runbook.Tags) != 2 || runbook.Tags[0] != "tag1" { + t.Errorf("expected Tags ['tag1', 'tag2'], got %v", runbook.Tags) + } if !runbook.RequiresSSH { t.Error("expected RequiresSSH to be true") } @@ -249,9 +258,18 @@ func TestStep_Fields(t *testing.T) { if step.ID != "step-id" { t.Errorf("expected ID 'step-id', got %q", step.ID) } + if step.Name != "Step Name" { + t.Errorf("expected Name 'Step Name', got %q", step.Name) + } + if step.Description != "Step description" { + t.Errorf("expected Description 'Step description', got %q", step.Description) + } if step.Type != StepTypeSSH { t.Errorf("expected type SSH, got %s", step.Type) } + if step.Command != "show clock" { + t.Errorf("expected Command 'show clock', got %q", step.Command) + } if !step.Required { t.Error("expected Required to be true") } @@ -274,9 +292,21 @@ func TestPattern_Fields(t *testing.T) { if pattern.ID != "pattern-id" { t.Errorf("expected ID 'pattern-id', got %q", pattern.ID) } + if pattern.Name != "Pattern Name" { + t.Errorf("expected Name 'Pattern Name', got %q", pattern.Name) + } + if pattern.Regex != `error\s+\d+` { + t.Errorf("expected Regex 'error\\s+\\d+', got %q", pattern.Regex) + } if pattern.Severity != SeverityCritical { t.Errorf("expected severity Critical, got %s", pattern.Severity) } + if pattern.Message != "Critical error found" { + t.Errorf("expected Message 'Critical error found', got %q", pattern.Message) + } + if pattern.Remediation != "Follow KB001 to resolve" { + t.Errorf("expected Remediation 'Follow KB001 to resolve', got %q", pattern.Remediation) + } if len(pattern.KBArticles) != 2 { t.Errorf("expected 2 KB articles, got %d", len(pattern.KBArticles)) } diff --git a/internal/tui/app.go b/internal/tui/app.go index ed52643..070791c 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -442,7 +442,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.dashboard = m.dashboard.SetDiskUsage(msg.Disks, msg.Err) case EnvironmentalsMsg: - m.dashboard = m.dashboard.SetEnvironmentals(msg.Environmentals, msg.Err) + m.dashboard = m.dashboard.SetEnvironmentals(msg.Environmentals, msg.Err) //nolint:misspell // "environmentals" is the PAN-OS XML API tag name case CertificatesMsg: m.dashboard = m.dashboard.SetCertificates(msg.Certificates, msg.Err) diff --git a/internal/tui/commands.go b/internal/tui/commands.go index 78490a4..6df0b48 100644 --- a/internal/tui/commands.go +++ b/internal/tui/commands.go @@ -2,6 +2,7 @@ package tui import ( tea "github.com/charmbracelet/bubbletea" + "github.com/jp2195/pyre/internal/api" "github.com/jp2195/pyre/internal/auth" "github.com/jp2195/pyre/internal/tui/views" @@ -244,6 +245,7 @@ func (m Model) fetchDiskUsage(conn *auth.Connection) tea.Cmd { } } +//nolint:misspell // "environmentals" is the PAN-OS XML API tag name func (m Model) fetchEnvironmentals(conn *auth.Connection) tea.Cmd { ctx := m.ctx return func() tea.Msg { diff --git a/internal/tui/handlers.go b/internal/tui/handlers.go index 0ad0f08..24fc807 100644 --- a/internal/tui/handlers.go +++ b/internal/tui/handlers.go @@ -3,6 +3,7 @@ package tui import ( "github.com/charmbracelet/bubbles/key" tea "github.com/charmbracelet/bubbletea" + "github.com/jp2195/pyre/internal/auth" "github.com/jp2195/pyre/internal/tui/views" ) diff --git a/internal/tui/messages.go b/internal/tui/messages.go index 3d12283..6f269c1 100644 --- a/internal/tui/messages.go +++ b/internal/tui/messages.go @@ -143,6 +143,7 @@ type DiskUsageMsg struct { Err error } +//nolint:misspell // "environmentals" is the PAN-OS XML API tag name type EnvironmentalsMsg struct { Environmentals []models.Environmental Err error diff --git a/internal/tui/navigation.go b/internal/tui/navigation.go index c31ec5a..ced4285 100644 --- a/internal/tui/navigation.go +++ b/internal/tui/navigation.go @@ -2,6 +2,7 @@ package tui import ( tea "github.com/charmbracelet/bubbletea" + "github.com/jp2195/pyre/internal/tui/views" ) diff --git a/internal/tui/render.go b/internal/tui/render.go index 4ebfa70..aefcc6a 100644 --- a/internal/tui/render.go +++ b/internal/tui/render.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/charmbracelet/lipgloss" + "github.com/jp2195/pyre/internal/tui/views" ) diff --git a/internal/tui/views/dashboard.go b/internal/tui/views/dashboard.go index aaabfcd..9f41a0f 100644 --- a/internal/tui/views/dashboard.go +++ b/internal/tui/views/dashboard.go @@ -50,7 +50,7 @@ type DashboardModel struct { licenses []models.LicenseInfo jobs []models.Job diskUsage []models.DiskUsage - environmentals []models.Environmental + environmentals []models.Environmental //nolint:misspell // "environmentals" is the PAN-OS XML API tag name certificates []models.Certificate natPools []models.NATPoolInfo @@ -195,12 +195,12 @@ func (m DashboardModel) View() string { } // Add disk usage panel to left column (health metric) - if m.diskUsage != nil && len(m.diskUsage) > 0 { + if len(m.diskUsage) > 0 { leftPanels = append(leftPanels, m.renderDiskUsage(leftColWidth)) } // Add hardware status panel to left column (health metric) - if m.environmentals != nil && len(m.environmentals) > 0 { + if len(m.environmentals) > 0 { //nolint:misspell // "environmentals" is the PAN-OS XML API tag name leftPanels = append(leftPanels, m.renderEnvironmentals(leftColWidth)) } @@ -216,7 +216,7 @@ func (m DashboardModel) View() string { } // NAT Pool Utilization (NEW) - if m.natPools != nil && len(m.natPools) > 0 { + if len(m.natPools) > 0 { rightPanels = append(rightPanels, m.renderNATPoolUtilization(rightColWidth)) } @@ -224,7 +224,7 @@ func (m DashboardModel) View() string { rightPanels = append(rightPanels, m.renderContentVersions(rightColWidth)) // Licenses - if m.licenses != nil && len(m.licenses) > 0 { + if len(m.licenses) > 0 { rightPanels = append(rightPanels, m.renderLicenses(rightColWidth)) } @@ -234,7 +234,7 @@ func (m DashboardModel) View() string { } // Admins Online - if m.admins != nil && len(m.admins) > 0 { + if len(m.admins) > 0 { rightPanels = append(rightPanels, m.renderLoggedInAdmins(rightColWidth)) } @@ -244,12 +244,12 @@ func (m DashboardModel) View() string { } // Recent Jobs - if m.jobs != nil && len(m.jobs) > 0 { + if len(m.jobs) > 0 { rightPanels = append(rightPanels, m.renderJobs(rightColWidth)) } // Certificates (expiring/expired) - if m.certificates != nil && len(m.certificates) > 0 { + if len(m.certificates) > 0 { rightPanels = append(rightPanels, m.renderCertificates(rightColWidth)) } @@ -266,12 +266,12 @@ func (m DashboardModel) renderSingleColumn(width int) string { } // Disk usage (health) - if m.diskUsage != nil && len(m.diskUsage) > 0 { + if len(m.diskUsage) > 0 { panels = append(panels, m.renderDiskUsage(width)) } // Hardware status (health) - if m.environmentals != nil && len(m.environmentals) > 0 { + if len(m.environmentals) > 0 { //nolint:misspell // "environmentals" is the PAN-OS XML API tag name panels = append(panels, m.renderEnvironmentals(width)) } @@ -281,19 +281,19 @@ func (m DashboardModel) renderSingleColumn(width int) string { } // NAT Pool Utilization - if m.natPools != nil && len(m.natPools) > 0 { + if len(m.natPools) > 0 { panels = append(panels, m.renderNATPoolUtilization(width)) } - if m.licenses != nil && len(m.licenses) > 0 { + if len(m.licenses) > 0 { panels = append(panels, m.renderLicenses(width)) } - if m.jobs != nil && len(m.jobs) > 0 { + if len(m.jobs) > 0 { panels = append(panels, m.renderJobs(width)) } - if m.certificates != nil && len(m.certificates) > 0 { + if len(m.certificates) > 0 { panels = append(panels, m.renderCertificates(width)) } @@ -1139,121 +1139,8 @@ func (m DashboardModel) renderNATPoolUtilization(width int) string { return panelStyle().Width(width).Render(b.String()) } -func (m DashboardModel) renderInterfaces(width int) string { - var b strings.Builder - b.WriteString(titleStyle().Render("Network Interfaces")) - b.WriteString("\n\n") - - if m.ifaceErr != nil { - b.WriteString(errorStyle().Render("Error: " + m.ifaceErr.Error())) - return panelStyle().Width(width).Render(b.String()) - } - if m.interfaces == nil { - b.WriteString(dimStyle().Render("Loading...")) - return panelStyle().Width(width).Render(b.String()) - } - - if len(m.interfaces) == 0 { - b.WriteString(dimStyle().Render("No interfaces configured")) - return panelStyle().Width(width).Render(b.String()) - } - - // Calculate column widths based on available space - availWidth := width - 8 - nameW := 16 - stateW := 6 - zoneW := 12 - ipW := availWidth - nameW - stateW - zoneW - 6 - - if ipW < 10 { - ipW = 15 - } - - // Header - headerStyle := DetailLabelStyle.Bold(true) - - header := fmt.Sprintf("%-*s %-*s %-*s %-*s", - nameW, "Interface", - stateW, "State", - zoneW, "Zone", - ipW, "IP Address") - b.WriteString(headerStyle.Render(header)) - b.WriteString("\n") - b.WriteString(dimStyle().Render(strings.Repeat("─", minInt(availWidth, len(header))))) - b.WriteString("\n") - - // Show interfaces, prioritizing those with IPs - maxRows := 8 - shown := 0 - upWithIP := []models.Interface{} - upNoIP := []models.Interface{} - downIfaces := []models.Interface{} - - for _, iface := range m.interfaces { - if iface.State == "up" { - if iface.IP != "" { - upWithIP = append(upWithIP, iface) - } else { - upNoIP = append(upNoIP, iface) - } - } else { - downIfaces = append(downIfaces, iface) - } - } - - // Display order: up with IP, up without IP, down - displayOrder := append(upWithIP, upNoIP...) - displayOrder = append(displayOrder, downIfaces...) - - for _, iface := range displayOrder { - if shown >= maxRows { - break - } - - stateStr := "up" - stStyle := highlightStyle() - if iface.State != "up" { - stateStr = "down" - stStyle = dimStyle() - } - - zone := iface.Zone - if zone == "" { - zone = "-" - } - - ip := iface.IP - if ip == "" { - ip = "-" - } - - row := fmt.Sprintf("%-*s %s %-*s %-*s", - nameW, truncateDash(iface.Name, nameW), - stStyle.Render(fmt.Sprintf("%-*s", stateW, stateStr)), - zoneW, truncateDash(zone, zoneW), - ipW, truncateDash(ip, ipW)) - b.WriteString(row) - b.WriteString("\n") - shown++ - } - - if len(m.interfaces) > maxRows { - remaining := len(m.interfaces) - maxRows - b.WriteString(dimStyle().Render(fmt.Sprintf("... and %d more", remaining))) - } - - return panelStyle().Width(width).Render(b.String()) -} - // Helper functions -func formatRow(label, value string, labelWidth int) string { - if value == "" { - return "" - } - return labelStyle().Width(labelWidth).Render(label+":") + " " + valueStyle().Render(value) + "\n" -} - func renderBar(percent float64, width int, color string) string { if percent < 0 { percent = 0 @@ -1310,10 +1197,6 @@ func formatThroughput(kbps int64) string { return fmt.Sprintf("%d Kbps", kbps) } -func truncateDash(s string, maxLen int) string { - return truncateEllipsis(s, maxLen) -} - func formatTimeAgo(t time.Time) string { if t.IsZero() { return "" diff --git a/internal/tui/views/dashboard_config.go b/internal/tui/views/dashboard_config.go index 20e4012..1735c08 100644 --- a/internal/tui/views/dashboard_config.go +++ b/internal/tui/views/dashboard_config.go @@ -126,9 +126,10 @@ func (m ConfigDashboardModel) renderPolicyStats(width int) string { if !rule.Disabled { enabledRules++ } - if rule.Action == "allow" { + switch rule.Action { + case "allow": allowRules++ - } else if rule.Action == "deny" || rule.Action == "drop" { + case "deny", "drop": denyRules++ } if rule.HitCount == 0 && !rule.Disabled { @@ -311,11 +312,14 @@ func (m ConfigDashboardModel) renderZeroHitRules(width int) string { rule := zeroHitRules[i] name := truncateEllipsis(rule.Name, nameWidth) - actionStyle := dimStyle() - if rule.Action == "allow" { + var actionStyle lipgloss.Style + switch rule.Action { + case "allow": actionStyle = highlightStyle() - } else if rule.Action == "deny" || rule.Action == "drop" { + case "deny", "drop": actionStyle = errorStyle() + default: + actionStyle = dimStyle() } b.WriteString(labelStyle().Render(fmt.Sprintf("%3d. ", rule.Position))) @@ -378,11 +382,14 @@ func (m ConfigDashboardModel) renderMostHitRules(width int) string { name := truncateEllipsis(rule.Name, nameWidth) - actionStyle := dimStyle() - if rule.Action == "allow" { + var actionStyle lipgloss.Style + switch rule.Action { + case "allow": actionStyle = highlightStyle() - } else if rule.Action == "deny" || rule.Action == "drop" { + case "deny", "drop": actionStyle = errorStyle() + default: + actionStyle = dimStyle() } b.WriteString(valueStyle().Render(fmt.Sprintf("%-*s ", nameWidth, name))) diff --git a/internal/tui/views/dashboard_security.go b/internal/tui/views/dashboard_security.go index a3a6e63..2320604 100644 --- a/internal/tui/views/dashboard_security.go +++ b/internal/tui/views/dashboard_security.go @@ -276,11 +276,14 @@ func (m SecurityDashboardModel) renderZeroHitRules(width int) string { rule := zeroHitRules[i] name := truncateEllipsis(rule.Name, nameWidth) - actionStyle := dimStyle() - if rule.Action == "allow" { + var actionStyle lipgloss.Style + switch rule.Action { + case "allow": actionStyle = highlightStyle() - } else if rule.Action == "deny" || rule.Action == "drop" { + case "deny", "drop": actionStyle = errorStyle() + default: + actionStyle = dimStyle() } b.WriteString(labelStyle().Render(fmt.Sprintf("%3d. ", rule.Position))) @@ -343,11 +346,14 @@ func (m SecurityDashboardModel) renderMostHitRules(width int) string { name := truncateEllipsis(rule.Name, nameWidth) - actionStyle := dimStyle() - if rule.Action == "allow" { + var actionStyle lipgloss.Style + switch rule.Action { + case "allow": actionStyle = highlightStyle() - } else if rule.Action == "deny" || rule.Action == "drop" { + case "deny", "drop": actionStyle = errorStyle() + default: + actionStyle = dimStyle() } b.WriteString(valueStyle().Render(fmt.Sprintf("%-*s ", nameWidth, name))) diff --git a/internal/tui/views/dashboard_vpn.go b/internal/tui/views/dashboard_vpn.go index 88f7c0d..4a6700f 100644 --- a/internal/tui/views/dashboard_vpn.go +++ b/internal/tui/views/dashboard_vpn.go @@ -187,14 +187,18 @@ func (m VPNDashboardModel) renderIPSecTunnels(width int) string { tunnel := m.tunnels[i] // State indicator - stateStyle := errorStyle() - stateIcon := "x" - if tunnel.State == "up" { + var stateStyle lipgloss.Style + var stateIcon string + switch tunnel.State { + case "up": stateStyle = highlightStyle() stateIcon = "o" - } else if tunnel.State == "init" { + case "init": stateStyle = warningStyle() stateIcon = "~" + default: + stateStyle = errorStyle() + stateIcon = "x" } name := truncateEllipsis(tunnel.Name, nameWidth) diff --git a/internal/tui/views/interfaces.go b/internal/tui/views/interfaces.go index e2c7852..4135fdb 100644 --- a/internal/tui/views/interfaces.go +++ b/internal/tui/views/interfaces.go @@ -132,7 +132,7 @@ func (m InterfacesModel) Update(msg tea.Msg) (InterfacesModel, tea.Cmd) { // Delegate to TableBase for common navigation visible := m.visibleCards() - base, handled, cmd := m.TableBase.HandleNavigation(msg, len(m.filtered), visible) + base, handled, cmd := m.HandleNavigation(msg, len(m.filtered), visible) if handled { m.TableBase = base return m, cmd @@ -142,7 +142,7 @@ func (m InterfacesModel) Update(msg tea.Msg) (InterfacesModel, tea.Cmd) { } func (m InterfacesModel) updateFilterMode(msg tea.Msg) (InterfacesModel, tea.Cmd) { - base, exited, cmd := m.TableBase.HandleFilterMode(msg) + base, exited, cmd := m.HandleFilterMode(msg) m.TableBase = base if exited { m.applyFilter() @@ -500,7 +500,7 @@ func (m InterfacesModel) renderHelp() string { {"r", "refresh"}, } - var parts []string + parts := make([]string, 0, len(keys)) for _, k := range keys { parts = append(parts, keyStyle.Render(k.key)+descStyle.Render(":"+k.desc)) } diff --git a/internal/tui/views/login_test.go b/internal/tui/views/login_test.go index 24ef413..e89e62c 100644 --- a/internal/tui/views/login_test.go +++ b/internal/tui/views/login_test.go @@ -153,10 +153,13 @@ func TestLoginModel_Update(t *testing.T) { // Type in host field msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("a")} - m, _ = m.Update(msg) + updated, _ := m.Update(msg) // The input should have processed the key - // (Actual value depends on textinput implementation) + // Verify the update returned a valid model + if updated.View() == "" { + t.Error("expected non-empty view after update") + } } func TestLoginModel_View(t *testing.T) { diff --git a/internal/tui/views/logs.go b/internal/tui/views/logs.go index b902e83..e1bdab2 100644 --- a/internal/tui/views/logs.go +++ b/internal/tui/views/logs.go @@ -318,7 +318,7 @@ func (m LogsModel) Update(msg tea.Msg) (LogsModel, tea.Cmd) { // Delegate to TableBase for common navigation visible := m.visibleRows() - base, handled, cmd := m.TableBase.HandleNavigation(msg, m.filteredCount(), visible) + base, handled, cmd := m.HandleNavigation(msg, m.filteredCount(), visible) if handled { m.TableBase = base return m, cmd @@ -681,7 +681,14 @@ func (m LogsModel) renderSystemDetail(log models.SystemLogEntry) string { panelStyle := DetailPanelStyle.Width(m.Width - 2) labelStyle := DetailLabelStyle.Width(12) - var lines []string + // Word wrap the description for better readability + descWidth := m.Width - 10 + if descWidth > 100 { + descWidth = 100 + } + wrapped := wrapText(log.Description, descWidth) + + lines := make([]string, 0, 7+len(wrapped)) lines = append(lines, ViewTitleStyle.Render("System Log Details")) lines = append(lines, "") lines = append(lines, labelStyle.Render("Time")+DetailValueStyle.Render(log.Time.Format("2006-01-02 15:04:05"))) @@ -690,12 +697,6 @@ func (m LogsModel) renderSystemDetail(log models.SystemLogEntry) string { lines = append(lines, "") lines = append(lines, ViewTitleStyle.Render("Message")) - // Word wrap the description for better readability - descWidth := m.Width - 10 - if descWidth > 100 { - descWidth = 100 - } - wrapped := wrapText(log.Description, descWidth) for _, line := range wrapped { lines = append(lines, DetailValueStyle.Render(line)) } @@ -819,7 +820,7 @@ func (m LogsModel) renderHelp() string { {"r", "refresh"}, } - var parts []string + parts := make([]string, 0, len(keys)) for _, k := range keys { parts = append(parts, HelpKeyStyle.Render(k.key)+HelpDescStyle.Render(":"+k.desc)) } diff --git a/internal/tui/views/nat_policies.go b/internal/tui/views/nat_policies.go index f373d6a..607dd1c 100644 --- a/internal/tui/views/nat_policies.go +++ b/internal/tui/views/nat_policies.go @@ -149,7 +149,7 @@ func (m NATPoliciesModel) Update(msg tea.Msg) (NATPoliciesModel, tea.Cmd) { // Delegate to TableBase for common navigation visible := m.visibleRows() - base, handled, cmd := m.TableBase.HandleNavigation(msg, len(m.filtered), visible) + base, handled, cmd := m.HandleNavigation(msg, len(m.filtered), visible) if handled { m.TableBase = base return m, cmd @@ -160,7 +160,7 @@ func (m NATPoliciesModel) Update(msg tea.Msg) (NATPoliciesModel, tea.Cmd) { } func (m NATPoliciesModel) updateFilter(msg tea.Msg) (NATPoliciesModel, tea.Cmd) { - base, exited, cmd := m.TableBase.HandleFilterMode(msg) + base, exited, cmd := m.HandleFilterMode(msg) m.TableBase = base if exited { m.applyFilter() diff --git a/internal/tui/views/policies.go b/internal/tui/views/policies.go index 5ae7c7c..a6ae464 100644 --- a/internal/tui/views/policies.go +++ b/internal/tui/views/policies.go @@ -158,7 +158,7 @@ func (m PoliciesModel) Update(msg tea.Msg) (PoliciesModel, tea.Cmd) { // Delegate to TableBase for common navigation visible := m.visibleRows() - base, handled, cmd := m.TableBase.HandleNavigation(msg, len(m.filtered), visible) + base, handled, cmd := m.HandleNavigation(msg, len(m.filtered), visible) if handled { m.TableBase = base return m, cmd @@ -169,7 +169,7 @@ func (m PoliciesModel) Update(msg tea.Msg) (PoliciesModel, tea.Cmd) { } func (m PoliciesModel) updateFilter(msg tea.Msg) (PoliciesModel, tea.Cmd) { - base, exited, cmd := m.TableBase.HandleFilterMode(msg) + base, exited, cmd := m.HandleFilterMode(msg) m.TableBase = base if exited { m.applyFilter() @@ -393,7 +393,7 @@ func (m PoliciesModel) renderDetail(p models.SecurityRule) string { b.WriteString("\n") b.WriteString(labelStyle.Render("Source Zones:") + " " + valueStyle.Render(formatListFull(p.SourceZones)) + "\n") b.WriteString(labelStyle.Render("Source Addr:") + " " + formatAddresses(p.Sources, p.NegateSource, valueStyle, dimValueStyle) + "\n") - if len(p.SourceUsers) > 0 && !(len(p.SourceUsers) == 1 && p.SourceUsers[0] == "any") { + if len(p.SourceUsers) > 0 && (len(p.SourceUsers) != 1 || p.SourceUsers[0] != "any") { b.WriteString(labelStyle.Render("Source Users:") + " " + valueStyle.Render(formatListFull(p.SourceUsers)) + "\n") } b.WriteString(labelStyle.Render("Dest Zones:") + " " + valueStyle.Render(formatListFull(p.DestZones)) + "\n") @@ -405,7 +405,7 @@ func (m PoliciesModel) renderDetail(p models.SecurityRule) string { b.WriteString("\n") b.WriteString(labelStyle.Render("Applications:") + " " + valueStyle.Render(formatListFull(p.Applications)) + "\n") b.WriteString(labelStyle.Render("Services:") + " " + valueStyle.Render(formatListFull(p.Services)) + "\n") - if len(p.URLCategories) > 0 && !(len(p.URLCategories) == 1 && p.URLCategories[0] == "any") { + if len(p.URLCategories) > 0 && (len(p.URLCategories) != 1 || p.URLCategories[0] != "any") { b.WriteString(labelStyle.Render("URL Categories:") + " " + valueStyle.Render(formatListFull(p.URLCategories)) + "\n") } diff --git a/internal/tui/views/sessions.go b/internal/tui/views/sessions.go index ce829da..8d585b9 100644 --- a/internal/tui/views/sessions.go +++ b/internal/tui/views/sessions.go @@ -143,7 +143,7 @@ func (m SessionsModel) Update(msg tea.Msg) (SessionsModel, tea.Cmd) { // Delegate to TableBase for common navigation visible := m.visibleRows() - base, handled, cmd := m.TableBase.HandleNavigation(msg, len(m.filtered), visible) + base, handled, cmd := m.HandleNavigation(msg, len(m.filtered), visible) if handled { m.TableBase = base return m, cmd @@ -154,7 +154,7 @@ func (m SessionsModel) Update(msg tea.Msg) (SessionsModel, tea.Cmd) { } func (m SessionsModel) updateFilter(msg tea.Msg) (SessionsModel, tea.Cmd) { - base, exited, cmd := m.TableBase.HandleFilterMode(msg) + base, exited, cmd := m.HandleFilterMode(msg) m.TableBase = base if exited { m.applyFilter() diff --git a/internal/tui/views/table_base_test.go b/internal/tui/views/table_base_test.go index d7b3590..10a7c41 100644 --- a/internal/tui/views/table_base_test.go +++ b/internal/tui/views/table_base_test.go @@ -184,15 +184,18 @@ func TestTableBase_HandleNavigation(t *testing.T) { tb := NewTableBase("") tb.Cursor = tt.cursor - msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(tt.key)} - if tt.key == "down" { + var msg tea.KeyMsg + switch tt.key { + case "down": msg = tea.KeyMsg{Type: tea.KeyDown} - } else if tt.key == "up" { + case "up": msg = tea.KeyMsg{Type: tea.KeyUp} - } else if tt.key == "home" { + case "home": msg = tea.KeyMsg{Type: tea.KeyHome} - } else if tt.key == "end" { + case "end": msg = tea.KeyMsg{Type: tea.KeyEnd} + default: + msg = tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(tt.key)} } result, handled, _ := tb.HandleNavigation(msg, tt.itemCount, tt.visibleRows) From f152359a393f8a5179fd8baa43493165a032a929 Mon Sep 17 00:00:00 2001 From: joshuamontgomery <24376525+jp2195@users.noreply.github.com> Date: Sat, 24 Jan 2026 20:25:45 -0500 Subject: [PATCH 5/6] fix(lint): resolve all golangci-lint errors - Fix prealloc by pre-allocating slices with known capacity --- internal/tui/views/dashboard_network.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/tui/views/dashboard_network.go b/internal/tui/views/dashboard_network.go index 29fe1dd..2cacd4a 100644 --- a/internal/tui/views/dashboard_network.go +++ b/internal/tui/views/dashboard_network.go @@ -264,7 +264,7 @@ func (m NetworkDashboardModel) renderARPSummary(width int) string { name string count int } - var counts []ifaceCount + counts := make([]ifaceCount, 0, len(ifaceCounts)) for name, count := range ifaceCounts { counts = append(counts, ifaceCount{name, count}) } From 50ebaa622e5c1f076ee1261b7ee750fc687de7f1 Mon Sep 17 00:00:00 2001 From: joshuamontgomery <24376525+jp2195@users.noreply.github.com> Date: Sat, 24 Jan 2026 20:29:42 -0500 Subject: [PATCH 6/6] fix(lint): add #nosec directives for standalone gosec compatibility The existing //nolint:gosec comments work with golangci-lint but are not recognized when gosec runs directly. Add #nosec directives to suppress intentional security exceptions in both tools. --- internal/api/client.go | 2 +- internal/auth/keygen.go | 2 +- internal/config/config.go | 2 +- internal/ssh/client.go | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 0ea5b14..490efb9 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -26,7 +26,7 @@ func WithInsecure(insecure bool) ClientOption { return func(c *Client) { if insecure { c.httpClient.Transport = &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // G402: InsecureSkipVerify required for self-signed firewall certificates when user enables --insecure + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // #nosec G402 -- InsecureSkipVerify required for self-signed firewall certificates when user enables --insecure } } } diff --git a/internal/auth/keygen.go b/internal/auth/keygen.go index c8d681d..b4e33e6 100644 --- a/internal/auth/keygen.go +++ b/internal/auth/keygen.go @@ -33,7 +33,7 @@ func GenerateAPIKey(ctx context.Context, host, username, password string, insecu client := &http.Client{ Timeout: 30 * time.Second, Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: insecure}, //nolint:gosec // G402: InsecureSkipVerify required for self-signed firewall certificates when user enables --insecure + TLSClientConfig: &tls.Config{InsecureSkipVerify: insecure}, //nolint:gosec // #nosec G402 -- InsecureSkipVerify required for self-signed firewall certificates when user enables --insecure }, } diff --git a/internal/config/config.go b/internal/config/config.go index d275637..f6436af 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -61,7 +61,7 @@ func Load() (*Config, error) { } configPath := filepath.Join(homeDir, ".pyre.yaml") - data, err := os.ReadFile(configPath) + data, err := os.ReadFile(configPath) // #nosec G304 -- Path is constructed from user's home directory if err != nil { if os.IsNotExist(err) { return cfg, nil diff --git a/internal/ssh/client.go b/internal/ssh/client.go index 2077fb4..8e7b73e 100644 --- a/internal/ssh/client.go +++ b/internal/ssh/client.go @@ -54,7 +54,7 @@ func NewClient(host string, cfg config.SSHConfig) (*Client, error) { // Try private key auth first if cfg.PrivateKeyPath != "" { keyPath := expandPath(cfg.PrivateKeyPath) - key, err := os.ReadFile(keyPath) + key, err := os.ReadFile(keyPath) // #nosec G304 -- Path is from user config, directory traversal not applicable if err != nil { return nil, fmt.Errorf("failed to read private key: %w", err) } @@ -282,7 +282,7 @@ func expandPath(path string) string { // Otherwise, it uses the known_hosts file for verification. func getHostKeyCallback(cfg config.SSHConfig) (ssh.HostKeyCallback, error) { if cfg.Insecure { - return ssh.InsecureIgnoreHostKey(), nil //nolint:gosec // G106: InsecureIgnoreHostKey used when user explicitly disables host key verification + return ssh.InsecureIgnoreHostKey(), nil //nolint:gosec // #nosec G106 -- InsecureIgnoreHostKey used when user explicitly disables host key verification } // Determine known_hosts path @@ -305,7 +305,7 @@ func getHostKeyCallback(cfg config.SSHConfig) (ssh.HostKeyCallback, error) { // Create known_hosts file if it doesn't exist if _, err := os.Stat(knownHostsPath); os.IsNotExist(err) { - f, err := os.OpenFile(knownHostsPath, os.O_CREATE|os.O_WRONLY, 0600) + f, err := os.OpenFile(knownHostsPath, os.O_CREATE|os.O_WRONLY, 0600) // #nosec G304 -- Path is user's .ssh directory or from config if err != nil { return nil, fmt.Errorf("creating known_hosts file: %w", err) } @@ -344,7 +344,7 @@ func getHostKeyCallback(cfg config.SSHConfig) (ssh.HostKeyCallback, error) { // addHostKey appends a host key to the known_hosts file. func addHostKey(knownHostsPath, hostname string, remote net.Addr, key ssh.PublicKey) error { - f, err := os.OpenFile(knownHostsPath, os.O_APPEND|os.O_WRONLY, 0600) + f, err := os.OpenFile(knownHostsPath, os.O_APPEND|os.O_WRONLY, 0600) // #nosec G304 -- Path is user's .ssh directory or from config if err != nil { return err }