No lint keeps raw bidi controls out of first-party sources, and the authoring path that introduces them was hit twice while fixing #610

[Yaraslaut]

#610 converted nine assertion lines in src/qt/forms/tests/tst_i18n.qml from raw
U+061C / U+200E / U+200F to \uXXXX escapes, and explicitly deferred the lint
that would keep them that way:

Optionally, a lint could keep it that way: a repo-wide scan rejecting raw
U+061C / U+200E / U+200F / U+202A–U+202E / U+2066–U+2069 outside comments would
also catch the "trojan source" shape in first-party code generally. That is a
bigger decision than this issue and should not block the mechanical fix.

Filing it, because while implementing #610 and #591 I hit the hazard the lint
would catch — twice, in the same session, in the opposite direction from the one
#610 describes.

Verification status

Reproduced, on myself, during PR #630. Not a hypothetical and not inferred
from reading.

Writing new source through a tool whose payload is JSON, I typed the six
characters ؜ intending them to land in the file as six characters. JSON
decoded the escape before the file was written, so what landed was the single
invisible character U+061C. It happened in two separate files before I checked:

$ grep -nP '[^\x00-\x7F]' include/morph/render/locale_format.hpp | cat -A
109:    /// The locale's negative-sign string, e.g. `"M-bM-^HM-^R"`. Empty is read as the$
112:    /// The locale's positive-sign string, e.g. `"M-XM-^\+"`. Entry-edge only:$
116:    /// The locale's DIGIT ZERO, e.g. `"M-YM- "`. The ten digits are the ten$
...
$ grep -nP '[^\x00-\x7F]' tests/test_render_locale_format.cpp
799:    // Before morph#591 this edge emitted "؜-1٬050٫25" -- the
856:    // A decision, not a consequence: "٥5" is rejected rather than read as

(M-XM-^\ is D8 9C, U+061C. The same measurement the #610 triage comment
used.) Baseline for comparison: the same header at 0e3b8823 had 2
non-ASCII lines, both pre-existing em-dashes in prose.

I caught it by grepping, fixed it, and both files are ASCII-clean in #630 apart
from the pre-existing em-dashes and box-drawing characters. But nothing in the
repository would have caught it if I had not thought to look, and in these two
cases the characters were inside comments, where the consequence is only
unreadability. In a string literal it is a silently wrong test, which is
precisely cost #2 of #610.

What I did not verify

• I did not write the lint, so I have not measured its false-positive rate
  against the tree. docs/ prose and the ladder fixtures are the obvious places
  where a legitimate non-ASCII character might live.
• I did not check whether any tool in CI or in a pre-commit hook already
  normalises or rejects these; .pre-commit-config.yaml has
  trailing-whitespace, end-of-file-fixer, check-yaml, check-json,
  mixed-line-ending, clang-format and codespell, none of which look at
  this.
• I did not establish that a human contributor hits this. My evidence is about
  a machine-authored edit path. That is a real and now-common authoring path for
  this repository, but it is one path, and the finding should be priced as such.

Why it matters

The failure is silent in both directions:

• ؜ → U+061C (what happened to me): the source stops saying what it
  means, and a reviewer reading the diff cannot tell.
• U+061C → nothing (what tst_i18n.qml embeds raw invisible bidi control characters in its assertions, while its C++ mirror escapes them #610 describes): the test keeps passing, because ASCII
  '+' and '-' are accepted in every locale by design (morph#596), so a
  mangled assertion measures the wrong thing and still reports success.

The tree-wide scan is known to be cheap and currently clean — the #610 triage
comment measured it over the full trojan-source class and found exactly one
file, which #630 has now fixed:

$ grep -rlP '[\x{061C}\x{200E}\x{200F}\x{202A}-\x{202E}\x{2066}-\x{2069}]' src/ tests/ examples/
(no output on this branch)

So the lint would go in green, which is the cheapest moment to add one.

What is actually being decided

Not "should we scan" but three narrower things, which is why this is its own
issue and not a line in #610:

1. Which code points. The bidi controls alone, or the full trojan-source
   class (adding U+202A–U+202E and U+2066–U+2069), or every non-ASCII character
   in src/, tests/ and include/ with an allowlist.
2. Where. First-party sources only, or docs/ too — docs/spec/ prose
   legitimately contains em-dashes and box drawing, so a naive scan over the
   whole tree fails on day one.
3. Comments or not. tst_i18n.qml embeds raw invisible bidi control characters in its assertions, while its C++ mirror escapes them #610's wording says "outside comments". That is harder
   to implement than it sounds for QML and C++, and the two cases above were
   in comments, where the cost is real but smaller. A simpler rule that
   covers comments too may be the better trade.

What would change the verdict

• Close as fixed when a CI step fails on a first-party file containing a raw
  character from the agreed class, and a deliberately-introduced one is shown to
  fail it (a check that passes over a clean tree proves nothing on its own).
• Close as won't-fix if the project decides the grep in tst_i18n.qml embeds raw invisible bidi control characters in its assertions, while its C++ mirror escapes them #610's acceptance
  criterion is enough, in which case that decision should be written where a
  reader of tst_i18n.qml or tests/test_render_locale_format.cpp will find
  it, since both files currently maintain the convention by hand.

[Yaraslaut]

Triage: valid — and the premise got stronger between filing and triage

Measured on master @ 513145a1.

No lint exists:

$ grep -rln "bidi\|U+202\|U+200F\|RLM\|LRM" scripts/*.sh scripts/*.py
(no match)

And raw controls are in the tree right now:

$ grep -rlP '[\x{202A}-\x{202E}\x{2066}-\x{2069}\x{200E}\x{200F}]' --include='*.cpp' --include='*.hpp' --include='*.qml' include src tests examples
src/qt/forms/tests/tst_i18n.qml

five of them, and they were added by #630 — the PR that closed #610, the ticket about exactly this hazard. The same file escapes 13 other occurrences as ‎. The negative-sign block (#610's subject) is escaped; the positive-sign block (#591's addition, same commit) is raw. Filed separately as #642, which is evidence for this ticket rather than a replacement for it.

This issue says the authoring path "was hit twice while fixing #610." Counting the merged result, it is three times, and the third occurrence survived a review that included mine. That is the argument for a lint rather than a fix: the characters are invisible in a diff, so review is not a control here — every human who looked at that diff, including the one who verified the PR, saw nothing.

triage: valid, enhancement, area: ci.

Design notes for whoever takes it, from what #642 turned up:

1. The gate needs a deliberate exemption mechanism, and render/forms: tst_i18n.qml still embeds five raw bidi controls, in the half of the file #630 added #642 is the reason. It is not yet established whether those five raw spellings are accidental or deliberate — nobody wrote it down. A lint that cannot express "this one is raw on purpose, here is why" will either be bypassed or will force a rewrite of a test whose intent nobody has yet stated.
2. Scope it by character class, not by file. The five found here are U+200E/U+200F; the isolates (U+2066–U+2069) and embeddings (U+202A–U+202E) are the ones that actually reorder rendered text and are the sharper hazard.
3. Self-test it first, as every lint in this repository now does — and note the specific vacuity trap: a scan that reports "0 files with raw controls" is satisfied by a broken regex just as well as by a clean tree. Pin a fixture containing each class and require the checker to find it.

What would change the verdict

Close as invalid if the five occurrences in #642 turn out to be deliberate and a reviewer can reliably see them without a lint — which would mean the hazard was overstated. I do not expect that, but it is the falsifiable form.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Correction to my triage above: the "five live examples" were not real

The verdict stands — triage: valid, no bidi lint exists, re-checked on c55ea5b7 (ls scripts/ | grep -i bidi → nothing). The supporting evidence does not.

I claimed five raw U+200E/U+200F were in src/qt/forms/tests/tst_i18n.qml after #630. They were not. My checkout was stale at a8511aa6 — the revision before #630 — so I was grepping the unfixed file and reporting it as the fixed one. Counted properly, by codepoint:

blob @ a8511aa6   lines_with_raw_bidi=9
blob @ 513145a1   lines_with_raw_bidi=0     <- #630
blob @ c55ea5b7   lines_with_raw_bidi=0     <- master when I wrote the triage

#630 escaped all nine. #642, which I filed off that mistake, is closed as invalid with the full measurement.

So strike this line from my triage: "the third time, in the merged result… the third occurrence survived a review that included mine." It did not survive; #630 caught it. The argument for the lint now rests on #628's own two occurrences during authoring, not on three.

What survives, and is if anything sharper: the lane implementing this gate reports that it hit the same hazard twice more while building it — a raw U+061C reached drift-guard.yml inside a comment explaining the escape, invisible in git show, and two more reached the PR body. Both were caught by the checker being written, not by review. That is the case for the lint made from live authoring, which is better evidence than a stale grep.

One practical note for the implementation: on this machine grep -P cannot be trusted for this. The same blob gave 5 lines as a file and 0 on a pipe, against a true count of 9 — grep here is ugrep. The checker should read codepoints, and its self-test should pin that it finds every declared codepoint in a fixture rather than trusting a regex engine.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW