Spun off from #935. The scanner test added there has now been too narrow four times in one PR review cycle.
Problem
tests/cli/test_cli_hardening_935.py::test_no_unescaped_user_text_reaches_rich_output guards against unescaped user text reaching Rich, where a title like Array indexing a[0] and b[1] raises MarkupError and crashes the command. It works, but every revision missed a category the reviewer then found:
| Version |
Approach |
Missed |
| v1 |
enumerated variable names anchored after { |
17 sites ({task.description}, {blocker.answer}, {t.title}, PRD/template/section titles) |
| v2 |
field names, line-based |
3 sites written across multiple physical lines, and all of tui/app.py |
| v3 |
field names, statement-aware, both modules |
amb.label, amb.source_node_title, amb.recommendation — field names not in the list |
| v4 |
expanded field denylist (current) |
unknown — it is still a denylist |
Deny-by-default was tried and rejected: flagging every interpolated attribute fires on 89 statements, most of them timestamps, counts and enum accessors, so it produces churn rather than safety.
Why a lint rule
The check needs to distinguish "this expression is free prose" from "this is an int/enum/timestamp", which is a type question. A denylist of field names cannot answer it and will keep drifting as fields are added.
Options worth evaluating:
- a
ruff custom rule / flake8 plugin over console.print / log.write / add_row call sites
- a typed wrapper —
console.print_user(text) that escapes internally — so the unsafe call is the one you have to reach for deliberately
rich.console.Console(markup=False) for output that never needs markup, escaping only where markup is intended
Acceptance criteria
Why P3
The concrete sites are all fixed in #935; this is about the guard's durability, not an open crash.
Problem
tests/cli/test_cli_hardening_935.py::test_no_unescaped_user_text_reaches_rich_outputguards against unescaped user text reaching Rich, where a title likeArray indexing a[0] and b[1]raisesMarkupErrorand crashes the command. It works, but every revision missed a category the reviewer then found:{{task.description},{blocker.answer},{t.title}, PRD/template/section titles)tui/app.pyamb.label,amb.source_node_title,amb.recommendation— field names not in the listDeny-by-default was tried and rejected: flagging every interpolated attribute fires on 89 statements, most of them timestamps, counts and enum accessors, so it produces churn rather than safety.
Why a lint rule
The check needs to distinguish "this expression is free prose" from "this is an int/enum/timestamp", which is a type question. A denylist of field names cannot answer it and will keep drifting as fields are added.
Options worth evaluating:
ruffcustom rule /flake8plugin overconsole.print/log.write/add_rowcall sitesconsole.print_user(text)that escapes internally — so the unsafe call is the one you have to reach for deliberatelyrich.console.Console(markup=False)for output that never needs markup, escaping only where markup is intendedAcceptance criteria
codeframe/cli/app.pyandcodeframe/tui/app.pyat minimumWhy P3
The concrete sites are all fixed in #935; this is about the guard's durability, not an open crash.