Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 29 additions & 21 deletions src/keboola_agent_cli/commands/project.py

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Invitation-list reason not escaped, unlike member-list pending reason

The PR escapes the pending-invitation reason field in _format_member_list (project.py:875), but the parallel _format_invitation_list renders inv.get("reason", "") unescaped at src/keboola_agent_cli/commands/project.py:897. Both render an API-sourced invitation reason into a Rich table cell, so this is the same display-truncation bug class the PR sets out to fix, left incomplete. Additionally the alias in the same function (data.get('alias') inside [cyan]...[/cyan] at project.py:883 and in the table title at project.py:886) is not escaped. This line is outside the PR's diff hunks so it is reported here rather than as a bug.

(Refers to line 897)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import typer
from rich.console import Console
from rich.markup import escape
from rich.table import Table

from ..constants import (
Expand Down Expand Up @@ -69,10 +70,10 @@ def _format_project_table(console: Console, projects: list[dict[str, Any]]) -> N
branch_id = p.get("active_branch_id")
branch_display = str(branch_id) if branch_id is not None else "[dim]main[/dim]"
table.add_row(
p["alias"],
p.get("project_name", ""),
escape(p["alias"]),
escape(p.get("project_name", "")),
str(p.get("project_id", "")),
p["stack_url"],
escape(p["stack_url"]),
p["token"],
default_marker,
branch_display,
Expand All @@ -99,16 +100,16 @@ def _format_status_table(console: Console, statuses: list[dict[str, Any]]) -> No
if s["status"] == "ok":
status_str = "[bold green]OK[/bold green]"
else:
status_str = f"[bold red]ERROR[/bold red]: {s.get('error', 'Unknown')}"
status_str = f"[bold red]ERROR[/bold red]: {escape(s.get('error', 'Unknown'))}"
response_time = f"{s.get('response_time_ms', 0)}ms"
branch_id = s.get("active_branch_id")
branch_display = str(branch_id) if branch_id is not None else "[dim]main[/dim]"
table.add_row(
s["alias"],
escape(s["alias"]),
status_str,
response_time,
s.get("project_name", ""),
s["stack_url"],
escape(s.get("project_name", "")),
escape(s["stack_url"]),
branch_display,
)

Expand Down Expand Up @@ -176,8 +177,8 @@ def project_add(
formatter.output(
result,
lambda c, d: c.print(
f"[bold green]Success:[/bold green] Project [bold]{d['alias']}[/bold] added "
f"(project: {d['project_name']}, id: {d['project_id']})"
f"[bold green]Success:[/bold green] Project [bold]{escape(d['alias'])}[/bold] "
f"added (project: {escape(d['project_name'])}, id: {d['project_id']})"
),
)
except KeboolaApiError as exc:
Expand Down Expand Up @@ -339,10 +340,13 @@ def _format_refresh_result(console: Console, data: dict) -> None:

for p in refreshed:
if dry_run:
table.add_row(p["alias"], str(p["project_id"]), p["project_name"])
table.add_row(escape(p["alias"]), str(p["project_id"]), escape(p["project_name"]))
else:
table.add_row(
p["alias"], str(p["project_id"]), p["project_name"], p.get("token", "")
escape(p["alias"]),
str(p["project_id"]),
escape(p["project_name"]),
p.get("token", ""),
)

console.print(table)
Expand All @@ -357,7 +361,7 @@ def _format_refresh_result(console: Console, data: dict) -> None:
table.add_column("Project Name")

for p in valid:
table.add_row(p["alias"], str(p["project_id"]), p["project_name"])
table.add_row(escape(p["alias"]), str(p["project_id"]), escape(p["project_name"]))

console.print(table)
console.print()
Expand Down Expand Up @@ -656,7 +660,9 @@ def project_description_get(

formatter.output(
result,
lambda c, d: c.print(d["description"] or "[dim](no description set)[/dim]"),
lambda c, d: c.print(
escape(d["description"]) if d["description"] else "[dim](no description set)[/dim]"
),
)


Expand Down Expand Up @@ -718,13 +724,13 @@ def _format_info_table(console: Console, data: dict[str, Any]) -> None:
table.add_column("Field", style="bold cyan", no_wrap=True)
table.add_column("Value")

table.add_row("Alias", str(data.get("alias", "")))
table.add_row("Alias", escape(str(data.get("alias", ""))))
table.add_row("Project ID", str(data.get("project_id", "")))
table.add_row("Project Name", str(data.get("project_name", "")))
table.add_row("Stack URL", str(data.get("stack_url", "")))
table.add_row("Project Name", escape(str(data.get("project_name", ""))))
table.add_row("Stack URL", escape(str(data.get("stack_url", ""))))
table.add_row("Default Backend", str(data.get("default_backend", "")))
table.add_row("Token ID", str(data.get("token_id", "")))
table.add_row("Token Description", str(data.get("token_description", "")))
table.add_row("Token Description", escape(str(data.get("token_description", ""))))
table.add_row("Master Token", "Yes" if data.get("is_master_token") else "No")

expires = data.get("token_expires")
Expand All @@ -746,7 +752,9 @@ def _format_info_table(console: Console, data: dict[str, Any]) -> None:
metric_lines = [f"{k}: {v}" for k, v in sorted(metrics.items())]
table.add_row("Metrics", "\n".join(metric_lines))

console.print(Panel(table, title=f"Project Info: {data.get('alias', '')}", expand=False))
console.print(
Panel(table, title=f"Project Info: {escape(str(data.get('alias', '')))}", expand=False)
)


@project_app.command("info")
Expand Down Expand Up @@ -830,7 +838,7 @@ def _format_bulk_invite_result(console: Console, data: dict[str, Any]) -> None:
row.get("email", ""),
row.get("project", ""),
row.get("role", ""),
row.get("note", ""),
escape(row.get("note", "")),
)
console.print(table)

Expand Down Expand Up @@ -864,7 +872,7 @@ def _format_member_list(console: Console, data: dict[str, Any]) -> None:
str(p.get("id", "")),
p.get("user", {}).get("email", ""),
p.get("role", ""),
p.get("reason", ""),
escape(p.get("reason", "")),
)
console.print(ptable)

Expand All @@ -886,7 +894,7 @@ def _format_invitation_list(console: Console, data: dict[str, Any]) -> None:
str(inv.get("id", "")),
inv.get("user", {}).get("email", ""),
inv.get("role", ""),
inv.get("reason", ""),
escape(inv.get("reason", "")),
)
console.print(table)

Expand Down
19 changes: 10 additions & 9 deletions src/keboola_agent_cli/commands/storage.py

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 bucket-detail source_project_name not escaped, unlike buckets list

The PR escapes source_project_name in the buckets command's "Linked From" column (storage.py:134), but the bucket-detail command renders result['source_project_name'] unescaped at src/keboola_agent_cli/commands/storage.py:211 (and source_bucket_id/display_name similarly). This is the same bug class (a project name with square brackets would be consumed as Rich markup), so the storage fix is incomplete. These lines fall outside the PR's diff hunks, hence reported as an analysis.

(Refers to line 211)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from typing import Any

import typer
from rich.markup import escape

from ..config_store import ConfigStore
from ..errors import ConfigError, ErrorCode, KeboolaApiError
Expand Down Expand Up @@ -130,7 +131,7 @@ def storage_buckets(
for b in proj_buckets:
linked = ""
if b["is_linked"]:
linked = f"{b['source_project_name']} (#{b['source_project_id']})"
linked = f"{escape(b['source_project_name'])} (#{b['source_project_id']})"
table.add_row(
b["id"],
b["stage"],
Expand Down Expand Up @@ -201,13 +202,13 @@ def storage_bucket_detail(
formatter.output(result)
else:
formatter.console.print(f"[bold]Bucket:[/bold] {result['bucket_id']}")
formatter.console.print(f" Display name: {result['display_name']}")
formatter.console.print(f" Display name: {escape(result['display_name'])}")
formatter.console.print(f" Backend: {result['backend']}")

if result["is_linked"]:
formatter.console.print(
f" [yellow]Linked from:[/yellow] "
f"{result['source_project_name']} (#{result['source_project_id']})"
f"{escape(result['source_project_name'])} (#{result['source_project_id']})"
)
formatter.console.print(f" Source bucket: {result['source_bucket_id']}")

Expand Down Expand Up @@ -409,7 +410,7 @@ def storage_table_detail(
formatter.output(result)
else:
formatter.console.print(f"[bold]Table:[/bold] {result['table_id']}")
formatter.console.print(f" Name: {result['display_name'] or result['name']}")
formatter.console.print(f" Name: {escape(result['display_name'] or result['name'])}")
formatter.console.print(f" Bucket: {result['bucket_id']}")
formatter.console.print(f" Rows: {result['rows_count']:,}")
size_mb = result["data_size_bytes"] / (1024 * 1024)
Expand Down Expand Up @@ -504,7 +505,7 @@ def storage_create_bucket(
formatter.console.print(f" Stage: {result['stage']}")
formatter.console.print(f" Backend: {result['backend']}")
if result["description"]:
formatter.console.print(f" Description: {result['description']}")
formatter.console.print(f" Description: {escape(result['description'])}")
if result.get("legacy_branch_storage"):
formatter.console.print(_LEGACY_BRANCH_STORAGE_WARNING)

Expand Down Expand Up @@ -1762,7 +1763,7 @@ def storage_describe_bucket(
formatter.output(result)
else:
formatter.console.print(f"[bold green]Description set:[/bold green] {bucket_id}")
formatter.console.print(f" {description[:120]}")
formatter.console.print(f" {escape(description[:120])}")


@storage_app.command("describe-table", rich_help_panel=_DESCRIBE)
Expand Down Expand Up @@ -1835,7 +1836,7 @@ def storage_describe_table(
formatter.output(result)
else:
formatter.console.print(f"[bold green]Description set:[/bold green] {table_id}")
formatter.console.print(f" {description[:120]}")
formatter.console.print(f" {escape(description[:120])}")


@storage_app.command("describe-column", rich_help_panel=_DESCRIBE)
Expand Down Expand Up @@ -1925,7 +1926,7 @@ def storage_describe_column(
f"({len(parsed)} column(s))"
)
for name, desc in parsed.items():
formatter.console.print(f" {name}: {desc[:80]}")
formatter.console.print(f" {name}: {escape(desc[:80])}")


@storage_app.command("describe-batch", rich_help_panel=_DESCRIBE)
Expand Down Expand Up @@ -2170,7 +2171,7 @@ def storage_file_list(
created = f.get("created", "")[:19] if f.get("created") else ""
table.add_row(
str(f.get("id", "")),
f.get("name", ""),
escape(f.get("name", "")),
_format_file_size(f.get("sizeBytes")),
tags_str,
permanent,
Expand Down
34 changes: 34 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,40 @@ def _setup_config_test(config_dir: Path, projects: dict[str, dict] | None = None
return store


class TestProjectNameMarkupEscape:
"""Regression: a project name containing square brackets ("[e2e] - ...")
must reach human-mode output verbatim, not be eaten as a Rich markup tag."""

BRACKETED_NAME = "[e2e] - kbagent bigquery"

def test_project_list_human_shows_bracketed_name(self, tmp_path: Path) -> None:
config_dir = tmp_path / "config"
config_dir.mkdir()

with (
patch("keboola_agent_cli.cli.ConfigStore") as MockStore,
patch("keboola_agent_cli.cli.ProjectService") as MockService,
):
store_instance = _setup_config_test(
config_dir,
{
"bq": {
"token": TEST_TOKEN,
"project_name": self.BRACKETED_NAME,
"project_id": 6100,
}
},
)
MockStore.return_value = store_instance
MockService.return_value = ProjectService(config_store=store_instance)

# Wide terminal so the Rich table does not wrap the name cell.
result = runner.invoke(app, ["project", "list"], env={"COLUMNS": "200"})

assert result.exit_code == 0
assert "[e2e]" in result.output


class TestConfigList:
"""Tests for `kbagent config list` command."""

Expand Down