From bf60dd0e16ae09415d6a97f5d2530004941bc6c2 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Mon, 17 Aug 2026 12:26:43 +0200 Subject: [PATCH 1/3] fix(sanitize): strip SS3 escapes and floor escape-only names (cli#516) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sanitizeClientName handled CSI (ESC '[' … final) only. SS3 (ESC 'O' final) is what the same arrow / Home / End / F-keys emit once the terminal is in DECCKM application-cursor mode — the state vim, less or tmux leave behind on an unclean exit. That residue was worse than the CSI residue fixed in cli#364 / client#362 (2026-07-21, not re-litigated here): CSI cleans to empty and re-prompts, while 'O' and the final byte are printable, so ESC OD ×3 ESC OA ×3 survived as the plausible name "ODODODOAOAOA" and minted the permanent namespace "odododoaoaoa". Nothing downstream can refuse it: is_dns1123_label validates by idempotence against the slug rule, so escape-derived garbage is a perfectly canonical label. Form is exactly what this input preserves. Two changes, both in sanitizeClientName — deliberately NOT in internal/slug, which must stay a faithful mirror of backend/common/utils/slug.py: 1. escSequence now matches CSI and SS3 in one pattern. 2. A post-sanitise floor. If an ESC SURVIVES step 1 the value carries an escape family we do not recognise — which is precisely how SS3 got here — so it must show one alphanumeric that did not come from an escape final byte, probed with a greedier pattern whose output is never returned. Nothing but residue returns "", the same path an omitted --name takes. Scoped to "an ESC survived" so a clean name never reaches it and real content beside an unknown escape is kept; the failure it chooses is the recoverable one. Tests: 10 new cases in the table (SS3 arrows / Home-End / F-keys / mixed with CSI / truncated / a bare O is not an escape; the floor with SS2 standing in for "the next family", including the non-Latin-content case) plus a test pinning the ticket's exact repro and the slug it used to mint. Mutation-proven, three anchors, each applied and each detected: • SS3 dropped from escSequence -> 2 cases red ("na\x1bODme", SS3+CSI mixed) • floor short-circuited to false -> 2 cases red (truncated SS3, unknown family) • hasAlphanumeric made ASCII-only -> 1 case red (non-Latin content) The "SS3 arrows only" case is green under anchor 1 because the floor also covers it; anchor 1 is carried by the mixed-content cases, which the floor cannot mask. The bash and PowerShell peers get the same two changes in tracebloc/client. Co-Authored-By: Claude Opus 5 --- internal/cli/sanitize.go | 97 +++++++++++++++++++++++++++-------- internal/cli/sanitize_test.go | 42 +++++++++++++++ 2 files changed, 119 insertions(+), 20 deletions(-) diff --git a/internal/cli/sanitize.go b/internal/cli/sanitize.go index f14ae13c..e4b38dd8 100644 --- a/internal/cli/sanitize.go +++ b/internal/cli/sanitize.go @@ -3,41 +3,90 @@ package cli import ( "regexp" "strings" + "unicode" ) -// csiSequence matches an ANSI CSI sequence: ESC '[' . This is what a terminal emits for arrow keys / cursor moves -// (ESC[A/B/C/D, ESC[1;5C, ESC[3~ …) and for bracketed-paste wrappers -// (ESC[200~ … ESC[201~). It is deliberately broader than submit.stripANSI, -// which only strips SGR colour codes (final byte 'm'). -var csiSequence = regexp.MustCompile("\x1b\\[[0-9;]*[A-Za-z~]") +// escSequence matches the two ANSI escape families a terminal actually sends for +// cursor / function keys: +// +// CSI ESC '[' +// ESC[A/B/C/D (arrows), ESC[1;5C (Ctrl+arrow), ESC[3~ (Delete), +// ESC[200~ … ESC[201~ (bracketed paste) +// SS3 ESC 'O' +// ESC OA/OB/OC/OD (arrows), ESC OH/OF (Home/End), ESC OP…OS (F1–F4) +// +// SS3 is what the *same* keys emit once the terminal is in DECCKM +// application-cursor mode — the state vim, less or tmux leave behind on an +// unclean exit (cli#516). Handling only CSI was the remaining hole: an ESC alone +// is a C0 byte and gets dropped below, but 'O' and the final byte are printable, +// so "\x1bOD\x1bOD\x1bOA" survived as the plausible-looking name "ODODOA" and +// minted the immutable namespace "ododoa". CSI residue never did: it cleans to +// empty, fails the non-empty check and re-prompts. +// +// Deliberately broader than submit.stripANSI, which only strips SGR colour codes +// (final byte 'm'). +var escSequence = regexp.MustCompile("\x1b(?:\\[[0-9;]*|O)[A-Za-z~]") + +// escResidue is the post-sanitise floor's *probe*, not a strip: ESC, any run of +// non-final intermediate bytes, then the whole run of bytes that could be escape +// final bytes. It is deliberately greedier than escSequence (a `+` on the final +// class, so it swallows both the 'O' and the 'D' of an unrecognised SS3-shaped +// pair) because its only job is to answer one question — "is there an +// alphanumeric here that did NOT come from an escape final byte?" Its output is +// never returned as a name. +var escResidue = regexp.MustCompile("\x1b[^A-Za-z0-9~]*[A-Za-z~]+") // sanitizeClientName strips terminal escape sequences and C0 control characters // from a user-supplied client name or location before it becomes the stored // display name and is slugified into an immutable Kubernetes namespace. // -// Defense-in-depth for the name-garble bug (customer-reported 2026-07-20): typing -// arrow keys at the installer's name prompt injected raw ESC[D/ESC[A bytes. The -// installer now strips them at the source, but a name can also arrive here -// directly via --name / $TRACEBLOC_CLIENT_NAME, and slug.Slugify would otherwise -// turn each ESC[ run into a "-" — ESC (0x1B) survives Slugify's ASCII pass, so -// "se-\e[D\e[D\e[A\e[A" mints the garbage namespace "se-d-d-a-a". Cleaning here, -// at the CLI boundary, keeps slug.Slugify a faithful mirror of the backend's -// slug.py (which must NOT strip — the backend validates exactly what it produces); -// input hygiene belongs at ingestion, not in the shared slug rule. +// Defense-in-depth for the name-garble bug (customer-reported 2026-07-20, fixed +// 2026-07-21 in cli#364 / client#362): typing arrow keys at the installer's name +// prompt injected raw ESC[D/ESC[A bytes. The installer strips them at the source, +// but a name can also arrive here directly via --name / $TRACEBLOC_CLIENT_NAME, +// and slug.Slugify would otherwise turn each ESC[ run into a "-" — ESC (0x1B) +// survives Slugify's ASCII pass, so "se-\e[D\e[D\e[A\e[A" mints the garbage +// namespace "se-d-d-a-a". Cleaning here, at the CLI boundary, keeps slug.Slugify +// a faithful mirror of the backend's slug.py (which must NOT strip — the backend +// validates exactly what it produces); input hygiene belongs at ingestion, not in +// the shared slug rule. +// +// Escape-derived garbage cannot be caught downstream: is_dns1123_label validates +// by idempotence against the slug rule, so "ododoa" is a perfectly canonical +// label. Form is exactly what this class of input preserves — hence the floor +// below, which is about *content*, not form. // // UTF-8 bytes (>= 0x80) are preserved so international names survive. func sanitizeClientName(s string) string { - // 1) Whole CSI sequences (arrow keys, paste wrappers). ReplaceAll handles - // consecutive sequences in one pass; any orphaned ESC left behind is a C0 - // byte and is removed by step 3. - s = csiSequence.ReplaceAllString(s, "") + // 1) Whole escape sequences (arrow keys in either cursor mode, function keys, + // paste wrappers). ReplaceAll handles consecutive sequences in one pass; + // any orphaned ESC left behind is a C0 byte and is removed by step 4. + s = escSequence.ReplaceAllString(s, "") // 2) Post-corruption case: an earlier (buggy) sanitizer dropped the ESC but // left the literal bracketed-paste markers. Only these two well-defined // markers are removed — a generic "[x~" could be real name content. s = strings.ReplaceAll(s, "[200~", "") s = strings.ReplaceAll(s, "[201~", "") - // 3) Drop any remaining C0 control characters and DEL; keep printable ASCII + // 3) The floor. Steps 1–2 know CSI, SS3 and the paste markers; they cannot + // know the escape family nobody has reported yet, and that is precisely how + // SS3 got here — one rule, three hand-copied implementations, no member of + // the family beyond CSI ever tested. So: if an ESC SURVIVED step 1, this + // value carries an escape shape we do not recognise, and the printable + // bytes around it are not trustworthy name content. Require at least one + // alphanumeric that did not come from an escape final byte; if there is + // none, the whole value was residue — return "" and let the caller + // auto-name (the same path an omitted --name takes). + // + // Scoped to "an ESC survived" on purpose. It is the narrowest trigger that + // still covers unknown families: a clean name never reaches it, a name with + // real content next to an unknown escape keeps that content, and only a + // value that is *nothing but* escape residue is rejected. The failure it + // chooses is the recoverable one — auto-naming a name is annoying; minting + // a permanent namespace from keyboard noise is not. + if strings.ContainsRune(s, 0x1b) && !hasAlphanumeric(escResidue.ReplaceAllString(s, "")) { + return "" + } + // 4) Drop any remaining C0 control characters and DEL; keep printable ASCII // and all multi-byte UTF-8 (>= 0x80). return strings.Map(func(r rune) rune { if r < 0x20 || r == 0x7f { @@ -46,3 +95,11 @@ func sanitizeClientName(s string) string { return r }, s) } + +// hasAlphanumeric reports whether s contains any letter or digit, Unicode +// included — a name written entirely in a non-Latin script is real content. +func hasAlphanumeric(s string) bool { + return strings.IndexFunc(s, func(r rune) bool { + return unicode.IsLetter(r) || unicode.IsDigit(r) + }) >= 0 +} diff --git a/internal/cli/sanitize_test.go b/internal/cli/sanitize_test.go index ec4c765c..a928b5cc 100644 --- a/internal/cli/sanitize_test.go +++ b/internal/cli/sanitize_test.go @@ -22,6 +22,25 @@ func TestSanitizeClientName(t *testing.T) { {"CSI with params (Ctrl+arrow)", "x\x1b[1;5Dy", "xy"}, {"Delete key (ESC[3~)", "ab\x1b[3~", "ab"}, + // SS3 (cli#516): the SAME keys once the terminal is in DECCKM + // application-cursor mode, which vim/less/tmux leave behind on an unclean + // exit. Unlike CSI residue these used to survive as a plausible name. + {"SS3 arrows around real content", "na\x1bODme", "name"}, + {"SS3 arrows only cleans to empty (→ auto-name)", "\x1bOD\x1bOD\x1bOD\x1bOA\x1bOA\x1bOA", ""}, + {"SS3 Home/End only", "\x1bOH\x1bOF", ""}, + {"SS3 F1/F2 only", "\x1bOP\x1bOQ", ""}, + {"SS3 and CSI mixed", "a\x1bODb\x1b[Dc", "abc"}, + {"truncated SS3 (ESC O, no final)", "\x1bO", ""}, + {"a bare O is not an escape", "OPTIMUS-01", "OPTIMUS-01"}, + + // The floor: an ESC that survived the known families means an escape shape + // we don't recognise, so what is left has to prove it is real content. + // SS2 (ESC N ) stands in for "the next family" — it is not stripped + // by escSequence, so these exercise the floor and nothing else. + {"unknown escape family, nothing but residue (→ auto-name)", "\x1bNB\x1bNC", ""}, + {"unknown escape family beside real content is kept", "box\x1bNC", "boxNC"}, + {"floor counts non-Latin letters as real content", "\x1bNC日本", "NC日本"}, + // Bracketed paste. {"bracketed-paste wrappers", "\x1b[200~hello\x1b[201~", "hello"}, {"post-corruption literal paste markers", "[200~hello[201~", "hello"}, @@ -63,3 +82,26 @@ func TestSanitizeClientName_neutralizesSlugGarble(t *testing.T) { t.Fatalf("slug.Slugify(sanitizeClientName(raw)) = %q, want %q", got, "se") } } + +// TestSanitizeClientName_ss3SurvivesAsAPlausibleName is the cli#516 half: SS3 +// residue was worse than the CSI residue fixed in cli#364 / client#362, because +// it did NOT clean to empty. 'O' and the final byte are printable, so dropping +// only the ESC left a non-empty, plausible-looking name that passed every +// downstream check — is_dns1123_label validates by idempotence against the slug +// rule, and "odododoaoaoa" is a perfectly canonical label. Form is exactly what +// this input preserves, so nothing but this function can refuse it. +func TestSanitizeClientName_ss3SurvivesAsAPlausibleName(t *testing.T) { + const garbled = "\x1bOD\x1bOD\x1bOD\x1bOA\x1bOA\x1bOA" // ← ← ← ↑ ↑ ↑ in DECCKM mode + + // What the CSI-only sanitizer produced: ESC removed as a C0 byte, the rest + // kept verbatim — and slugifying that mints a permanent namespace. + if got := slug.Slugify("ODODODOAOAOA"); got != "odododoaoaoa" { + t.Fatalf("precondition: slug.Slugify(%q) = %q, want the documented garble %q", "ODODODOAOAOA", got, "odododoaoaoa") + } + + // With SS3 handled, the value cleans to empty, which client create treats + // exactly like an omitted --name: it auto-names instead of minting garbage. + if got := sanitizeClientName(garbled); got != "" { + t.Fatalf("sanitizeClientName(%q) = %q, want %q (the auto-name path)", garbled, got, "") + } +} From 91859695323f669051943d59a9f859796c1e23bc Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Mon, 17 Aug 2026 12:31:39 +0200 Subject: [PATCH 2/3] chore(release): VERSION 0.10.8 -> 0.10.9 (cli#516) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit version-bump-gate is a required check and it refuses a PR that touches internal/* while VERSION still names an already-released version: v0.10.8 is out, so shipping this fix under it would put different bytes under an existing release. 0.10.9 is untagged and above every released final version, and it is the same target the other two open PRs on develop bump to — identical one-line changes merge without conflict, and all three then ship under the pending 0.10.9. Not a hand-cut release: the release train still reads this file and cuts the tag from it at the prod hop. The gate's own message is explicit that it never bumps for you, and that a stale VERSION fails days later on somebody else's hop (backend#1561) rather than here. Co-Authored-By: Claude Opus 5 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 1a46c7f1..f314d020 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.10.8 +0.10.9 From 6bb9f41fe76fe11283b132a9f564c90616c46583 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Mon, 17 Aug 2026 12:46:24 +0200 Subject: [PATCH 3/3] fix(sanitize): bound the floor's probe to two final bytes (cli#516) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot, Medium, on tracebloc/client#736: the floor's probe used an unbounded `[A-Za-z~]+` after an unrecognised ESC, so every ASCII letter following the escape was swallowed into the probe and the value read as residue-only. It is right, and the sharper half of it is the part I had not seen: `\x1bNChello` was refused while `\x1bNC日本` was kept, which makes keep-vs-reject depend on the script the user's name is written in. I had accepted the over-strictness on purpose; I had not noticed it was inconsistent. Bounded to `{1,2}`. Two, not one and not unbounded: one leaves the 'D' of an unrecognised SS3-shaped pair behind and the floor stops firing on the exact family shape this ticket is about, while unbounded eats a whole name. An escape final is one byte, an intro plus a final is two, and every keyboard-input escape family (SS2, SS3, the 7-bit C1 forms) fits in that — so the bound is a statement about escapes rather than a tuning constant. Every case the floor is meant to catch is unaffected: ESC N B / ESC N C, a truncated ESC O, and ESC [ ; ] A all still collapse to empty. Applied to all three copies so the rule stays one rule. Mutation-proven: reverting `{1,2}` to `+` turns the new case red in Go ("\x1bNChello" -> "") and in bats. Co-Authored-By: Claude Opus 5 --- internal/cli/sanitize.go | 28 ++++++++++++++++++---------- internal/cli/sanitize_test.go | 5 +++++ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/internal/cli/sanitize.go b/internal/cli/sanitize.go index e4b38dd8..0847bf26 100644 --- a/internal/cli/sanitize.go +++ b/internal/cli/sanitize.go @@ -28,13 +28,20 @@ import ( var escSequence = regexp.MustCompile("\x1b(?:\\[[0-9;]*|O)[A-Za-z~]") // escResidue is the post-sanitise floor's *probe*, not a strip: ESC, any run of -// non-final intermediate bytes, then the whole run of bytes that could be escape -// final bytes. It is deliberately greedier than escSequence (a `+` on the final -// class, so it swallows both the 'O' and the 'D' of an unrecognised SS3-shaped -// pair) because its only job is to answer one question — "is there an -// alphanumeric here that did NOT come from an escape final byte?" Its output is -// never returned as a name. -var escResidue = regexp.MustCompile("\x1b[^A-Za-z0-9~]*[A-Za-z~]+") +// non-final intermediate bytes, then AT MOST TWO bytes that could be escape final +// bytes. Its only job is to answer one question — "is there an alphanumeric here +// that did NOT come from an escape final byte?" — and its output is never +// returned as a name. +// +// Two, not one and not unbounded. One is too few: it leaves the 'D' of an +// unrecognised SS3-shaped pair behind, and the floor stops firing on exactly the +// family shape this ticket is about. Unbounded is too many: `\x1bNChello` would +// have the whole name swallowed and be refused, while `\x1bNC日本` survives — +// making keep-vs-reject depend on the script the user's name is written in +// (Bugbot, tracebloc/client#736). Two is the honest bound: an escape final is one +// byte, an intro plus a final is two, and every keyboard-input escape family +// (SS2, SS3, the 7-bit C1 forms) fits in that. +var escResidue = regexp.MustCompile("\x1b[^A-Za-z0-9~]*[A-Za-z~]{1,2}") // sanitizeClientName strips terminal escape sequences and C0 control characters // from a user-supplied client name or location before it becomes the stored @@ -73,9 +80,10 @@ func sanitizeClientName(s string) string { // the family beyond CSI ever tested. So: if an ESC SURVIVED step 1, this // value carries an escape shape we do not recognise, and the printable // bytes around it are not trustworthy name content. Require at least one - // alphanumeric that did not come from an escape final byte; if there is - // none, the whole value was residue — return "" and let the caller - // auto-name (the same path an omitted --name takes). + // alphanumeric that did not come from an escape final byte (at most two of + // them — see escResidue); if there is none, the whole value was residue — + // return "" and let the caller auto-name (the same path an omitted --name + // takes). // // Scoped to "an ESC survived" on purpose. It is the narrowest trigger that // still covers unknown families: a clean name never reaches it, a name with diff --git a/internal/cli/sanitize_test.go b/internal/cli/sanitize_test.go index a928b5cc..5e9915af 100644 --- a/internal/cli/sanitize_test.go +++ b/internal/cli/sanitize_test.go @@ -40,6 +40,11 @@ func TestSanitizeClientName(t *testing.T) { {"unknown escape family, nothing but residue (→ auto-name)", "\x1bNB\x1bNC", ""}, {"unknown escape family beside real content is kept", "box\x1bNC", "boxNC"}, {"floor counts non-Latin letters as real content", "\x1bNC日本", "NC日本"}, + // The probe's final-byte run is bounded at two, so an ASCII name after an + // unknown escape is kept just like a non-Latin one — keep-vs-reject must + // not depend on the script the name is written in (Bugbot, + // tracebloc/client#736). + {"floor keeps an ASCII name after an unknown escape", "\x1bNChello", "NChello"}, // Bracketed paste. {"bracketed-paste wrappers", "\x1b[200~hello\x1b[201~", "hello"},