diff --git a/docs/design-system.md b/docs/design-system.md index b435d22e..2c046a71 100644 --- a/docs/design-system.md +++ b/docs/design-system.md @@ -44,6 +44,20 @@ semantic styles (`StyleTitle`, `StyleSuccess`, `StyleError`, `StyleDim`, …). | `Render(v)` | humanized fields | any API object (JSON only under `--json`) | | `RenderTable` / `RenderCard` | | lists / result summaries | +Every one of those writes its text through `output.Sanitize` first. Most of +what the CLI shows came from somewhere else — API responses, store metadata, +an App User ID a customer picked for themselves — and a terminal *acts* on +control bytes rather than drawing them (OSC 52 rewrites the clipboard, CSI +moves the cursor, CR overwrites the line above). Sanitized, remote text can +only ever be shown: controls become their escaped literal (`\x1b`), newline +and tab pass through. The interactive browser sanitizes on the way into a +frame, so lazily-loaded children are covered too. `--json` is untouched: the +JSON encoder already escapes control bytes, so agents keep the exact value. + +Strings the CLI styled itself (`Paint`, `Panel`, `Link`) carry deliberate +escapes and are built from our own literals, so they skip sanitizing — never +route remote text through them. + ### Interaction (`internal/tui`) - `tui.Form(...)` and `tui.Confirm*` are the only ways to prompt. They apply @@ -67,6 +81,10 @@ Simple creates stay promptless. colors outside the token layer, raw `huh.NewForm`, or hand-rolled `AssumeYes` checks fail CI. Deliberate exceptions live in an allow list with a written reason. +- Escape-neutralization tests — `internal/output/untrusted_test.go` (every + `Renderer` slot), `internal/tui/browser_escape_test.go` (every browser + view), and `internal/cli/customers_escape_test.go` (end to end, from an + API response to stdout). A writer that prints remote text raw fails here. - `TestOutputSnapshots` — layout and copy of representative commands are locked into golden files (`UPDATE_SNAPSHOTS=1` to regenerate after intentional changes). diff --git a/internal/cli/customers_escape_test.go b/internal/cli/customers_escape_test.go new file mode 100644 index 00000000..beabf2d3 --- /dev/null +++ b/internal/cli/customers_escape_test.go @@ -0,0 +1,86 @@ +package cli_test + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// A Customer's ID is whatever App User ID the app handed RevenueCat, and the +// v2 schema constrains it only by length (docs/specs/v2-developer.yaml — +// `Customer.id`: string, maxLength 1500, no pattern). So an ID can carry +// terminal control bytes, and human-mode output must show them rather than let +// the terminal act on them: OSC 52 replaces the reader's clipboard. +const osc52CustomerID = "rcbb_target\x1b]52;c;UkNCQjE5MQ==\x07" + +// JSON-encoded form of the same ID, as the API would send it on the wire. +const osc52CustomerIDJSON = `rcbb_target\u001b]52;c;UkNCQjE5MQ==\u0007` + +func customerEscapeServer(t *testing.T) *httptest.Server { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + customer := `{"object":"customer","id":"` + osc52CustomerIDJSON + `","project_id":"proj_esc",` + + `"first_seen_at":1700000000000,"last_seen_at":1700000000000,"last_seen_platform":"ios",` + + `"last_seen_country":"` + osc52CustomerIDJSON + `","last_seen_app_version":"1.0.0",` + + `"active_entitlements":{"object":"list","items":[{"object":"customer.active_entitlement","entitlement_id":"` + osc52CustomerIDJSON + `"}]}}` + switch { + case strings.HasSuffix(r.URL.Path, "/subscriptions"): + io.WriteString(w, `{"object":"list","items":[{"object":"subscription","id":"`+osc52CustomerIDJSON+`","store":"app_store","status":"active"}]}`) + case strings.HasSuffix(r.URL.Path, "/purchases"): + io.WriteString(w, `{"object":"list","items":[]}`) + case strings.HasSuffix(r.URL.Path, "/customers"): + io.WriteString(w, `{"object":"list","items":[`+customer+`],"next_page":"/v2/projects/proj_esc/customers?starting_after=x"}`) + default: + io.WriteString(w, customer) + } + })) + t.Cleanup(server.Close) + return server +} + +func TestCustomers_HumanOutputNeverLetsACustomerIDDriveTheTerminal(t *testing.T) { + server := customerEscapeServer(t) + t.Setenv("RC_BASE_URL", server.URL) + + for _, args := range [][]string{ + {"customers", "list"}, + {"customers", "show", "cus_escape"}, + } { + t.Run(strings.Join(args, " "), func(t *testing.T) { + args = append(args, "--no-input", "--no-color", "--project-id", "proj_esc", "--api-key", "sk_esc") + stdout, stderr, err := runAgentCmd(t, args...) + if err != nil { + t.Fatalf("execute: %v\nstderr: %s", err, stderr) + } + for name, stream := range map[string]string{"stdout": stdout, "stderr": stderr} { + if strings.ContainsRune(stream, 0x1b) || strings.ContainsRune(stream, 0x07) { + t.Errorf("%s hands the terminal a control sequence from the customer ID:\n%q", name, stream) + } + } + if !strings.Contains(stdout, `\x1b]52`) { + t.Errorf("the ID should still be readable as an escaped literal:\n%s", stdout) + } + }) + } +} + +// The agent contract is unchanged: --json carries the exact bytes, encoded. +func TestCustomersList_JSONStillCarriesTheRawIDEncoded(t *testing.T) { + server := customerEscapeServer(t) + t.Setenv("RC_BASE_URL", server.URL) + + stdout, _, err := runAgentCmd(t, "customers", "list", "--json", "--no-input", + "--project-id", "proj_esc", "--api-key", "sk_esc") + if err != nil { + t.Fatalf("execute: %v", err) + } + if strings.ContainsRune(stdout, 0x1b) { + t.Errorf("--json leaked a raw escape byte:\n%q", stdout) + } + if !strings.Contains(stdout, osc52CustomerIDJSON) { + t.Errorf("--json must keep the exact ID:\n%s", stdout) + } +} diff --git a/internal/output/card.go b/internal/output/card.go index 465ea335..89629c12 100644 --- a/internal/output/card.go +++ b/internal/output/card.go @@ -72,16 +72,16 @@ func (r *Renderer) RenderCard(c Card) error { emptyStyle := lipgloss.NewStyle().Faint(true).Italic(true) if c.Title != "" { - fmt.Fprintln(r.stdout, r.style(StyleAccent, "▍ ")+r.style(titleStyle, c.Title)) + fmt.Fprintln(r.stdout, r.style(StyleAccent, "▍ ")+r.style(titleStyle, Sanitize(c.Title))) } if c.Subtitle != "" { - fmt.Fprintln(r.stdout, " "+r.style(subtitleStyle, c.Subtitle)) + fmt.Fprintln(r.stdout, " "+r.style(subtitleStyle, Sanitize(c.Subtitle))) } for _, s := range c.Sections { fmt.Fprintln(r.stdout) if s.Heading != "" { - fmt.Fprintln(r.stdout, r.style(headingStyle, s.Heading)) + fmt.Fprintln(r.stdout, r.style(headingStyle, Sanitize(s.Heading))) } switch { case len(s.Chips) > 0: @@ -95,7 +95,7 @@ func (r *Renderer) RenderCard(c Card) error { if msg == "" { msg = "none" } - fmt.Fprintln(r.stdout, r.style(emptyStyle, " "+msg)) + fmt.Fprintln(r.stdout, r.style(emptyStyle, " "+Sanitize(msg))) } } return nil @@ -110,8 +110,9 @@ func (r *Renderer) writeChips(chips []Chip) { } func (r *Renderer) styleChip(c Chip) string { + label := Sanitize(c.Label) if r.noColor { - return "[" + c.Label + "]" + return "[" + label + "]" } base := lipgloss.NewStyle().Padding(0, 1).Bold(true) white := lipgloss.Color("15") @@ -129,15 +130,20 @@ func (r *Renderer) styleChip(c Chip) string { default: base = base.Background(NeutralGray).Foreground(white) } - return base.Render(c.Label) + return base.Render(label) } func (r *Renderer) writeCardTable(t CardTable) { - widths := make([]int, len(t.Columns)) - for i, c := range t.Columns { + columns := sanitizeAll(t.Columns) + rows := make([][]string, len(t.Rows)) + for i, row := range t.Rows { + rows[i] = sanitizeAll(row) + } + widths := make([]int, len(columns)) + for i, c := range columns { widths[i] = len(c) } - for _, row := range t.Rows { + for _, row := range rows { for i, cell := range row { if i >= len(widths) { continue @@ -149,14 +155,14 @@ func (r *Renderer) writeCardTable(t CardTable) { } headerStyle := lipgloss.NewStyle().Bold(true) fmt.Fprint(r.stdout, " ") - for i, c := range t.Columns { + for i, c := range columns { if i > 0 { fmt.Fprint(r.stdout, " ") } fmt.Fprint(r.stdout, r.style(headerStyle, padRight(c, widths[i]))) } fmt.Fprintln(r.stdout) - for _, row := range t.Rows { + for _, row := range rows { fmt.Fprint(r.stdout, " ") for i, cell := range row { if i > 0 { @@ -169,14 +175,16 @@ func (r *Renderer) writeCardTable(t CardTable) { } func (r *Renderer) writeLines(lines []CardLine) { + safe := make([]CardLine, len(lines)) keyWidth := 0 - for _, l := range lines { - if len(l.Key) > keyWidth { - keyWidth = len(l.Key) + for i, l := range lines { + safe[i] = CardLine{Key: Sanitize(l.Key), Value: Sanitize(l.Value)} + if len(safe[i].Key) > keyWidth { + keyWidth = len(safe[i].Key) } } keyStyle := lipgloss.NewStyle().Faint(true) - for _, l := range lines { + for _, l := range safe { fmt.Fprintf(r.stdout, " %s %s\n", r.style(keyStyle, padRight(l.Key+":", keyWidth+1)), l.Value) } } diff --git a/internal/output/output.go b/internal/output/output.go index 7533c1a8..9f480e92 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -187,13 +187,15 @@ func (r *Renderer) renderHuman(v any) error { } keys := humanKeyOrder(m) width := 0 - for _, k := range keys { - if len(k) > width { - width = len(k) + labels := make([]string, len(keys)) + for i, k := range keys { + labels[i] = Sanitize(k) + if len(labels[i]) > width { + width = len(labels[i]) } } - for _, k := range keys { - fmt.Fprintf(r.stdout, "%s %s\n", r.style(r.dim, padRight(k, width)), humanFieldValue(k, m[k])) + for i, k := range keys { + fmt.Fprintf(r.stdout, "%s %s\n", r.style(r.dim, padRight(labels[i], width)), humanFieldValue(k, m[k])) } return nil } @@ -232,14 +234,16 @@ func humanKeyOrder(m map[string]json.RawMessage) []string { } // humanValue renders one JSON value on one line: scalars verbatim, short -// composites as compact JSON, long ones summarized. +// composites as compact JSON, long ones summarized. Composites stay +// JSON-encoded, which already escapes control bytes; bare strings go through +// Sanitize. func humanValue(raw json.RawMessage) string { var s string if json.Unmarshal(raw, &s) == nil { if s == "" { return "—" } - return s + return Sanitize(s) } trimmed := string(raw) if trimmed == "null" { @@ -322,11 +326,16 @@ func (r *Renderer) RenderTable(t Table) error { fmt.Fprintln(r.stderr, r.style(r.info, "• ")+"no results") return nil } - widths := make([]int, len(t.Columns)) - for i, c := range t.Columns { + columns := sanitizeAll(t.Columns) + rows := make([][]string, len(t.Rows)) + for i, row := range t.Rows { + rows[i] = sanitizeAll(row) + } + widths := make([]int, len(columns)) + for i, c := range columns { widths[i] = len(c) } - for _, row := range t.Rows { + for _, row := range rows { for i, cell := range row { if i >= len(widths) { continue @@ -337,14 +346,14 @@ func (r *Renderer) RenderTable(t Table) error { } } headerStyle := lipgloss.NewStyle().Bold(true) - for i, c := range t.Columns { + for i, c := range columns { if i > 0 { fmt.Fprint(r.stdout, " ") } 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, " ") @@ -378,27 +387,30 @@ func (r *Renderer) Success(msg string) { if r.json || r.quiet { return } - fmt.Fprintln(r.stderr, r.style(r.success, "✓ ")+msg) + fmt.Fprintln(r.stderr, r.style(r.success, "✓ ")+Sanitize(msg)) } func (r *Renderer) Info(msg string) { if r.json || r.quiet { return } - fmt.Fprintln(r.stderr, r.style(r.info, "· ")+msg) + fmt.Fprintln(r.stderr, r.style(r.info, "· ")+Sanitize(msg)) } // Hyperlink wraps styledLabel in an OSC 8 terminal hyperlink pointing at url. // Supporting terminals make it clickable; others render the label text. This is -// the one place the OSC 8 escape lives. +// the one place the OSC 8 escape lives. url is sanitized: a control byte in a +// server-supplied URL would close this sequence early and leave the rest of +// the value to be interpreted as a new one. func Hyperlink(styledLabel, url string) string { - return "\x1b]8;;" + url + "\x1b\\" + styledLabel + "\x1b]8;;\x1b\\" + 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 { + url = Sanitize(url) if r.noColor { return label + " (" + url + ")" } @@ -409,6 +421,7 @@ func (r *Renderer) LinkText(label, url string) string { // back to the plain URL when color is off (our proxy for a dumb/non-interactive // terminal), so nothing leaks escape codes into piped or --no-color output. func (r *Renderer) Link(url string) string { + url = Sanitize(url) if r.noColor { return url } @@ -430,7 +443,7 @@ func (r *Renderer) Hint(msg string) { if r.json || r.quiet { return } - fmt.Fprintln(r.stderr, r.style(r.dim, " "+msg)) + fmt.Fprintln(r.stderr, r.style(r.dim, " "+Sanitize(msg))) } // Title starts a visually distinct section: a brand-colored bar plus a bold @@ -440,7 +453,7 @@ func (r *Renderer) Title(msg string) { return } fmt.Fprintln(r.stderr) - fmt.Fprintln(r.stderr, r.style(r.accent, "▍ ")+r.style(StyleTitle, msg)) + fmt.Fprintln(r.stderr, r.style(r.accent, "▍ ")+r.style(StyleTitle, Sanitize(msg))) } // Lead is the orienting sentence(s) under a Title: what this flow is for @@ -452,7 +465,7 @@ func (r *Renderer) Lead(text string) { return } const width = 76 - words := strings.Fields(text) + words := strings.Fields(Sanitize(text)) line := " " for _, w := range words { if len(line)+1+len(w) > width { @@ -478,7 +491,7 @@ func (r *Renderer) Notice(lines ...string) { fmt.Fprintln(r.stderr) bar := lipgloss.NewStyle().Foreground(InfoBlue).Bold(true) for _, line := range lines { - fmt.Fprintln(r.stderr, r.style(bar, "▐ ")+line) + fmt.Fprintln(r.stderr, r.style(bar, "▐ ")+Sanitize(line)) } fmt.Fprintln(r.stderr) } @@ -490,7 +503,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(Sanitize(key), 26)), Sanitize(value)) } // Plan renders the guided-command plan: a titled, numbered list of the @@ -512,12 +525,13 @@ func (r *Renderer) Field(key, value string, note ...string) { if r.json || r.quiet { return } + value = Sanitize(value) if len(note) > 0 && note[0] != "" { // Pad the value only when a note follows so notes column-align and // bare values carry no trailing whitespace. - value = padRight(value, 15) + " " + r.style(r.dim, "· "+note[0]) + value = padRight(value, 15) + " " + r.style(r.dim, "· "+Sanitize(note[0])) } - fmt.Fprintf(r.stderr, " %s %s\n", r.style(r.dim, padRight(key, 26)), value) + fmt.Fprintf(r.stderr, " %s %s\n", r.style(r.dim, padRight(Sanitize(key), 26)), value) } // Blank prints an empty separator line between logical sections. @@ -532,7 +546,7 @@ func (r *Renderer) Warn(msg string) { if r.json || r.quiet { return } - fmt.Fprintln(r.stderr, r.style(r.warn, "! ")+msg) + fmt.Fprintln(r.stderr, r.style(r.warn, "! ")+Sanitize(msg)) } // AlwaysWarn writes a warning to stderr even in --json mode. @@ -540,12 +554,12 @@ func (r *Renderer) AlwaysWarn(msg string) { if r.quiet { return } - fmt.Fprintln(r.stderr, r.style(r.warn, "! ")+msg) + fmt.Fprintln(r.stderr, r.style(r.warn, "! ")+Sanitize(msg)) } 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/output/untrusted.go b/internal/output/untrusted.go new file mode 100644 index 00000000..b0912ce5 --- /dev/null +++ b/internal/output/untrusted.go @@ -0,0 +1,58 @@ +package output + +import ( + "fmt" + "strings" +) + +// Sanitize renders terminal control sequences inert. Every value the Renderer +// and the interactive browser show passes through here first, because most of +// what they show came from somewhere else: API responses, store metadata, App +// User IDs a customer chose. A terminal *acts* on those bytes rather than +// printing them — OSC 52 rewrites the clipboard, CSI moves the cursor, CR +// overwrites the line just printed — so an identifier is enough to drive the +// reader's terminal. After this, remote text can only ever be shown. +// +// Newline and tab survive: they move the cursor the same way ordinary text +// does, and some payloads legitimately carry them. Every other C0/C1 control +// and DEL becomes its escaped literal (`\x1b`), which is also what a human +// needs to see to understand what the value actually contains. +// +// Sanitize is not applied to strings the CLI styled itself (Paint, Panel, +// Link) — those carry deliberate escapes and are built from our own literals. +func Sanitize(s string) string { + if !strings.ContainsFunc(s, isControl) { + return s + } + var b strings.Builder + b.Grow(len(s) + 8) + for _, r := range s { + switch { + case !isControl(r): + b.WriteRune(r) + case r > 0x7f: + fmt.Fprintf(&b, `\u%04x`, r) + default: + fmt.Fprintf(&b, `\x%02x`, r) + } + } + return b.String() +} + +// sanitizeAll is the slice form, for table rows and other cell collections. +func sanitizeAll(values []string) []string { + out := make([]string, len(values)) + for i, v := range values { + out[i] = Sanitize(v) + } + return out +} + +// isControl reports whether r drives the terminal instead of printing. +// C1 (0x80–0x9f) counts: terminals in 8-bit mode read 0x9b as CSI. +func isControl(r rune) bool { + if r == '\n' || r == '\t' { + return false + } + return r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f) +} diff --git a/internal/output/untrusted_test.go b/internal/output/untrusted_test.go new file mode 100644 index 00000000..e9a22b6c --- /dev/null +++ b/internal/output/untrusted_test.go @@ -0,0 +1,152 @@ +package output_test + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/revenuecat/cli/internal/output" +) + +// osc52 asks the terminal to replace the clipboard with "RCBB191". An App User +// ID is enough to carry it, so it stands in here for any remote string. +const osc52 = "rcbb_target\x1b]52;c;UkNCQjE5MQ==\x07" + +func TestSanitize_NeutralizesControlsAndLeavesTextAlone(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"plain text untouched", "cus_abc123", "cus_abc123"}, + {"unicode untouched", "日本語 café 🐈", "日本語 café 🐈"}, + {"newline and tab survive", "line\nnext\tcell", "line\nnext\tcell"}, + {"clipboard write", osc52, `rcbb_target\x1b]52;c;UkNCQjE5MQ==\x07`}, + {"cursor movement", "a\x1b[2Jb", `a\x1b[2Jb`}, + {"carriage return overwrite", "real\rfake", `real\x0dfake`}, + {"del", "a\x7fb", `a\x7fb`}, + {"c1 csi", "a\u009bb", `a\u009bb`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := output.Sanitize(tc.in); got != tc.want { + t.Errorf("Sanitize(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestRenderTable_EscapesControlSequencesAndKeepsAlignment(t *testing.T) { + r, out, _ := newR(false) + err := r.RenderTable(output.Table{ + Columns: []string{"ID", "PLATFORM"}, + Rows: [][]string{{osc52, "ios"}, {"cus_plain", "android"}}, + }) + if err != nil { + t.Fatal(err) + } + assertNoEscapes(t, "table stdout", out.String()) + if !strings.Contains(out.String(), `\x1b]52`) { + t.Errorf("expected the sequence shown as an escaped literal:\n%q", out.String()) + } + lines := strings.Split(strings.TrimRight(out.String(), "\n"), "\n") + for i, l := range lines[1:] { + if len(l) != len(lines[0]) { + t.Errorf("row %d width %d != header width %d (escaping must be included in the width)", i+1, len(l), len(lines[0])) + } + } +} + +// Escaping is not a side effect of --no-color: the colored path styles the +// value after it has been neutralized. +func TestRenderTable_EscapesControlSequencesWithColorEnabled(t *testing.T) { + var out, errb strings.Builder + r := output.NewRenderer(&out, &errb, false, false, false, "") + err := r.RenderTable(output.Table{Columns: []string{"ID"}, Rows: [][]string{{osc52}}}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(out.String(), "\x1b]52") || strings.ContainsRune(out.String(), 0x07) { + t.Errorf("colored table leaked the clipboard sequence:\n%q", out.String()) + } +} + +func TestRenderCard_EscapesControlSequencesInEverySlot(t *testing.T) { + r, out, _ := newR(false) + err := r.RenderCard(output.Card{ + Title: osc52, + Subtitle: osc52, + Sections: []output.CardSection{ + {Heading: "Active entitlements", Chips: []output.Chip{{Label: osc52}}}, + {Heading: "Subscriptions", Table: &output.CardTable{Columns: []string{"ID"}, Rows: [][]string{{osc52}}}}, + {Heading: "Attributes", Lines: []output.CardLine{{Key: osc52, Value: osc52}}}, + }, + }) + if err != nil { + t.Fatal(err) + } + assertNoEscapes(t, "card stdout", out.String()) +} + +func TestRenderHuman_EscapesControlSequencesInKeysAndValues(t *testing.T) { + r, out, _ := newR(false) + if err := r.Render(map[string]string{"id": osc52, osc52: "value"}); err != nil { + t.Fatal(err) + } + assertNoEscapes(t, "human stdout", out.String()) +} + +func TestChatter_EscapesControlSequences(t *testing.T) { + cases := map[string]func(r *output.Renderer){ + "success": func(r *output.Renderer) { r.Success(osc52) }, + "info": func(r *output.Renderer) { r.Info(osc52) }, + "warn": func(r *output.Renderer) { r.Warn(osc52) }, + "always": func(r *output.Renderer) { r.AlwaysWarn(osc52) }, + "error": func(r *output.Renderer) { r.Error(osc52) }, + "hint": func(r *output.Renderer) { r.Hint(osc52) }, + "title": func(r *output.Renderer) { r.Title(osc52) }, + "lead": func(r *output.Renderer) { r.Lead(osc52) }, + "notice": func(r *output.Renderer) { r.Notice(osc52) }, + "answer": func(r *output.Renderer) { r.Answer("Customer", osc52) }, + "field": func(r *output.Renderer) { r.Field("Customer", osc52, osc52) }, + "plan": func(r *output.Renderer) { r.Plan([]string{osc52}) }, + "link": func(r *output.Renderer) { r.LinkLine("https://example.com/" + osc52) }, + } + for name, call := range cases { + t.Run(name, func(t *testing.T) { + r, out, errb := newR(false) + call(r) + assertNoEscapes(t, name+" stderr", errb.String()) + assertNoEscapes(t, name+" stdout", out.String()) + }) + } +} + +// --json is the agent contract and must keep carrying the exact bytes, safely: +// the JSON encoder escapes them, so nothing reaches the terminal as a sequence. +func TestRenderJSON_KeepsControlBytesEncodedNotExecuted(t *testing.T) { + r, out, _ := newR(true) + if err := r.Render(map[string]string{"id": osc52}); err != nil { + t.Fatal(err) + } + assertNoEscapes(t, "json stdout", out.String()) + var got struct { + Data map[string]string `json:"data"` + } + if err := json.Unmarshal(out.Bytes(), &got); err != nil { + t.Fatal(err) + } + if got.Data["id"] != osc52 { + t.Errorf("--json must round-trip the raw value; got %q", got.Data["id"]) + } +} + +func assertNoEscapes(t *testing.T, where, s string) { + t.Helper() + if strings.ContainsRune(s, 0x1b) { + t.Errorf("%s carries a raw escape byte a terminal would act on:\n%q", where, s) + } + if strings.ContainsRune(s, 0x07) { + t.Errorf("%s carries a raw BEL:\n%q", where, s) + } +} diff --git a/internal/tui/browser.go b/internal/tui/browser.go index 04cf27f3..a163bda0 100644 --- a/internal/tui/browser.go +++ b/internal/tui/browser.go @@ -130,21 +130,75 @@ type bframe struct { } func newListFrame(title string, items []BrowserItem) bframe { - return bframe{kind: kindList, title: title, all: items} + return bframe{kind: kindList, title: output.Sanitize(title), all: sanitizeItems(items)} } func newTableFrame(title string, cols []string, items []BrowserItem) bframe { - return bframe{kind: kindTable, title: title, all: items, tableCols: cols} + return bframe{kind: kindTable, title: output.Sanitize(title), all: sanitizeItems(items), tableCols: cols} } func newDetailFrame(item BrowserItem) bframe { return bframe{ kind: kindDetail, - item: item, + item: sanitizeItem(item), autoLoading: item.AutoLoad != nil, } } +// Frames are the only way data enters the browser, so sanitizing here covers +// every view: remote text (customer IDs, product names) would otherwise reach +// the terminal as escape sequences it acts on rather than characters it draws. +// Lazily-loaded children come back through these same constructors. +func sanitizeItems(items []BrowserItem) []BrowserItem { + out := make([]BrowserItem, len(items)) + for i, it := range items { + out[i] = sanitizeItem(it) + } + return out +} + +func sanitizeItem(it BrowserItem) BrowserItem { + it.ID = output.Sanitize(it.ID) + it.Label = output.Sanitize(it.Label) + it.Meta = output.Sanitize(it.Meta) + it.Row = sanitizeCells(it.Row) + fields := make([]BrowserField, len(it.Fields)) + for i, f := range it.Fields { + fields[i] = BrowserField{Key: output.Sanitize(f.Key), Value: output.Sanitize(f.Value)} + } + it.Fields = fields + links := make([]BrowserLink, len(it.Links)) + for i, l := range it.Links { + l.Label = output.Sanitize(l.Label) + links[i] = l + } + it.Links = links + return it +} + +func sanitizeSections(sections []BrowserSection) []BrowserSection { + out := make([]BrowserSection, len(sections)) + for i, sec := range sections { + sec.Title = output.Sanitize(sec.Title) + sec.Empty = output.Sanitize(sec.Empty) + rows := make([]BrowserSectionRow, len(sec.Rows)) + for j, row := range sec.Rows { + rows[j] = BrowserSectionRow{Cells: sanitizeCells(row.Cells), Item: row.Item} + } + sec.Rows = rows + out[i] = sec + } + return out +} + +func sanitizeCells(cells []string) []string { + out := make([]string, len(cells)) + for i, c := range cells { + out[i] = output.Sanitize(c) + } + return out +} + func (f *bframe) visible() []BrowserItem { if f.filter == "" { return f.all @@ -302,7 +356,7 @@ func (m *browser) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if msg.err != nil { f.autoErr = msg.err.Error() } else { - f.sections = msg.sections + f.sections = sanitizeSections(msg.sections) } // Clamp cursor so it stays within the new slot list. slots := detailSlots(f) @@ -484,7 +538,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() @@ -780,7 +834,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") diff --git a/internal/tui/browser_escape_test.go b/internal/tui/browser_escape_test.go new file mode 100644 index 00000000..f3296b3b --- /dev/null +++ b/internal/tui/browser_escape_test.go @@ -0,0 +1,56 @@ +package tui + +import ( + "strings" + "testing" +) + +// The interactive browser draws the same remote strings the non-interactive +// renderer does — a customer ID carrying OSC 52 would replace the reader's +// clipboard the moment the list paints. +const browserOSC52 = "rcbb_target\x1b]52;c;UkNCQjE5MQ==\x07" + +func TestBrowserViews_NeverDrawRemoteControlSequences(t *testing.T) { + item := BrowserItem{ + ID: browserOSC52, + Label: browserOSC52, + Meta: browserOSC52, + Row: []string{browserOSC52, "ios"}, + Fields: []BrowserField{{Key: "ID", Value: browserOSC52}}, + Links: []BrowserLink{{Label: browserOSC52}}, + } + sections := []BrowserSection{{ + Title: browserOSC52, + Cols: []string{"PRODUCT", "STORE"}, + Rows: []BrowserSectionRow{{Cells: []string{browserOSC52, "app_store"}}}, + }} + + frames := map[string]bframe{ + "list": newListFrame(browserOSC52, []BrowserItem{item}), + "table": newTableFrame(browserOSC52, []string{"ID", "PLATFORM"}, []BrowserItem{item}), + "detail": newDetailFrame(item), + } + for name, frame := range frames { + t.Run(name, func(t *testing.T) { + m := &browser{stack: []bframe{frame}, width: 120, height: 40} + // Sub-resources arrive asynchronously, after the frame was built. + m.Update(autoLoadedMsg{frameIdx: 0, sections: sections}) + view := m.View() + if strings.Contains(view, "\x1b]52") || strings.ContainsRune(view, 0x07) { + t.Errorf("%s view draws the clipboard sequence:\n%q", name, view) + } + if !strings.Contains(view, `\x1b]52`) { + t.Errorf("%s view should show the value as an escaped literal:\n%s", name, view) + } + }) + } +} + +func TestBrowserErrors_NeverDrawRemoteControlSequences(t *testing.T) { + frame := newListFrame("Customers", nil) + frame.autoErr = "server said: " + browserOSC52 + m := &browser{stack: []bframe{frame}, width: 120, height: 40, loadErr: "server said: " + browserOSC52} + if view := m.View(); strings.Contains(view, "\x1b]52") { + t.Errorf("error view draws the clipboard sequence:\n%q", view) + } +}