From b86fcb2ff55bfc184dfa56a5907d90580d0c5e90 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 11 Oct 2026 21:49:14 +0200 Subject: [PATCH] fix(pr): strip terminal control sequences from pr diff on a terminal pr diff copied the server's patch bytes straight to stdout, so a pull request could change the terminal title, clear the screen or move the cursor of whoever viewed its diff. Add terminal.SafeWriter, which filters streamed server text like terminal.SafeText when stdout is a terminal, and use it for pr diff. Piped output stays byte-exact, so `tea pr diff N | git apply` keeps working. Signed-off-by: bircni --- internal/app/pull_mutations.go | 2 +- internal/app/pull_mutations_test.go | 27 +++++++++ internal/terminal/safe_writer.go | 85 +++++++++++++++++++++++++++ internal/terminal/safe_writer_test.go | 45 ++++++++++++++ 4 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 internal/app/pull_mutations_test.go create mode 100644 internal/terminal/safe_writer.go create mode 100644 internal/terminal/safe_writer_test.go diff --git a/internal/app/pull_mutations.go b/internal/app/pull_mutations.go index b5eb3e98..9b8a707b 100644 --- a/internal/app/pull_mutations.go +++ b/internal/app/pull_mutations.go @@ -96,7 +96,7 @@ func (a *application) addPullMutations(group *cobra.Command) { return err } defer body.Close() - _, err = io.Copy(cmd.OutOrStdout(), body) + _, err = io.Copy(terminal.SafeWriter(cmd.OutOrStdout(), a.deps.Streams.OutTTY, false), body) return err }}) group.AddCommand(a.pullReviewCommand()) diff --git a/internal/app/pull_mutations_test.go b/internal/app/pull_mutations_test.go new file mode 100644 index 00000000..b393f201 --- /dev/null +++ b/internal/app/pull_mutations_test.go @@ -0,0 +1,27 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package app + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPullDiffStripsTerminalControlsOnlyOnATerminal(t *testing.T) { + const diff = "+hello \x1b]0;PWNED\x07 \x1b[31mred\x1b[0m\r\n-old\tline\n" + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/api/v1/repos/alice/project/pulls/2.diff", r.URL.Path) + _, _ = w.Write([]byte(diff)) + })) + defer server.Close() + for tty, want := range map[bool]string{false: diff, true: "+hello ]0;PWNED [31mred[0m\n-old\tline\n"} { + deps, out, _ := serverDeps(t, server.URL) + deps.Streams.OutTTY = tty + require.NoError(t, Execute(context.Background(), deps, []string{"pr", "diff", "2", "-R", "alice/project"})) + require.Equal(t, want, out.String(), "tty=%t", tty) + } +} diff --git a/internal/terminal/safe_writer.go b/internal/terminal/safe_writer.go new file mode 100644 index 00000000..ee45ed71 --- /dev/null +++ b/internal/terminal/safe_writer.go @@ -0,0 +1,85 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package terminal + +import ( + "io" + "unicode" + "unicode/utf8" +) + +// maxSGRLength bounds how many bytes of a possible SGR sequence are held back between writes. +const maxSGRLength = 32 + +// SafeWriter returns w unchanged when tty is false, so piped output stays byte-exact. +// On a terminal it filters streamed server text like SafeText, and with allowSGR it keeps +// SGR color sequences (ESC [ ... m) while dropping every other escape sequence's ESC. +func SafeWriter(w io.Writer, tty, allowSGR bool) io.Writer { + if !tty { + return w + } + return &safeWriter{w: w, allowSGR: allowSGR} +} + +type safeWriter struct { + w io.Writer + allowSGR bool + // pending holds a rune or SGR sequence split across writes. + pending []byte +} + +func (s *safeWriter) Write(p []byte) (int, error) { + data := append(s.pending, p...) + out := make([]byte, 0, len(data)) + i := 0 + for i < len(data) { + if s.allowSGR && data[i] == '\x1b' { + n, complete := sgrLength(data[i:]) + if !complete { + break + } + if n == 0 { + i++ + continue + } + out = append(out, data[i:i+n]...) + i += n + continue + } + if !utf8.FullRune(data[i:]) { + break + } + r, size := utf8.DecodeRune(data[i:]) + if r == '\t' || r == '\n' || !(unicode.IsControl(r) || unicode.Is(unicode.Cf, r)) { + out = utf8.AppendRune(out, r) + } + i += size + } + s.pending = append(s.pending[:0], data[i:]...) + if _, err := s.w.Write(out); err != nil { + return 0, err + } + return len(p), nil +} + +// sgrLength returns the length of the SGR sequence at the start of b, which begins with ESC, +// or 0 when b starts another escape sequence. complete is false while b may be a truncated SGR sequence. +func sgrLength(b []byte) (n int, complete bool) { + if len(b) < 2 { + return 0, false + } + if b[1] != '[' { + return 0, true + } + for j := 2; j < len(b); j++ { + c := b[j] + if c == 'm' { + return j + 1, true + } + if j >= maxSGRLength || ((c < '0' || c > '9') && c != ';' && c != ':') { + return 0, true + } + } + return 0, false +} diff --git a/internal/terminal/safe_writer_test.go b/internal/terminal/safe_writer_test.go new file mode 100644 index 00000000..05bb9525 --- /dev/null +++ b/internal/terminal/safe_writer_test.go @@ -0,0 +1,45 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package terminal + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSafeWriter(t *testing.T) { + const hostile = "+hello \x1b]0;PWNED\x07 \x1b[1;31mred\x1b[0m\x1b[2J\r\n\u009b2J\u202eend\t\xff\x1b[1111111111111111111111111111111111m\n" + var piped bytes.Buffer + _, err := SafeWriter(&piped, false, false).Write([]byte(hostile)) + require.NoError(t, err) + require.Equal(t, hostile, piped.String()) + for _, tc := range []struct { + name string + allowSGR bool + want string + }{ + {"drops every escape", false, "+hello ]0;PWNED [1;31mred[0m[2J\n2Jend\t\uFFFD[1111111111111111111111111111111111m\n"}, + {"keeps colors", true, "+hello ]0;PWNED \x1b[1;31mred\x1b[0m[2J\n2Jend\t\uFFFD[1111111111111111111111111111111111m\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + if !tc.allowSGR { + require.Equal(t, SafeText(hostile), tc.want) + } + for _, chunk := range []int{len(hostile), 1, 3} { + var out bytes.Buffer + w := SafeWriter(&out, true, tc.allowSGR) + for data := []byte(hostile); len(data) > 0; { + n := min(chunk, len(data)) + written, err := w.Write(data[:n]) + require.NoError(t, err) + require.Equal(t, n, written) + data = data[n:] + } + require.Equal(t, tc.want, out.String(), "chunk size %d", chunk) + } + }) + } +}