Skip to content

fix(project): escape Rich markup in names rendered to human output#495

Merged
padak merged 2 commits into
mainfrom
fix/escape-rich-markup-in-names
Jul 20, 2026
Merged

fix(project): escape Rich markup in names rendered to human output#495
padak merged 2 commits into
mainfrom
fix/escape-rich-markup-in-names

Conversation

@padak

@padak padak commented Jul 20, 2026

Copy link
Copy Markdown
Member

What

Project names (and other user/API-sourced text) containing square brackets were silently truncated in human-mode output because they were rendered as Rich markup unescaped. Live repro (2026-07-20, project 6100): a Keboola project named [e2e] - kbagent bigquery displayed as - kbagent bigquery in both the project add success message and the project list table -- [e2e] was consumed as a Rich tag. --json always carried the correct stored value; display-only bug.

Fix

Apply rich.markup.escape() in human formatters, following the pattern already used by config.py / flow.py / data_app.py / workspace.py:

  • project group (the confirmed bug): list and status tables, add success message, refresh tables, info panel (alias, project_name, stack_url, token_description), status error text, pending-invitation reason, description-get output.
  • storage buckets: source_project_name in the "Linked From" column -- same bug class (it renders a project name).

Deliberately not escaped: markup built by the code itself ([dim]main[/dim], status badges), masked tokens, and identifier-charset values where brackets cannot occur. The audit of the other command groups found flow/data-app/workspace already escape consistently; semantic-layer names are constrained to [a-z0-9_] so they cannot carry markup.

Tests

  • New TestProjectNameMarkupEscape regression test: seeded project named [e2e] - kbagent bigquery, human project list must contain [e2e]. Verified to FAIL against the unfixed code and pass with the fix.
  • make check green (4315 passed).
  • Live smoke test against the real config: kbagent project list now renders [e2e] - kbagent bigquery in full.

No version bump (display-only fix, rides with the next release).


Open in Devin Review

A project named "[e2e] - kbagent bigquery" displayed as
" - kbagent bigquery" in project add/list/status/info/refresh output:
the name was interpolated into Rich-rendered strings unescaped, so
"[e2e]" was consumed as a markup tag. --json always carried the
correct stored value; display-only bug (live repro on project 6100,
2026-07-20).

- project group: escape alias, project_name, stack_url,
  token_description, status error text, member-invitation reason and
  the project description in every human formatter (rich.markup.escape,
  the same pattern config.py/flow.py/data_app.py already use).
- storage buckets: escape source_project_name in the "Linked From"
  column (same bug class -- it renders a project name).
- Regression test: seeded project with a bracketed name, human
  "project list" output must contain "[e2e]" (fails without the fix).

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Open in Devin Review

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.

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.

@padak padak left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Review of #495 — fix(project): escape Rich markup in names rendered to human output

Generated by kbagent-pr-reviewer subagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed via make check, not duplicated here.

Summary

This PR fixes a real, confirmed bug: user/API-sourced text containing square brackets (e.g. a project named [e2e] - kbagent bigquery) was silently mangled in human-mode Rich output because it was interpreted as Rich markup instead of literal text. The fix correctly applies rich.markup.escape() in the project command group and one field in storage buckets, with a solid regression test and a clean make check. However, the fix is incomplete within the very files it touches: the identical fields it just fixed in one function are left unescaped in sibling functions in the same file (storage bucket-detail's source_project_name/display_name, project invitation-list's reason), which I reproduced live. Verdict: REQUEST CHANGES — the core idea is right, but two spots reproduce the exact same bug this PR exists to close.

Verdict

  • Verdict: REQUEST CHANGES
  • Blocking findings: 2
  • Non-blocking findings: 4
  • Nits: 1

Blocking findings

[B-1] src/keboola_agent_cli/commands/storage.py:205,210-211storage bucket-detail leaves display_name and source_project_name unescaped

storage_buckets (the storage buckets list command) escapes source_project_name at storage.py:134 in this very PR, but the sibling storage_bucket_detail command prints the identical field unescaped at line 211 (f"{result['source_project_name']} (#{result['source_project_id']})"), and also prints the equally free-text display_name unescaped at line 205. Live-reproduced with the fixed code checked out: a bucket detail for a linked bucket whose source project is named [e2e] source proj renders "Linked from: source proj (#999)" — the exact same silent-truncation bug the PR claims to close. Fix: wrap both fields with escape(), same as the list command.

[B-2] src/keboola_agent_cli/commands/project.py:897project invitation-list leaves reason unescaped while the identical field IS escaped three lines of diff away

This PR escapes the pending-invitation reason field inside _format_member_list (project.py:875, part of this diff), but _format_invitation_list's own Reason column (line 897) renders the same --reason TEXT-sourced free text unescaped. Live-reproduced: project invitation-list with reason="[e2e] please approve" prints "Reason: please approve" in the table, dropping [e2e]. Since --reason is arbitrary user-supplied text (see project invite --reason), this is not a hypothetical — it's the same bug, one function over. Fix: escape(inv.get("reason", "")).

Non-blocking findings

[NB-1] src/keboola_agent_cli/commands/storage.py:413storage table-detail's display_name unescaped

Same bug class as NB bucket cases above: display_name is set via storage create-table --name (free text) and rendered unescaped at f" Name: {result['display_name'] or result['name']}". Lower urgency than B-1/B-2 since it wasn't already fixed in a sibling function, but it's the same root cause and should be swept in the same PR.

[NB-2] src/keboola_agent_cli/commands/storage.py:508,1766,1839,1929 — bucket/table/column description text echoed unescaped

create-bucket's "Description:" line (508), describe-bucket/describe-table's echoed text (1766, 1839), and describe-column's echoed desc (1929) all print free text the user just supplied via --text/--file/--stdin/--column NAME=DESC straight into an f-string console.print. Lower severity than B-1/B-2 (it's a same-invocation self-echo, not persisted data shown to a different user later), but it is the same underlying markup-injection bug and will silently truncate any description containing [...].

[NB-3] src/keboola_agent_cli/commands/storage.py:2174storage files table lists uploaded file name unescaped

f.get("name", "") in the files table comes from storage file-upload --name NAME (free text, visible to any teammate who later runs storage files). Same bug class as the fixed project_name — a file named [prod] export.csv would render as export.csv for a different user later, which is exactly the "silent, persisted, shown-to-someone-else" pattern the PR is meant to fix.

[NB-4] src/keboola_agent_cli/commands/project.py:841_format_invite_batch_result's note column unescaped

row.get("note", "") in the bulk-invite per-row results table can carry raw API/exception message text (member_service.py:146,471,567 pass str(exc.message) into note) or the same --reason text via the CSV bulk-invite path. Not yet reproduced with a concrete bracket-bearing example, but the data path is the same shape as the already-fixed fields.

Nits

  • [NIT-1] PR title uses scope fix(project): ... but the diff also touches src/keboola_agent_cli/commands/storage.py (one field). Conventional-commit scope is a minor mismatch given the PR body does call out the storage change explicitly; not worth a re-title but flagging since the checklist asks for it.

Verification log

  • gh auth status → authenticated as padak, scopes gist read:org repo workflow
  • gh pr view 495 --json ... → state OPEN, +63/-20, 3 files (commands/project.py, commands/storage.py, tests/test_cli.py), conventional fix(project): prefix matches bug-fix nature ✓
  • git rev-parse --abbrev-ref HEADfix/escape-rich-markup-in-names, matches <branch> input ✓
  • gh pr diff 495 → 198-line diff, reviewed in full ✓
  • Read CONTRIBUTING.md (Plugin synchronization map, Checklist: Adding a New CLI Command, Releasing a new version) ✓ — not applicable in full: this PR adds zero new/removed/renamed commands, so OPERATION_REGISTRY, AGENT_CONTEXT, CLAUDE.md command list, keboola-expert.md matrix/gotchas, commands-reference.md, gotchas.md sync obligations do not apply to this change.
  • Read CLAUDE.md convention #17 and ## All CLI Commands — no command signatures changed, no drift ✓
  • Read plugins/kbagent/agents/keboola-expert.md §1 and §3 — no non-negotiable rule or inline gotcha touched by this display-only fix ✓
  • Layer-violation greps (httpx/requests in commands, typer/formatter in services) on the diff → empty ✓ (change is entirely within commands/, correct layer for a formatter-only fix)
  • Magic-number / raw-error_code / bare-except / print() / new-tuple[...] greps on the diff → all empty ✓
  • Token-leak grep on diff → one hit (p.get("token", "") in project.py refresh table) but confirmed pre-existing (only reformatted due to adjacent escape() calls, not a new exposure introduced by this PR) — not flagged per "stay in the diff"
  • make check → exit 0, 4315 passed, 8 skipped, 131 deselected ✓ (matches PR description's claim)
  • Reproduced the claimed fix: uv run python3 calling _format_invitation_list and inline snippets mirroring storage bucket-detail / bucket-detail code paths with a [e2e]-bracketed value → confirmed the already-fixed fields (list-mode source_project_name, member-list pending reason) now render correctly; confirmed the not-yet-fixed sibling fields (bucket-detail's source_project_name/display_name, invitation-list's reason) still silently drop the [e2e] text — this is the basis for B-1/B-2 ✓
  • Spot-checked the PR's audit claim ("flow/data-app/workspace/... already escape consistently") via grep -c "escape(" src/keboola_agent_cli/commands/{flow,data_app,workspace,config}.py → 31/6/3/45 hits respectively, confirming those groups do use escape() extensively, supporting the PR's claim for those files ✓

Open questions for the author

(none)

…and fields

Review findings on #495 (all reproduced by the reviewer): the identical
fields fixed in one function were still unescaped in sibling commands.

- B-1 storage bucket-detail: display_name + source_project_name
- B-2 project invitation-list: reason
- NB-1 storage table-detail: display_name
- NB-2 create-bucket / describe-bucket / describe-table / describe-column
  description echoes
- NB-3 storage files table: file name
- NB-4 project invite (bulk) results table: note
@padak

padak commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

All review findings addressed in 1c2f89b:

  • B-1 storage bucket-detail: display_name + source_project_name now escaped (matching the list command).
  • B-2 project invitation-list: reason now escaped (matching the member-list pending table).
  • NB-1 storage table-detail: display_name escaped.
  • NB-2 description echoes escaped in create-bucket, describe-bucket, describe-table, describe-column.
  • NB-3 storage files table: file name escaped.
  • NB-4 bulk-invite results table: note escaped.
  • NIT-1: keeping the title scope as-is per the review's own suggestion (storage change is called out in the body).

make check green after the sweep (4315 passed).

@padak
padak merged commit b10c6ad into main Jul 20, 2026
4 checks passed
@padak
padak deleted the fix/escape-rich-markup-in-names branch July 20, 2026 13:40
padak added a commit that referenced this pull request Jul 20, 2026
Backfills changelog entries for ten PRs merged since v0.66.1 without a
version bump (#465 #486 #487 #488 #490 #492 #493 #494 #495 + #500),
attributed to their bump windows (0.67.0 / 0.70.0 / 0.70.1 / 0.71.0),
and aligns the version at 0.71.0 as the catch-up release: 0.67.0-0.70.1
were merged to main but never tagged or published, so auto-update users
are still on 0.66.1. The v0.71.0 tag + GitHub Release follow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant