fix(sanitize): strip SS3 escapes and floor escape-only names (#516)

[LukasWodka]

Closes #516. Companion PR (bash + PowerShell copies of the same rule): tracebloc/client#736 — land both, they are one rule.

The bug

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.

SS3 residue was worse than the CSI residue fixed in #364 / tracebloc/client#362 on 2026-07-21 (not re-litigated here — that fix is correct and stays). CSI residue cleans to empty, fails the non-empty check and re-prompts. SS3 does not: ESC is dropped as a C0 byte but O and the final byte are printable, so

"\x1bOD\x1bOD\x1bOD\x1bOA\x1bOA\x1bOA" -> "ODODODOAOAOA" -> namespace "odododoaoaoa"
"\x1bOH\x1bOF" -> "OHOF"      "\x1bOP\x1bOQ" -> "OPOQ"

and the namespace is immutable. 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 — so the defence has to be about content, and it has to live here.

The fix

Both changes are in sanitizeClientName, not in internal/slug — that must stay a faithful mirror of backend/common/utils/slug.py, and both files say so. Hygiene belongs at ingestion.

1. escSequence matches CSI and SS3 in one pattern.

var escSequence = regexp.MustCompile("\x1b(?:\\[[0-9;]*|O)[A-Za-z~]")

2. A post-sanitise floor. The strip knows CSI, SS3 and the paste markers; it cannot know the family nobody has reported yet — and that is exactly how SS3 got here. So: if an ESC survives step 1, the value carries a shape we do not recognise, and it must show one alphanumeric that did not come from an escape final byte. The probe is ESC + intermediates + at most two final-class bytes; its output is a yes/no and is never returned. Nothing but residue returns "", which client create already treats exactly like an omitted --name.

Why that rule, of the ones the ticket floated. The ticket suggested "cleaned ≠ raw and remainder under N characters". I picked a different trigger because "cleaned ≠ raw" fires on every ordinary paste and every stray tab, so an N large enough to catch residue also auto-names short real names, and an N small enough to spare them catches nothing. Scoping to "an ESC survived the known families" is narrower and needs no magic number: a clean name never reaches it, --name box beside an unknown escape keeps box, and only a value that is nothing but residue is refused. Where it is wrong it is wrong in the recoverable direction — auto-naming a name is annoying; minting a permanent namespace out of keyboard noise is not.

Why the probe's run is bounded at two — this came out of a Bugbot Medium on the companion PR (tracebloc/client#736). One is too few: it leaves the D of an unrecognised SS3-shaped pair behind, and the floor stops firing on the exact family shape this ticket is about. Unbounded is too many: \x1bNChello had the whole name swallowed and was refused while \x1bNC日本 was kept, so keep-vs-reject depended on the script the user's name is written in. Two is a statement about escapes rather than a tuning constant — an escape final is one byte, an intro plus a final is two, and every keyboard-input family (SS2, SS3, the 7-bit C1 forms) fits in that.

Mutation evidence

Four anchors, each applied to the source, each confirmed to redden, each restored:

mutation	result
SS3 dropped from escSequence	2 red: na\x1bODme → "naODme", SS3+CSI mixed → "aODbc"
floor short-circuited (if false && …)	2 red: truncated SS3 → "O", unknown family → "NBNC"
hasAlphanumeric made ASCII-only	1 red: \x1bNC日本 → ""
probe's run reverted {1,2} → +	1 red: \x1bNChello → ""

Worth stating rather than hiding: the "SS3 arrows only → empty" case stays green under the first mutation, because the floor catches it too. It is kept as the ticket's documented repro, and the SS3 strip itself is carried by the mixed-content cases, which the floor cannot mask.

Tests

internal/cli/sanitize_test.go: 26 table cases (was 15) plus a new TestSanitizeClientName_ss3SurvivesAsAPlausibleName pinning the ticket's exact repro and the slug it used to mint. The floor cases use SS2 (ESC N <final>) as a stand-in for "the next family" — genuinely not matched by escSequence, so they exercise the floor and nothing else.

make check and make check-all green (go test ./...: 17 packages ok; lint, staticcheck, golangci-lint 0 issues, govulncheck clean).

VERSION 0.10.8 → 0.10.9, in its own commit: version-bump-gate is a required check and refuses a PR touching internal/* while VERSION names an already-released version. 0.10.9 is the same target #518 and #519 bump to — identical one-line changes merge without conflict, and all three then ship under the pending 0.10.9. The train still cuts the tag from the file; nothing is hand-released here.

On the shared fixture (ticket item 4) — deferred, deliberately

The ticket is right that three hand-maintained copies with no shared corpus is why all three missed SS3 at once. I built exactly that corpus while doing this (18 raw→expected pairs, run against all three implementations, 18/18 each) — but I did not land it as a shared fixture, for two reasons:

1. The three implementations live in two repos. A fixture genuinely shared between cli and client needs a vendoring + drift-check mechanism (the shape of client's check-drift.sh), which is a larger and riskier change than the bug fix and belongs in its own PR.
2. A fixture that only two of the three consume is worse than none. The PowerShell peer's tests live in scripts/tests/install-k8s.Tests.ps1, outside this change's scope — wiring two implementations to a corpus and leaving the third out produces exactly the "looks like a check, is not connected to what it claims to check" shape the org standards warn about.

Follow-up filed: tracebloc/backend#2084 (it carries the "add the PowerShell SS3 cases" sub-task explicitly). Until it lands, all three copies now carry a "change all three together" pointer to the other two.

One gap this PR does not close, stated plainly rather than left to be discovered: the PowerShell copy's SS3 behaviour is verified (18/18 out of tree, and the existing Pester suite is 672 passed / 0 failed with the change in) but has no committed SS3 test, because install-k8s.Tests.ps1 was outside this change's scope. That is a real hole and is the first sub-task on the follow-up.

[saadqbal]

Careful, well-reasoned PR. The SS3 grammar (ESC O + one final) is right, escSequence still strips CSI/paste, and a bare O is untouched. Nice touch that the real backstop stays step 4: strings.Map drops every ESC on every non-empty path, so even families nobody stripped as a whole (OSC/DCS/APC, truncated ESC O, ESC O ESC) can't reach the terminal — the floor and escSequence only keep the name clean. The two-final-byte bound and its script-independence argument hold up, and the tests are non-vacuous (SS3->floor, legit-O, other families intact, the ASCII/non-Latin symmetry). Ran the suite + an adversarial "no ESC survives" sweep, both green. 👍