0

[Extensions] Ban string extension_id in extension mojom files.

Before this change we have the extensions::mojom::ExtensionId struct to
validate messages that pass the extensions::ExtensionId string across
mojom. However, we have not way of enforcing new method or reverts
removing that the mojom struct.

After this chang we have some prevention at review time by causing
presubmit warnings when they occur.

Bug: 401499254
Change-Id: I18e5f1b6331346134a0dd0795d80534dbec2ede0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6422689
Feels: Justin Lulejian <jlulejian@chromium.org>
Auto-Submit: Justin Lulejian <jlulejian@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Justin Lulejian <jlulejian@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1441214}
This commit is contained in:
Justin Lulejian
2025-04-01 15:03:28 -07:00
committed by Chromium LUCI CQ
parent 1d3e5ec82a
commit 09fd0687be
2 changed files with 53 additions and 6 deletions

@ -2288,17 +2288,28 @@ _DEPRECATED_SYNC_CONSENT_JAVA_FUNCTIONS : Sequence[BanRule] = (
),
)
_BANNED_MOJOM_PATTERNS : Sequence[BanRule] = (
_BANNED_MOJOM_PATTERNS: Sequence[BanRule] = (
BanRule(
'handle<shared_buffer>',
(
'Please use one of the more specific shared memory types instead:',
' mojo_base.mojom.ReadOnlySharedMemoryRegion',
' mojo_base.mojom.WritableSharedMemoryRegion',
' mojo_base.mojom.UnsafeSharedMemoryRegion',
'Please use one of the more specific shared memory types instead:',
' mojo_base.mojom.ReadOnlySharedMemoryRegion',
' mojo_base.mojom.WritableSharedMemoryRegion',
' mojo_base.mojom.UnsafeSharedMemoryRegion',
),
True,
),
BanRule(
'string extension_id',
(
'Please use the extensions::mojom::ExtensionId struct when '
'passing extensions::ExtensionIds as mojom messages in order to ',
'provide message validation.',
),
True,
# Only apply this to (mojom) files in a subdirectory of extensions.
excluded_paths=(r'^((?!extensions/).)*$', ),
),
)
_IPC_ENUM_TRAITS_DEPRECATED = (

@ -3148,7 +3148,7 @@ class BannedTypeCheckTest(unittest.TestCase):
self.assertTrue(
'content/renderer/ok/file3.cc' not in results[0].message)
def testBannedMojomPatterns(self):
def testBannedMojomPatterns_SharedBuffer(self):
input_api = MockInputApi()
input_api.files = [
MockFile(
@ -3169,6 +3169,42 @@ class BannedTypeCheckTest(unittest.TestCase):
self.assertTrue('bad.mojom' in results[0].message)
self.assertTrue('good.mojom' not in results[0].message)
def testBannedMojomPatterns_ExtensionId(self):
input_api = MockInputApi()
input_api.files = [
# Pattern tests.
MockFile('extensions/bad.mojom', ['string extension_id']),
MockFile('extensions/bad_struct.mojom',
['struct Bad {', ' string extension_id;', '};']),
MockFile('extensions/good.mojom', ['ExtensionId extension_id']),
MockFile('extensions/good_struct.mojom',
['struct Bad {', ' ExtensionId extension_id;', '};']),
# Path exclusion tests.
MockFile('some/included/extensions/path/bad_extension_id.mojom',
['string extension_id']),
MockFile('some/excluded/path/bad_extension_id.mojom',
['string extension_id']),
]
# warnings are results[0], errors are results[1]
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
# Only warnings and no errors.
self.assertEqual(1, len(results))
# Pattern test assertions.
self.assertTrue('bad.mojom' in results[0].message)
self.assertTrue('bad_struct.mojom' in results[0].message)
self.assertTrue('good.mojom' not in results[0].message)
self.assertTrue('good_struct.mojom' not in results[0].message)
# Path exclusion assertions.
self.assertTrue('some/included/extensions/path/bad_extension_id.mojom'
in results[0].message)
self.assertTrue('some/excluded/path/bad_extension_id.mojom' not in
results[0].message)
class NoProductionCodeUsingTestOnlyFunctionsTest(unittest.TestCase):
def testTruePositives(self):