Add option to disable ambiguous unicode characters detection

[wxiaoguang]

• Close Add configuration option to disable check of "This file contains ambiguous Unicode characters!" #24483
• Close Implement option to disable globally "This file contains ambiguous Unicode characters!" annoying message. #28123
• Close Wrong unicode char reporting. #23682
• Close Incorrect report of ambiguous unicode characters #23149

(maybe more)

[silverwind]

Maybe it should even be disabled by default because as I see it, the current detection is almost pure false-positives for the "warnings", only the "errors" are real problems.

[lunny]

Maybe it should even be disabled by default because as I see it, the current detection is almost pure false-positives for the "warnings", only the "errors" are real problems.

Then we need a break label.

[silverwind]

Maybe it should even be disabled by default because as I see it, the current detection is almost pure false-positives for the "warnings", only the "errors" are real problems.

Then we need a break label.

It wouldn't break anyone's workflow I assume. Not a fan of unnecessary breaking labels.

[wxiaoguang]

Maybe it should even be disabled by default because as I see it, the current detection is almost pure false-positives for the "warnings", only the "errors" are real problems.

I won't do so at the moment, because otherwise some extremists would catch this point and criticize that "Gitea doesn't take security seriously“

[silverwind]

I think ideally we just remove the warning category (which warns on any irregular whitespace), and just keep the error category (which are the real bidi exploits). Doesn't have to be in this PR.

[wxiaoguang]

I think ideally we just remove the warning category (which warns on any irregular whitespace), and just keep the error category (which are the real bidi exploits). Doesn't have to be in this PR.

That's why I used AMBIGUOUS_UNICODE_DETECTION = true, in the future it could be improved to : AMBIGUOUS_UNICODE_DETECTION = error or AMBIGUOUS_UNICODE_DETECTION = bidi, etc

[JakobDev]

I think ideally we just remove the warning category

Sounds like a good idea. Currently this feature is kinda useless. If you have almost only false positives, Users do not care about the Warning and will ignore it, even when there's a real threat.

You should also add a User setting. Not everyone is Admin.

[wxiaoguang]

I think ideally we just remove the warning category

Sounds like a good idea. Currently this feature is kinda useless. If you have almost only false positives, Users do not care about the Warning and will ignore it, even when there's a real threat.

You should also add a User setting. Not everyone is Admin.

It doesn't work that way at the moment, every detected problem is rendered as "warning".

Any improvement could be done later.

[silverwind]

Does this hide the "Escape" button in file view and diffs? I think it should when the feature is disabled as the button is only useful for that feature.

[wxiaoguang]

Does this hide the "Escape" button in file view and diffs? I think it should when the feature is disabled as the button is only useful for that feature.

Yes. There won't be "escape" button if there is no detected ambiguous unicode character.

---

update: due to some old code problem, see #28454 (comment)

[GiteaBot]

I was unable to create a backport for 1.21. @wxiaoguang, please send one manually. 🍵

go run ./contrib/backport 28454
...  // fix git conflicts if any
go run ./contrib/backport --continue

[wxiaoguang]

Does this hide the "Escape" button in file view and diffs? I think it should when the feature is disabled as the button is only useful for that feature.

Yes. There won't be "escape" button if there is no detected ambiguous unicode character.

Just realized that some "escape" buttons are hidden correctly, while some are not, because some of the old code doesn't respect EscapeStatus.Escaped .... so some buttons are always there.

[silverwind]

Does this hide the "Escape" button in file view and diffs? I think it should when the feature is disabled as the button is only useful for that feature.

Yes. There won't be "escape" button if there is no detected ambiguous unicode character.

Just realized that some "escape" buttons are hidden correctly, while some are not, because some of the old code doesn't respect EscapeStatus.Escaped .... so some buttons are always there.

We should get rid of all those unneccesary and confusing escape buttons, e.g. only show if there is actually escapable content.

[github-actions]

Automatically locked because of our CONTRIBUTING guidelines