mirror of
https://gitea.com/gitea/tea.git
synced 2026-10-26 11:01:25 +01:00
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 <bircni@icloud.com>
This commit is contained in:
4 files changed
+58
-3
No files matched your search
@@ -12,6 +12,7 @@ import (
|
|||||||
|
|
||||||
"gitea.dev/tea/internal/clierr"
|
"gitea.dev/tea/internal/clierr"
|
||||||
"gitea.dev/tea/internal/services"
|
"gitea.dev/tea/internal/services"
|
||||||
|
"gitea.dev/tea/internal/terminal"
|
||||||
"github.com/spf13/cobra"
|
"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)
|
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 follow {
|
||||||
if len(jobs) != 1 {
|
if len(jobs) != 1 {
|
||||||
return clierr.Wrap(clierr.Usage, errors.New("--follow requires exactly one job; select --job"))
|
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)
|
ctx, cancel := context.WithTimeout(cmd.Context(), timeout)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
return service.FollowJobLogs(ctx, run, jobs[0].ID, interval, func(chunk []byte) error {
|
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) {
|
if err == nil && n != len(chunk) {
|
||||||
return io.ErrShortWrite
|
return io.ErrShortWrite
|
||||||
}
|
}
|
||||||
@@ -84,14 +86,14 @@ func (a *application) runLogsCommand() *cobra.Command {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
for _, job := range jobs {
|
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
|
return err
|
||||||
}
|
}
|
||||||
logs, err := service.JobLogs(cmd.Context(), job.ID)
|
logs, err := service.JobLogs(cmd.Context(), job.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
_, copyErr := io.Copy(cmd.OutOrStdout(), logs)
|
_, copyErr := io.Copy(out, logs)
|
||||||
closeErr := logs.Close()
|
closeErr := logs.Close()
|
||||||
if copyErr != nil {
|
if copyErr != nil {
|
||||||
return copyErr
|
return copyErr
|
||||||
|
|||||||
@@ -64,3 +64,30 @@ func TestRunLogsStreamWithoutMixingDiagnosticsAndVerifyJobMembership(t *testing.
|
|||||||
require.Empty(t, out.String())
|
require.Empty(t, out.String())
|
||||||
require.Empty(t, diagnostics.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())
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -17,6 +17,7 @@ import (
|
|||||||
|
|
||||||
"gitea.dev/tea/internal/clierr"
|
"gitea.dev/tea/internal/clierr"
|
||||||
"gitea.dev/tea/internal/services"
|
"gitea.dev/tea/internal/services"
|
||||||
|
"gitea.dev/tea/internal/terminal"
|
||||||
"github.com/spf13/cobra"
|
"github.com/spf13/cobra"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -209,6 +210,9 @@ func (a *application) apiCommand() *cobra.Command {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
return writeOutput(cmd, output, func(destination io.Writer) error {
|
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 {
|
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 include {
|
||||||
if _, err := fmt.Fprintf(cmd.ErrOrStderr(), "%s %s\n", response.Proto, response.Status); err != nil {
|
if _, err := fmt.Fprintf(cmd.ErrOrStderr(), "%s %s\n", response.Proto, response.Status); err != nil {
|
||||||
|
|||||||
@@ -161,6 +161,28 @@ func TestAPIHeadersPreserveRawResponseAndRejectCredentialOverrides(t *testing.T)
|
|||||||
require.Equal(t, 1, calls)
|
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 }
|
type rejectAPIInput struct{ t *testing.T }
|
||||||
|
|
||||||
func (r rejectAPIInput) Read([]byte) (int, error) {
|
func (r rejectAPIInput) Read([]byte) (int, error) {
|
||||||
|
|||||||
Reference in new issue
Block a user