From 005a4c1c4d37b3ea344c6ed1ad8fa8e2985320c1 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Fri, 18 Sep 2026 09:53:43 -0500 Subject: [PATCH 1/3] fix(output): render values as visible text in human output Values coming back from the API are arbitrary text; table cells, human key/value output, browser rows and detail fields, and select option labels now drop non-printing characters before rendering. --json output is unchanged (the encoder already escapes losslessly). Co-Authored-By: Claude Fable 5 --- internal/output/output.go | 15 ++++-- internal/output/sanitize.go | 28 ++++++++++ internal/output/sanitize_test.go | 73 +++++++++++++++++++++++++++ internal/tui/browser.go | 13 +++-- internal/tui/browser_sanitize_test.go | 21 ++++++++ internal/tui/prompt_rail.go | 6 +-- 6 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 internal/output/sanitize.go create mode 100644 internal/output/sanitize_test.go create mode 100644 internal/tui/browser_sanitize_test.go diff --git a/internal/output/output.go b/internal/output/output.go index 7533c1a8..d984146a 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -182,7 +182,7 @@ func (r *Renderer) renderHuman(v any) error { var m map[string]json.RawMessage if err := json.Unmarshal(raw, &m); err != nil { // Not an object (array/scalar): print compactly. - fmt.Fprintln(r.stdout, humanValue(raw)) + fmt.Fprintln(r.stdout, Sanitize(humanValue(raw))) return nil } keys := humanKeyOrder(m) @@ -193,7 +193,7 @@ func (r *Renderer) renderHuman(v any) error { } } for _, k := range keys { - fmt.Fprintf(r.stdout, "%s %s\n", r.style(r.dim, padRight(k, width)), humanFieldValue(k, m[k])) + fmt.Fprintf(r.stdout, "%s %s\n", r.style(r.dim, padRight(Sanitize(k), width)), Sanitize(humanFieldValue(k, m[k]))) } return nil } @@ -322,11 +322,18 @@ func (r *Renderer) RenderTable(t Table) error { fmt.Fprintln(r.stderr, r.style(r.info, "• ")+"no results") return nil } + rows := make([][]string, len(t.Rows)) + for ri, row := range t.Rows { + rows[ri] = make([]string, len(row)) + for i, cell := range row { + rows[ri][i] = Sanitize(cell) + } + } widths := make([]int, len(t.Columns)) for i, c := range t.Columns { widths[i] = len(c) } - for _, row := range t.Rows { + for _, row := range rows { for i, cell := range row { if i >= len(widths) { continue @@ -344,7 +351,7 @@ func (r *Renderer) RenderTable(t Table) error { fmt.Fprint(r.stdout, r.style(headerStyle, padRight(c, widths[i]))) } fmt.Fprintln(r.stdout) - for _, row := range t.Rows { + for _, row := range rows { for i, cell := range row { if i > 0 { fmt.Fprint(r.stdout, " ") diff --git a/internal/output/sanitize.go b/internal/output/sanitize.go new file mode 100644 index 00000000..f19600b9 --- /dev/null +++ b/internal/output/sanitize.go @@ -0,0 +1,28 @@ +package output + +import "strings" + +// Sanitize strips control characters (C0 except \n and \t, DEL, and C1) from a +// value before it is rendered as human output. API values are arbitrary text +// and must render as visible characters only — they must never be able to move +// the cursor or address the terminal. +func Sanitize(s string) string { + if !strings.ContainsFunc(s, isControlRune) { + return s + } + var b strings.Builder + b.Grow(len(s)) + for _, r := range s { + if !isControlRune(r) { + b.WriteRune(r) + } + } + return b.String() +} + +func isControlRune(r rune) bool { + if r == '\n' || r == '\t' { + return false + } + return r < 0x20 || r == 0x7F || (r >= 0x80 && r <= 0x9F) +} diff --git a/internal/output/sanitize_test.go b/internal/output/sanitize_test.go new file mode 100644 index 00000000..963bc60b --- /dev/null +++ b/internal/output/sanitize_test.go @@ -0,0 +1,73 @@ +package output + +import ( + "bytes" + "strings" + "testing" +) + +func TestSanitize(t *testing.T) { + cases := map[string]string{ + "plain-id_1.2": "plain-id_1.2", + "keep\nnewlines\tand tabs": "keep\nnewlines\tand tabs", + "esc\x1b]52;c;UkNCQg==\x07seq": "esc]52;c;UkNCQg==seq", + "csi\x1b[31mred\x1b[0m": "csi[31mred[0m", + "bell\x07 backspace\x08 del\x7f": "bell backspace del", + "c1\u0085dev\u009bice\u0090str": "c1devicestr", + "unicode üñî remains": "unicode üñî remains", + "\x00\x01\x02only-controls\x1f": "only-controls", + "carriage\rreturn": "carriagereturn", + } + for in, want := range cases { + if got := Sanitize(in); got != want { + t.Errorf("Sanitize(%q) = %q, want %q", in, got, want) + } + } +} + +func TestRenderTable_ValuesRenderAsVisibleText(t *testing.T) { + var out, errBuf bytes.Buffer + r := NewRenderer(&out, &errBuf, false, true, false, "") + err := r.RenderTable(Table{ + Columns: []string{"ID", "NAME"}, + Rows: [][]string{ + {"cus_\x1b]52;c;UkNCQg==\x07x", "a\rb"}, + }, + }) + if err != nil { + t.Fatal(err) + } + if s := out.String(); strings.ContainsAny(s, "\x1b\x07\r") { + t.Errorf("table output contains raw control bytes: %q", s) + } +} + +func TestRenderHuman_ValuesRenderAsVisibleText(t *testing.T) { + var out, errBuf bytes.Buffer + r := NewRenderer(&out, &errBuf, false, true, false, "") + err := r.Render(map[string]any{ + "id": "cus_\x1b]52;c;UkNCQg==\x07x", + "na\x1bme_key": "value\x07", + }) + if err != nil { + t.Fatal(err) + } + if s := out.String(); strings.ContainsAny(s, "\x1b\x07") { + t.Errorf("human output contains raw control bytes: %q", s) + } +} + +func TestRenderJSON_LeavesValuesEncoded(t *testing.T) { + var out, errBuf bytes.Buffer + r := NewRenderer(&out, &errBuf, true, true, false, "") + if err := r.Render(map[string]any{"id": "a\x1bb"}); err != nil { + t.Fatal(err) + } + s := out.String() + if strings.Contains(s, "\x1b") { + t.Errorf("json output contains a raw escape byte: %q", s) + } + if !strings.Contains(s, `\u001b`) { + t.Errorf("json output should keep the value losslessly escaped, got %q", s) + } +} diff --git a/internal/tui/browser.go b/internal/tui/browser.go index 04cf27f3..189c6c79 100644 --- a/internal/tui/browser.go +++ b/internal/tui/browser.go @@ -746,7 +746,7 @@ func (m *browser) viewDetail(f *bframe) string { if f.item.ID != "" && f.item.ID != f.item.Label { label = f.item.ID } - sb.WriteString(m.renderHeader(label)) + sb.WriteString(m.renderHeader(output.Sanitize(label))) sb.WriteString("\n") keyW := 0 @@ -759,8 +759,8 @@ func (m *browser) viewDetail(f *bframe) string { if field.Value == "" { continue } - k := brPadRight(field.Key, keyW) - sb.WriteString(" " + brDim.Render(k) + " " + field.Value + "\n") + k := brPadRight(output.Sanitize(field.Key), keyW) + sb.WriteString(" " + brDim.Render(k) + " " + output.Sanitize(field.Value) + "\n") } slotIdx := 0 @@ -866,10 +866,13 @@ var ( // ── helpers ────────────────────────────────────────────────────────────────── +// brTrunc caps a display value at maxLen runes. Values pass through +// output.Sanitize on the way: everything shown here came from the API and must +// render as visible characters only. func brTrunc(s string, maxLen int) string { - runes := []rune(s) + runes := []rune(output.Sanitize(s)) if maxLen <= 3 || len(runes) <= maxLen { - return s + return string(runes) } return string(runes[:maxLen-1]) + "…" } diff --git a/internal/tui/browser_sanitize_test.go b/internal/tui/browser_sanitize_test.go new file mode 100644 index 00000000..d6c51082 --- /dev/null +++ b/internal/tui/browser_sanitize_test.go @@ -0,0 +1,21 @@ +package tui + +import ( + "strings" + "testing" +) + +// Every list, table, and detail path renders values through brTrunc or an +// explicit Sanitize call; brTrunc is the shared chokepoint. +func TestBrTrunc_StripsControlBytes(t *testing.T) { + got := brTrunc("cus_\x1b]52;c;UkNCQg==\x07x", 80) + if strings.ContainsAny(got, "\x1b\x07") { + t.Errorf("brTrunc left raw control bytes: %q", got) + } + if got != "cus_]52;c;UkNCQg==x" { + t.Errorf("brTrunc = %q", got) + } + if short := brTrunc("plain", 3); short != "plain" { + t.Errorf("short passthrough broken: %q", short) + } +} diff --git a/internal/tui/prompt_rail.go b/internal/tui/prompt_rail.go index c3e813f5..b542c93e 100644 --- a/internal/tui/prompt_rail.go +++ b/internal/tui/prompt_rail.go @@ -219,7 +219,7 @@ func (m selectModel) View() string { b.WriteString(railSpacer() + "\n") if m.done { b.WriteString(railHead("◇", m.title) + "\n") - b.WriteString(railBody(prOKSty.Render("✓")+" "+m.opts[m.cursor].Label) + "\n") + b.WriteString(railBody(prOKSty.Render("✓")+" "+output.Sanitize(m.opts[m.cursor].Label)) + "\n") return b.String() } b.WriteString(railHead("◆", m.title) + "\n") @@ -228,9 +228,9 @@ func (m selectModel) View() string { } for i, o := range m.opts { if i == m.cursor { - b.WriteString(railBody(prSelSty.Render("▸ "+o.Label)) + "\n") + b.WriteString(railBody(prSelSty.Render("▸ "+output.Sanitize(o.Label))) + "\n") } else { - b.WriteString(railBody(" "+o.Label) + "\n") + b.WriteString(railBody(" "+output.Sanitize(o.Label)) + "\n") } } return b.String() From d0b8b6b4f1399be3b61fe71cfa1c87943132b5b7 Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Fri, 18 Sep 2026 10:09:01 -0500 Subject: [PATCH 2/3] fix(output): widen the visible-text pass after review - cards (rc customers show), browser breadcrumbs/error lines, chart titles, huh picker labels, cursor hints, Answer values, and the top-level error printer now go through the same helper - single-line variant for cells/labels/crumbs so values can't span rows or shift columns - JSON keeps values losslessly escaped, now including C1 codepoints, which encoding/json passes through as raw bytes - OSC 8 link URLs can no longer terminate their own sequence Co-Authored-By: Claude Fable 5 --- internal/cli/customers.go | 4 +- internal/cli/pagination.go | 3 +- internal/cli/resolve.go | 3 +- internal/cli/run.go | 2 +- internal/output/card.go | 44 +++++++++++++++++++++ internal/output/output.go | 51 +++++++++++++++++++----- internal/output/sanitize.go | 16 ++++++++ internal/output/sanitize_test.go | 56 +++++++++++++++++++++++---- internal/tui/browser.go | 27 +++++++------ internal/tui/browser_sanitize_test.go | 25 ++++++++++-- internal/tui/chartview.go | 4 +- 11 files changed, 196 insertions(+), 39 deletions(-) diff --git a/internal/cli/customers.go b/internal/cli/customers.go index 90733007..c906f134 100644 --- a/internal/cli/customers.go +++ b/internal/cli/customers.go @@ -575,7 +575,7 @@ func pickProjectInteractive(ctx context.Context, rt *Runtime) (string, error) { const noDefault = "__no_default__" projectOpts := make([]huh.Option[string], len(page.Items)) for i, p := range page.Items { - projectOpts[i] = huh.NewOption(fmt.Sprintf("%s (%s)", p.Name, p.ID), p.ID) + projectOpts[i] = huh.NewOption(output.SanitizeLine(fmt.Sprintf("%s (%s)", p.Name, p.ID)), p.ID) } allOpts := append([]huh.Option[string]{ huh.NewOption("Ask me every time (don't save a default)", noDefault), @@ -672,7 +672,7 @@ pass --json for machine-readable output or --no-input to disable the browser.`, return err } if page.NextPage != "" && !rt.Globals.JSON { - rt.Out.Info(fmt.Sprintf("more results — pass --cursor %s for the next page", lastID(page.Items))) + rt.Out.Info("more results — pass --cursor " + output.SanitizeLine(lastID(page.Items)) + " for the next page") } return nil }, diff --git a/internal/cli/pagination.go b/internal/cli/pagination.go index 9718b645..677b39c1 100644 --- a/internal/cli/pagination.go +++ b/internal/cli/pagination.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" "github.com/revenuecat/cli/internal/api" + "github.com/revenuecat/cli/internal/output" ) // addListPaginationFlags binds --limit / --cursor. @@ -18,6 +19,6 @@ func hintMoreResults[T any](rt *Runtime, page *api.Page[T]) { return } if cursor := page.NextCursor(); cursor != "" { - rt.Out.Info("more results — pass --cursor " + cursor + " for the next page") + rt.Out.Info("more results — pass --cursor " + output.SanitizeLine(cursor) + " for the next page") } } diff --git a/internal/cli/resolve.go b/internal/cli/resolve.go index 6c75f318..23bf233f 100644 --- a/internal/cli/resolve.go +++ b/internal/cli/resolve.go @@ -6,6 +6,7 @@ import ( "github.com/charmbracelet/huh" + "github.com/revenuecat/cli/internal/output" "github.com/revenuecat/cli/internal/tui" ) @@ -51,7 +52,7 @@ func requireID(rt *Runtime, arg, noun string, fetch func() ([]PickerItem, error) func selectID(rt *Runtime, noun string, items []PickerItem, defaultID string) (string, error) { opts := make([]huh.Option[string], len(items)) for i, item := range items { - opts[i] = huh.NewOption(item.Label, item.ID) + opts[i] = huh.NewOption(output.SanitizeLine(item.Label), item.ID) } chosen := defaultID sel := huh.NewSelect[string](). diff --git a/internal/cli/run.go b/internal/cli/run.go index 442b3a5b..ee627cd2 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -43,7 +43,7 @@ func Run(version string) int { if jsonMode { writeJSONError(os.Stderr, err) } else { - fmt.Fprintln(os.Stderr, output.StyleError.Render("✗")+" "+err.Error()) + fmt.Fprintln(os.Stderr, output.StyleError.Render("✗")+" "+output.Sanitize(err.Error())) if hint := hintFor(err); hint != "" { fmt.Fprintln(os.Stderr, output.StyleDim.Render("Hint: "+hint)) } diff --git a/internal/output/card.go b/internal/output/card.go index 465ea335..02e80fbe 100644 --- a/internal/output/card.go +++ b/internal/output/card.go @@ -65,6 +65,7 @@ func (r *Renderer) RenderCard(c Card) error { if r.json { return r.Render(c.Raw) } + c = sanitizeCard(c) titleStyle := lipgloss.NewStyle().Bold(true) subtitleStyle := StyleDim @@ -101,6 +102,49 @@ func (r *Renderer) RenderCard(c Card) error { return nil } +// sanitizeCard runs every displayed card value through SanitizeLine. Raw is +// left alone — it's the --json payload and never printed here. +func sanitizeCard(c Card) Card { + c.Title = SanitizeLine(c.Title) + c.Subtitle = SanitizeLine(c.Subtitle) + sections := make([]CardSection, len(c.Sections)) + for i, s := range c.Sections { + s.Heading = SanitizeLine(s.Heading) + if len(s.Chips) > 0 { + chips := make([]Chip, len(s.Chips)) + for j, ch := range s.Chips { + ch.Label = SanitizeLine(ch.Label) + chips[j] = ch + } + s.Chips = chips + } + if s.Table != nil { + t := *s.Table + rows := make([][]string, len(t.Rows)) + for ri, row := range t.Rows { + rows[ri] = make([]string, len(row)) + for ci, cell := range row { + rows[ri][ci] = SanitizeLine(cell) + } + } + t.Rows = rows + s.Table = &t + } + if len(s.Lines) > 0 { + lines := make([]CardLine, len(s.Lines)) + for j, l := range s.Lines { + l.Key = SanitizeLine(l.Key) + l.Value = SanitizeLine(l.Value) + lines[j] = l + } + s.Lines = lines + } + sections[i] = s + } + c.Sections = sections + return c +} + func (r *Renderer) writeChips(chips []Chip) { var parts []string for _, c := range chips { diff --git a/internal/output/output.go b/internal/output/output.go index d984146a..a8a5c516 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -11,6 +11,7 @@ package output import ( + "bytes" "encoding/json" "errors" "fmt" @@ -149,9 +150,7 @@ func (r *Renderer) Render(v any) error { if r.format != "" { return r.renderJSONFiltered(env) } - enc := json.NewEncoder(r.stdout) - enc.SetIndent("", " ") - return enc.Encode(env) + return encodeJSON(r.stdout, env) } if r.format != "" { // --format without --json: warn on stderr, fall through to pretty. @@ -166,9 +165,40 @@ func (r *Renderer) RenderJSON(v any) error { if r.json { return r.Render(v) } - enc := json.NewEncoder(r.stdout) + return encodeJSON(r.stdout, v) +} + +// encodeJSON writes v as indented JSON with C1 codepoints \u-escaped: +// encoding/json escapes C0 controls but emits U+0080–U+009F as raw UTF-8 +// bytes, and JSON output frequently lands on a terminal. +func encodeJSON(w io.Writer, v any) error { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) enc.SetIndent("", " ") - return enc.Encode(v) + if err := enc.Encode(v); err != nil { + return err + } + _, err := w.Write(escapeC1(buf.Bytes())) + return err +} + +// escapeC1 rewrites UTF-8-encoded C1 codepoints (0xC2 0x80–0x9F; in valid +// UTF-8, 0xC2 only ever appears as that lead byte) as JSON \u escapes. +func escapeC1(b []byte) []byte { + if bytes.IndexByte(b, 0xC2) < 0 { + return b + } + var out bytes.Buffer + out.Grow(len(b) + 16) + for i := 0; i < len(b); i++ { + if b[i] == 0xC2 && i+1 < len(b) && b[i+1] >= 0x80 && b[i+1] <= 0x9F { + fmt.Fprintf(&out, `\u%04x`, b[i+1]) + i++ + continue + } + out.WriteByte(b[i]) + } + return out.Bytes() } // renderHuman is the human-mode fallback for structured results: aligned @@ -193,7 +223,7 @@ func (r *Renderer) renderHuman(v any) error { } } for _, k := range keys { - fmt.Fprintf(r.stdout, "%s %s\n", r.style(r.dim, padRight(Sanitize(k), width)), Sanitize(humanFieldValue(k, m[k]))) + fmt.Fprintf(r.stdout, "%s %s\n", r.style(r.dim, padRight(SanitizeLine(k), width)), SanitizeLine(humanFieldValue(k, m[k]))) } return nil } @@ -326,7 +356,7 @@ func (r *Renderer) RenderTable(t Table) error { for ri, row := range t.Rows { rows[ri] = make([]string, len(row)) for i, cell := range row { - rows[ri][i] = Sanitize(cell) + rows[ri][i] = SanitizeLine(cell) } } widths := make([]int, len(t.Columns)) @@ -399,13 +429,16 @@ func (r *Renderer) Info(msg string) { // Supporting terminals make it clickable; others render the label text. This is // the one place the OSC 8 escape lives. func Hyperlink(styledLabel, url string) string { - return "\x1b]8;;" + url + "\x1b\\" + styledLabel + "\x1b]8;;\x1b\\" + // A control character in url would terminate the OSC 8 sequence early and + // leave the rest to the terminal. + return "\x1b]8;;" + Sanitize(url) + "\x1b\\" + styledLabel + "\x1b]8;;\x1b\\" } // LinkText renders a clickable hyperlink (OSC 8) with a custom label instead of // the raw URL, so long auth URLs don't dominate the output. With color off it // falls back to "label (url)" so the URL stays copyable. func (r *Renderer) LinkText(label, url string) string { + label, url = SanitizeLine(label), Sanitize(url) if r.noColor { return label + " (" + url + ")" } @@ -497,7 +530,7 @@ func (r *Renderer) Answer(key, value string) { if r.json || r.quiet { return } - fmt.Fprintf(r.stderr, "%s %s %s\n", r.style(r.success, "✓"), r.style(r.dim, padRight(key, 26)), value) + fmt.Fprintf(r.stderr, "%s %s %s\n", r.style(r.success, "✓"), r.style(r.dim, padRight(key, 26)), SanitizeLine(value)) } // Plan renders the guided-command plan: a titled, numbered list of the diff --git a/internal/output/sanitize.go b/internal/output/sanitize.go index f19600b9..aa1a5aa5 100644 --- a/internal/output/sanitize.go +++ b/internal/output/sanitize.go @@ -26,3 +26,19 @@ func isControlRune(r rune) bool { } return r < 0x20 || r == 0x7F || (r >= 0x80 && r <= 0x9F) } + +// SanitizeLine is Sanitize for single-line contexts — table cells, labels, +// breadcrumbs, chips — where a newline would fake extra rows and a tab would +// shift columns; both collapse to a space. +func SanitizeLine(s string) string { + s = Sanitize(s) + if !strings.ContainsAny(s, "\n\t") { + return s + } + return strings.Map(func(r rune) rune { + if r == '\n' || r == '\t' { + return ' ' + } + return r + }, s) +} diff --git a/internal/output/sanitize_test.go b/internal/output/sanitize_test.go index 963bc60b..b557951f 100644 --- a/internal/output/sanitize_test.go +++ b/internal/output/sanitize_test.go @@ -10,7 +10,7 @@ func TestSanitize(t *testing.T) { cases := map[string]string{ "plain-id_1.2": "plain-id_1.2", "keep\nnewlines\tand tabs": "keep\nnewlines\tand tabs", - "esc\x1b]52;c;UkNCQg==\x07seq": "esc]52;c;UkNCQg==seq", + "osc\x1b]0;title\x07seq": "osc]0;titleseq", "csi\x1b[31mred\x1b[0m": "csi[31mred[0m", "bell\x07 backspace\x08 del\x7f": "bell backspace del", "c1\u0085dev\u009bice\u0090str": "c1devicestr", @@ -25,13 +25,26 @@ func TestSanitize(t *testing.T) { } } +func TestSanitizeLine(t *testing.T) { + cases := map[string]string{ + "one line": "one line", + "two\nlines\tand tab": "two lines and tab", + "ctrl\x1b[2Jhere": "ctrl[2Jhere", + } + for in, want := range cases { + if got := SanitizeLine(in); got != want { + t.Errorf("SanitizeLine(%q) = %q, want %q", in, got, want) + } + } +} + func TestRenderTable_ValuesRenderAsVisibleText(t *testing.T) { var out, errBuf bytes.Buffer r := NewRenderer(&out, &errBuf, false, true, false, "") err := r.RenderTable(Table{ Columns: []string{"ID", "NAME"}, Rows: [][]string{ - {"cus_\x1b]52;c;UkNCQg==\x07x", "a\rb"}, + {"id_\x1b]0;title\x07x", "a\rb\nc"}, }, }) if err != nil { @@ -42,11 +55,31 @@ func TestRenderTable_ValuesRenderAsVisibleText(t *testing.T) { } } +func TestRenderCard_ValuesRenderAsVisibleText(t *testing.T) { + var out, errBuf bytes.Buffer + r := NewRenderer(&out, &errBuf, false, true, false, "") + err := r.RenderCard(Card{ + Title: "id_\x1b]0;title\x07x", + Subtitle: "sub\x1b[2J", + Sections: []CardSection{ + {Heading: "chips\x07", Chips: []Chip{{Label: "ent\x1b[31m"}}}, + {Heading: "table", Table: &CardTable{Columns: []string{"A"}, Rows: [][]string{{"v\x1b[2J"}}}}, + {Heading: "lines", Lines: []CardLine{{Key: "k\x1b", Value: "v\x07"}}}, + }, + }) + if err != nil { + t.Fatal(err) + } + if s := out.String(); strings.ContainsAny(s, "\x1b\x07") { + t.Errorf("card output contains raw control bytes: %q", s) + } +} + func TestRenderHuman_ValuesRenderAsVisibleText(t *testing.T) { var out, errBuf bytes.Buffer r := NewRenderer(&out, &errBuf, false, true, false, "") err := r.Render(map[string]any{ - "id": "cus_\x1b]52;c;UkNCQg==\x07x", + "id": "id_\x1b]0;title\x07x", "na\x1bme_key": "value\x07", }) if err != nil { @@ -57,17 +90,26 @@ func TestRenderHuman_ValuesRenderAsVisibleText(t *testing.T) { } } +// JSON output must stay losslessly escaped rather than stripped — including C1 +// codepoints, which encoding/json would otherwise emit as raw UTF-8 bytes. func TestRenderJSON_LeavesValuesEncoded(t *testing.T) { var out, errBuf bytes.Buffer r := NewRenderer(&out, &errBuf, true, true, false, "") - if err := r.Render(map[string]any{"id": "a\x1bb"}); err != nil { + if err := r.Render(map[string]any{"id": "a\x1bb\u0085c"}); err != nil { t.Fatal(err) } s := out.String() - if strings.Contains(s, "\x1b") { - t.Errorf("json output contains a raw escape byte: %q", s) + if strings.Contains(s, "\x1b") || strings.Contains(s, "\u0085") { + t.Errorf("json output contains a raw control byte: %q", s) } - if !strings.Contains(s, `\u001b`) { + if !strings.Contains(s, `\u001b`) || !strings.Contains(s, `\u0085`) { t.Errorf("json output should keep the value losslessly escaped, got %q", s) } } + +func TestHyperlink_URLCannotTerminateSequence(t *testing.T) { + got := Hyperlink("label", "https://example.com/\x1b\\x\x07") + if strings.Count(got, "\x1b]8;;") != 2 || strings.Contains(got, "\x07") { + t.Errorf("hyperlink URL broke out of the OSC 8 sequence: %q", got) + } +} diff --git a/internal/tui/browser.go b/internal/tui/browser.go index 189c6c79..f2a08801 100644 --- a/internal/tui/browser.go +++ b/internal/tui/browser.go @@ -484,7 +484,7 @@ func (m *browser) View() string { } if m.loadErr != "" { return m.renderHeader("Error") + - "\n " + brErr.Render("Error: "+m.loadErr) + + "\n " + brErr.Render("Error: "+output.Sanitize(m.loadErr)) + "\n\n Press any key to dismiss.\n" } f := m.top() @@ -499,19 +499,21 @@ func (m *browser) View() string { return "" } +// renderHeader sanitizes every crumb and the current title itself: frame +// titles and detail labels carry API values (project names, customer IDs). func (m *browser) renderHeader(current string) string { var crumbs []string for i := 0; i < len(m.stack)-1; i++ { f := m.stack[i] switch f.kind { case kindList, kindTable: - crumbs = append(crumbs, f.title) + crumbs = append(crumbs, output.SanitizeLine(f.title)) case kindDetail: lbl := f.item.ID if lbl == "" { lbl = f.item.Label } - crumbs = append(crumbs, lbl) + crumbs = append(crumbs, output.SanitizeLine(lbl)) } } var sb strings.Builder @@ -519,7 +521,7 @@ func (m *browser) renderHeader(current string) string { if len(crumbs) > 0 { sb.WriteString(brDim.Render(strings.Join(crumbs, " › ") + " › ")) } - sb.WriteString(brTitle.Render(current)) + sb.WriteString(brTitle.Render(output.SanitizeLine(current))) sb.WriteString("\n ") sb.WriteString(brDim.Render(strings.Repeat("─", brMax(m.width-4, 10)))) sb.WriteString("\n") @@ -673,7 +675,7 @@ func (m *browser) viewTable(f *bframe) string { if j > 0 { row.WriteString(" ") } - row.WriteString(brTrunc(brPadRight(cell, colW[j]), colW[j])) + row.WriteString(brTrunc(brPadRight(output.SanitizeLine(cell), colW[j]), colW[j])) } rowStr := row.String() if i == f.cursor { @@ -746,7 +748,7 @@ func (m *browser) viewDetail(f *bframe) string { if f.item.ID != "" && f.item.ID != f.item.Label { label = f.item.ID } - sb.WriteString(m.renderHeader(output.Sanitize(label))) + sb.WriteString(m.renderHeader(label)) sb.WriteString("\n") keyW := 0 @@ -780,7 +782,7 @@ func (m *browser) viewDetail(f *bframe) string { if f.autoLoading { sb.WriteString("\n " + brDim.Render("Loading…") + "\n") } else if f.autoErr != "" { - sb.WriteString("\n " + brErr.Render("Error: "+f.autoErr) + "\n") + sb.WriteString("\n " + brErr.Render("Error: "+output.Sanitize(f.autoErr)) + "\n") } else { for _, sec := range f.sections { sb.WriteString("\n " + brSection.Render(sec.Title) + "\n") @@ -816,7 +818,7 @@ func (m *browser) viewDetail(f *bframe) string { if i > 0 { cells.WriteString(" ") } - cells.WriteString(brTrunc(brPadRight(cell, colW[i]), colW[i])) + cells.WriteString(brTrunc(brPadRight(output.SanitizeLine(cell), colW[i]), colW[i])) } cellStr := cells.String() @@ -867,12 +869,13 @@ var ( // ── helpers ────────────────────────────────────────────────────────────────── // brTrunc caps a display value at maxLen runes. Values pass through -// output.Sanitize on the way: everything shown here came from the API and must -// render as visible characters only. +// output.SanitizeLine on the way: everything shown here came from the API and +// must render as one line of visible characters. func brTrunc(s string, maxLen int) string { - runes := []rune(output.Sanitize(s)) + s = output.SanitizeLine(s) + runes := []rune(s) if maxLen <= 3 || len(runes) <= maxLen { - return string(runes) + return s } return string(runes[:maxLen-1]) + "…" } diff --git a/internal/tui/browser_sanitize_test.go b/internal/tui/browser_sanitize_test.go index d6c51082..0cca4e99 100644 --- a/internal/tui/browser_sanitize_test.go +++ b/internal/tui/browser_sanitize_test.go @@ -5,17 +5,34 @@ import ( "testing" ) -// Every list, table, and detail path renders values through brTrunc or an -// explicit Sanitize call; brTrunc is the shared chokepoint. +// List, table, and section cells render through brTrunc; detail fields, +// breadcrumbs, and error lines carry their own Sanitize calls in the view code. func TestBrTrunc_StripsControlBytes(t *testing.T) { - got := brTrunc("cus_\x1b]52;c;UkNCQg==\x07x", 80) + got := brTrunc("id_\x1b]0;title\x07x", 80) if strings.ContainsAny(got, "\x1b\x07") { t.Errorf("brTrunc left raw control bytes: %q", got) } - if got != "cus_]52;c;UkNCQg==x" { + if got != "id_]0;titlex" { t.Errorf("brTrunc = %q", got) } if short := brTrunc("plain", 3); short != "plain" { t.Errorf("short passthrough broken: %q", short) } + if multi := brTrunc("a\nb", 80); multi != "a b" { + t.Errorf("newline should collapse to a space in cells, got %q", multi) + } +} + +// Breadcrumbs carry API values (frame titles, detail IDs) and must be +// sanitized inside renderHeader, not by each caller. +func TestRenderHeader_SanitizesCrumbsAndTitle(t *testing.T) { + m := &browser{width: 80, stack: []bframe{ + newListFrame("Customers\x1b[2J", nil), + newDetailFrame(BrowserItem{ID: "id_\x1b]0;t\x07x"}), + newListFrame("child", nil), + }} + got := m.renderHeader("Current\x07Title") + if strings.ContainsAny(got[1:], "\x07") || strings.Contains(got[1:], "\x1b]") || strings.Contains(got[1:], "\x1b[2J") { + t.Errorf("header contains raw control bytes from API values: %q", got) + } } diff --git a/internal/tui/chartview.go b/internal/tui/chartview.go index 4c9505e0..f6ca8d8b 100644 --- a/internal/tui/chartview.go +++ b/internal/tui/chartview.go @@ -439,7 +439,7 @@ func (m *chartApp) View() string { sb.WriteString("\n") if m.fetchErr != nil && !m.loading { - sb.WriteString(fmt.Sprintf(" error: %v\n", m.fetchErr)) + sb.WriteString(" error: " + output.Sanitize(m.fetchErr.Error()) + "\n") } else { if m.chartTypeIdx == 0 { sb.WriteString(m.buildBarView()) @@ -535,7 +535,7 @@ func newChartApp( bars: bars, maxVal: maxVal, unit: unit, - title: data.DisplayName, + title: output.SanitizeLine(data.DisplayName), fetchFn: fetchFn, noColor: noColor, completeStyle: completeStyle, From 9739b37ffb91f22c854b940e77011fd23230e42e Mon Sep 17 00:00:00 2001 From: Josh Holtz Date: Fri, 18 Sep 2026 10:40:24 -0500 Subject: [PATCH 3/3] fix(output): cover the streaming, chat, and --format paths too - rico plain stream deltas, chat transcript entries (the markdown renderer passes control characters through), tool/approval labels, conversation show, and error lines - --format string results decode JSON escapes back to raw bytes; keep them visible-text like every other path - rc api raw bodies get the same C1 escaping as encoded JSON - paywalls AI activity lines, chart refetch title/unit, single-option picker echo Co-Authored-By: Claude Fable 5 --- internal/cli/api.go | 4 +++- internal/cli/paywalls_ai.go | 7 ++++--- internal/cli/resolve.go | 2 +- internal/cli/rico.go | 18 +++++++++--------- internal/output/output.go | 11 +++++++++-- internal/tui/chartview.go | 4 ++-- internal/tui/chat.go | 11 +++++++---- 7 files changed, 35 insertions(+), 22 deletions(-) diff --git a/internal/cli/api.go b/internal/cli/api.go index 27c9facc..2fb81c0e 100644 --- a/internal/cli/api.go +++ b/internal/cli/api.go @@ -7,6 +7,8 @@ import ( "strings" "github.com/spf13/cobra" + + "github.com/revenuecat/cli/internal/output" ) func newAPICmd() *cobra.Command { @@ -63,7 +65,7 @@ Exit code reflects the HTTP status: non-2xx responses exit non-zero.`, return err } if len(data) > 0 { - if _, werr := cmd.OutOrStdout().Write(data); werr != nil { + if _, werr := cmd.OutOrStdout().Write(output.EscapeC1JSON(data)); werr != nil { return werr } // Ensure trailing newline for shell friendliness. diff --git a/internal/cli/paywalls_ai.go b/internal/cli/paywalls_ai.go index 5efc0e1e..102a8d6f 100644 --- a/internal/cli/paywalls_ai.go +++ b/internal/cli/paywalls_ai.go @@ -16,6 +16,7 @@ import ( "github.com/revenuecat/cli/internal/api" "github.com/revenuecat/cli/internal/config" + "github.com/revenuecat/cli/internal/output" "github.com/revenuecat/cli/internal/paywallai" "github.com/revenuecat/cli/internal/tui" ) @@ -819,11 +820,11 @@ func reportPaywallAIActivity(rt *Runtime, activity []paywallai.ToolActivity, alr for _, item := range activity[min(alreadyReported, len(activity)):] { switch item.Type { case "assistant_message": - rt.Out.Info("Paywalls AI: " + item.Content) + rt.Out.Info("Paywalls AI: " + output.Sanitize(item.Content)) default: - text := item.Display.Text + text := output.SanitizeLine(item.Display.Text) if text == "" { - text = item.ToolName + text = output.SanitizeLine(item.ToolName) } if item.Status == "error" { rt.Out.Warn("⚙ " + text + " (errored — the Paywalls AI Editor retries these itself)") diff --git a/internal/cli/resolve.go b/internal/cli/resolve.go index 23bf233f..2427b96a 100644 --- a/internal/cli/resolve.go +++ b/internal/cli/resolve.go @@ -40,7 +40,7 @@ func requireID(rt *Runtime, arg, noun string, fetch func() ([]PickerItem, error) return "", fmt.Errorf("no %ss found — pass an ID explicitly", noun) } if len(items) == 1 { - rt.Out.Info(fmt.Sprintf("Only one %s available: %s", noun, items[0].Label)) + rt.Out.Info(fmt.Sprintf("Only one %s available: %s", noun, output.SanitizeLine(items[0].Label))) return items[0].ID, nil } return selectID(rt, noun, items, "") diff --git a/internal/cli/rico.go b/internal/cli/rico.go index afc60d84..386e9343 100644 --- a/internal/cli/rico.go +++ b/internal/cli/rico.go @@ -216,7 +216,7 @@ func pickRicoConversation(ctx context.Context, rt *Runtime, client *rico.Client) } options := make([]huh.Option[string], len(items)) for i, item := range items { - options[i] = huh.NewOption(item.Label, item.ID) + options[i] = huh.NewOption(output.SanitizeLine(item.Label), item.ID) } var chosen string selectField := huh.NewSelect[string](). @@ -372,7 +372,7 @@ func (s *ricoSession) repl(ctx context.Context) error { return nil } if err := s.turn(ctx, message); err != nil { - s.rt.Out.Error(err.Error()) + s.rt.Out.Error(output.Sanitize(err.Error())) } } } @@ -473,9 +473,9 @@ func (s *ricoSession) streamRun(ctx context.Context, input rico.RunAgentInput, r func (s *ricoSession) resolveInterrupts(interrupts []rico.Interrupt, result *ricoTurnResult, sink ricoSink) ([]rico.ResumeEntry, error) { entries := make([]rico.ResumeEntry, 0, len(interrupts)) for _, interrupt := range interrupts { - label := interrupt.Message + label := output.SanitizeLine(interrupt.Message) if label == "" { - label = interrupt.Reason + label = output.SanitizeLine(interrupt.Reason) } approved, err := sink.Approve(interrupt, label) if err != nil { @@ -507,7 +507,7 @@ func (s *ricoPlainSink) Delta(text string) { if s.silent { return } - fmt.Print(text) + fmt.Print(output.Sanitize(text)) s.midLine = true } @@ -523,7 +523,7 @@ func (s *ricoPlainSink) Tool(name string) { return } s.endLine() - s.session.rt.Out.Info("⚙ " + name) + s.session.rt.Out.Info("⚙ " + output.SanitizeLine(name)) } func (s *ricoPlainSink) Approve(interrupt rico.Interrupt, label string) (bool, error) { @@ -606,15 +606,15 @@ scope to a single Project.`, for _, message := range snapshot.Messages { text := message.Text() for _, call := range message.ToolCalls { - rt.Out.Info("⚙ " + call.Function.Name) + rt.Out.Info("⚙ " + output.SanitizeLine(call.Function.Name)) } if text == "" { continue } - fmt.Printf("%s: %s\n", message.Role, text) + fmt.Printf("%s: %s\n", message.Role, output.Sanitize(text)) } for _, interrupt := range snapshot.PendingInterrupts { - rt.Out.Warn("Pending approval: " + interrupt.Reason) + rt.Out.Warn("Pending approval: " + output.SanitizeLine(interrupt.Reason)) } return nil }, diff --git a/internal/output/output.go b/internal/output/output.go index a8a5c516..4cce74fc 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -182,6 +182,11 @@ func encodeJSON(w io.Writer, v any) error { return err } +// EscapeC1JSON exposes escapeC1 for callers that stream raw JSON bodies to +// stdout (rc api): JSON guarantees C0 is escaped on the wire, but C1 arrives +// as raw UTF-8 bytes. +func EscapeC1JSON(b []byte) []byte { return escapeC1(b) } + // escapeC1 rewrites UTF-8-encoded C1 codepoints (0xC2 0x80–0x9F; in valid // UTF-8, 0xC2 only ever appears as that lead byte) as JSON \u escapes. func escapeC1(b []byte) []byte { @@ -320,7 +325,9 @@ func (r *Renderer) renderJSONFiltered(env any) error { } switch t := v.(type) { case string: - fmt.Fprintln(r.stdout, t) + // Unmarshal decoded the API's \u escapes back into real control + // bytes; keep --format output to visible text like every other path. + fmt.Fprintln(r.stdout, Sanitize(t)) case nil: // jq emits nil for `.missing`; skip rather than print "null". default: @@ -587,5 +594,5 @@ func (r *Renderer) Error(msg string) { if r.json { return } - fmt.Fprintln(r.stderr, r.style(r.errSty, "✗ ")+msg) + fmt.Fprintln(r.stderr, r.style(r.errSty, "✗ ")+Sanitize(msg)) } diff --git a/internal/tui/chartview.go b/internal/tui/chartview.go index f6ca8d8b..55b3b749 100644 --- a/internal/tui/chartview.go +++ b/internal/tui/chartview.go @@ -122,7 +122,7 @@ func (m *chartApp) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if msg.err == nil && msg.data != nil { m.bars, m.maxVal, m.unit = processBars(msg.data, m.completeStyle, m.incompleteStyle) if m.title == "" { - m.title = msg.data.DisplayName + m.title = output.SanitizeLine(msg.data.DisplayName) } } // Reset scroll to end (overshoots intentionally; clampBarOffset pins to max). @@ -477,7 +477,7 @@ func processBars(data *api.ChartData, completeStyle, incompleteStyle lipgloss.St if maxVal == 0 { maxVal = 1 } - unit := data.YAxis + unit := output.SanitizeLine(data.YAxis) return bars, maxVal, unit } diff --git a/internal/tui/chat.go b/internal/tui/chat.go index cc5a4e80..02c4f1b2 100644 --- a/internal/tui/chat.go +++ b/internal/tui/chat.go @@ -383,7 +383,7 @@ func (m *chatModel) renderTranscript() string { // stretches of the stream never look frozen. label := "thinking…" if m.activity != "" { - label = "running " + m.activity + "…" + label = "running " + output.SanitizeLine(m.activity) + "…" } b.WriteString("\n " + m.spin.View() + " " + chatDimStyle.Render(label) + "\n") } @@ -391,13 +391,16 @@ func (m *chatModel) renderTranscript() string { } func (m *chatModel) renderEntry(entry ChatEntry) string { + // Transcript text is untrusted (assistant/server-composed, and it quotes + // API data); the markdown renderer passes control characters through. + entry.Text = output.Sanitize(entry.Text) switch entry.Role { case ChatUser: return "\n" + chatUserStyle.Render("❯ ") + entry.Text + "\n" case ChatTool: - return chatToolStyle.Render(" ⚙ "+entry.Text) + "\n" + return chatToolStyle.Render(" ⚙ "+output.SanitizeLine(entry.Text)) + "\n" case ChatNotice: - return chatNoticeStyle.Render(" "+entry.Text) + "\n" + return chatNoticeStyle.Render(" "+output.SanitizeLine(entry.Text)) + "\n" default: // assistant text := entry.Text if m.cfg.RelativeLinkBase != "" { @@ -424,7 +427,7 @@ func (m *chatModel) View() string { footer := chatDimStyle.Render("enter send · ctrl+j newline · pgup/pgdn scroll · esc quit") switch { case m.approval != nil: - label := "Allow: " + m.approval.prompt + " " + label := "Allow: " + output.SanitizeLine(m.approval.prompt) + " " hint := chatApproveStyle.Render("[y] approve") + " " + chatDestructStyle.Render("[n] reject") if m.approval.destructive { label = chatDestructStyle.Render("⚠ destructive · ") + label