From 48e13f9b5734090af816cffef26a4ba957700346 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 22 Oct 2026 03:52:37 +0200 Subject: [PATCH] fix(bonfire): strip control characters from Process's plain item lines and the view's label (WR-14) --- modules/bonfire/README.md | 2 +- modules/bonfire/helpers.go | 2 +- modules/bonfire/live.go | 2 +- modules/bonfire/live_test.go | 43 ++++++++++++++++++++++++++++++++---- 4 files changed, 42 insertions(+), 7 deletions(-) diff --git a/modules/bonfire/README.md b/modules/bonfire/README.md index c2323aab..d8a7aac2 100644 --- a/modules/bonfire/README.md +++ b/modules/bonfire/README.md @@ -27,7 +27,7 @@ Use cases: - Usage strings and argument-count checks derived from the declared arguments (`` for required, `[name]` for optional). - Scalar flags, bare flags (`bonfire.Flag.Bare`, so `--force` alone stores `true`) and ordered repeatable flags (`bonfire.Flag.Repeatable`, read back through `bonfire.Input.Flags`). - Styled status lines: `bonfire.Output.Info`, `bonfire.Output.Success`, `bonfire.Output.Warning` and `bonfire.Output.Error` (the last one writes to the error stream). -- Widgets on the charm stack, ported from SSU's terminal UI: box-drawn tables (`bonfire.Output.Table`, lipgloss), a spinner around a function (`bonfire.Output.Spinner`), a gradient progress bar (`bonfire.Output.Progress`, whose `bonfire.Progress` also implements `bonfire.ItemProgress`) and a multi-row process view for bulk loops (`bonfire.Process`), each a bubbletea program that keeps stdin and signals to the command: Ctrl+C or SIGTERM during a view restores the cursor and the terminal, then stops the command as before (or reaches its own signal handler). Log lines, and output the command prints through the console that started the view, appear above a running view (`bonfire.PrintAbove`); other consoles (a `bonfire.Call` buffer, a console on a file) keep writing to their own writers. Table cells, process-view item names and errors, and failure lines are stripped of control characters (`bonfire.Sanitize`). +- Widgets on the charm stack, ported from SSU's terminal UI: box-drawn tables (`bonfire.Output.Table`, lipgloss), a spinner around a function (`bonfire.Output.Spinner`), a gradient progress bar (`bonfire.Output.Progress`, whose `bonfire.Progress` also implements `bonfire.ItemProgress`) and a multi-row process view for bulk loops (`bonfire.Process`), each a bubbletea program that keeps stdin and signals to the command: Ctrl+C or SIGTERM during a view restores the cursor and the terminal, then stops the command as before (or reaches its own signal handler). Log lines, and output the command prints through the console that started the view, appear above a running view (`bonfire.PrintAbove`); other consoles (a `bonfire.Call` buffer, a console on a file) keep writing to their own writers. Table cells, the item names and errors of `bonfire.Process` (in the process view and in its plain lines), the process view's operation label, and failure lines are stripped of control characters (`bonfire.Sanitize`). - Prompts: `bonfire.Output.Ask`, `bonfire.Output.Confirm` and `bonfire.Output.Secret` use huh fields on an interactive terminal, and `bonfire.Output.Choice` an arrow-key selector with a confirmation screen. Off a terminal they read lines from stdin as before (a piped password still works), return their defaults when input ends, and `bonfire.Output.Confirm` returns its default without asking. A cancelled prompt returns `bonfire.ErrAborted`. `bonfire.ConfirmProceed` asks before a destructive step only on an interactive terminal. - Degradation: every widget follows the terminal and the global options, and the `bonfire.Output` interface is unchanged for plugins: diff --git a/modules/bonfire/helpers.go b/modules/bonfire/helpers.go index f3548c8f..abce5fcb 100644 --- a/modules/bonfire/helpers.go +++ b/modules/bonfire/helpers.go @@ -180,7 +180,7 @@ func Process(ctx context.Context, out Output, operation string, items []string, out.Error(failureText(item, err)) continue } - out.Success(item) + out.Success(Sanitize(item)) } return errors.Join(errs...) } diff --git a/modules/bonfire/live.go b/modules/bonfire/live.go index 788d45f7..f9939773 100644 --- a/modules/bonfire/live.go +++ b/modules/bonfire/live.go @@ -310,7 +310,7 @@ func (m processModel) View() tea.View { if total > 0 { pct = float64(m.done) / float64(total) } - op := m.operation + op := Sanitize(m.operation) if op == "" { op = "Processing" } diff --git a/modules/bonfire/live_test.go b/modules/bonfire/live_test.go index b9e845b9..43a796ab 100644 --- a/modules/bonfire/live_test.go +++ b/modules/bonfire/live_test.go @@ -531,11 +531,13 @@ func TestLiveViewPanicUnregisters(t *testing.T) { } } -// TestProcessViewStripsControl (WR-05): an item name and an item error -// carrying control sequences render without them. +// TestProcessViewStripsControl (WR-05, WR-14): an item name, an item error +// and the operation label carrying control sequences render without them, +// in the live view and on the plain path (-n on a terminal, forced color +// off a terminal). func TestProcessViewStripsControl(t *testing.T) { name := "evil\x1b]0;pwned\x07item" - var m tea.Model = newProcessModel(DefaultPalette(), "Pushing", []string{name, "next"}) + var m tea.Model = newProcessModel(DefaultPalette(), "Push\x1b]0;pwned\x07ing", []string{name, "next"}) m, _ = m.Update(tea.WindowSizeMsg{Width: 120, Height: 24}) m, _ = m.Update(stepMsg{item: name, state: itemFailed, err: errors.New("rejected \x1b[2J by the remote"), done: 1}) m, _ = m.Update(stepMsg{item: "next", state: itemRunning, done: 1}) @@ -543,9 +545,42 @@ func TestProcessViewStripsControl(t *testing.T) { if bad := carriesInjected(out); bad != "" { t.Errorf("process view carries %q:\n%q", bad, out) } - for _, want := range []string{"evil", "item", "rejected", "by the remote"} { + for _, want := range []string{"evil", "item", "rejected", "by the remote", "Push", "ing"} { if !strings.Contains(ansi.Strip(out), want) { t.Errorf("process view lacks %q:\n%q", want, out) } } + + // The plain path: no live view, a colored writer that passes control + // sequences through (TestTableStripsControl). + for _, tc := range []struct { + name string + tty bool + globals Globals + }{ + {"terminal with -n", true, Globals{Color: ColorAlways, NoInteraction: true}}, + {"no terminal, color forced", false, Globals{Color: ColorAlways}}, + } { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + c := newConsole(strings.NewReader(""), &buf, &buf, noEnv, boolPtr(tc.tty), boolPtr(tc.tty)) + c.setGlobals(tc.globals) + if c.live() { + t.Fatal("the console runs live views; the plain path is not exercised") + } + err := Process(context.Background(), c, "Push\x1b]0;pwned\x07ing", []string{name}, func(context.Context, string) error { return nil }) + if err != nil { + t.Fatal(err) + } + got := buf.String() + if bad := carriesInjected(got); bad != "" { + t.Errorf("plain Process line carries %q:\n%q", bad, got) + } + for _, want := range []string{"evil", "item"} { + if !strings.Contains(ansi.Strip(got), want) { + t.Errorf("plain Process line lacks %q:\n%q", want, got) + } + } + }) + } }