Summary

Add base support for code safety checkers.

Review Request #11904 — Created Jan. 4, 2022 and submitted June 14, 2022, 5:52 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

Blocks

11907, 11906

Reviewers

Groups

reviewboard

People

Description

There are issues that are best caught before code goes up for review,
and there are also issues that are best caught by default without
needing to install a tool like Review Bot.

Two examples would be credentials accidentally left in code and
so-called "Trojan Source" attacks (where code is either accidentally or
intentionally added to a file that displays one way to a user but
executes another way).

This begins laying the foundation for code safety checkers, which can
look for suspicious content in code before it's ready for review,
flagging issues that are found.

This will be used in the diff validation API and the diff viewer to
highlight any issues that are found. Specific code safety checkers will
be implemented in future changes.

Testing Done

Unit tests pass.

Tested this along with a code checker implementation and the upcoming
diff viewer updates.

Commits

Summary	ID
Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	22e7af692bbf34511d35ef76a0ba3979a7fbde4b

Summary

Add base support for code safety checkers.

Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.

22e7af692bbf34511d35ef76a0ba3979a7fbde4b

Issues

Description	From	Last Updated
E501 line too long (94 > 79 characters)	reviewbot	Jan. 4, 2022, 8:45 p.m.
F401 'reviewboard.codesafety.checkers.trojan_source.TrojanSourceCodeSafetyChecker' imported but unused	reviewbot	Jan. 4, 2022, 8:45 p.m.
F811 redefinition of unused 'test_render_file_alert_html' from line 38	reviewbot	Jan. 4, 2022, 8:45 p.m.
E501 line too long (94 > 79 characters)	reviewbot	Jan. 4, 2022, 10:52 p.m.
Seems like "unicode" probably shouldn't be capitalized here?	david	May 23, 2022, 7:06 p.m.
And here.	david	May 23, 2022, 7:06 p.m.
Seems like an edit got messed up here.	david	May 23, 2022, 7:01 p.m.
E501 line too long (94 > 79 characters)	reviewbot	May 23, 2022, 7:08 p.m.
SafeString instead of SafeText?	maubin	June 8, 2022, 3:58 p.m.
SafeString instead of SafeText?	maubin	June 8, 2022, 3:58 p.m.
Just a thought--do we want to make this use an EntryPointRegistry instead?	david	June 1, 2022, 3:45 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/codesafety/checkers/base.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (94 > 79 characters)
```

reviewboard/codesafety/checkers/registry.py (Diff revision 1)

The issue has been resolved. Show all issues

F401 'reviewboard.codesafety.checkers.trojan_source.TrojanSourceCodeSafetyChecker' imported but unused

reviewboard/codesafety/tests/test_base_code_safety_checker.py (Diff revision 1)
The issue has been resolved. Show all issues
```
F811 redefinition of unused 'test_render_file_alert_html' from line 38
```

Change Summary:


Removed a duplicate unit test.
Removed an import that should have been in the next change in this series.

Commits:

	Summary	ID
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	8e538985f072f2dfea5d740cf31bfa78e4042d8c
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	fe0dbeb83b9ef724e9c1cd3b05d8f5262dbe8343

Diff:

Revision 2 (+750)

Show changes

	reviewboard/codesafety/__init__.py
	reviewboard/codesafety/checkers/__init__.py
	reviewboard/codesafety/checkers/base.py
	reviewboard/codesafety/checkers/registry.py
	reviewboard/codesafety/tests/__init__.py
	reviewboard/codesafety/tests/test_base_code_safety_checker.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/codesafety/checkers/base.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (94 > 79 characters)
```

reviewboard/codesafety/checkers/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Seems like "unicode" probably shouldn't be capitalized here?
```
reviewboard/codesafety/checkers/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
And here.
```
reviewboard/codesafety/checkers/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Seems like an edit got messed up here.
```

Change Summary:

Updated for release-5.0.x:

Changed version numbers in docstrings
Changed docs to use Keys and str.
Removed six usage
Removed __future__ imports

Commits:

	Summary	ID
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	fe0dbeb83b9ef724e9c1cd3b05d8f5262dbe8343
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	2e025b3d97869424457f9dca7897d63837635d70

Branch:

release-4.0.x

release-5.0.x

Diff:

Revision 3 (+730)

Show changes

	reviewboard/codesafety/__init__.py
	reviewboard/codesafety/checkers/__init__.py
	reviewboard/codesafety/checkers/base.py
	reviewboard/codesafety/checkers/registry.py
	reviewboard/codesafety/tests/__init__.py
	reviewboard/codesafety/tests/test_base_code_safety_checker.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/codesafety/checkers/base.py (Diff revision 3)
The issue has been resolved. Show all issues
```
E501 line too long (94 > 79 characters)
```

Change Summary:

Update another docstring to use Keys.

Commits:

	Summary	ID
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	2e025b3d97869424457f9dca7897d63837635d70
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	28120f5a718e57cecc12c70c5fef231a5e52951d

Diff:

Revision 4 (+734)

Show changes

	reviewboard/codesafety/__init__.py
	reviewboard/codesafety/checkers/__init__.py
	reviewboard/codesafety/checkers/base.py
	reviewboard/codesafety/checkers/registry.py
	reviewboard/codesafety/tests/__init__.py
	reviewboard/codesafety/tests/test_base_code_safety_checker.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Changed ugettext_lazy to gettext_lazy.

Commits:

	Summary	ID
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	28120f5a718e57cecc12c70c5fef231a5e52951d
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	d0544db2e6101d30bff7e94660d937f6d302650e

Diff:

Revision 5 (+734)

Show changes

	reviewboard/codesafety/__init__.py
	reviewboard/codesafety/checkers/__init__.py
	reviewboard/codesafety/checkers/base.py
	reviewboard/codesafety/checkers/registry.py
	reviewboard/codesafety/tests/__init__.py
	reviewboard/codesafety/tests/test_base_code_safety_checker.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/codesafety/checkers/registry.py (Diff revision 5)

The issue has been dropped. Show all issues

Just a thought--do we want to make this use an EntryPointRegistry instead?

chipx86 May 26, 2022, 11:43 a.m.

I think we should discourage entrypoints in favor of extensions where possible. Extensions give us better control all around and let us manage initialization order.

reviewboard/codesafety/checkers/base.py (Diff revision 5)
The issue has been resolved. Show all issues
```
SafeString instead of SafeText?
```
reviewboard/codesafety/checkers/base.py (Diff revision 5)
The issue has been resolved. Show all issues
```
SafeString instead of SafeText?
```

Change Summary:

Changed SafeText to SafeString in docs.

Commits:

	Summary	ID
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	d0544db2e6101d30bff7e94660d937f6d302650e
	Add base support for code safety checkers. Review Board's focus historically has been to provide tools for automatically or manually catching problems with code before it goes into a product. The automatic checks come by way of extensions like Review Bot, and these can only check code once it's been published. There are issues that are best caught before code goes up for review, and there are also issues that are best caught by default without needing to install a tool like Review Bot. Two examples would be credentials accidentally left in code and so-called "Trojan Source" attacks (where code is either accidentally or intentionally added to a file that displays one way to a user but executes another way). This begins laying the foundation for code safety checkers, which can look for suspicious content in code before it's ready for review, flagging issues that are found. This will be used in the diff validation API and the diff viewer to highlight any issues that are found. Specific code safety checkers will be implemented in future changes.	22e7af692bbf34511d35ef76a0ba3979a7fbde4b

Diff:

Revision 6 (+734)

Show changes

	reviewboard/codesafety/__init__.py
	reviewboard/codesafety/checkers/__init__.py
	reviewboard/codesafety/checkers/base.py
	reviewboard/codesafety/checkers/registry.py
	reviewboard/codesafety/tests/__init__.py
	reviewboard/codesafety/tests/test_base_code_safety_checker.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-5.0.x (0547356)