golem15/summercms
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(bonfire): strip control characters from error blocks, process views and tables

Browse Source 
- bonfire.Sanitize (additive), moved from diary's console channel: tab
  kept, CR and LF to a space, every other control character dropped
- applied before styling to every error-block line (message,
  explanation, Try lines, -v chain), process-view item names and errors,
  the current item of spinner and progress, failure lines, and table
  headers and cells in both layouts
- diary's console handler calls bonfire.Sanitize; README and the
  writing-commands page say so (WR-05)
This commit is contained in:
Jakub Zych
2026-10-22 02:31:55 +02:00
parent 39677f1819
commit a9025c0d12
10 changed files with 181 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
docs/console/writing-commands.md
View File

@@ -152,7 +152,7 @@ The framework's own commands follow these rules, and so should a plugin's:
## Errors and hints
When `Run` returns an error, the binary prints a block on stderr: the ERROR badge and the message, then "Run with -v for details.". With `-v` the block lists the wrapped errors under "Caused by:" instead. Messages pass through `sunscreen.Scrub`, so a token inside an error text is not printed.
When `Run` returns an error, the binary prints a block on stderr: the ERROR badge and the message, then "Run with -v for details.". With `-v` the block lists the wrapped errors under "Caused by:" instead. Messages pass through `sunscreen.Scrub`, so a token inside an error text is not printed, and through `bonfire.Sanitize`, which drops control characters: an error text from a database or a remote service cannot move the cursor, clear the screen or retitle the terminal. Table cells and the item names and errors of `bonfire.Process` are cleaned the same way.
Wrap an error with `bonfire.WithHint` to explain it and to say what to run next. `{bin}` in the Try text becomes `bin/<name>` or `summer`, so the hint names the right program:

5
modules/bonfire/README.md
View File

@@ -27,7 +27,7 @@ Use cases:
- Usage strings and argument-count checks derived from the declared arguments (`<name>` 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.
- 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`).
- 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:
@@ -46,7 +46,7 @@ Use cases:
- Hidden commands (`bonfire.Command.Hidden`) run but stay out of the list unless `list --all`.
- JSON catalog: `list --format=json` prints every command, hidden ones flagged, with arguments, options, defaults and examples as a `bonfire.Listing` (`bonfire.Describe` builds it from a command slice).
- Documentation coverage: `bonfire.CheckDocs` reports commands lacking a description, long help, examples, or argument and option descriptions.
- Error block and exit codes: `bonfire.Execute` runs the root through fang and returns `bonfire.ExitOK`, `bonfire.ExitFailure`, or `bonfire.ExitUsage` for an unknown command or flag and a wrong number of arguments (`bonfire.UsageError`). Errors print on the error stream as a block (`bonfire.RenderError`): the ERROR badge and the message, the explanation and Try lines of `bonfire.WithHint` (`{bin}` becomes `bin/<name>` or `summer`), cobra's "did you mean" suggestions as Try lines, and "Run with -v for details." or, at `-v`, the wrapped error chain; every message passes through `sunscreen.Scrub`. Errors wrapped by `bonfire.Reported` print nothing. An error carrying a code through `bonfire.WithExitCode` exits with that code; summer uses it to exit with the code of an app binary it ran. `bonfire.Fail` prints errors that happen before the root exists. For an application binary (any root other than `summer`) that fails, other than with a usage error, while the working directory has no `config/` directory and a parent directory holds `summer.yaml`, both add the hint "This command runs from the application root." with `cd <relative path>; bin/<name> <args>` (only the message changes); the values of flags named like a password, secret, token or key are masked as `***` in that line.
- Error block and exit codes: `bonfire.Execute` runs the root through fang and returns `bonfire.ExitOK`, `bonfire.ExitFailure`, or `bonfire.ExitUsage` for an unknown command or flag and a wrong number of arguments (`bonfire.UsageError`). Errors print on the error stream as a block (`bonfire.RenderError`): the ERROR badge and the message, the explanation and Try lines of `bonfire.WithHint` (`{bin}` becomes `bin/<name>` or `summer`), cobra's "did you mean" suggestions as Try lines, and "Run with -v for details." or, at `-v`, the wrapped error chain; every message passes through `sunscreen.Scrub` and every line through `bonfire.Sanitize`, which strips control characters. Errors wrapped by `bonfire.Reported` print nothing. An error carrying a code through `bonfire.WithExitCode` exits with that code; summer uses it to exit with the code of an app binary it ran. `bonfire.Fail` prints errors that happen before the root exists. For an application binary (any root other than `summer`) that fails, other than with a usage error, while the working directory has no `config/` directory and a parent directory holds `summer.yaml`, both add the hint "This command runs from the application root." with `cd <relative path>; bin/<name> <args>` (only the message changes); the values of flags named like a password, secret, token or key are masked as `***` in that line.
- Name checks at boot: two commands with the same name, or a command named `list` or `help`, make `bonfire.NewRoot` fail with `bonfire.ErrDuplicateCommand`; a command flag that reuses a global option's name or shorthand fails with `bonfire.ErrReservedFlag` instead of panicking.
- Injectable streams (`bonfire.NewRootIO`, `bonfire.NewOutput`) so commands can be tested against buffers.
- In-process calls: `bonfire.Call` runs a named command with arguments against any writer (Laravel `Artisan::call`), and `bonfire.Catalog` holds an application binary's final command list so code outside the Cobra root, such as the conga scheduler, can call any registered command.
@@ -140,6 +140,7 @@ if catalog.Has("blog:import") {
| `bonfire.WithHint` | Wraps an error in a `bonfire.HintError`; `{bin}` in the Try text becomes `bin/<name>` or `summer`. |
| `bonfire.ErrReported` | Marks an error whose details the command already printed. |
| `bonfire.Reported` | Wraps an error so it matches `bonfire.ErrReported`: exit code 1, or the code of `bonfire.WithExitCode`, and no error block. |
| `bonfire.Sanitize` | Removes control characters other than tab (line breaks become spaces), so untrusted text cannot move the cursor or retitle the terminal; error blocks, process views, failure lines, tables and diary's console use it. |
| `bonfire.WithExitCode` | Makes an error carry the exit code `bonfire.Execute` returns for it (1 to 255; other codes give 1). |
| `bonfire.Listing` | The JSON catalog of `list --format=json`: `Application` (`bonfire.ApplicationInfo`), `GlobalOptions` and `Commands`. |
| `bonfire.ApplicationInfo` | Name, version and framework version of a binary in a `bonfire.Listing`. |

53
modules/bonfire/errors.go
View File

@@ -6,6 +6,7 @@ import (
"os"
"path/filepath"
"strings"
"unicode"
"git.golem15.com/golem15/summercms/modules/sunscreen"
"github.com/charmbracelet/colorprofile"
@@ -93,6 +94,47 @@ func Reported(err error) error {
return &reportedError{err: err}
}
// Sanitize removes control characters other than tab from s; carriage
// returns and line feeds become spaces, so a multi-line value stays on one
// line. Text from outside the operator's control (database or remote error
// text, names an administrator typed) then cannot move the cursor, clear
// the screen or set the window title. Error blocks, process views, failure
// lines and tables pass their text through it, and so does diary's console
// channel. A clean string is returned unchanged.
func Sanitize(s string) string {
clean := true
for _, r := range s {
if r != '\t' && unicode.IsControl(r) {
clean = false
break
}
}
if clean {
return s
}
var b strings.Builder
for _, r := range s {
switch {
case r == '\n' || r == '\r':
b.WriteByte(' ')
case r != '\t' && unicode.IsControl(r):
default:
b.WriteRune(r)
}
}
return b.String()
}
// sanitizeLines applies Sanitize to every line of s, keeping the line
// breaks.
func sanitizeLines(s string) string {
lines := strings.Split(s, "\n")
for i, line := range lines {
lines[i] = Sanitize(line)
}
return strings.Join(lines, "\n")
}
// exitCodeError carries the exit code Execute returns for an error.
type exitCodeError struct {
err error
@@ -121,7 +163,8 @@ func WithExitCode(err error, code int) error {
// -v for details." at verbosity 0 or the wrapped error chain under "Caused
// by:" at -v and above. cobra's unknown command error becomes `Command "x"
// is not defined.` with its suggestions as Try lines, and other usage errors
// suggest the command's help. Every message passes through sunscreen.Scrub.
// suggest the command's help. Every message passes through sunscreen.Scrub,
// and every line through Sanitize.
// Errors wrapped by Reported print nothing.
func RenderError(w io.Writer, name string, err error, g Globals) {
if err == nil || w == nil || errors.Is(err, ErrReported) {
@@ -164,7 +207,7 @@ func RenderError(w io.Writer, name string, err error, g Globals) {
var b strings.Builder
b.WriteString("\n")
for i, line := range strings.Split(sunscreen.Scrub(strings.TrimRight(message, "\n")), "\n") {
for i, line := range strings.Split(sanitizeLines(sunscreen.Scrub(strings.TrimRight(message, "\n"))), "\n") {
if i == 0 {
b.WriteString(" " + p.ErrorBold.Reverse(true).Render(" ERROR ") + " " + line + "\n")
continue
@@ -173,21 +216,21 @@ func RenderError(w io.Writer, name string, err error, g Globals) {
}
if explanation != "" {
b.WriteString("\n")
for _, line := range strings.Split(sunscreen.Scrub(explanation), "\n") {
for _, line := range strings.Split(sanitizeLines(sunscreen.Scrub(explanation)), "\n") {
b.WriteString(" " + line + "\n")
}
}
if try != "" {
b.WriteString("\n " + p.Header.Render("Try:") + "\n")
for _, line := range strings.Split(strings.ReplaceAll(try, "{bin}", bin), "\n") {
b.WriteString(" " + p.Success.Render(sunscreen.Scrub(line)) + "\n")
b.WriteString(" " + p.Success.Render(Sanitize(sunscreen.Scrub(line))) + "\n")
}
}
if g.Verbosity > 0 {
if chain := causes(err, err.Error()); len(chain) > 0 {
b.WriteString("\n " + p.Header.Render("Caused by:") + "\n")
for _, c := range chain {
b.WriteString(" " + p.Muted.Render("- "+sunscreen.Scrub(c)) + "\n")
b.WriteString(" " + p.Muted.Render("- "+sanitizeLines(sunscreen.Scrub(c))) + "\n")
}
}
} else {

61
modules/bonfire/errors_test.go
View File

@@ -262,3 +262,64 @@ func TestWrongDirectoryMasksSecrets(t *testing.T) {
}
}
}
// injected are terminal control sequences carried by untrusted text: an
// OSC window title and a CSI screen clear.
var injected = []string{"\x1b]0;pwned\x07", "\x1b[2J", "\x07"}
// carriesInjected reports which injected sequence s still holds.
func carriesInjected(s string) string {
for _, bad := range injected {
if strings.Contains(s, bad) {
return bad
}
}
return ""
}
// TestRenderErrorStripsControl (WR-05): with color on, control sequences
// in the message, explanation, Try lines and the -v chain never reach the
// terminal; the visible text stays.
func TestRenderErrorStripsControl(t *testing.T) {
plainEnv(t)
cause := errors.New("cause \x1b]0;pwned\x07 from the database\x1b[2J")
err := &HintError{
Err: fmt.Errorf("sync failed \x1b]0;pwned\x07 here: %w", cause),
Explanation: "the remote said \x1b[2J no",
Try: "{bin} sync --retry \x1b]0;pwned\x07",
}
var buf bytes.Buffer
RenderError(&buf, "acme", err, Globals{Color: ColorAlways, Verbosity: 1})
out := buf.String()
if bad := carriesInjected(out); bad != "" {
t.Errorf("error block carries %q:\n%q", bad, out)
}
if !strings.Contains(out, "\x1b[") {
t.Errorf("color is off, the test does not exercise styled output:\n%q", out)
}
for _, want := range []string{"sync failed", "the remote said", "sync --retry", "from the database"} {
if !strings.Contains(out, want) {
t.Errorf("error block lacks %q:\n%q", want, out)
}
}
}
// TestSanitize (WR-05): tab stays, CR and LF become spaces, every other
// control character (ESC, BEL, C0, DEL) is removed; clean text is returned
// unchanged. The rules are those of diary's console channel.
func TestSanitize(t *testing.T) {
for in, want := range map[string]string{
"": "",
"plain text ✓": "plain text ✓",
"a\tb": "a\tb",
"line one\nline two\rthree": "line one line two three",
"evil \x1b[31mred\x1b[0m\a bell": "evil [31mred[0m bell",
"a\x1b]0;title\x07b": "a]0;titleb",
"nul\x00 and del\x7f and \x01": "nul and del and ",
"boom\x1b[2Jclear": "boom[2Jclear",
} {
if got := Sanitize(in); got != want {
t.Errorf("Sanitize(%q) = %q, want %q", in, got, want)
}
}
}

2
modules/bonfire/helpers.go
View File

@@ -177,7 +177,7 @@ func Process(ctx context.Context, out Output, operation string, items []string,
}
if err := fn(ctx, item); err != nil {
errs = append(errs, failure(item, err))
out.Error(failureLine(item, err))
out.Error(failureText(item, err))
continue
}
out.Success(item)

15
modules/bonfire/live.go
View File

@@ -140,7 +140,7 @@ func (m spinnerModel) View() tea.View {
line += fmt.Sprintf(" %d/%d", m.done, m.total)
}
if m.current != "" {
line += " " + p.Muted.Render(m.current+"…")
line += " " + p.Muted.Render(Sanitize(m.current)+"…")
}
if m.failed > 0 {
line += " " + p.ErrorBold.Render(fmt.Sprintf("! %d failed", m.failed))
@@ -193,7 +193,7 @@ func (m progressModel) View() tea.View {
}
line := " " + m.bar.ViewAs(pct) + m.palette.Muted.Render(fmt.Sprintf(" %d/%d", m.done, m.total))
if m.current != "" && !m.finished {
line += " " + m.palette.Muted.Render(m.current)
line += " " + m.palette.Muted.Render(Sanitize(m.current))
}
if m.failed > 0 {
line += " " + m.palette.ErrorBold.Render(fmt.Sprintf("! %d failed", m.failed))
@@ -289,17 +289,18 @@ func (m processModel) View() tea.View {
var b strings.Builder
start, end := m.window()
for _, it := range m.items[start:end] {
name := Sanitize(it.name)
switch it.state {
case itemWaiting:
b.WriteString(p.Muted.Render(" "+symWaiting) + " " + p.Muted.Render(it.name))
b.WriteString(p.Muted.Render(" "+symWaiting) + " " + p.Muted.Render(name))
case itemRunning:
b.WriteString(" " + m.spin.View() + " " + it.name)
b.WriteString(" " + m.spin.View() + " " + name)
case itemDone:
b.WriteString(p.Success.Render(" "+symCheck) + " " + it.name)
b.WriteString(p.Success.Render(" "+symCheck) + " " + name)
case itemFailed:
b.WriteString(p.Error.Render(" "+symCross) + " " + it.name)
b.WriteString(p.Error.Render(" "+symCross) + " " + name)
if it.err != nil {
b.WriteString(" " + p.Error.Render(it.err.Error()))
b.WriteString(" " + p.Error.Render(Sanitize(it.err.Error())))
}
}
b.WriteString("\n")

19
modules/bonfire/live_test.go
View File

@@ -530,3 +530,22 @@ func TestLiveViewPanicUnregisters(t *testing.T) {
t.Errorf("the next spinner did not run as a live view:\n%s", ansi.Strip(next.String()))
}
}
// TestProcessViewStripsControl (WR-05): an item name and an item error
// carrying control sequences render without them.
func TestProcessViewStripsControl(t *testing.T) {
name := "evil\x1b]0;pwned\x07item"
var m tea.Model = newProcessModel(DefaultPalette(), "Pushing", []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})
out := m.View().Content
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"} {
if !strings.Contains(ansi.Strip(out), want) {
t.Errorf("process view lacks %q:\n%q", want, out)
}
}
}

17
modules/bonfire/widgets.go
View File

@@ -96,7 +96,7 @@ func (p *progressBar) Advance(step int) {
func (p *progressBar) Current(string) {}
func (p *progressBar) Fail(item string, err error) {
p.c.Error(failureLine(item, err))
p.c.Error(failureText(item, err))
}
// quietProgress prints nothing but failures (-q).
@@ -109,7 +109,7 @@ func (p *quietProgress) Advance(int) {}
func (p *quietProgress) Current(string) {}
func (p *quietProgress) Fail(item string, err error) {
p.c.Error(failureLine(item, err))
p.c.Error(failureText(item, err))
}
// liveProgress drives the live progress model.
@@ -135,7 +135,7 @@ func (p *liveProgress) Current(item string) {
func (p *liveProgress) Fail(item string, err error) {
p.send(itemMsg{done: p.current, total: p.total, item: item, err: err})
p.send(printLine(failureLine(item, err)))
p.send(printLine(failureText(item, err)))
}
func failureLine(item string, err error) string {
@@ -145,7 +145,14 @@ func failureLine(item string, err error) string {
return item + ": " + err.Error()
}
// failureText is the failure line as printed: one line without control
// characters.
func failureText(item string, err error) string {
return Sanitize(failureLine(item, err))
}
func (c *console) Table(headers []string, rows [][]string) {
headers = padRow(headers, len(headers))
if !c.tty {
c.Println(strings.Join(headers, "\t"))
for _, row := range rows {
@@ -171,11 +178,13 @@ func (c *console) Table(headers []string, rows [][]string) {
c.emit(c.out, t.String()+"\n")
}
// padRow returns n cells of row (missing cells empty), each passed through
// Sanitize so a cell cannot move the cursor or retitle the terminal.
func padRow(row []string, n int) []string {
out := make([]string, n)
for i := 0; i < n; i++ {
if i < len(row) {
out[i] = row[i]
out[i] = Sanitize(row[i])
}
}
return out

23
modules/bonfire/widgets_test.go
View File

@@ -295,3 +295,26 @@ func TestPrintAbove(t *testing.T) {
t.Errorf("the spinner did not run as a live view:\n%s", got)
}
}
// TestTableStripsControl (WR-05): table headers and cells carrying control
// sequences print without them, boxed with color on a terminal and as
// tab-separated lines off one.
func TestTableStripsControl(t *testing.T) {
headers := []string{"Name\x1b]0;pwned\x07", "Scope"}
rows := [][]string{{"token \x1b[2J one", "journal\x07.posts"}}
for _, tty := range []bool{true, false} {
var out bytes.Buffer
c := newConsole(strings.NewReader(""), &out, &out, noEnv, boolPtr(tty), boolPtr(tty))
c.setGlobals(Globals{Color: ColorAlways})
c.Table(headers, rows)
got := out.String()
if bad := carriesInjected(got); bad != "" {
t.Errorf("terminal %v: table carries %q:\n%q", tty, bad, got)
}
for _, want := range []string{"Name", "token", "one", "journal", "posts"} {
if !strings.Contains(got, want) {
t.Errorf("terminal %v: table lacks %q:\n%q", tty, want, got)
}
}
}
}

35
modules/diary/console.go
View File

@@ -12,7 +12,6 @@ import (
"sync"
"sync/atomic"
"time"
"unicode"
"charm.land/lipgloss/v2"
"git.golem15.com/golem15/summercms/modules/bonfire"
@@ -141,12 +140,12 @@ func (h *consoleHandler) Handle(ctx context.Context, r slog.Record) error {
label, style := levelLabel(r.Level, p)
b.WriteString(style.Render(label))
b.WriteString(strings.Repeat(" ", max(0, 5-len(label))+1))
name := sanitize(component)
name := bonfire.Sanitize(component)
if name != "" {
b.WriteString(p.Header.Render(name))
}
b.WriteString(strings.Repeat(" ", max(0, componentWidth-len(name))+1))
b.WriteString(sanitize(r.Message))
b.WriteString(bonfire.Sanitize(r.Message))
for _, list := range [][]consoleAttr{h.attrs, recAttrs} {
for _, a := range list {
b.WriteByte(' ')
@@ -247,7 +246,7 @@ func appendAttr(out []consoleAttr, prefix string, a slog.Attr) []consoleAttr {
}
return out
}
return append(out, consoleAttr{key: sanitize(prefix + a.Key), value: formatValue(a.Value)})
return append(out, consoleAttr{key: bonfire.Sanitize(prefix + a.Key), value: formatValue(a.Value)})
}
func formatValue(v slog.Value) string {
@@ -266,35 +265,9 @@ func formatValue(v slog.Value) string {
default:
s = v.String()
}
s = sanitize(s)
s = bonfire.Sanitize(s)
if s == "" || strings.ContainsAny(s, " \t\"=") {
return strconv.Quote(s)
}
return s
}
// sanitize removes control characters other than tab; line breaks become
// spaces so a multi-line value stays on one line.
func sanitize(s string) string {
clean := true
for _, r := range s {
if r != '\t' && unicode.IsControl(r) {
clean = false
break
}
}
if clean {
return s
}
var b strings.Builder
for _, r := range s {
switch {
case r == '\n' || r == '\r':
b.WriteByte(' ')
case r != '\t' && unicode.IsControl(r):
default:
b.WriteRune(r)
}
}
return b.String()
}