This is an automated email from the ASF dual-hosted git repository.
shahar1 pushed a commit to branch airflow-ctl/v0-1-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/airflow-ctl/v0-1-test by this
push:
new cee1c582cd7 [airflow-ctl/v0-1-test] Escape server and user text in
airflowctl command messages (#76265) (#76621)
cee1c582cd7 is described below
commit cee1c582cd767658e69cbf8e792dd99baa8b4944
Author: Shahar Epstein <[email protected]>
AuthorDate: Fri Nov 6 19:38:46 2026 +0200
[airflow-ctl/v0-1-test] Escape server and user text in airflowctl command
messages (#76265) (#76621)
Several airflowctl commands interpolated server error text, Dag run ids
and logical dates into Rich markup unescaped. A value containing a
tag-like fragment such as "[/x]" made rich.print raise MarkupError, so
the user saw a traceback instead of the actual error.
(cherry picked from commit abc3a20f080a06dbd81b55fea028c22179cd6380)
Conflict resolution: this branch has no ctl/utils/dag_run.py, so the escapes
from that module go to the lookup helpers it keeps inline in dag_command.py
and
task_command.py, and its tests are rewritten against `dags state` and
`tasks states-for-dag-run`. `dags clear` and `tasks failed-deps`, `state`
and
`logs` do not exist here, so their escapes and tests are dropped.
"Error retrieving DAG" keeps this branch's wording.
Generated-by: Claude Code (Opus 5.5)
Claude-Session: https://claude.ai/code/session_01BVx6gA44HQY4GEf2fEAQwE
Co-authored-by: Rishabh <[email protected]>
---
.../src/airflowctl/ctl/commands/auth_command.py | 5 +-
.../src/airflowctl/ctl/commands/config_command.py | 3 +-
.../airflowctl/ctl/commands/connection_command.py | 5 +-
.../src/airflowctl/ctl/commands/dag_command.py | 16 ++--
.../src/airflowctl/ctl/commands/task_command.py | 11 ++-
.../airflow_ctl/ctl/commands/test_auth_command.py | 52 +++++++++++-
.../ctl/commands/test_config_command.py | 14 +++-
.../ctl/commands/test_connections_command.py | 25 ++++++
.../airflow_ctl/ctl/commands/test_dag_command.py | 93 +++++++++++++++++++++-
.../airflow_ctl/ctl/commands/test_task_command.py | 40 ++++++++++
10 files changed, 248 insertions(+), 16 deletions(-)
diff --git a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py
b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py
index ee2a8512de7..6440000fce0 100644
--- a/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py
+++ b/airflow-ctl/src/airflowctl/ctl/commands/auth_command.py
@@ -27,6 +27,7 @@ import sys
import keyring
import rich
from keyring.errors import NoKeyringError
+from rich.markup import escape
from airflowctl.api.client import (
NEW_API_CLIENT,
@@ -93,7 +94,7 @@ def login(args, api_client=NEW_API_CLIENT) -> None:
rich.print(success_message)
return
except Exception as e:
- rich.print(f"[red]Login failed: {e}")
+ rich.print(f"[red]Login failed: {escape(str(e))}")
sys.exit(1)
# Token-based login
@@ -156,7 +157,7 @@ def get_token(args, api_client=NEW_API_CLIENT) -> None:
)
print(login_response.access_token)
except Exception as e:
- rich.print(f"[red]Token generation failed: {e}[/red]", file=sys.stderr)
+ rich.print(f"[red]Token generation failed: {escape(str(e))}[/red]",
file=sys.stderr)
sys.exit(1)
diff --git a/airflow-ctl/src/airflowctl/ctl/commands/config_command.py
b/airflow-ctl/src/airflowctl/ctl/commands/config_command.py
index 30d77d1b298..eeb6e66aa5f 100644
--- a/airflow-ctl/src/airflowctl/ctl/commands/config_command.py
+++ b/airflow-ctl/src/airflowctl/ctl/commands/config_command.py
@@ -23,6 +23,7 @@ from dataclasses import dataclass
from typing import TYPE_CHECKING, Any, NamedTuple
import rich
+from rich.markup import escape
from airflowctl.api.client import NEW_API_CLIENT, ClientKind,
provide_api_client
@@ -838,5 +839,5 @@ def lint(args, api_client=NEW_API_CLIENT) -> None:
rich.print("[green]No issues found in your airflow.cfg. It is
ready for Airflow 3![/green]")
except Exception as e:
- rich.print(f"[red]Lint configs failed: {e}")
+ rich.print(f"[red]Lint configs failed: {escape(str(e))}")
sys.exit(1)
diff --git a/airflow-ctl/src/airflowctl/ctl/commands/connection_command.py
b/airflow-ctl/src/airflowctl/ctl/commands/connection_command.py
index f99bd81cc7b..040f4e0960c 100644
--- a/airflow-ctl/src/airflowctl/ctl/commands/connection_command.py
+++ b/airflow-ctl/src/airflowctl/ctl/commands/connection_command.py
@@ -23,6 +23,7 @@ from typing import Any
import rich
from pydantic import ValidationError
+from rich.markup import escape
from airflowctl.api.client import NEW_API_CLIENT, ClientKind,
provide_api_client
from airflowctl.api.datamodels.generated import (
@@ -91,9 +92,9 @@ def import_(args, api_client=NEW_API_CLIENT) -> None:
)
response =
api_client.connections.bulk(BulkBodyConnectionBody(actions=[connection_create_action]))
if response.create.errors:
- rich.print(f"[red]Failed to import connections:
{response.create.errors}[/red]")
+ rich.print(f"[red]Failed to import connections:
{escape(str(response.create.errors))}[/red]")
raise SystemExit(1)
rich.print(f"[green]Successfully imported {response.create.success}
connection(s)[/green]")
except Exception as e:
- rich.print(f"[red]Failed to import connections: {e}[/red]")
+ rich.print(f"[red]Failed to import connections:
{escape(str(e))}[/red]")
raise SystemExit(1)
diff --git a/airflow-ctl/src/airflowctl/ctl/commands/dag_command.py
b/airflow-ctl/src/airflowctl/ctl/commands/dag_command.py
index 0027a36e36a..92750715c9d 100644
--- a/airflow-ctl/src/airflowctl/ctl/commands/dag_command.py
+++ b/airflow-ctl/src/airflowctl/ctl/commands/dag_command.py
@@ -23,6 +23,7 @@ import sys
from typing import Literal
import rich
+from rich.markup import escape
from rich.text import Text
from airflowctl.api.client import (
@@ -48,7 +49,10 @@ def update_dag_state(
dag_id=dag_id, dag_body=DAGPatchBody(is_paused=operation ==
"pause")
)
except ServerResponseError as e:
- rich.print(f"[red]Error while trying to {operation} Dag {dag_id}:
{e}[/red]", file=sys.stderr)
+ rich.print(
+ f"[red]Error while trying to {operation} Dag {escape(dag_id)}:
{escape(str(e))}[/red]",
+ file=sys.stderr,
+ )
sys.exit(1)
response_dict = response.model_dump(mode="json")
@@ -98,7 +102,7 @@ def next_execution(args, api_client=NEW_API_CLIENT) -> dict
| None:
try:
response = api_client.dags.get(dag_id=args.dag_id)
except ServerResponseError as e:
- rich.print(f"[red]Error retrieving DAG {args.dag_id}: {e}[/red]")
+ rich.print(f"[red]Error retrieving DAG {escape(args.dag_id)}:
{escape(str(e))}[/red]")
sys.exit(1)
response_dict = response.model_dump(mode="json")
@@ -123,7 +127,7 @@ def _get_dag_run_by_run_id(api_client: Client, dag_id: str,
run_id: str) -> DAGR
except ServerResponseError as e:
if e.response.status_code != 404:
raise
- rich.print(f"[red]Dag run {run_id!r} of Dag {dag_id!r} not
found[/red]")
+ rich.print(f"[red]Dag run {escape(repr(run_id))} of Dag
{escape(repr(dag_id))} not found[/red]")
sys.exit(1)
@@ -132,7 +136,7 @@ def _get_dag_run_by_logical_date(api_client: Client,
dag_id: str, value: str) ->
try:
logical_date = datetime.datetime.fromisoformat(value.replace("Z",
"+00:00"))
except ValueError:
- rich.print(f"[red]Invalid --logical-date: {value!r}[/red]")
+ rich.print(f"[red]Invalid --logical-date: {escape(repr(value))}[/red]")
sys.exit(1)
if logical_date.tzinfo is None:
rich.print("[red]--logical-date must include a timezone offset[/red]")
@@ -152,7 +156,9 @@ def _get_dag_run_by_logical_date(api_client: Client,
dag_id: str, value: str) ->
if e.response.status_code != 404:
raise
if not dag_runs:
- rich.print(f"[red]Dag run for {dag_id} with logical date {value!r} not
found[/red]")
+ rich.print(
+ f"[red]Dag run for {escape(dag_id)} with logical date
{escape(repr(value))} not found[/red]"
+ )
sys.exit(1)
return dag_runs[0]
diff --git a/airflow-ctl/src/airflowctl/ctl/commands/task_command.py
b/airflow-ctl/src/airflowctl/ctl/commands/task_command.py
index 5881a1bd556..530ff10fbce 100644
--- a/airflow-ctl/src/airflowctl/ctl/commands/task_command.py
+++ b/airflow-ctl/src/airflowctl/ctl/commands/task_command.py
@@ -22,6 +22,7 @@ import sys
from typing import TYPE_CHECKING
import rich
+from rich.markup import escape
from airflowctl.api.client import NEW_API_CLIENT, ClientKind,
ServerResponseError, provide_api_client
from airflowctl.ctl.console_formatting import AirflowConsole
@@ -35,7 +36,7 @@ def _find_run_id_by_logical_date(api_client, dag_id: str,
value: str) -> str:
try:
logical_date = datetime.datetime.fromisoformat(value.replace("Z",
"+00:00"))
except ValueError:
- rich.print(f"[red]Invalid --logical-date: {value!r}[/red]")
+ rich.print(f"[red]Invalid --logical-date: {escape(repr(value))}[/red]")
sys.exit(1)
if logical_date.tzinfo is None:
rich.print("[red]--logical-date must include a timezone offset[/red]")
@@ -55,7 +56,9 @@ def _find_run_id_by_logical_date(api_client, dag_id: str,
value: str) -> str:
if e.response.status_code != 404:
raise
if not dag_runs:
- rich.print(f"[red]Dag run for {dag_id} with logical date {value!r} not
found[/red]")
+ rich.print(
+ f"[red]Dag run for {escape(dag_id)} with logical date
{escape(repr(value))} not found[/red]"
+ )
sys.exit(1)
return dag_runs[0].dag_run_id
@@ -87,7 +90,9 @@ def states_for_dag_run(args, api_client=NEW_API_CLIENT) ->
None:
task_instances = api_client.task_instances.list(dag_id=args.dag_id,
dag_run_id=run_id).task_instances
except ServerResponseError as e:
if e.response.status_code == 404:
- rich.print(f"[red]Dag run {run_id!r} of Dag {args.dag_id!r} not
found[/red]")
+ rich.print(
+ f"[red]Dag run {escape(repr(run_id))} of Dag
{escape(repr(args.dag_id))} not found[/red]"
+ )
sys.exit(1)
raise
diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py
b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py
index 2856394ac40..11a30dd3741 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_auth_command.py
@@ -24,7 +24,7 @@ from unittest.mock import patch
import pytest
-from airflowctl.api.client import ClientKind
+from airflowctl.api.client import Client, ClientKind
from airflowctl.api.datamodels.auth_generated import LoginResponse
from airflowctl.ctl import cli_parser
from airflowctl.ctl.commands import auth_command
@@ -647,3 +647,53 @@ class TestListEnvs:
auth_command.list_envs(args)
mock_get_password.assert_called_once_with("airflowctl",
"api_token_prod.json.region")
+
+
+def test_login_prints_failure_containing_markup(capsys):
+ api_client = mock.MagicMock(spec=Client)
+ api_client.login.login_with_username_and_password.side_effect =
RuntimeError("bad [/x]")
+
+ with pytest.raises(SystemExit) as exc_info:
+ auth_command.login(
+ cli_parser.get_parser().parse_args(
+ [
+ "auth",
+ "login",
+ "--api-url",
+ "http://localhost:8080",
+ "--username",
+ "user",
+ "--password",
+ "wrong",
+ ]
+ ),
+ api_client=api_client,
+ )
+
+ assert exc_info.value.code == 1
+ assert "bad [/x]" in capsys.readouterr().out
+
+
+def test_token_prints_failure_containing_markup(capsys):
+ api_client = mock.MagicMock(spec=Client)
+ api_client.login.login_with_username_and_password.side_effect =
RuntimeError("bad [/x]")
+
+ with pytest.raises(SystemExit) as exc_info:
+ auth_command.get_token(
+ cli_parser.get_parser().parse_args(
+ [
+ "auth",
+ "token",
+ "--api-url",
+ "http://localhost:8080",
+ "--username",
+ "user",
+ "--password",
+ "wrong",
+ ]
+ ),
+ api_client=api_client,
+ )
+
+ assert exc_info.value.code == 1
+ assert "bad [/x]" in capsys.readouterr().err
diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_config_command.py
b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_config_command.py
index d7b7662e4ce..d7d68b8aee6 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_config_command.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_config_command.py
@@ -17,11 +17,12 @@
from __future__ import annotations
import os
+from unittest import mock
from unittest.mock import patch
import pytest
-from airflowctl.api.client import ClientKind
+from airflowctl.api.client import Client, ClientKind
from airflowctl.api.datamodels.generated import Config, ConfigOption,
ConfigSection
from airflowctl.ctl import cli_parser
from airflowctl.ctl.commands import config_command
@@ -611,3 +612,14 @@ class TestCliConfigCommands:
assert "fernet_key" in output_str
assert "< hidden >" in output_str
assert "sql_alchemy_conn" in output_str
+
+
+def test_lint_prints_failure_containing_markup(capsys):
+ api_client = mock.MagicMock(spec=Client)
+ api_client.configs.list.side_effect = RuntimeError("bad [/x]")
+
+ with pytest.raises(SystemExit) as exc_info:
+ config_command.lint(cli_parser.get_parser().parse_args(["config",
"lint"]), api_client=api_client)
+
+ assert exc_info.value.code == 1
+ assert "bad [/x]" in capsys.readouterr().out
diff --git
a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_connections_command.py
b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_connections_command.py
index 3178a08339a..7646e074914 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_connections_command.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_connections_command.py
@@ -321,3 +321,28 @@ class TestCliConnectionCommands:
bulk_body = mock_client.connections.bulk.call_args[0][0]
action = bulk_body.actions[0]
assert action.action_on_existence == expected_enum
+
+
[email protected](
+ "bulk_setup",
+ [
+ pytest.param(
+ lambda bulk: setattr(bulk.return_value.create, "errors", ["bad
[/x]"]), id="bulk-errors"
+ ),
+ pytest.param(lambda bulk: setattr(bulk, "side_effect",
RuntimeError("bad [/x]")), id="exception"),
+ ],
+)
+def test_import_prints_failure_containing_markup(bulk_setup, tmp_path, capsys):
+ connections_file = tmp_path / "connections.json"
+ connections_file.write_text(json.dumps({"my_conn": {"conn_type": "http"}}))
+ api_client = mock.MagicMock(spec=Client)
+ bulk_setup(api_client.connections.bulk)
+
+ with pytest.raises(SystemExit) as exc_info:
+ connection_command.import_(
+ cli_parser.get_parser().parse_args(["connections", "import",
connections_file.as_posix()]),
+ api_client=api_client,
+ )
+
+ assert exc_info.value.code == 1
+ assert "bad [/x]" in capsys.readouterr().out
diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_dag_command.py
b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_dag_command.py
index cbc11fd44bc..3ac3534f2e8 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_dag_command.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_dag_command.py
@@ -23,7 +23,7 @@ from unittest import mock
import httpx
import pytest
-from airflowctl.api.client import ClientKind
+from airflowctl.api.client import Client, ClientKind
from airflowctl.api.datamodels.generated import DAGResponse, DagRunType
from airflowctl.api.operations import ServerResponseError
from airflowctl.ctl import cli_parser
@@ -163,6 +163,33 @@ class TestDagCommands:
api_client=api_client,
)
+ def test_pause_fail_prints_server_error_containing_markup(self,
api_client_maker, capsys):
+ api_client = api_client_maker(
+ path=f"/api/v2/dags/{self.dag_id}",
+ response_json={"detail": "bad [/x] value"},
+ expected_http_status_code=404,
+ kind=ClientKind.CLI,
+ )
+ with pytest.raises(SystemExit):
+ dag_command.pause(
+ self.parser.parse_args(["dags", "pause", self.dag_id]),
+ api_client=api_client,
+ )
+
+ assert "[/x]" in capsys.readouterr().err
+
+ def test_pause_fail_prints_dag_id_containing_markup(self, capsys):
+ api_client = mock.Mock(spec=Client)
+ api_client.dags.update.side_effect = _make_server_error(404)
+
+ with pytest.raises(SystemExit):
+ dag_command.pause(
+ self.parser.parse_args(["dags", "pause", "my_dag[/y]"]),
+ api_client=api_client,
+ )
+
+ assert "my_dag[/y]" in capsys.readouterr().err
+
def test_unpause_dag(self, api_client_maker, monkeypatch):
api_client = api_client_maker(
path=f"/api/v2/dags/{self.dag_id}",
@@ -272,6 +299,33 @@ class TestDagCommands:
)
assert result is None
+ def test_next_execution_fail_prints_dag_id_containing_markup(self, capsys):
+ api_client = mock.Mock(spec=Client)
+ api_client.dags.get.side_effect = _make_server_error(404)
+
+ with pytest.raises(SystemExit):
+ dag_command.next_execution(
+ self.parser.parse_args(["dags", "next-execution",
"my_dag[/y]"]),
+ api_client=api_client,
+ )
+
+ assert "my_dag[/y]" in capsys.readouterr().out
+
+ def test_next_execution_fail_prints_server_error_containing_markup(self,
api_client_maker, capsys):
+ api_client = api_client_maker(
+ path=f"/api/v2/dags/{self.dag_id}",
+ response_json={"detail": "bad [/x] value"},
+ expected_http_status_code=404,
+ kind=ClientKind.CLI,
+ )
+ with pytest.raises(SystemExit):
+ dag_command.next_execution(
+ self.parser.parse_args(["dags", "next-execution",
self.dag_id]),
+ api_client=api_client,
+ )
+
+ assert "[/x]" in capsys.readouterr().out
+
def test_next_execution_fail(self, api_client_maker):
api_client = api_client_maker(
path=f"/api/v2/dags/{self.dag_id}",
@@ -409,6 +463,43 @@ class TestDagCommands:
"Dag run 'missing_run' of Dag 'test_dag' not found"
)
+ def test_state_not_found_by_run_id_prints_ids_containing_markup(self,
capsys):
+ api_client = mock.Mock(spec=Client)
+ api_client.dag_runs.get.side_effect = _make_server_error(404)
+
+ with pytest.raises(SystemExit):
+ dag_command.state(
+ self.parser.parse_args(["dags", "state", "my_dag[/y]",
"run_[/x]"]),
+ api_client=api_client,
+ )
+
+ output = capsys.readouterr().out
+ assert "run_[/x]" in output
+ assert "my_dag[/y]" in output
+
+ def test_state_prints_invalid_logical_date_containing_markup(self, capsys):
+ with pytest.raises(SystemExit):
+ dag_command.state(
+ self.parser.parse_args(["dags", "state", self.dag_id,
"--logical-date", "[/x]"]),
+ api_client=mock.Mock(spec=Client),
+ )
+
+ assert "[/x]" in capsys.readouterr().out
+
+ def
test_state_not_found_by_logical_date_prints_dag_id_containing_markup(self,
capsys):
+ api_client = mock.Mock(spec=Client)
+ api_client.dag_runs.list.return_value = mock.Mock(dag_runs=[])
+
+ with pytest.raises(SystemExit):
+ dag_command.state(
+ self.parser.parse_args(
+ ["dags", "state", "my_dag[/y]", "--logical-date",
"2026-01-01T00:00:00Z"]
+ ),
+ api_client=api_client,
+ )
+
+ assert "my_dag[/y]" in capsys.readouterr().out
+
def test_state_reraises_non_404_dag_run_get_error(self):
api_client = mock.MagicMock()
api_client.dag_runs.get.side_effect = error = _make_server_error(500)
diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_task_command.py
b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_task_command.py
index cb134726711..f245b9e0796 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_task_command.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_task_command.py
@@ -23,6 +23,7 @@ from unittest import mock
import httpx
import pytest
+from airflowctl.api.client import Client
from airflowctl.api.datamodels.generated import (
TaskInstanceCollectionResponse,
TaskInstanceResponse,
@@ -264,6 +265,45 @@ class TestStatesForDagRun:
"Dag run 'test_run' of Dag 'test_dag' not found"
)
+ def test_states_for_dag_run_not_found_prints_ids_containing_markup(self,
capsys):
+ api_client = mock.MagicMock(spec=Client)
+ api_client.task_instances.list.side_effect = _make_server_error(404)
+
+ with pytest.raises(SystemExit, match="1"):
+ task_command.states_for_dag_run(
+ self.parser.parse_args(["tasks", "states-for-dag-run",
"my_dag[/y]", "run_[/x]"]),
+ api_client=api_client,
+ )
+
+ output = capsys.readouterr().out
+ assert "run_[/x]" in output
+ assert "my_dag[/y]" in output
+
+ def
test_states_for_dag_run_prints_invalid_logical_date_containing_markup(self,
capsys):
+ with pytest.raises(SystemExit, match="1"):
+ task_command.states_for_dag_run(
+ self.parser.parse_args(
+ ["tasks", "states-for-dag-run", self.dag_id,
"--logical-date", "[/x]"]
+ ),
+ api_client=mock.MagicMock(spec=Client),
+ )
+
+ assert "[/x]" in capsys.readouterr().out
+
+ def
test_states_for_dag_run_not_found_by_logical_date_prints_dag_id_containing_markup(self,
capsys):
+ api_client = mock.MagicMock(spec=Client)
+ api_client.dag_runs.list.return_value.dag_runs = []
+
+ with pytest.raises(SystemExit, match="1"):
+ task_command.states_for_dag_run(
+ self.parser.parse_args(
+ ["tasks", "states-for-dag-run", "my_dag[/y]",
"--logical-date", "2026-01-01T00:00:00Z"]
+ ),
+ api_client=api_client,
+ )
+
+ assert "my_dag[/y]" in capsys.readouterr().out
+
@pytest.mark.parametrize(
"failing_call",
["dag_runs_list", "task_instances_list"],