From 9a3dc9fce7bb5997f96fbaa04847dd15b2494930 Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 23 Apr 2026 13:41:26 +0200 Subject: [PATCH 1/2] Fix log terminal injection --- .../run-view-log-escape-sequences.txtar | 70 +++++++++++++++++++ pkg/cmd/run/view/view.go | 5 +- pkg/cmd/run/view/view_test.go | 40 +++++++++++ 3 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 acceptance/testdata/workflow/run-view-log-escape-sequences.txtar diff --git a/acceptance/testdata/workflow/run-view-log-escape-sequences.txtar b/acceptance/testdata/workflow/run-view-log-escape-sequences.txtar new file mode 100644 index 000000000..14c75cd86 --- /dev/null +++ b/acceptance/testdata/workflow/run-view-log-escape-sequences.txtar @@ -0,0 +1,70 @@ +# This test ensure that a malicious workflow which emit terminal control sequences (ESC, OSC, CSI) in +# its log output does not result in terminal injection when logs are displayed using `gh run view --log` + +# Use gh as a credential helper +exec gh auth setup-git + +# Create a repository with a file so it has a default branch +exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private + +# Defer repo cleanup +defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Clone the repo +exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING + +# Commit the workflow file +cd $SCRIPT_NAME-$RANDOM_STRING +mkdir .github/workflows +mv ../workflow.yml .github/workflows/workflow.yml +exec git add .github/workflows/workflow.yml +exec git commit -m 'Create workflow with escape sequences' +exec git push -u origin main + +# Sleep because it takes a second for the workflow to register +sleep 1 + +# Run the workflow +exec gh workflow run 'Escape Sequence PoC' + +# It takes some time for a workflow run to register +sleep 10 + +# Get the run ID we want to view +exec gh run list --json databaseId --jq '.[0].databaseId' +stdout2env RUN_ID + +# Wait for workflow to complete +exec gh run watch $RUN_ID --exit-status + +# View the logs and check that raw ESC bytes (0x1b) are NOT present in output. +# If this assertion fails, it means terminal escape sequences from the workflow +# log are being passed through to the user's terminal unsanitised. +exec gh run view $RUN_ID --log + +# The output should contain the safe/visible text but not raw ESC bytes. +# \x1b is the ESC byte - it must not appear in the output. +! stdout '\x1b' + +# The log output should still contain the non-escape parts of the log lines. +stdout 'ESCAPE_MARKER_START' +stdout 'ESCAPE_MARKER_END' + +-- workflow.yml -- +name: Escape Sequence PoC + +on: + workflow_dispatch: + +jobs: + emit-escape-sequences: + runs-on: ubuntu-latest + steps: + - name: Emit terminal escape sequences + run: | + # OSC title set: \x1b]0;TITLE\x07 + printf 'ESCAPE_MARKER_START \033]0;HIJACKED_TITLE\007 ESCAPE_MARKER_END\n' + # CSI color: \x1b[31m ... \x1b[0m + printf 'ESCAPE_MARKER_START \033[31mRED_TEXT\033[0m ESCAPE_MARKER_END\n' + # Screen title set (from original PoC): \x1bk ... \x1b\\ + printf 'ESCAPE_MARKER_START \033k;malicious command;\033\\ ESCAPE_MARKER_END\n' diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index bed9e3bfa..3e5199452 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -22,7 +22,9 @@ import ( "github.com/cli/cli/v2/pkg/cmd/run/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/go-gh/v2/pkg/asciisanitizer" "github.com/spf13/cobra" + "golang.org/x/text/transform" ) type RunLogCache struct { @@ -579,7 +581,8 @@ func displayLogSegments(w io.Writer, segments []logSegment) error { } func copyLogWithLinePrefix(w io.Writer, r io.Reader, prefix string) error { - scanner := bufio.NewScanner(r) + sanitized := transform.NewReader(r, &asciisanitizer.Sanitizer{}) + scanner := bufio.NewScanner(sanitized) for scanner.Scan() { fmt.Fprintf(w, "%s%s\n", prefix, scanner.Text()) } diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 14749fcf6..c3ee9a54a 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -2759,6 +2759,46 @@ var expectedRunLogOutput = fmt.Sprintf("%s%s", coolJobRunLogOutput, sadJobRunLog var expectedRunLogOutputWithNoSteps = fmt.Sprintf("%s%s", coolJobRunWithNoStepLogsLogOutput, sadJobRunWithNoStepLogsLogOutput) var expectedLegacyRunLogOutputWithNoSteps = fmt.Sprintf("%s%s", legacyCoolJobRunWithNoStepLogsLogOutput, legacySadJobRunWithNoStepLogsLogOutput) +func TestCopyLogWithLinePrefix_TerminalEscapeSequences(t *testing.T) { + tests := []struct { + name string + input string + }{ + { + name: "OSC title set sequence", + input: "normal prefix\x1b]0;HIJACKED TITLE\x07trailing text\n", + }, + { + name: "CSI color sequence", + input: "\x1b[31mRED TEXT\x1b[0m normal text\n", + }, + { + name: "screen title set sequence used in original report", + input: "\x1bk;echo this is an arbitrary command;\x1b\\\n", + }, + { + name: "CSI window title query", + input: "before\x1b[21tafter\n", + }, + { + name: "multiple escape sequences", + input: "\x1b]0;title\x07\x1b[31mred\x1b[0m\x1b[21t\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + err := copyLogWithLinePrefix(&buf, strings.NewReader(tt.input), "jobname\tstep\t") + require.NoError(t, err) + + output := buf.String() + assert.NotContains(t, output, "\x1b", + "output should not contain raw ESC (0x1b) bytes, got: %q", output) + }) + } +} + func TestRunLog(t *testing.T) { t.Run("when the cache dir doesn't exist, exists return false", func(t *testing.T) { cacheDir := t.TempDir() + "/non-existent-dir" From c8e013991948fa17c3ec1b5141eebebdde9f872c Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 23 Apr 2026 15:31:38 +0200 Subject: [PATCH 2/2] Update acceptance/testdata/workflow/run-view-log-escape-sequences.txtar Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../testdata/workflow/run-view-log-escape-sequences.txtar | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/testdata/workflow/run-view-log-escape-sequences.txtar b/acceptance/testdata/workflow/run-view-log-escape-sequences.txtar index 14c75cd86..47978cf4d 100644 --- a/acceptance/testdata/workflow/run-view-log-escape-sequences.txtar +++ b/acceptance/testdata/workflow/run-view-log-escape-sequences.txtar @@ -1,4 +1,4 @@ -# This test ensure that a malicious workflow which emit terminal control sequences (ESC, OSC, CSI) in +# This test ensures that a malicious workflow which emit terminal control sequences (ESC, OSC, CSI) in # its log output does not result in terminal injection when logs are displayed using `gh run view --log` # Use gh as a credential helper