From edc02d48d4ff767309e7d590e833fccd255e2fd0 Mon Sep 17 00:00:00 2001 From: Stefan Koelle Date: Sun, 9 Aug 2026 19:23:37 +0200 Subject: [PATCH] Phase 2 hardening: binary secret handling, tool checks, input validation, verbose flag, diff scroll --- cmd/kctl-tui/full.go | 15 ++++++++ cmd/kctl-tui/main.go | 22 ++++++++++-- cmd/kctl-tui/panel.go | 67 ++++++++++++++++++++++++++++++++--- internal/kctl/diff.go | 33 ++++++++++++++--- internal/kctl/diff_test.go | 40 +++++++++++++++++++++ internal/kubeexec/kubeexec.go | 19 ++++++++-- internal/kubeexec/log.go | 56 +++++++++++++++++++++++++++++ 7 files changed, 238 insertions(+), 14 deletions(-) create mode 100644 internal/kubeexec/log.go diff --git a/cmd/kctl-tui/full.go b/cmd/kctl-tui/full.go index 10ea1b6..c928555 100644 --- a/cmd/kctl-tui/full.go +++ b/cmd/kctl-tui/full.go @@ -67,6 +67,9 @@ func (m *fullModel) bootstrap() tea.Msg { if len(cfg.Envs) == 0 { return errMsg{fmt.Errorf("no 'envs' configured in ~/.kctl-tui/config.yaml (see config.example.yaml)")} } + if err := kubeexec.CheckTool("kubectl"); err != nil { + return errMsg{err} + } return bootstrapMsg{cfg: cfg, context: cfg.EffectiveDefaultContext()} } @@ -254,6 +257,18 @@ func (m *fullModel) loadNamespacesFor(teamValue string) tea.Cmd { // configured envs, resolved via the context template, so both are // visible side by side. func (m *fullModel) startTmuxSession() tea.Cmd { + return func() tea.Msg { + if err := kubeexec.CheckTool("tmux"); err != nil { + return errMsg{err} + } + if err := kubeexec.CheckTool("k9s"); err != nil { + return errMsg{err} + } + return m.buildAndRunTmux() + } +} + +func (m *fullModel) buildAndRunTmux() tea.Msg { selfPath := "kctl-tui" // resolved via PATH; see README for install instructions panelCmd := fmt.Sprintf("%s panel --context=%s --ns=%s --team=%s", selfPath, m.selectedContext, m.selectedNamespace, m.selectedTeam) diff --git a/cmd/kctl-tui/main.go b/cmd/kctl-tui/main.go index 56b5bb3..5fba0c3 100644 --- a/cmd/kctl-tui/main.go +++ b/cmd/kctl-tui/main.go @@ -5,11 +5,29 @@ import ( "os" tea "github.com/charmbracelet/bubbletea" + + "github.com/skoelle/kctl-tui/internal/kubeexec" ) func main() { - if len(os.Args) > 1 && os.Args[1] == "panel" { - if err := runPanel(os.Args[2:]); err != nil { + args := os.Args[1:] + + // Extract --verbose before delegating to panel or full mode. + verbose := false + filtered := make([]string, 0, len(args)) + for _, a := range args { + if a == "--verbose" { + verbose = true + } else { + filtered = append(filtered, a) + } + } + if verbose { + kubeexec.SetVerbose(true, os.Stderr) + } + + if len(filtered) > 0 && filtered[0] == "panel" { + if err := runPanel(filtered[1:]); err != nil { fmt.Fprintln(os.Stderr, "kctl-tui panel error:", err) os.Exit(1) } diff --git a/cmd/kctl-tui/panel.go b/cmd/kctl-tui/panel.go index c04d31d..9d5bbfc 100644 --- a/cmd/kctl-tui/panel.go +++ b/cmd/kctl-tui/panel.go @@ -51,6 +51,7 @@ type panelModel struct { awsValues map[string]string k8sValues map[string]string diffEntries []kctl.SecretDiffEntry + diffOffset int // scroll position for diff table message string err error @@ -144,6 +145,14 @@ func (m *panelModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m.handleEsc() case "enter": return m.handleEnter() + case "up", "k": + if m.step == stepDiffResult { + return m.scrollDiff(-1) + } + case "down", "j": + if m.step == stepDiffResult { + return m.scrollDiff(1) + } } } @@ -255,6 +264,9 @@ func (m *panelModel) fromActionMenu() (tea.Model, tea.Cmd) { // interactively instead of letting the user hit a confusing failure // several steps later. func (m *panelModel) checkAWSAuthAndProceed() (tea.Model, tea.Cmd) { + if err := kubeexec.CheckTool("aws"); err != nil { + return m.showError(err) + } if err := kubeexec.CheckAWSAuth(); err != nil { m.err = err m.list.SetItems([]list.Item{ @@ -363,7 +375,8 @@ func (m *panelModel) compareAllFields() (tea.Model, tea.Cmd) { } m.k8sValues = k8sValues m.diffEntries = diffSecretValues(m.awsValues, m.k8sValues) - m.message = renderDiffTable(m.currentEnv, m.awsSecretName, m.k8sSecretName, m.diffEntries) + m.diffOffset = 0 + m.message = renderDiffTable(m.currentEnv, m.awsSecretName, m.k8sSecretName, m.diffEntries, m.diffOffset, 0) if anyMismatch(m.diffEntries) { m.list.SetItems([]list.Item{ @@ -378,16 +391,52 @@ func (m *panelModel) compareAllFields() (tea.Model, tea.Cmd) { return m, nil } -func renderDiffTable(env, awsSecretName, k8sSecretName string, entries []kctl.SecretDiffEntry) string { +func (m *panelModel) scrollDiff(delta int) (tea.Model, tea.Cmd) { + newOff := m.diffOffset + delta + if newOff < 0 { + newOff = 0 + } + maxOff := len(m.diffEntries) - 1 + if maxOff < 0 { + maxOff = 0 + } + if newOff > maxOff { + newOff = maxOff + } + m.diffOffset = newOff + m.message = renderDiffTable(m.currentEnv, m.awsSecretName, m.k8sSecretName, m.diffEntries, m.diffOffset, 0) + return m, nil +} + +func renderDiffTable(env, awsSecretName, k8sSecretName string, entries []kctl.SecretDiffEntry, offset, visibleHeight int) string { var b strings.Builder fmt.Fprintf(&b, "env: %s AWS secret: %s Kubernetes secret: %s\n\n", env, awsSecretName, k8sSecretName) fmt.Fprintf(&b, "%-25s %-20s %-20s %s\n", "KEY", "AWS", "KUBERNETES", "STATUS") - for _, e := range entries { + start := offset + if start > len(entries) { + start = len(entries) + } + end := len(entries) + if visibleHeight > 0 && start+visibleHeight < end { + end = start + visibleHeight + } + for _, e := range entries[start:end] { status := "OK" if !e.Match { status = "MISMATCH" } - fmt.Fprintf(&b, "%-25s %-20s %-20s %s\n", e.Key, truncate(e.Left, 20), truncate(e.Right, 20), status) + left := e.Left + if e.LeftBin { + left = fmt.Sprintf("", len(e.Left)) + } + right := e.Right + if e.RightBin { + right = fmt.Sprintf("", len(e.Right)) + } + fmt.Fprintf(&b, "%-25s %-20s %-20s %s\n", e.Key, truncate(left, 20), truncate(right, 20), status) + } + if len(entries) > 0 { + fmt.Fprintf(&b, "\n Showing %d-%d of %d fields (j/k or arrow keys to scroll)", start+1, end, len(entries)) } return b.String() } @@ -415,7 +464,15 @@ func (m *panelModel) fromForceSyncConfirm() (tea.Model, tea.Cmd) { } func (m *panelModel) doForceSync() (tea.Model, tea.Cmd) { - name := m.input.Value() + name := strings.TrimSpace(m.input.Value()) + if name == "" { + return m.showError(fmt.Errorf("ExternalSecret name must not be empty")) + } + for _, r := range name { + if r < 0x20 || r > 0x7e || r == '/' || r == ' ' { + return m.showError(fmt.Errorf("ExternalSecret name contains invalid character: %q", r)) + } + } ts := time.Now().Unix() _, err := kubeexec.AnnotateForceSync(m.resolvedContext(), m.ns, name, ts) if err != nil { diff --git a/internal/kctl/diff.go b/internal/kctl/diff.go index afb5ecb..9479ac8 100644 --- a/internal/kctl/diff.go +++ b/internal/kctl/diff.go @@ -5,10 +5,27 @@ import "sort" // SecretDiffEntry represents the comparison of one key between two secret // sources (e.g. AWS Secrets Manager vs. a Kubernetes Secret). type SecretDiffEntry struct { - Key string - Left string // e.g. the AWS Secrets Manager value - Right string // e.g. the decoded Kubernetes secret value - Match bool + Key string + Left string // e.g. the AWS Secrets Manager value + Right string // e.g. the decoded Kubernetes secret value + Match bool + LeftBin bool // true if Left contains non-printable (binary) data + RightBin bool // true if Right contains non-printable (binary) data +} + +// IsBinary reports whether s contains non-printable bytes (i.e. is likely +// binary data rather than human-readable text). Control characters below +// space (0x20) are excluded, except for common whitespace (\t, \n, \r). +func IsBinary(s string) bool { + for _, r := range s { + if r > 0x7f { + return true + } + if r < 0x20 && r != '\t' && r != '\n' && r != '\r' { + return true + } + } + return false } // DiffSecretValues compares two key/value maps and returns a sorted list of @@ -35,7 +52,13 @@ func DiffSecretValues(left, right map[string]string) []SecretDiffEntry { for _, k := range keys { l := left[k] r := right[k] - result = append(result, SecretDiffEntry{Key: k, Left: l, Right: r, Match: l == r}) + lb := IsBinary(l) + rb := IsBinary(r) + match := l == r + if lb || rb { + match = l == r + } + result = append(result, SecretDiffEntry{Key: k, Left: l, Right: r, Match: match, LeftBin: lb, RightBin: rb}) } return result } diff --git a/internal/kctl/diff_test.go b/internal/kctl/diff_test.go index 59aaf63..11937af 100644 --- a/internal/kctl/diff_test.go +++ b/internal/kctl/diff_test.go @@ -57,3 +57,43 @@ func TestDiffSecretValues_EmptyMaps(t *testing.T) { t.Fatalf("expected no mismatch for empty maps") } } + +func TestIsBinary_Plaintext(t *testing.T) { + if IsBinary("hello world") { + t.Fatal("expected plaintext to not be binary") + } + if IsBinary("line1\nline2\ttab") { + t.Fatal("expected newline/tab to not be binary") + } +} + +func TestIsBinary_BinaryData(t *testing.T) { + if !IsBinary("hello\x00world") { + t.Fatal("expected null byte to be binary") + } + if !IsBinary("key=\xff\xfe") { + t.Fatal("expected non-ASCII bytes to be binary") + } +} + +func TestDiffSecretValues_BinaryDetection(t *testing.T) { + left := map[string]string{"ok": "text", "bin": "data\x00here"} + right := map[string]string{"ok": "text", "bin": "data\x00here"} + + entries := DiffSecretValues(left, right) + for _, e := range entries { + if e.Key == "bin" { + if !e.LeftBin || !e.RightBin { + t.Fatalf("expected binary flags set for key 'bin', got LeftBin=%v RightBin=%v", e.LeftBin, e.RightBin) + } + if !e.Match { + t.Fatalf("expected binary values to match") + } + } + if e.Key == "ok" { + if e.LeftBin || e.RightBin { + t.Fatalf("expected binary flags unset for key 'ok'") + } + } + } +} diff --git a/internal/kubeexec/kubeexec.go b/internal/kubeexec/kubeexec.go index b6ee7e5..d72667c 100644 --- a/internal/kubeexec/kubeexec.go +++ b/internal/kubeexec/kubeexec.go @@ -19,13 +19,16 @@ import ( ) func runOutput(name string, args ...string) (string, error) { + logCmd(name, args...) cmd := exec.Command(name, args...) var stdout, stderr strings.Builder cmd.Stdout = &stdout cmd.Stderr = &stderr err := cmd.Run() if err != nil { - msg := fmt.Sprintf("%s %s failed: %v", name, strings.Join(args, " "), err) + logErr(name, err) + prefix := fmt.Sprintf("%s %s failed: %v", name, strings.Join(args, " "), err) + msg := prefix if s := strings.TrimSpace(stderr.String()); s != "" { msg += "\n" + s } @@ -34,7 +37,9 @@ func runOutput(name string, args ...string) (string, error) { } return "", fmt.Errorf("%s", msg) } - return strings.TrimSpace(stdout.String()), nil + out := strings.TrimSpace(stdout.String()) + logOutput(name, out) + return out, nil } // kubectlArgs prepends a --context flag when context is non-empty. @@ -172,3 +177,13 @@ func RunAWSLogin(loginCommand string) *exec.Cmd { } return exec.Command(parts[0], parts[1:]...) } + +// CheckTool verifies that a named executable is available in PATH. +// Returns nil if found, or a descriptive error if not. +func CheckTool(name string) error { + _, err := exec.LookPath(name) + if err != nil { + return fmt.Errorf("%q not found in PATH — please install it first", name) + } + return nil +} diff --git a/internal/kubeexec/log.go b/internal/kubeexec/log.go new file mode 100644 index 0000000..359d4f7 --- /dev/null +++ b/internal/kubeexec/log.go @@ -0,0 +1,56 @@ +package kubeexec + +import ( + "fmt" + "io" + "strings" + "sync" +) + +var ( + verbose bool + logOut io.Writer = io.Discard + mu sync.Mutex +) + +// SetVerbose enables or disables debug logging of executed commands. +// When enabled, commands and their outputs are written to the provided +// writer (typically os.Stderr). When disabled (the default), all logging +// is discarded. +func SetVerbose(enabled bool, w io.Writer) { + mu.Lock() + defer mu.Unlock() + verbose = enabled + if w != nil { + logOut = w + } +} + +func logCmd(name string, args ...string) { + mu.Lock() + defer mu.Unlock() + if !verbose { + return + } + fmt.Fprintf(logOut, "[cmd] %s %s\n", name, strings.Join(args, " ")) +} + +func logOutput(name string, output string) { + mu.Lock() + defer mu.Unlock() + if !verbose { + return + } + if output != "" { + fmt.Fprintf(logOut, "[out] %s: %s\n", name, output) + } +} + +func logErr(name string, err error) { + mu.Lock() + defer mu.Unlock() + if !verbose { + return + } + fmt.Fprintf(logOut, "[err] %s: %v\n", name, err) +}