Skip to content

Commit 9103d13

Browse files
committed
Strip ANSI/OSC sequences from API-controlled output
API-controlled strings (names, summaries, notices, headlines, affordance commands) reached styled/markdown sinks and the --watch TUI without ANSI stripping, allowing terminal/OSC injection. Sanitize WithSummary/WithNotice/ WithDiagnostic at the source, the timeline watch TUI fields (creator name stripped before the empty check so an all-escape name still falls back to a placeholder), and presenter format helpers, with defense-in-depth at the render sinks. Stripping is centralized in RenderTemplate + RenderHeadline so schema headlines ({{.name}}/{{.subject}}/{{.content}}) and affordance commands are all covered, including the identity-label headline fallback.
1 parent 447cbf3 commit 9103d13

7 files changed

Lines changed: 90 additions & 24 deletions

File tree

internal/commands/timeline.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
tea "charm.land/bubbletea/v2"
1515
"charm.land/lipgloss/v2"
1616
"github.com/basecamp/basecamp-sdk/go/pkg/basecamp"
17+
"github.com/charmbracelet/x/ansi"
1718
"github.com/spf13/cobra"
1819

1920
"github.com/basecamp/basecamp-cli/internal/appctx"
@@ -395,19 +396,26 @@ func formatEvent(e basecamp.TimelineEvent) string {
395396
timeStr = "--:--"
396397
}
397398

399+
// API-controlled fields are ANSI-stripped before rendering: rune truncation
400+
// preserves escape bytes, and the alt-screen watch TUI would otherwise
401+
// execute embedded OSC/ANSI sequences (terminal injection).
402+
// Strip before the empty check so a name that's only escape sequences
403+
// still falls back to the placeholder rather than rendering blank.
398404
creatorName := "Someone"
399-
if e.Creator != nil && e.Creator.Name != "" {
400-
creatorName = e.Creator.Name
405+
if e.Creator != nil {
406+
if name := ansi.Strip(e.Creator.Name); name != "" {
407+
creatorName = name
408+
}
401409
}
402410

403-
action := e.Action
411+
action := ansi.Strip(e.Action)
404412
if action == "" {
405413
action = "updated"
406414
}
407415

408-
title := e.Title
416+
title := ansi.Strip(e.Title)
409417
if title == "" {
410-
title = e.SummaryExcerpt
418+
title = ansi.Strip(e.SummaryExcerpt)
411419
}
412420
// Truncate at rune boundary for proper Unicode handling
413421
if len([]rune(title)) > 40 {
@@ -507,7 +515,7 @@ func runTimelineWatch(cmd *cobra.Command, args []string, project, person string,
507515
if err != nil {
508516
return output.ErrUsage("Invalid person ID")
509517
}
510-
description = fmt.Sprintf("activity for %s", personName)
518+
description = fmt.Sprintf("activity for %s", ansi.Strip(personName))
511519
fetchFn = func(ctx context.Context) ([]basecamp.TimelineEvent, error) {
512520
result, err := app.Account().Timeline().PersonProgress(ctx, personID, opts)
513521
if err != nil {
@@ -525,7 +533,7 @@ func runTimelineWatch(cmd *cobra.Command, args []string, project, person string,
525533
if err != nil {
526534
return output.ErrUsage("Invalid project ID")
527535
}
528-
description = fmt.Sprintf("activity in %s", projectName)
536+
description = fmt.Sprintf("activity in %s", ansi.Strip(projectName))
529537
fetchFn = func(ctx context.Context) ([]basecamp.TimelineEvent, error) {
530538
r, err := app.Account().Timeline().ProjectTimeline(ctx, projectIDInt, opts)
531539
if err != nil {

internal/output/envelope.go

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"os"
88
"strings"
99

10+
"github.com/charmbracelet/x/ansi"
1011
"github.com/itchyny/gojq"
1112

1213
clioutput "github.com/basecamp/cli/output"
@@ -421,22 +422,26 @@ func (w *Writer) writeLiteralMarkdown(v any) error {
421422
type ResponseOption func(*Response)
422423

423424
// WithSummary adds a summary to the response.
425+
// Summaries frequently interpolate API-controlled strings (project/person/
426+
// entity names), so ANSI/OSC escape sequences are stripped at the source to
427+
// prevent terminal injection in every styled/markdown sink.
424428
func WithSummary(s string) ResponseOption {
425-
return func(r *Response) { r.Summary = s }
429+
return func(r *Response) { r.Summary = ansi.Strip(s) }
426430
}
427431

428432
// WithNotice adds an informational notice to the response.
429433
// Use this for non-error messages like truncation warnings.
434+
// Like WithSummary, the value is ANSI-stripped at the source.
430435
func WithNotice(s string) ResponseOption {
431-
return func(r *Response) { r.Notice = s; r.noticeDiagnostic = false }
436+
return func(r *Response) { r.Notice = ansi.Strip(s); r.noticeDiagnostic = false }
432437
}
433438

434439
// WithDiagnostic sets a notice that is also emitted to stderr in quiet mode.
435440
// Use this for degraded-operation warnings (e.g. unresolved mentions) that
436441
// automation consumers need to detect. Truncation and other informational
437442
// notices should use WithNotice instead.
438443
func WithDiagnostic(s string) ResponseOption {
439-
return func(r *Response) { r.Notice = s; r.noticeDiagnostic = true }
444+
return func(r *Response) { r.Notice = ansi.Strip(s); r.noticeDiagnostic = true }
440445
}
441446

442447
// WithBreadcrumbs adds breadcrumbs to the response.
@@ -530,13 +535,16 @@ func (w *Writer) presentStyledEntity(resp *Response) bool {
530535
var out strings.Builder
531536
r := NewRenderer(w.opts.Writer, true)
532537

538+
// ansi.Strip defends against terminal injection from API-controlled
539+
// summary/notice content (already stripped at the WithSummary/WithNotice
540+
// source; repeated here as defense-in-depth at the render sink).
533541
if resp.Summary != "" {
534-
out.WriteString(r.Summary.Render(resp.Summary))
542+
out.WriteString(r.Summary.Render(ansi.Strip(resp.Summary)))
535543
out.WriteString("\n")
536544
}
537545

538546
if resp.Notice != "" {
539-
out.WriteString(r.Hint.Render(resp.Notice))
547+
out.WriteString(r.Hint.Render(ansi.Strip(resp.Notice)))
540548
out.WriteString("\n")
541549
}
542550

@@ -589,12 +597,13 @@ func (w *Writer) presentMarkdownEntity(resp *Response) bool {
589597
var out strings.Builder
590598
mr := NewMarkdownRenderer(w.opts.Writer)
591599

600+
// Defense-in-depth ANSI stripping (see presentStyledEntity).
592601
if resp.Summary != "" {
593-
out.WriteString("## " + resp.Summary + "\n")
602+
out.WriteString("## " + ansi.Strip(resp.Summary) + "\n")
594603
}
595604

596605
if resp.Notice != "" {
597-
out.WriteString("*" + resp.Notice + "*\n")
606+
out.WriteString("*" + ansi.Strip(resp.Notice) + "*\n")
598607
}
599608

600609
if resp.Summary != "" || resp.Notice != "" {

internal/output/output_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3565,3 +3565,34 @@ func TestPluralNoun(t *testing.T) {
35653565
assert.Equal(t, tt.expected, PluralNoun(tt.input), "PluralNoun(%q)", tt.input)
35663566
}
35673567
}
3568+
3569+
// TestWithSummaryStripsANSI verifies that API-controlled summary/notice content
3570+
// is sanitized of terminal escape sequences at the source, preventing terminal
3571+
// injection in every styled/markdown sink.
3572+
func TestWithSummaryStripsANSI(t *testing.T) {
3573+
// OSC 8 hyperlink + CSI color sequence wrapping a hostile payload.
3574+
payload := "\x1b]8;;http://evil\x07click\x1b]8;;\x07\x1b[31mpwn\x1b[0m"
3575+
3576+
t.Run("WithSummary", func(t *testing.T) {
3577+
r := &Response{}
3578+
WithSummary(payload)(r)
3579+
assert.Equal(t, ansi.Strip(payload), r.Summary)
3580+
assert.NotContains(t, r.Summary, "\x1b")
3581+
})
3582+
3583+
t.Run("WithNotice", func(t *testing.T) {
3584+
r := &Response{}
3585+
WithNotice(payload)(r)
3586+
assert.Equal(t, ansi.Strip(payload), r.Notice)
3587+
assert.NotContains(t, r.Notice, "\x1b")
3588+
assert.False(t, r.noticeDiagnostic)
3589+
})
3590+
3591+
t.Run("WithDiagnostic", func(t *testing.T) {
3592+
r := &Response{}
3593+
WithDiagnostic(payload)(r)
3594+
assert.Equal(t, ansi.Strip(payload), r.Notice)
3595+
assert.NotContains(t, r.Notice, "\x1b")
3596+
assert.True(t, r.noticeDiagnostic)
3597+
})
3598+
}

internal/output/render.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,17 @@ func terminalInfo(w io.Writer) (width int, isTTY bool) {
112112
func (r *Renderer) RenderResponse(w io.Writer, resp *Response) error {
113113
var b strings.Builder
114114

115-
// Summary line
115+
// Summary line. ansi.Strip guards against terminal injection from
116+
// API-controlled summary/notice content (defense-in-depth: also stripped
117+
// at the WithSummary/WithNotice source).
116118
if resp.Summary != "" {
117-
b.WriteString(r.Summary.Render(resp.Summary))
119+
b.WriteString(r.Summary.Render(ansi.Strip(resp.Summary)))
118120
b.WriteString("\n")
119121
}
120122

121123
// Notice (e.g., truncation warning)
122124
if resp.Notice != "" {
123-
b.WriteString(r.Hint.Render(resp.Notice))
125+
b.WriteString(r.Hint.Render(ansi.Strip(resp.Notice)))
124126
b.WriteString("\n")
125127
}
126128

@@ -1116,14 +1118,15 @@ func NewMarkdownRenderer(w io.Writer) *MarkdownRenderer {
11161118
func (r *MarkdownRenderer) RenderResponse(w io.Writer, resp *Response) error {
11171119
var b strings.Builder
11181120

1119-
// Summary as heading
1121+
// Summary as heading. ansi.Strip guards against terminal injection
1122+
// (defense-in-depth; also stripped at the WithSummary/WithNotice source).
11201123
if resp.Summary != "" {
1121-
b.WriteString("## " + resp.Summary + "\n")
1124+
b.WriteString("## " + ansi.Strip(resp.Summary) + "\n")
11221125
}
11231126

11241127
// Notice (e.g., truncation warning)
11251128
if resp.Notice != "" {
1126-
b.WriteString("*" + resp.Notice + "*\n")
1129+
b.WriteString("*" + ansi.Strip(resp.Notice) + "*\n")
11271130
}
11281131

11291132
if resp.Summary != "" || resp.Notice != "" {

internal/presenter/format.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"strings"
99
"time"
1010

11+
"github.com/charmbracelet/x/ansi"
12+
1113
"github.com/basecamp/basecamp-cli/internal/richtext"
1214
)
1315

@@ -129,7 +131,7 @@ func formatPeople(val any) string {
129131
for _, item := range arr {
130132
if m, ok := item.(map[string]any); ok {
131133
if name, ok := m["name"].(string); ok {
132-
names = append(names, name)
134+
names = append(names, ansi.Strip(name))
133135
}
134136
}
135137
}
@@ -263,7 +265,7 @@ func dockPosition(m map[string]any) int {
263265
func formatPerson(val any) string {
264266
if m, ok := val.(map[string]any); ok {
265267
if name, ok := m["name"].(string); ok {
266-
return name
268+
return ansi.Strip(name)
267269
}
268270
}
269271
return ""
@@ -295,6 +297,9 @@ func formatText(val any) string {
295297
case nil:
296298
return ""
297299
case string:
300+
// Strip terminal escape sequences from API-controlled strings before
301+
// they reach a styled/markdown sink (terminal injection defense).
302+
v = ansi.Strip(v)
298303
if richtext.IsHTML(v) {
299304
return richtext.HTMLToMarkdown(v)
300305
}

internal/presenter/render.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,8 @@ func renderAffordances(b *strings.Builder, schema *EntitySchema, data map[string
335335
b.WriteString("\n")
336336

337337
// Find max command width for alignment
338+
// RenderTemplate strips terminal escapes from the interpolated, API-controlled
339+
// command (centralized in template.go).
338340
maxCmd := 0
339341
renderedCmds := make([]string, len(visible))
340342
for i, a := range visible {

internal/presenter/template.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"regexp"
88
"text/template"
99

10+
"github.com/charmbracelet/x/ansi"
11+
1012
"github.com/basecamp/basecamp-cli/internal/richtext"
1113
)
1214

@@ -34,7 +36,11 @@ func RenderTemplate(tmpl string, data map[string]any) string {
3436
if err := t.Execute(&buf, sanitizeNumericValues(data)); err != nil {
3537
return "<template error>"
3638
}
37-
return buf.String()
39+
// Strip terminal escape sequences: templates interpolate API-controlled
40+
// data (names, subjects, content) and the result reaches lipgloss.Render
41+
// via headlines and affordance commands. Centralizing here covers every
42+
// RenderTemplate caller (terminal-injection defense).
43+
return ansi.Strip(buf.String())
3844
}
3945

4046
// EvalCondition evaluates a template condition (from affordance "when" field).
@@ -73,7 +79,9 @@ func sanitizeNumericValues(data map[string]any) map[string]any {
7379
// or an outer **...** wrapper), so nested markers would produce visual noise
7480
// like ****word****.
7581
func RenderHeadline(schema *EntitySchema, data map[string]any) string {
76-
raw := renderHeadlineRaw(schema, data)
82+
// Strip here too: the identity-label fallback in renderHeadlineRaw formats
83+
// a raw data value without going through RenderTemplate.
84+
raw := ansi.Strip(renderHeadlineRaw(schema, data))
7785
if richtext.IsHTML(raw) {
7886
md := singleLine(richtext.HTMLToMarkdown(raw))
7987
return reBoldWrap.ReplaceAllString(md, "$1")

0 commit comments

Comments
 (0)