From 8fd803ab5cb52ef3341e1d7ebdea72fc1cf689fb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 1 May 2026 17:16:05 -0700 Subject: [PATCH] buildkite: strip terminal sequences from the logs --- provider_buildkite.go | 97 ++++++++++++++++++++++++++++++++++++++ provider_buildkite_test.go | 84 +++++++++++++++++++++++++++++++++ 2 files changed, 181 insertions(+) diff --git a/provider_buildkite.go b/provider_buildkite.go index df0655d..7a7516b 100644 --- a/provider_buildkite.go +++ b/provider_buildkite.go @@ -434,6 +434,14 @@ func (p *buildkiteProvider) Logs( body = "" } + // Buildkite injects per-line timestamp metadata as + // ANSI APC sequences (ESC "_" "bk;t=" BEL) and + // some renderers downstream don't recognise the APC + // envelope, leaking the inner "_bk;t=…" payload into + // the displayed text. Strip them here so consumers + // only ever see the actual log content. + body = stripTerminal(body) + for _, line := range strings.Split(strings.TrimRight(body, "\n"), "\n") { if line == "" { // Skip the leading empty entry that @@ -653,6 +661,95 @@ func refToBranch(ref string) string { return ref } +// stripTerminal removes ANSI/ECMA-48 escape sequences from a log +// payload, leaving only the displayable text. We need this because +// Buildkite ships its plain-text log API with the agent's full +// terminal output — per-line timestamp APC envelopes +// (`ESC _ "bk;t=" BEL`), CSI colour codes, clear-to-EOL +// (`ESC [ K`), OSC title sets, etc. — and our consumers are not +// terminal emulators; they render the bytes verbatim. +// +// We recognise the standard escape families described by ECMA-48: +// +// - CSI: ESC '[' parameters intermediates final +// - OSC/DCS/APC/SOS/PM: ESC (']'|'P'|'_'|'X'|'^') … (BEL | ESC '\') +// - everything else: ESC +// +// As a safety belt we also strip the bare "_bk;t=" residue +// that appears when an upstream processor has stripped the ESC/BEL +// framing without understanding the APC envelope inside it. +// +// We should use libghostty for obvious reasons. +func stripTerminal(s string) string { + if !strings.ContainsAny(s, "\x1b_") { + return s + } + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); { + if s[i] == 0x1b && i+1 < len(s) { + switch s[i+1] { + case '[': + // CSI: parameter bytes 0x30-0x3F, then + // intermediate bytes 0x20-0x2F, then a + // single final byte 0x40-0x7E. Anything + // that doesn't conform we drop minimally + // (just the ESC) so we don't swallow + // legitimate text. + j := i + 2 + for j < len(s) && s[j] >= 0x30 && s[j] <= 0x3F { + j++ + } + for j < len(s) && s[j] >= 0x20 && s[j] <= 0x2F { + j++ + } + if j < len(s) && s[j] >= 0x40 && s[j] <= 0x7E { + i = j + 1 + continue + } + i += 2 + continue + case ']', 'P', '_', 'X', '^': + // OSC/DCS/APC/SOS/PM: terminated by BEL or + // ST (ESC '\'). Drop the entire envelope. + j := i + 2 + for j < len(s) { + if s[j] == 0x07 { + j++ + break + } + if s[j] == 0x1b && j+1 < len(s) && s[j+1] == '\\' { + j += 2 + break + } + j++ + } + i = j + continue + default: + // Two-byte escape (RIS, DECSC, charset + // selection, …). Drop both bytes. + i += 2 + continue + } + } + // Bare residue: "_bk;t=" with no ESC/BEL framing. + if s[i] == '_' && strings.HasPrefix(s[i:], "_bk;t=") { + j := i + len("_bk;t=") + for j < len(s) && s[j] >= '0' && s[j] <= '9' { + j++ + } + if j > i+len("_bk;t=") { + i = j + continue + } + } + b.WriteByte(s[i]) + i++ + } + return b.String() +} + // sendLine pushes one LogLine into out, returning false if ctx // fired first. Centralised so the per-job loop in Logs stays // focused on the wire-shape decisions. diff --git a/provider_buildkite_test.go b/provider_buildkite_test.go index fc3d896..286a68b 100644 --- a/provider_buildkite_test.go +++ b/provider_buildkite_test.go @@ -505,3 +505,87 @@ func TestTriggerCommitAndBranch(t *testing.T) { }) } } + +// TestStripTerminal exercises both shapes of Buildkite's inline +// timestamp metadata: the well-formed APC envelope ESC _ … BEL, and +// the bare "_bk;t=" residue we sometimes see when an upstream +// processor strips control bytes without understanding the envelope. +func TestStripTerminal(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"empty", "", ""}, + {"no metadata", "hello world\n", "hello world\n"}, + { + "framed apc", + "\x1b_bk;t=1777680702553\x07~~~ Running agent environment hook\n", + "~~~ Running agent environment hook\n", + }, + { + "multiple framed apc per line", + "\x1b_bk;t=1\x07a\x1b_bk;t=2\x07b\n", + "ab\n", + }, + { + "bare residue", + "_bk;t=1777680702553~~~ Running agent environment hook\n", + "~~~ Running agent environment hook\n", + }, + { + "bare residue mid-line", + "remote: Counting objects: 0% _bk;t=1777680705851\n", + "remote: Counting objects: 0% \n", + }, + { + "unterminated apc dropped", + "keep\x1b_bk;t=1no-bel-here", + "keep", + }, + { + // Underscores unrelated to bk;t= must survive untouched. + "underscore not metadata", + "foo_bar baz_qux\n", + "foo_bar baz_qux\n", + }, + { + // CSI colour codes are stripped down to plain text. + "strips csi colour", + "\x1b[90m# comment\x1b[0m\n", + "# comment\n", + }, + { + // Clear-to-EOL embedded mid-line is removed too. + "strips csi clear-to-eol", + "remote: Counting objects: 1% (2/144) \x1b[K\n", + "remote: Counting objects: 1% (2/144) \n", + }, + { + // OSC title-set sequences (terminated by BEL) drop. + "strips osc bel terminated", + "before\x1b]0;window title\x07after\n", + "beforeafter\n", + }, + { + // OSC terminated by ST (ESC '\') also drops. + "strips osc st terminated", + "before\x1b]0;title\x1b\\after\n", + "beforeafter\n", + }, + { + // Two-byte escapes like ESC '=' (DECKPAM) drop. + "strips two byte escape", + "a\x1b=b\n", + "ab\n", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := stripTerminal(c.in) + if got != c.want { + t.Fatalf("got %q; want %q", got, c.want) + } + }) + } +} -- knotmirror 0.0.1