text_dir_codepoint lint update #16452

enthropy7 · 2025-12-31T17:36:00Z

What does this PR try to resolve?

This PR implements the text_direction_codepoint lint for Cargo.toml files:
Fixes #16373 and #16374

lint detects Unicode BiDi (bidirectional) control codepoints that can be used in "Trojan Source" attacks. These invisible characters can alter the visual display of text without changing its underlying representation, potentially making malicious manifest content appear benign. All like descritped in rustc's text_direction_codepoint_in_comment/literal lint but for Cargo manifests.

Detected codepoints:

U+202A LEFT-TO-RIGHT EMBEDDING
U+202B RIGHT-TO-LEFT EMBEDDING
U+202C POP DIRECTIONAL FORMATTING
U+202D LEFT-TO-RIGHT OVERRIDE
U+202E RIGHT-TO-LEFT OVERRIDE
U+2066 LEFT-TO-RIGHT ISOLATE
U+2067 RIGHT-TO-LEFT ISOLATE
U+2068 FIRST STRONG ISOLATE
U+2069 POP DIRECTIONAL ISOLATE

Default level: deny

How to test and review this PR?

cargo test -p cargo --test testsuite -- lints::text_direction_codepoint

Manual testing:

Create a manifest with BiDi codepoint (U+202E)

printf '[package]\nname = "foo"\nversion = "0.1.0"\nedition = "2024"\ndescription = "A \xE2\x80\xAEtest"\n' > Cargo.toml
mkdir -p src && echo 'fn main(){}' > src/main.rs
cargo +nightly check -Zcargo-lints

The lint can be configured via [lints.cargo]:

[lints.cargo]
text_direction_codepoint = "allow"  # or "warn"

rustbot · 2025-12-31T17:36:06Z

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

epage · 2025-12-31T17:49:39Z

Feel free to clean up the commits for how they should be reviewed and merged, rather than reflecting the implementation.

On top of that, our contrib guide suggests adding tests in a commit before the feature, showing the current behavior. The feature commit would then update the tests to show the new behavior. When reviewing the feature commit, this helps remove all of the boilerplate from the view and see exactly what changed in behavior.

epage · 2025-12-31T17:51:16Z

src/cargo/core/workspace.rs

Fixes #16373 and #16374

I had forgotten that we had separate issues. The question is whether we should mirror rustc or lint them together. I had overlooked this conversation in #16374, focusing on the other parts.

A good starting point would be to check the history of rustc as to why they made them separate lints.

i think it’s easier to keep lints together cause we parse cargo in raw format so, but i will check and come back later after new year :)

rustc separated the lints because in Rust code, BiDi in string literals might be legitimate (RTL language support) while in comments it's always suspicious. In TOML there's no such distinction - BiDi anywhere in a manifest is suspicious (no legitimate RTL use case for dependency names/versions). Single lint makes sense for Cargo.

This sounds reasonable to me. Thanks for the investigation!

package.description, package.metadata, and workspace.metadata do up some possibilities.

For those cases users can set text_direction_codepoint = "warn" or "allow" in [lints.cargo]. Deny-by-default is still appropriate since legitimate RTL in manifests is rare

There are likely valid reasons for package.description to have text direction code points, e.g. if they are written in a non-english language that uses a different direction. This becomes even bigger of a deal for *.metadata.

My main concern is actually around performance. I was going to suggest we start this as one lint and if there is enough of a motivating case, we split this into two lints with the current lint becoming a lint group. However, in thinking that through, I realized we can easily implement this lint without much of a performance impact.

The way I see this working is we have one function for checking both lints. This function would do a string search for the different code points in question in the Cargo.toml, collecting their spans. If they are not present, we bail out. If they are present, we use toml_parser to create Events. We then iterate through all of the Events to see if they include the span in question and based on EventKind, we choose which lint to fire.

i already started to work on your suggestion, forgot to answer. thanks, i like your idea, will push new method soon

src/cargo/lints/rules/text_direction_codepoint.rs

enthropy7 · 2025-12-31T19:10:41Z

Feel free to clean up the commits for how they should be reviewed and merged, rather than reflecting the implementation.

On top of that, our contrib guide suggests adding tests in a commit before the feature, showing the current behavior. The feature commit would then update the tests to show the new behavior. When reviewing the feature commit, this helps remove all of the boilerplate from the view and see exactly what changed in behavior.

I can restructure commits if needed, but the tests wouldn't show meaningful info because before there was no lint to trigger. sorry btw, for next PR's i would take that info

epage · 2025-12-31T20:51:50Z

I can restructure commits if needed, but the tests wouldn't show meaningful info because before there was no lint to trigger. sorry btw, for next PR's i would take that info

"before there was no lint to trigger": that is exactly what we want to see with this.

enthropy7 · 2025-12-31T20:53:45Z

I can restructure commits if needed, but the tests wouldn't show meaningful info because before there was no lint to trigger. sorry btw, for next PR's i would take that info

"before there was no lint to trigger": that is exactly what we want to see with this.

k, when i get to my PC in an hour will do it correctly 💯

enthropy7 · 2025-12-31T22:33:29Z

I can restructure commits if needed, but the tests wouldn't show meaningful info because before there was no lint to trigger. sorry btw, for next PR's i would take that info

"before there was no lint to trigger": that is exactly what we want to see with this.

Done, restructured commits

epage · 2025-12-31T22:42:11Z

Thanks!

enthropy7 · 2025-12-31T22:51:33Z

Thanks!

np, for future PR's - will keep it in mind. thanks for marking this

enthropy7 · 2026-01-01T17:58:41Z

if anything is need - i’m here 24/7 :3 just wanna merge it and take the next task

weihanglo

You might need to fix CI job failures (rustfmt).

View changes since this review

weihanglo · 2026-01-01T22:43:28Z

src/cargo/core/workspace.rs

+            // Only check for BiDi codepoints in virtual workspace manifests.
+            // For package roots, the lint is run in emit_pkg_lints.
+            if matches!(self.root_maybe(), MaybePackage::Virtual(_)) {


Should we move this logic into the lint function itself?

In the future we might want to make the lint infra a LintContext, and register lints all at once, instead of the current manual calling functions.

corrected it, thanks, i haven't thought about it (my bad)

weihanglo · 2026-01-01T23:00:39Z

tests/testsuite/lints/text_direction_codepoint.rs

        .with_stderr_data(str![[r#"
-[CHECKING] foo v0.0.1 ([ROOT]/foo)
-[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
+[ERROR] unicode codepoint changing visible direction of text present in manifest: `/u{202E}` (RIGHT-TO-LEFT OVERRIDE)


Suggested change

[ERROR] unicode codepoint changing visible direction of text present in manifest: `/u{202E}` (RIGHT-TO-LEFT OVERRIDE)

[ERROR] unicode codepoint changing visible direction of text present in manifest

Can we instead putting "/u{202E} (RIGHT-TO-LEFT OVERRIDE)" as a label of the annotated span, like what rustc lint displays?

https://doc.rust-lang.org/rustc/lints/listing/deny-by-default.html#text-direction-codepoint-in-comment

weihanglo · 2026-01-01T23:09:35Z

tests/testsuite/lints/text_direction_codepoint.rs

+[ERROR] unicode codepoint changing visible direction of text present in manifest: `/u{202E}` (RIGHT-TO-LEFT OVERRIDE)
+ --> Cargo.toml:6:18
+  |
+6 | description = "A �test� package"
+  |                  ^ this invisible unicode codepoint changes text flow direction
+  |
+  = [NOTE] `cargo::text_direction_codepoint` is set to `deny` by default
+  = [HELP] if their presence wasn't intentional, you can remove them
+[ERROR] unicode codepoint changing visible direction of text present in manifest: `/u{202D}` (LEFT-TO-RIGHT OVERRIDE)
+ --> Cargo.toml:6:23
+  |
+6 | description = "A �test� package"
+  |                       ^ this invisible unicode codepoint changes text flow direction
+  |
+  = [HELP] if their presence wasn't intentional, you can remove them
+[ERROR] encountered 2 errors while running lints


(Not a blocker)

Do we want to have one error per finding, or one per line, or whathever rustc does if making sense to cargo? I found rustc's approach looks nicer btw

error: unicode codepoint changing visible direction of text present in comment --> $DIR/unicode-control-codepoints.rs:40:1 | LL | //"/*� } �if isAdmin� � begin admins only */" | ^^^^^-^^^-^^^^^^^^^^-^-^^^^^^^^^^^^^^^^^^^^^^ | | | | | | | | | | | '\u{2066}' | | | | '\u{2069}' | | | '\u{2066}' | | '\u{202e}' | this comment contains invisible unicode text flow control codepoints | = note: these kind of unicode codepoints change the way text flows on applications that support them, but can cause confusion because they change the order of characters on the screen = help: if their presence wasn't intentional, you can remove them

https://github.com/rust-lang/rust/blob/fcd630976c460c819c4bbcaf107d0c94501205d8/tests/ui/parser/unicode-control-codepoints.stderr#L212-L225

weihanglo · 2026-01-01T23:11:04Z

tests/testsuite/lints/text_direction_codepoint.rs

+6 | description = "A �test package"
+  |                  ^ this invisible unicode codepoint changes text flow direction
+  |
+  = [NOTE] `cargo::text_direction_codepoint` is set to `deny` by default


rustc has this note, which I found useful explaining why the lint.

= note: these kind of unicode codepoints change the way text flows on applications that support them, but can cause confusion because they change the order of characters on the screen

enthropy7 · 2026-01-02T00:03:36Z

@weihanglo thanks for your suggestions! it was very helpful. small diffs make final work more beautiful and user-friendly.

weihanglo · 2026-01-02T02:40:29Z

Nice. Feel free to force-push. The commit history is expected to be what it should be merged and reviewed, not how it was developed.

enthropy7 · 2026-01-02T10:14:36Z

done, i think , thanks for all the help

enthropy7 · 2026-01-02T22:04:33Z

@epage hi, please take a look if you have some time, waiting for your opinion :)

weihanglo

Other than the question, looks good to me.

View changes since this review

weihanglo · 2026-01-06T21:42:04Z

src/cargo/lints/rules/text_direction_codepoint.rs

+    let mut line_map = Vec::new();
+    line_map.push(0);
+    for (idx, ch) in contents.char_indices() {
+        if ch == '\n' {


Does this match how annotate-snippets detect new lines?

cc @Muscraft

Tests for detecting Unicode BiDi control codepoints in Cargo.toml. Currently these pass as there is no lint implementation yet.

rustbot · 2026-01-06T23:59:01Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Muscraft · 2026-01-07T16:06:45Z

src/cargo/lints/rules/text_direction_codepoint.rs

+        group = group.element(
+            Level::HELP.message("if their presence wasn't intentional, you can remove them"),
+        );
+


Rust suggests escaping the codepoints in literals, and we should do the same.

Note: Basic strings are the only strings that support Unicode literal escaping

If we wanted to provide suggestions for literal strings and bare keys, you can decode the string and then re-encode it using the equivalent basic string type using

https://docs.rs/toml_writer/latest/toml_writer/struct.TomlKeyBuilder.html

https://docs.rs/toml_writer/latest/toml_writer/struct.TomlStringBuilder.html

I should have been more specific in the style of suggestion I was referring to, as I didn't intend for this to get added to the "presence wasn't intentional" help message. I intended for it to be its own "group" in annotate-snippets terms, utilizing Patch, like we do here or here.

Muscraft · 2026-01-07T16:15:19Z

src/cargo/lints/rules/text_direction_codepoint.rs

+    let mut findings_by_line: std::collections::BTreeMap<usize, Vec<_>> =
+        std::collections::BTreeMap::new();
+    for finding in disallowed_findings {
+        let line_num = line_map
+            .iter()
+            .rposition(|&line_start| line_start <= finding.byte_idx)
+            .map(|i| i + 1)
+            .unwrap_or(1);
+        findings_by_line
+            .entry(line_num)
+            .or_insert_with(Vec::new)
+            .push(finding);
+    }


Rust groups codepoints by comment/literal, and we should match that. In our case, we would match by toml_parser::Event.

enthropy7 · 2026-01-08T12:44:04Z

i think now it's closed all of the suggestions. looks pretty nice to me, thanks

rustbot assigned weihanglo Dec 31, 2025

rustbot added A-workspaces Area: workspaces S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 31, 2025

epage reviewed Dec 31, 2025

View reviewed changes

src/cargo/lints/rules/text_direction_codepoint.rs Show resolved Hide resolved

enthropy7 force-pushed the master branch from 079fe86 to 32eb709 Compare December 31, 2025 22:31

enthropy7 requested a review from epage January 1, 2026 12:53

weihanglo reviewed Jan 1, 2026

View reviewed changes

enthropy7 force-pushed the master branch from f411302 to b20cc70 Compare January 2, 2026 03:44

weihanglo reviewed Jan 6, 2026

View reviewed changes

enthropy7 force-pushed the master branch 2 times, most recently from 34744de to 209c2cc Compare January 6, 2026 23:38

This comment has been minimized.

Sign in to view

Add tests for text_direction_codepoint lint

d1bcb39

Tests for detecting Unicode BiDi control codepoints in Cargo.toml. Currently these pass as there is no lint implementation yet.

enthropy7 force-pushed the master branch from 209c2cc to 939e345 Compare January 6, 2026 23:58

Muscraft reviewed Jan 7, 2026

View reviewed changes

enthropy7 force-pushed the master branch from 939e345 to cbb8a46 Compare January 7, 2026 16:16

rustbot added A-credential-provider Area: credential provider for storing and retreiving credentials A-testing-cargo-itself Area: cargo's tests labels Jan 7, 2026

enthropy7 force-pushed the master branch 3 times, most recently from a026552 to 0c21d4f Compare January 7, 2026 16:21

Implement text_direction_codepoint lint

c2c3e3e

enthropy7 force-pushed the master branch from f6881b9 to c2c3e3e Compare January 7, 2026 17:28

	[ERROR] unicode codepoint changing visible direction of text present in manifest: `/u{202E}` (RIGHT-TO-LEFT OVERRIDE)
	[ERROR] unicode codepoint changing visible direction of text present in manifest

text_dir_codepoint lint update #16452

Are you sure you want to change the base?

text_dir_codepoint lint update #16452

Conversation

enthropy7 commented Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How to test and review this PR?

Manual testing:

Uh oh!

rustbot commented Dec 31, 2025

Uh oh!

epage commented Dec 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

enthropy7 commented Dec 31, 2025

Uh oh!

epage commented Dec 31, 2025

Uh oh!

enthropy7 commented Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enthropy7 commented Dec 31, 2025

Uh oh!

epage commented Dec 31, 2025

Uh oh!

enthropy7 commented Dec 31, 2025

Uh oh!

enthropy7 commented Jan 1, 2026

Uh oh!

weihanglo left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

enthropy7 commented Jan 2, 2026

Uh oh!

weihanglo commented Jan 2, 2026

Uh oh!

enthropy7 commented Jan 2, 2026

Uh oh!

enthropy7 commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weihanglo left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

enthropy7 commented Dec 31, 2025 •

edited

Loading

enthropy7 commented Dec 31, 2025 •

edited

Loading

weihanglo left a comment •

edited by rustbot

Loading

enthropy7 commented Jan 2, 2026 •

edited

Loading

weihanglo left a comment •

edited by rustbot

Loading

enthropy7 commented Jan 8, 2026 •

edited

Loading