It's not ideal, but we could have a content-sniffing presubmit that checks for a top-level key named "interface_provider_specs" in json files, and require that a per-file OWNERS rule be added for that. Does that seem reasonable?

(Naming convention overall seems a bit hard to enforce--even today with struct traits, it's a bit of a problem)

I don't know how to make GN work in PRESUBMIT, since there's no guaranteed out/ directory to run this from. But one can gather all the service_manifest() targets like so:

gn refs out/debug "//services/service_manager/public/tools/manifest/manifest_collator.py"

You can't `gn refs` a template() so instead you have to put a ref on the script the service_manifest() action uses here: https://cs.chromium.org/chromium/src/services/service_manager/public/service_manifest.gni

GN also doesn't let you do prefix wildcards, so you can't do `gn refs out/debug "*__is_service_manifest"`, which is an empty group target that the template also defines.

Both of those would of course also ignore conditionally-defined service_manifest() targets that are not defined for the current out/-dir configuration.

Chatting with dcheng, it's not clear how we'd invoke gn from PRESUBMIT, and #6 means we could get inconsistent cross-platform results. So it seems like checking for an interface_provider_specs top-level key may be the best thing to do. (Though parsing every JSON file in Python/as PRESUBMIT seems yucky…).

I was able to verify that all but one (https://cs.chromium.org/chromium/src/services/service_manager/runner/host/host_test_service_manifest.json) manifest file contains "interface_provider_specs", so that does seem like a strong signal. I did that with these commands:

for f in $(gn refs out/debug "//services/service_manager/public/tools/manifest/manifest_collator.py"); do gn desc out/debug $f inputs; done > inputs.txt
for f in $(grep -v //out/ inputs.txt | sed s@^//@@); do grep interface_provider_specs $f || echo "FAIL $f"; done

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1

commit d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1
Author: Daniel Cheng <dcheng@chromium.org>
Date: Thu Aug 24 07:39:55 2017

Add per-file security owner rules for mojo manifests.

Since these files control what interfaces are visible between different
processes, make sure these manifests go through a security review.
OWNERS rules were updating by grepping for 'interface_provider_specs' in
all json files and running the result through a Python script to add
per-file rules to the corresponding OWNERS directory if needed.

Bug:  695922 
Cq-Include-Trybots: master.tryserver.chromium.mac:mac_optional_gpu_tests_rel
Change-Id: Ibb6cc655c2aceaeda572f1a9f77862516fd0ea90
Reviewed-on: https://chromium-review.googlesource.com/630238
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496973}
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/android_webview/browser/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/ash/autoclick/mus/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/ash/mus/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/ash/mus/standalone/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/ash/touch_hud/mus/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/chrome/app/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/chrome/browser/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/chrome/browser/chromeos/prefs/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/chrome/profiling/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/chrome/test/base/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/chromecast/browser/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/components/filesystem/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/components/font_service/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/components/leveldb/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/components/nacl/broker/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/components/nacl/loader/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/components/printing/service/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/content/network/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/content/public/app/mojo/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/content/shell/browser/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/content/test/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/extensions/test/data/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/headless/lib/browser/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/headless/lib/renderer/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/ios/web/public/app/mojo/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/ios/web/shell/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/mash/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/mash/catalog_viewer/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/mash/example/views_examples/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/mash/example/window_type_launcher/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/mash/quick_launch/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/mash/session/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/mash/simple_wm/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/mash/task_viewer/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/media/mojo/services/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/catalog/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/data_decoder/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/device/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/file/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/identity/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/preferences/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/resource_coordinator/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/service_manager/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/service_manager/background/tests/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/service_manager/runner/host/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/service_manager/tests/connect/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/service_manager/tests/lifecycle/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/service_manager/tests/service_manager/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/service_manager/tests/shutdown/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/shape_detection/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/test/echo/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/test/user_id/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/ui/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/ui/clipboard/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/ui/demo/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/ui/ime/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/ui/ime/test_ime_driver/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/ui/test_wm/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/ui/ws/OWNERS
[modify] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/video_capture/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/services/video_capture/test/OWNERS
[add] https://crrev.com/d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1/ui/views/mus/OWNERS

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/13ca61a886be9dc1e3b862e06f2ae52040aa073e

commit 13ca61a886be9dc1e3b862e06f2ae52040aa073e
Author: Daniel Cheng <dcheng@chromium.org>
Date: Fri Aug 25 15:11:25 2017

Enforce that mojo manifests are covered by security reviewers.

Since mojo manifests don't have a consistent naming convention, this
simply looks for JSON files with the "interface_provider_specs" key:
there are some manifests that don't specify this, but the important
part for security review is auditing what's exposed between processes.

Bug:  695922 
Change-Id: Id30dae51ecc0cbfa35650ead14ef2dfd081c23d7
Reviewed-on: https://chromium-review.googlesource.com/621707
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497410}
[modify] https://crrev.com/13ca61a886be9dc1e3b862e06f2ae52040aa073e/PRESUBMIT.py
[modify] https://crrev.com/13ca61a886be9dc1e3b862e06f2ae52040aa073e/PRESUBMIT_test.py
[modify] https://crrev.com/13ca61a886be9dc1e3b862e06f2ae52040aa073e/PRESUBMIT_test_mocks.py

Unfortunately, I totally shot myself in the foot when making sure that no new manifests were introduced that could cause violations of the newly added presubmit rule by typing > instead of |.

I did a git log -S interface_provider_specs d395d7eb8383c6ee9fe46a176e52ae4ad51c31f1..13ca61a886be9dc1e3b862e06f2ae52040aa073e instead to see if anything landed which added (or removed) interface_provider_specs other than my CL in question, and it seems like we're clear.

I'm going to mark this fixed, though I think we'll want to make sure this presubmit check covers sandbox levels when that moves into the manifest.

This breaks if policy_templates.json is changed because it's not actually a well formed JSON file.  I've uploaded a change https://chromium-review.googlesource.com/636186 to exclude this file.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46e29bc430c269d11774ebdcb722cc7cd76bec8b

commit 46e29bc430c269d11774ebdcb722cc7cd76bec8b
Author: Sean Kau <skau@chromium.org>
Date: Mon Aug 28 16:31:16 2017

Exclude invalid JSON files from _CheckIpcOwners.

Some JSON files are not parseable and are also not a Mojo
manifest so they should be skipped.  Otherwise,
PRESUBMIT.py crashes when trying to parse them.

Bug:  695922 
Change-Id: I6549116508e4da43e6781146a266064d6b72b6ac
Reviewed-on: https://chromium-review.googlesource.com/636186
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Sean Kau <skau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497774}
[modify] https://crrev.com/46e29bc430c269d11774ebdcb722cc7cd76bec8b/PRESUBMIT.py