Skip to content

Commit

Permalink
Ensure newly-added security-sensitive files are security reviewed.
Browse files Browse the repository at this point in the history
The current security owners check ensures that the per-file rules are
present in the various OWNERS files; however, for security reasons [1],
the actual owners checks evaluates owners requirements against the
*currently* checked-in version of OWNERS files and not the updated
version of OWNERS files being reviewed. The net result is that it was
a fairly common occurrence to add a new .mojom file, add new per-file
restrictions to OWNERS, but not actually send it to a security reviewer.

It turns out that there is already logic to check for security reviewers
for changes that include calls to security-sensitive functions. This
logic has been factored out so it can be reused by the per-file security
owners checker. In addition:

- The required per-file rules checker has been refactored so that IPC
  and Fuchsia can re-use the same core logic.
- ipc-security-reviews@chromium.org is no longer CCed on changes that
  are security-sensitive for Fuchsia but not IPC.
- Test logic consolidation and cleanup.

[1] If the owners requirements were evaluated against the updated
version of OWNERS files being reviewed, it would be possible for someone
to add a new OWNER and for that newly-added OWNER to approve the CL
adding themselves.

Bug: 801315
Change-Id: If824e4698368a089474ba992601edbcf4c7d009c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3641150
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002697}
  • Loading branch information
zetafunction authored and Chromium LUCI CQ committed May 12, 2022
1 parent f009ece commit a37c03d
Show file tree
Hide file tree
Showing 2 changed files with 394 additions and 220 deletions.

0 comments on commit a37c03d

Please sign in to comment.