rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.31k stars 1.53k forks source link

Add a lint to warn on github URLs in panic strings #12622

Closed raggi closed 2 months ago

raggi commented 6 months ago

What it does

There are a growing number of anti-virus products that have ingested variants of YARA rules that declare any program containing github URLs as being malware. This is triggering a high volume of false-positives on rust programs when libraries happen to embed these strings. One common path of this embedding is github URLs in error and panic strings. I recently removed from winit for example: https://github.com/rust-windowing/winit/commit/c4415009c04990d04549a971f01e150e1d5ef79e

Advantage

Drawbacks

Example

.expect("Unexpected GetWindowRect failure; please report this error to https://github.com/rust-windowing/winit");

Could be written as:

.expect("Unexpected GetWindowRect failure; please report this error to rust-windowing/winit");
J-ZhengLi commented 5 months ago

The problem this trying to prevent seems very specific (also very odd).

...declare any program containing github URLs as being malware...

that would mean any github urls outside of expect have the same problem right? like a str variable.

If this is decided, maybe it could be a configurable lint that prevents a certain set of string Lits and lint accordingly? But then again, something like that could be easily done using Regex, so... idk

PatchMixolydic commented 5 months ago

A growing number of libraries seem to be cargo-culting adding issue URLs into panic strings, which while helpful in principle is adding tons of false-positive busywork.

One such cargo-culting project that's had a GitHub URL mindlessly thrown into a panic message is rust— um, no, wait...

This seems like a bizarre characterization of helping the user find precisely where to file a bug report. Indeed, this lint's suggestion would render such panic messages flat-out unhelpful:

please report this error to rust-windowing/winit

From a user's perspective, I'd have no idea "rust-windowing/winit" refers to https://github.com/rust-windowing/winit, https://codeberg.org/rust-windowing/winit, https://git.windowing.rs/rust-windowing/winit, or something else. Introducing any friction into reporting a bug is much more likely to cause developers on a deadline to just sigh and work around it rather than go hunt for where winit happens to live, which could cause serious bugs to persist for much longer than they would otherwise.

It doesn't really make sense to make the UX of an unrecoverable error any worse just because purported antivirus products use broken heuristics.

Alexendoo commented 2 months ago

Clippy itself also embeds a bunch of GitHub links but doesn't seem to trip anything

Trojan:Win32/Wacatac.B!ml from the linked issue means it's based on machine learning right? I don't think we should be offering this as a suggestion when we can't be confident it's correct, it could be that no longer triggering came down to an arbitrary change rather than GitHub URLs specifically