Skip to content

Commit 7b1d3ea

Browse files
committed
Strip ANSI/OSC sequences from API-controlled output
API-controlled strings (project/person/entity names, summaries, notices) 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, and the presenter format/affordance paths, with defense-in-depth ansi.Strip at the styled and markdown render sinks.
1 parent b6d427d commit 7b1d3ea

6 files changed

Lines changed: 78 additions & 23 deletions

File tree

internal/commands/timeline.go

Lines changed: 10 additions & 6 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,22 @@ 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).
398402
creatorName := "Someone"
399403
if e.Creator != nil && e.Creator.Name != "" {
400-
creatorName = e.Creator.Name
404+
creatorName = ansi.Strip(e.Creator.Name)
401405
}
402406

403-
action := e.Action
407+
action := ansi.Strip(e.Action)
404408
if action == "" {
405409
action = "updated"
406410
}
407411

408-
title := e.Title
412+
title := ansi.Strip(e.Title)
409413
if title == "" {
410-
title = e.SummaryExcerpt
414+
title = ansi.Strip(e.SummaryExcerpt)
411415
}
412416
// Truncate at rune boundary for proper Unicode handling
413417
if len([]rune(title)) > 40 {
@@ -507,7 +511,7 @@ func runTimelineWatch(cmd *cobra.Command, args []string, project, person string,
507511
if err != nil {
508512
return output.ErrUsage("Invalid person ID")
509513
}
510-
description = fmt.Sprintf("activity for %s", personName)
514+
description = fmt.Sprintf("activity for %s", ansi.Strip(personName))
511515
fetchFn = func(ctx context.Context) ([]basecamp.TimelineEvent, error) {
512516
result, err := app.Account().Timeline().PersonProgress(ctx, personID, opts)
513517
if err != nil {
@@ -525,7 +529,7 @@ func runTimelineWatch(cmd *cobra.Command, args []string, project, person string,
525529
if err != nil {
526530
return output.ErrUsage("Invalid project ID")
527531
}
528-
description = fmt.Sprintf("activity in %s", projectName)
532+
description = fmt.Sprintf("activity in %s", ansi.Strip(projectName))
529533
fetchFn = func(ctx context.Context) ([]basecamp.TimelineEvent, error) {
530534
r, err := app.Account().Timeline().ProjectTimeline(ctx, projectIDInt, opts)
531535
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: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"strings"
88

99
"charm.land/lipgloss/v2"
10+
"github.com/charmbracelet/x/ansi"
1011

1112
"github.com/basecamp/basecamp-cli/internal/richtext"
1213
"github.com/basecamp/basecamp-cli/internal/tui"
@@ -335,10 +336,12 @@ func renderAffordances(b *strings.Builder, schema *EntitySchema, data map[string
335336
b.WriteString("\n")
336337

337338
// Find max command width for alignment
339+
// Affordance commands interpolate API-controlled data values via
340+
// RenderTemplate; strip terminal escapes before rendering them.
338341
maxCmd := 0
339342
renderedCmds := make([]string, len(visible))
340343
for i, a := range visible {
341-
renderedCmds[i] = RenderTemplate(a.Cmd, data)
344+
renderedCmds[i] = ansi.Strip(RenderTemplate(a.Cmd, data))
342345
if len(renderedCmds[i]) > maxCmd {
343346
maxCmd = len(renderedCmds[i])
344347
}
@@ -759,7 +762,7 @@ func renderAffordancesMarkdown(b *strings.Builder, schema *EntitySchema, data ma
759762

760763
b.WriteString("\n#### Hints\n\n")
761764
for _, a := range visible {
762-
cmd := RenderTemplate(a.Cmd, data)
765+
cmd := ansi.Strip(RenderTemplate(a.Cmd, data))
763766
b.WriteString("- `" + cmd + "` — " + a.Label + "\n")
764767
}
765768
}

0 commit comments

Comments
 (0)