Add base support for code safety checkers.

Review Request #11904 — Created Jan. 4, 2022 and submitted

Information

Review Board
release-5.0.x

Reviewers

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.

Unit tests pass.

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

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
Description From Last Updated

E501 line too long (94 > 79 characters)

reviewbotreviewbot

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

reviewbotreviewbot

F811 redefinition of unused 'test_render_file_alert_html' from line 38

reviewbotreviewbot

E501 line too long (94 > 79 characters)

reviewbotreviewbot

Seems like "unicode" probably shouldn't be capitalized here?

daviddavid

And here.

daviddavid

Seems like an edit got messed up here.

daviddavid

E501 line too long (94 > 79 characters)

reviewbotreviewbot

SafeString instead of SafeText?

maubinmaubin

SafeString instead of SafeText?

maubinmaubin

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

daviddavid
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

chipx86
Review request changed

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

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
  1. 
      
  2. reviewboard/codesafety/checkers/base.py (Diff revision 2)
     
     

    Seems like "unicode" probably shouldn't be capitalized here?

  3. reviewboard/codesafety/checkers/base.py (Diff revision 2)
     
     

    And here.

  4. reviewboard/codesafety/checkers/base.py (Diff revision 2)
     
     
     
     

    Seems like an edit got messed up here.

  5. 
      
chipx86
Review request changed

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

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

chipx86
chipx86
david
  1. 
      
  2. Just a thought--do we want to make this use an EntryPointRegistry instead?

    1. 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.

  3. 
      
maubin
  1. 
      
  2. reviewboard/codesafety/checkers/base.py (Diff revision 5)
     
     

    SafeString instead of SafeText?

  3. reviewboard/codesafety/checkers/base.py (Diff revision 5)
     
     

    SafeString instead of SafeText?

  4. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (0547356)
Loading...