From ab6228e25b7c82398d70c8a612a86cd72a3007fc Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 11 Oct 2026 21:50:36 +0200 Subject: [PATCH] fix: sanitize run logs and api responses written to a terminal run logs and api copied server bytes straight to stdout. CI logs are attacker-influenced through pull request workflows, so they could drive the terminal of whoever read them. On a terminal, run logs now keep only SGR color sequences, job names are reduced to plain text, and api drops control characters from the response. Piped output and --output-file stay byte-exact. Signed-off-by: bircni --- internal/app/action_logs.go | 8 +++++--- internal/app/action_logs_test.go | 27 +++++++++++++++++++++++++++ internal/app/api.go | 4 ++++ internal/app/api_test.go | 22 ++++++++++++++++++++++ 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/internal/app/action_logs.go b/internal/app/action_logs.go index 5a93e73b..bd1ea60f 100644 --- a/internal/app/action_logs.go +++ b/internal/app/action_logs.go @@ -12,6 +12,7 @@ import ( "gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/services" + "gitea.dev/tea/internal/terminal" "github.com/spf13/cobra" ) @@ -69,6 +70,7 @@ func (a *application) runLogsCommand() *cobra.Command { return fmt.Errorf("job %d does not belong to run %d", selected, run) } } + out := terminal.SafeWriter(cmd.OutOrStdout(), a.deps.Streams.OutTTY, true) if follow { if len(jobs) != 1 { return clierr.Wrap(clierr.Usage, errors.New("--follow requires exactly one job; select --job")) @@ -76,7 +78,7 @@ func (a *application) runLogsCommand() *cobra.Command { ctx, cancel := context.WithTimeout(cmd.Context(), timeout) defer cancel() return service.FollowJobLogs(ctx, run, jobs[0].ID, interval, func(chunk []byte) error { - n, err := cmd.OutOrStdout().Write(chunk) + n, err := out.Write(chunk) if err == nil && n != len(chunk) { return io.ErrShortWrite } @@ -84,14 +86,14 @@ func (a *application) runLogsCommand() *cobra.Command { }) } for _, job := range jobs { - if _, err := fmt.Fprintf(cmd.ErrOrStderr(), "Job %d: %s\n", job.ID, job.Name); err != nil { + if _, err := fmt.Fprintf(cmd.ErrOrStderr(), "Job %d: %s\n", job.ID, terminal.SafeLine(job.Name)); err != nil { return err } logs, err := service.JobLogs(cmd.Context(), job.ID) if err != nil { return err } - _, copyErr := io.Copy(cmd.OutOrStdout(), logs) + _, copyErr := io.Copy(out, logs) closeErr := logs.Close() if copyErr != nil { return copyErr diff --git a/internal/app/action_logs_test.go b/internal/app/action_logs_test.go index 1229d248..901852bc 100644 --- a/internal/app/action_logs_test.go +++ b/internal/app/action_logs_test.go @@ -64,3 +64,30 @@ func TestRunLogsStreamWithoutMixingDiagnosticsAndVerifyJobMembership(t *testing. require.Empty(t, out.String()) require.Empty(t, diagnostics.String()) } + +func TestRunLogsKeepOnlyColorsOnATerminal(t *testing.T) { + const logs = "\x1b[36mstep\x1b[0m \x1b]0;PWNED\x07done\x1b[2J\r\n" + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/repos/owner/repo/actions/runs/7/jobs": + if r.URL.Query().Get("page") == "1" { + _, _ = w.Write([]byte(`{"jobs":[{"id":9,"name":"Build\u001b]0;PWNED\u0007"}]}`)) + } else { + _, _ = w.Write([]byte(`{"jobs":[]}`)) + } + case "/api/v1/repos/owner/repo/actions/jobs/9/logs": + _, _ = w.Write([]byte(logs)) + default: + t.Errorf("unexpected endpoint %s", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + for tty, want := range map[bool]string{false: logs, true: "\x1b[36mstep\x1b[0m ]0;PWNEDdone[2J\n"} { + deps, out, diagnostics := serverDeps(t, server.URL) + deps.Streams.OutTTY = tty + require.NoError(t, Execute(context.Background(), deps, []string{"run", "logs", "7", "-R", "owner/repo", "--job=9", "--no-input"})) + require.Equal(t, want, out.String(), "tty=%t", tty) + require.Equal(t, "Job 9: Build]0;PWNED\n", diagnostics.String()) + } +} diff --git a/internal/app/api.go b/internal/app/api.go index 22b1c79c..8aac7cda 100644 --- a/internal/app/api.go +++ b/internal/app/api.go @@ -17,6 +17,7 @@ import ( "gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/services" + "gitea.dev/tea/internal/terminal" "github.com/spf13/cobra" ) @@ -209,6 +210,9 @@ func (a *application) apiCommand() *cobra.Command { } } return writeOutput(cmd, output, func(destination io.Writer) error { + if output == "" || output == "-" { + destination = terminal.SafeWriter(destination, a.deps.Streams.OutTTY, false) + } return (services.API{Client: target.client}).Visit(cmd.Context(), services.APIOptions{Endpoint: endpoint, Method: method, Body: body, Headers: headers, Paginate: paginate, MaxPages: maxPages}, func(response *http.Response) error { if include { if _, err := fmt.Fprintf(cmd.ErrOrStderr(), "%s %s\n", response.Proto, response.Status); err != nil { diff --git a/internal/app/api_test.go b/internal/app/api_test.go index 0d683a30..b27fbd03 100644 --- a/internal/app/api_test.go +++ b/internal/app/api_test.go @@ -161,6 +161,28 @@ func TestAPIHeadersPreserveRawResponseAndRejectCredentialOverrides(t *testing.T) require.Equal(t, 1, calls) } +func TestAPIStripsTerminalControlsOnlyOnATerminalStdout(t *testing.T) { + const body = "plain \x1b]0;PWNED\x07\x1b[31mtext\x00\n" + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(body)) + })) + defer server.Close() + for tty, want := range map[bool]string{false: body, true: "plain ]0;PWNED[31mtext\n"} { + deps, out, _ := serverDeps(t, server.URL) + deps.Streams.OutTTY = tty + require.NoError(t, Execute(context.Background(), deps, []string{"api", "user/data"})) + require.Equal(t, want, out.String(), "tty=%t", tty) + } + deps, out, _ := serverDeps(t, server.URL) + deps.Streams.OutTTY = true + path := filepath.Join(t.TempDir(), "response") + require.NoError(t, Execute(context.Background(), deps, []string{"api", "user/data", "--output-file", path})) + data, err := os.ReadFile(path) + require.NoError(t, err) + require.Equal(t, body, string(data)) + require.Empty(t, out.String()) +} + type rejectAPIInput struct{ t *testing.T } func (r rejectAPIInput) Read([]byte) (int, error) {