From ae5b9d9414476d68531b5801c569ca7649e7e93e Mon Sep 17 00:00:00 2001 From: Dima Kotlyarov Date: Fri, 13 Nov 2026 09:13:13 +0000 Subject: [PATCH 1/2] air: strip control characters from server-supplied names in air list and air get `air list` and `air get` showed the MLflow run name, experiment name and user name as the server sent them, so a name carrying terminal escape sequences reached the terminal of anyone who listed the run. A new `stripControl` helper drops escape sequences (CSI, OSC, DCS, ...) and C0/C1 control characters, and is applied where those names become display values: the MLflow run name as it is fetched (before it is shortened or cached), each `air list` table cell, and the Experiment, MLflow Run and User fields of the `air get` box and the `air list` info pane. JSON output and link targets are unchanged. Internal ticket: CNXT-2620. Co-authored-by: Isaac --- acceptance/air/control-chars/out.test.toml | 1 + acceptance/air/control-chars/output.txt | 54 +++++++++++++++++ acceptance/air/control-chars/script | 8 +++ acceptance/air/control-chars/test.toml | 69 ++++++++++++++++++++++ cmd/air/list_tui_render.go | 9 +-- cmd/air/list_tui_test.go | 23 ++++++++ cmd/air/mlflow.go | 5 +- cmd/air/mlflow_test.go | 6 ++ cmd/air/render.go | 7 ++- cmd/air/render_test.go | 23 ++++++++ cmd/air/sanitize.go | 29 +++++++++ cmd/air/sanitize_test.go | 33 +++++++++++ 12 files changed, 260 insertions(+), 7 deletions(-) create mode 100644 acceptance/air/control-chars/out.test.toml create mode 100644 acceptance/air/control-chars/output.txt create mode 100644 acceptance/air/control-chars/script create mode 100644 acceptance/air/control-chars/test.toml create mode 100644 cmd/air/sanitize.go create mode 100644 cmd/air/sanitize_test.go diff --git a/acceptance/air/control-chars/out.test.toml b/acceptance/air/control-chars/out.test.toml new file mode 100644 index 00000000000..18b1a88417e --- /dev/null +++ b/acceptance/air/control-chars/out.test.toml @@ -0,0 +1 @@ +Cloud = false diff --git a/acceptance/air/control-chars/output.txt b/acceptance/air/control-chars/output.txt new file mode 100644 index 00000000000..b320060ce29 --- /dev/null +++ b/acceptance/air/control-chars/output.txt @@ -0,0 +1,54 @@ + +=== get (text): control characters are stripped from the Metadata box + +>>> [CLI] air get 123 + +╭─ Metadata ─────────────────────────────────────────────────────╮ +│ │ +│ Run ID 123 │ +│ Status ● SUCCESS │ +│ Submitted 2023-11-14 22:13 UTC │ +│ Retries 0 │ +│ Max Retries 0 │ +│ Duration 12s │ +│ Experiment my-exp │ +│ MLflow Run my-run │ +│ User user@example.com │ +│ Accelerators 1x A10 │ +│ Environment N/A │ +│ │ +╰────────────────────────────────────────────────────────────────╯ + +Run URL: [DATABRICKS_URL]/jobs/runs/123?o=[WORKSPACE_ID] +MLflow URL: [DATABRICKS_URL]/ml/experiments/exp1/runs/run1 + +=== list (text): control characters are stripped from the table + +>>> [CLI] air list --all-users + Run ID Experiment Status Started Duration Progress MLflow User Accelerators + 123 my-exp ● SUCCESS [TIMESTAMP] 12s - my-run user@example.com 1x A10 + +=== list (json): strings are passed through as the server sent them + +>>> [CLI] air list --all-users -o json +{ + "v": 1, + "ts": "[TIMESTAMP]", + "data": { + "runs": [ + { + "run_id": "123", + "run_name": "qwen\u001b[2J-train", + "user": "user\u001b[31m@example.com", + "status": "SUCCESS", + "started_at": "[TIMESTAMP]", + "is_sweep": false, + "experiment_name": "my\u001b]0;pwned\u0007-exp", + "duration_seconds": 12, + "accelerator_type": "GPU_1xA10", + "num_accelerators": 1, + "dashboard_url": "[DATABRICKS_URL]/jobs/runs/123?o=[WORKSPACE_ID]" + } + ] + } +} diff --git a/acceptance/air/control-chars/script b/acceptance/air/control-chars/script new file mode 100644 index 00000000000..43bbf50aa93 --- /dev/null +++ b/acceptance/air/control-chars/script @@ -0,0 +1,8 @@ +title "get (text): control characters are stripped from the Metadata box" +trace $CLI air get 123 + +title "list (text): control characters are stripped from the table" +trace $CLI air list --all-users + +title "list (json): strings are passed through as the server sent them" +trace $CLI air list --all-users -o json diff --git a/acceptance/air/control-chars/test.toml b/acceptance/air/control-chars/test.toml new file mode 100644 index 00000000000..770413a86a4 --- /dev/null +++ b/acceptance/air/control-chars/test.toml @@ -0,0 +1,69 @@ +# Server-supplied names carrying terminal control characters: an OSC +# set-window-title sequence, a CSI clear-screen sequence, an SGR colour code and +# a carriage return. The text views must drop them; JSON output keeps the strings +# as the server sent them (encoded as \u001b etc. by the JSON encoder). +Env.DATABRICKS_CACHE_ENABLED = "false" + +[[Server]] +Pattern = "HEAD /" +Response.Body = '' + +# Experiment and creator names with embedded escapes, for `air get` and `air list`. +[[Server]] +Pattern = "GET /api/2.2/jobs/runs/get" +Response.Body = ''' +{ + "run_id": 123, + "run_name": "qwen\u001b[2J-train", + "creator_user_name": "user\u001b[31m@example.com", + "start_time": 1700000000000, + "end_time": 1700000012000, + "state": {"life_cycle_state": "TERMINATED", "result_state": "SUCCESS"}, + "tasks": [{ + "task_key": "train", + "run_id": 456, + "attempt_number": 0, + "ai_runtime_task": { + "experiment": "/Users/user@example.com/my\u001b]0;pwned\u0007-exp", + "deployments": [{"compute": {"accelerator_type": "GPU_1xA10", "accelerator_count": 1}}] + } + }] +} +''' + +[[Server]] +Pattern = "GET /api/2.2/jobs/runs/list" +Response.Body = ''' +{ + "runs": [{ + "run_id": 123, + "run_name": "qwen\u001b[2J-train", + "creator_user_name": "user\u001b[31m@example.com", + "start_time": 1700000000000, + "end_time": 1700000012000, + "state": {"life_cycle_state": "TERMINATED", "result_state": "SUCCESS"}, + "tasks": [{ + "task_key": "train", + "run_id": 456, + "attempt_number": 0, + "ai_runtime_task": { + "experiment": "/Users/user@example.com/my\u001b]0;pwned\u0007-exp", + "deployments": [{"compute": {"accelerator_type": "GPU_1xA10", "accelerator_count": 1}}] + } + }] + }] +} +''' + +[[Server]] +Pattern = "GET /api/2.2/jobs/runs/get-output" +Response.Body = ''' +{"ai_runtime_task_output": {"mlflow_experiment_id": "exp1", "mlflow_run_id": "run1"}} +''' + +# The MLflow run name carries all three kinds of control character. +[[Server]] +Pattern = "GET /api/2.0/mlflow/runs/get" +Response.Body = ''' +{"run": {"info": {"run_name": "\u001b]0;pwned\u0007my\u001b[2J-run\r"}}} +''' diff --git a/cmd/air/list_tui_render.go b/cmd/air/list_tui_render.go index 2152663430d..b893f1ee51d 100644 --- a/cmd/air/list_tui_render.go +++ b/cmd/air/list_tui_render.go @@ -184,11 +184,12 @@ func listRowStatus(r listRow) string { return r.Status } -// cell renders one padded, colored cell. The text is truncated to width, then -// padded with (background-only) spaces so columns align even when the text is -// styled or hyperlinked. +// cell renders one padded, colored cell. The text is stripped of control +// characters (it may come from the server, or from a cache written before this +// stripping existed), truncated to width, then padded with (background-only) +// spaces so columns align even when the text is styled or hyperlinked. func (s listStyles) cell(base lipgloss.Style, text string, width int, fg lipgloss.Color, right, underline bool, link string) string { - text = truncate(text, width) + text = truncate(stripControl(text), width) style := base.Foreground(fg) if underline { style = style.Underline(true) diff --git a/cmd/air/list_tui_test.go b/cmd/air/list_tui_test.go index 7117aabaec6..09dea8dc008 100644 --- a/cmd/air/list_tui_test.go +++ b/cmd/air/list_tui_test.go @@ -7,6 +7,7 @@ import ( "testing" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" "github.com/charmbracelet/x/ansi" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" @@ -238,6 +239,28 @@ func TestRenderRowHyperlinks(t *testing.T) { assert.NotContains(t, plain, "\x1b]8;;", "no links when links are disabled") } +func TestRenderRowStripsControlCharacters(t *testing.T) { + r, _ := cmdio.NewRenderer(cmdio.MockDiscard(t.Context()), io.Discard) + styles := newListStyles(r) + row := listRow{ + RunID: "1", Status: "SUCCESS", Duration: "-", Accelerators: "-", + Experiment: "\x1b]0;pwned\x07exp\x1b[2Jeriment", + User: "user\r@example.com", + MLflowURL: "https://h/ml/experiments/E/runs/rid", + MLflowLabel: "my\x1b[31m-run", + } + cols := computeListCols([]listRow{row}) + + out := styles.renderRow(cols, row, false, false) + assert.Contains(t, out, "experiment") + assert.Contains(t, out, "user@example.com") + assert.Contains(t, out, "my-run") + assert.NotContains(t, out, "\x1b") + assert.NotContains(t, out, "\r") + // Stripping happens before width is measured, so the columns stay aligned. + assert.Equal(t, cols.experiment, lipgloss.Width(stripControl(row.Experiment))) +} + func TestListModelInfoKeyOpensDetail(t *testing.T) { r, _ := cmdio.NewRenderer(cmdio.MockDiscard(t.Context()), io.Discard) f := &runFetcher{ctx: t.Context(), w: newTestWorkspaceClient(t, "https://x.test")} diff --git a/cmd/air/mlflow.go b/cmd/air/mlflow.go index 82a0a96ca3e..70570c030a3 100644 --- a/cmd/air/mlflow.go +++ b/cmd/air/mlflow.go @@ -142,7 +142,8 @@ func mlflowExperimentURL(host string, ids *mlflowIdentifiers) string { // fetchMLflowRunName fetches a run's MLflow run_name via the MLflow REST API, // returning "" if it can't be obtained. Best-effort, like the rest of the MLflow -// enrichment. +// enrichment. The name is stripped of control characters here, before it is +// shortened to a column width or cached, so no caller can show it raw. func fetchMLflowRunName(ctx context.Context, w *databricks.WorkspaceClient, mlflowRunID string) string { resp, err := w.Experiments.GetRun(ctx, ml.GetRunRequest{RunId: mlflowRunID}) if err != nil { @@ -152,7 +153,7 @@ func fetchMLflowRunName(ctx context.Context, w *databricks.WorkspaceClient, mlfl if resp.Run == nil || resp.Run.Info == nil { return "" } - return resp.Run.Info.RunName + return stripControl(resp.Run.Info.RunName) } // mlflowRunLabel is the text shown for the MLflow Run cell: the run's name, or diff --git a/cmd/air/mlflow_test.go b/cmd/air/mlflow_test.go index 1728ab22111..6edc209537a 100644 --- a/cmd/air/mlflow_test.go +++ b/cmd/air/mlflow_test.go @@ -229,4 +229,10 @@ func TestFetchMLflowRunName(t *testing.T) { srv := mlflowServer(`{}`) assert.Empty(t, fetchMLflowRunName(ctx, newTestWorkspaceClient(t, srv.URL), "run1")) }) + + t.Run("strips control characters from the name", func(t *testing.T) { + // A set-window-title OSC, a clear-screen CSI and a carriage return. + srv := mlflowServer(`{"run":{"info":{"run_name":"\u001b]0;pwned\u0007sunny\u001b[2J-cat\r-42"}}}`) + assert.Equal(t, "sunny-cat-42", fetchMLflowRunName(ctx, newTestWorkspaceClient(t, srv.URL), "run1")) + }) } diff --git a/cmd/air/render.go b/cmd/air/render.go index 755e8540fb9..6e2f3f5b1a3 100644 --- a/cmd/air/render.go +++ b/cmd/air/render.go @@ -376,8 +376,13 @@ func renderBox(p palette, title, body string) string { // renderFields draws the two-column summary: muted labels right-padded to the // longest one, neutral values, a status-colored Status, and blue Run ID / MLflow -// Run hyperlinks. +// Run hyperlinks. The experiment, MLflow run and user values are server-supplied +// names, so they are stripped of control characters before they reach the +// terminal. func renderFields(p palette, colorOn bool, v runView) string { + v.experiment = stripControl(v.experiment) + v.mlflowLabel = stripControl(v.mlflowLabel) + v.user = stripControl(v.user) status := statusStyle(p, v.status).Render("● " + v.status) rows := []string{ field(p, "Run ID", link(colorOn, p.blue, v.runID, v.dashboardURL)), diff --git a/cmd/air/render_test.go b/cmd/air/render_test.go index 66e5b09deb7..29ad7cc6913 100644 --- a/cmd/air/render_test.go +++ b/cmd/air/render_test.go @@ -171,6 +171,29 @@ func TestRenderFields(t *testing.T) { assert.NotContains(t, out, "\n\n") } +func TestRenderFieldsStripsControlCharacters(t *testing.T) { + p := asciiPalette(t) + out := renderFields(p, false, runView{ + runID: "1", + status: "SUCCESS", + submitted: "2026-06-03 04:17 UTC", + maxRetries: "3", + duration: "1m 13s", + experiment: "\x1b]0;pwned\x07exp\x1b[2Jeriment", + mlflowLabel: "run\rname", + mlflowURL: "https://h.test/ml/experiments/E1/runs/R1", + user: "user\x1b[31m@example.com", + accelerators: "1x A10", + environment: "4", + }) + + assert.Contains(t, out, "Experiment experiment") + assert.Contains(t, out, "MLflow Run runname") + assert.Contains(t, out, "User user@example.com") + assert.NotContains(t, out, "\x1b") + assert.NotContains(t, out, "\r") +} + func TestLink(t *testing.T) { p := asciiPalette(t) // Color off: the bare label, no URL. diff --git a/cmd/air/sanitize.go b/cmd/air/sanitize.go new file mode 100644 index 00000000000..f84544a6bc4 --- /dev/null +++ b/cmd/air/sanitize.go @@ -0,0 +1,29 @@ +package aircmd + +import ( + "strings" + "unicode" + + "github.com/charmbracelet/x/ansi" +) + +// stripControl removes terminal escape sequences (CSI, OSC, DCS, ...) and C0/C1 +// control characters from s, so a server-supplied string such as an MLflow run +// name can be shown without letting whoever chose it clear the screen, retitle +// the window, write the clipboard or overwrite a line of output. Printable text, +// including non-ASCII, is kept. Apply it to display values only: JSON output and +// link targets are left as the server sent them. +func stripControl(s string) string { + // Drop whole escape sequences first, so "\x1b[2J" does not leave "[2J" + // behind once the ESC is removed below. + s = ansi.Strip(s) + if !strings.ContainsFunc(s, unicode.IsControl) { + return s + } + return strings.Map(func(r rune) rune { + if unicode.IsControl(r) { + return -1 + } + return r + }, s) +} diff --git a/cmd/air/sanitize_test.go b/cmd/air/sanitize_test.go new file mode 100644 index 00000000000..1d0b0818b85 --- /dev/null +++ b/cmd/air/sanitize_test.go @@ -0,0 +1,33 @@ +package aircmd + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestStripControl(t *testing.T) { + tests := []struct { + name, in, want string + }{ + {"plain text is unchanged", "sunny-cat-42", "sunny-cat-42"}, + {"non-ASCII text is kept", "résumé 日本語 🚀", "résumé 日本語 🚀"}, + {"OSC set-title terminated by BEL", "\x1b]0;pwned\x07name", "name"}, + {"OSC set-title terminated by ST", "\x1b]0;pwned\x1b\\name", "name"}, + {"OSC 52 clipboard write", "a\x1b]52;c;cHduZWQ=\x07b", "ab"}, + {"CSI clear screen", "a\x1b[2Jb", "ab"}, + {"CSI color", "\x1b[31mred\x1b[0m", "red"}, + {"carriage return", "real-name\rfake", "real-namefake"}, + {"tab and newline", "a\tb\nc", "abc"}, + {"DEL", "a\x7fb", "ab"}, + {"C1 control", "a\u0085b\u009bc", "abc"}, + {"two-byte escape sequence", "a\x1bbz", "az"}, + {"trailing ESC", "a\x1b", "a"}, + {"NUL", "a\x00b", "ab"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, stripControl(tt.in)) + }) + } +} From 2118b25de1f4d2c8e7da57699d3f11ed0d410591 Mon Sep 17 00:00:00 2001 From: Dima Kotlyarov Date: Fri, 13 Nov 2026 13:20:41 +0000 Subject: [PATCH 2/2] air: add changelog fragment for #7601 Co-authored-by: Isaac --- .nextchanges/air/7601.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 .nextchanges/air/7601.md diff --git a/.nextchanges/air/7601.md b/.nextchanges/air/7601.md new file mode 100644 index 00000000000..ce40c3b3372 --- /dev/null +++ b/.nextchanges/air/7601.md @@ -0,0 +1 @@ +* Strip terminal control characters from the MLflow run, experiment and user names shown by `air list` and `air get`, so a run name cannot clear or retitle the terminal of whoever lists it. ([#7601](https://github.com/databricks/cli/pull/7601))