Summary

Run code safety checks when generating a diff and show results.

Review Request #11907 — Created Jan. 4, 2022 and submitted June 16, 2022, 1:24 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

~~11904~~, ~~11902~~

Reviewers

Groups

reviewboard

People

Description

The diff viewer now interfaces with the new code safety checkers,
looking for unsafe content during diff generation, before anyone but the
owner is able to view the diff.

Scanners are run per-line, and any warnings or errors from the checker
will be stored along with that line and in a global list in the chunk
generator.

Anything found on a line will cause that line to display a red warning
symbol. Hovering over that will show a summary of what was found. The
top of the file will also show an alert from the safety checker
detailing what was found.

The per-line and per-file information is also available through the Diff
Context API, since it's just part of the resulting file/chunk
information.

The current trojan code safety checker we have deals with
possibly-malicious Unicode characters. As such, there's also support for
highlighting these characters, using a new CSS component, rb-o-duc.
DUCs are Displayed Unicode Characters. They are a simple <span>
containing the codepoint and the character entity.

DUCs default to safely showing the codepoint in a little box, instead of
rendering the character. They can be toggled to render instead, by
setting the -hide-ducs CSS modifier class on the .sidebyside element
for the diff viewer. The render toggling happens purely in CSS.

Hovering over a DUC will show information on the Unicode character. Note
that DUCs are not provided for all Unicode characters, only those that a
code safety checker has highlighted.

The trojan code checker provides the button used for toggling those
characters.

Testing Done

Unit tests pass on Python 2 and 3.

Tested this with the Trojan Source code checker, using the source files
at https://github.com/nickboucher/trojan-source/

Commits

Summary	ID
Run code safety checks when generating a diff and show results. The diff viewer now interfaces with the new code safety checkers, looking for unsafe content during diff generation, before anyone but the owner is able to view the diff. Scanners are run per-line, and any warnings or errors from the checker will be stored along with that line and in a global list in the chunk generator. Anything found on a line will cause that line to display a red warning symbol. Hovering over that will show a summary of what was found. The top of the file will also show an alert from the safety checker detailing what was found. The per-line and per-file information is also available through the Diff Context API, since it's just part of the resulting file/chunk information. The current trojan code safety checker we have deals with possibly-malicious Unicode characters. As such, there's also support for highlighting these characters, using a new CSS component, `rb-o-duc`. DUCs are Displayed Unicode Characters. They are a simple `<span>` containing the codepoint and the character entity. DUCs default to safely showing the codepoint in a little box, instead of rendering the character. They can be toggled to render instead, by setting the `-hide-ducs` CSS modifier class on the `.sidebyside` element for the diff viewer. The render toggling happens purely in CSS. Hovering over a DUC will show information on the Unicode character. Note that DUCs are not provided for all Unicode characters, only those that a code safety checker has highlighted. The trojan code checker provides the button used for toggling those characters.	e24e621013c89f6c5cc8e2d8da54b99aa1ea5d08

Summary

Run code safety checks when generating a diff and show results.

The diff viewer now interfaces with the new code safety checkers, looking for unsafe content during diff generation, before anyone but the owner is able to view the diff. Scanners are run per-line, and any warnings or errors from the checker will be stored along with that line and in a global list in the chunk generator. Anything found on a line will cause that line to display a red warning symbol. Hovering over that will show a summary of what was found. The top of the file will also show an alert from the safety checker detailing what was found. The per-line and per-file information is also available through the Diff Context API, since it's just part of the resulting file/chunk information. The current trojan code safety checker we have deals with possibly-malicious Unicode characters. As such, there's also support for highlighting these characters, using a new CSS component, `rb-o-duc`. DUCs are Displayed Unicode Characters. They are a simple `<span>` containing the codepoint and the character entity. DUCs default to safely showing the codepoint in a little box, instead of rendering the character. They can be toggled to render instead, by setting the `-hide-ducs` CSS modifier class on the `.sidebyside` element for the diff viewer. The render toggling happens purely in CSS. Hovering over a DUC will show information on the Unicode character. Note that DUCs are not provided for all Unicode characters, only those that a code safety checker has highlighted. The trojan code checker provides the button used for toggling those characters.

e24e621013c89f6c5cc8e2d8da54b99aa1ea5d08

Files

Issues

Description	From	Last Updated
F821 undefined name 'format_html'	reviewbot	Jan. 4, 2022, 11:22 p.m.
I'm not sure what this variable name means.	david	May 23, 2022, 6:42 p.m.
Seems like something that should be logged as well?	david	May 23, 2022, 6:43 p.m.
This seems like it should be part of a different change. Same with the other JS/CSS related to ducs.	david	May 23, 2022, 6:44 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/templatetags/difftags.py (Diff revision 1)
Show all issues
```
F821 undefined name 'format_html'
```

Change Summary:

Added a missing import.

Commits:

	Summary	ID
	Run code safety checks when generating a diff and show results. The diff viewer now interfaces with the new code safety checkers, looking for unsafe content during diff generation, before anyone but the owner is able to view the diff. Scanners are run per-line, and any warnings or errors from the checker will be stored along with that line and in a global list in the chunk generator. Anything found on a line will cause that line to display a red warning symbol. Hovering over that will show a summary of what was found. The top of the file will also show an alert from the safety checker detailing what was found. The per-line and per-file information is also available through the Diff Context API, since it's just part of the resulting file/chunk information. The current trojan code safety checker we have deals with possibly-malicious Unicode characters. As such, there's also support for highlighting these characters, using a new CSS component, `rb-o-duc`. DUCs are Displayed Unicode Characters. They are a simple `<span>` containing the codepoint and the character entity. DUCs default to safely showing the codepoint in a little box, instead of rendering the character. They can be toggled to render instead, by setting the `-hide-ducs` CSS modifier class on the `.sidebyside` element for the diff viewer. The render toggling happens purely in CSS. Hovering over a DUC will show information on the Unicode character. Note that DUCs are not provided for all Unicode characters, only those that a code safety checker has highlighted. The trojan code checker provides the button used for toggling those characters.	cc4205d48495603fc03d88494a57c2388c86acf1
	Run code safety checks when generating a diff and show results. The diff viewer now interfaces with the new code safety checkers, looking for unsafe content during diff generation, before anyone but the owner is able to view the diff. Scanners are run per-line, and any warnings or errors from the checker will be stored along with that line and in a global list in the chunk generator. Anything found on a line will cause that line to display a red warning symbol. Hovering over that will show a summary of what was found. The top of the file will also show an alert from the safety checker detailing what was found. The per-line and per-file information is also available through the Diff Context API, since it's just part of the resulting file/chunk information. The current trojan code safety checker we have deals with possibly-malicious Unicode characters. As such, there's also support for highlighting these characters, using a new CSS component, `rb-o-duc`. DUCs are Displayed Unicode Characters. They are a simple `<span>` containing the codepoint and the character entity. DUCs default to safely showing the codepoint in a little box, instead of rendering the character. They can be toggled to render instead, by setting the `-hide-ducs` CSS modifier class on the `.sidebyside` element for the diff viewer. The render toggling happens purely in CSS. Hovering over a DUC will show information on the Unicode character. Note that DUCs are not provided for all Unicode characters, only those that a code safety checker has highlighted. The trojan code checker provides the button used for toggling those characters.	5788d8ec60100733a44e8e5db63441621cc75eca

Diff:

Revision 2 (+1166 -28)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/templatetags/difftags.py
	reviewboard/diffviewer/tests/test_raw_diff_chunk_generator.py
	reviewboard/static/rb/css/pages/diffviewer.less
	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js
	reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.es6.js
	reviewboard/static/rb/js/views/textCommentRowSelector.es6.js
	1 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/diffviewer/chunk_generator.py (Diff revision 2)
Show all issues
```
I'm not sure what this variable name means.
```
reviewboard/diffviewer/templatetags/difftags.py (Diff revision 2)
Show all issues
```
Seems like something that should be logged as well?
```

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)

Show all issues

This seems like it should be part of a different change. Same with the other JS/CSS related to ducs.

chipx86 May 23, 2022, 6:44 p.m.

These are part of the change. This is the client-side rendering of the results of Unicode-related results from the safety checkers. It's in the change description.

Change Summary:


Added some documentation around the code checker result normalization process when rendering the diff.
Added logging when failing to find a code checker result when rendering a file alert.
Removed six usage.
Changed docstring versions to Review Board 5.
Changed unicode types in docstrings to str
Changed SafeText to SafeString.
Changed unit tests to check the renamed trojan code result IDs.
Changed unit tests for newer Pygments HTML output.

Commits:

	Summary	ID
	Run code safety checks when generating a diff and show results. The diff viewer now interfaces with the new code safety checkers, looking for unsafe content during diff generation, before anyone but the owner is able to view the diff. Scanners are run per-line, and any warnings or errors from the checker will be stored along with that line and in a global list in the chunk generator. Anything found on a line will cause that line to display a red warning symbol. Hovering over that will show a summary of what was found. The top of the file will also show an alert from the safety checker detailing what was found. The per-line and per-file information is also available through the Diff Context API, since it's just part of the resulting file/chunk information. The current trojan code safety checker we have deals with possibly-malicious Unicode characters. As such, there's also support for highlighting these characters, using a new CSS component, `rb-o-duc`. DUCs are Displayed Unicode Characters. They are a simple `<span>` containing the codepoint and the character entity. DUCs default to safely showing the codepoint in a little box, instead of rendering the character. They can be toggled to render instead, by setting the `-hide-ducs` CSS modifier class on the `.sidebyside` element for the diff viewer. The render toggling happens purely in CSS. Hovering over a DUC will show information on the Unicode character. Note that DUCs are not provided for all Unicode characters, only those that a code safety checker has highlighted. The trojan code checker provides the button used for toggling those characters.	5788d8ec60100733a44e8e5db63441621cc75eca
	Run code safety checks when generating a diff and show results. The diff viewer now interfaces with the new code safety checkers, looking for unsafe content during diff generation, before anyone but the owner is able to view the diff. Scanners are run per-line, and any warnings or errors from the checker will be stored along with that line and in a global list in the chunk generator. Anything found on a line will cause that line to display a red warning symbol. Hovering over that will show a summary of what was found. The top of the file will also show an alert from the safety checker detailing what was found. The per-line and per-file information is also available through the Diff Context API, since it's just part of the resulting file/chunk information. The current trojan code safety checker we have deals with possibly-malicious Unicode characters. As such, there's also support for highlighting these characters, using a new CSS component, `rb-o-duc`. DUCs are Displayed Unicode Characters. They are a simple `<span>` containing the codepoint and the character entity. DUCs default to safely showing the codepoint in a little box, instead of rendering the character. They can be toggled to render instead, by setting the `-hide-ducs` CSS modifier class on the `.sidebyside` element for the diff viewer. The render toggling happens purely in CSS. Hovering over a DUC will show information on the Unicode character. Note that DUCs are not provided for all Unicode characters, only those that a code safety checker has highlighted. The trojan code checker provides the button used for toggling those characters.	e24e621013c89f6c5cc8e2d8da54b99aa1ea5d08

Branch:

-	release-4.0.x
+	release-5.0.x

Diff:

Revision 3 (+1216 -30)

Show changes

	reviewboard/diffviewer/chunk_generator.py
	reviewboard/diffviewer/diffutils.py
	reviewboard/diffviewer/templatetags/difftags.py
	reviewboard/diffviewer/tests/test_raw_diff_chunk_generator.py
	reviewboard/static/rb/css/pages/diffviewer.less
	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js
	reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.es6.js
	reviewboard/static/rb/js/views/textCommentRowSelector.es6.js
	1 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (4553213)