Skip to content

Commit 2f79718

Browse files
WebMCP: Update registerTool() to match the spec
This CL brings Blink up to date with webmachinelearning/webmcp#179, by: - Supporting potentially-trustworthy *origins* (not URLs) in `exposedTo`, closing https://crbug.com/509983801 - Changing the ordering of `exposedTo` process with respect to `options.signal` processing - Using `NotAllowedError` instead of `SecurityError` when the `tools` permission is not present. (Subsequent spec PRs and impl CLs will do this for `getTools()` and `executeTool()`). R=mfoltz Bug: 489045948,509983801 Change-Id: I42d915412a7ef1881c4fa35fe63870979d739921 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7863576 Reviewed-by: Mark Foltz <mfoltz@chromium.org> Commit-Queue: Dominic Farolino <dom@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Mark Pearson <mpearson@chromium.org> Cr-Commit-Position: refs/heads/main@{#1635540}
1 parent d7fa41f commit 2f79718

2 files changed

Lines changed: 45 additions & 20 deletions

File tree

webmcp/imperative/exposedTo-invalid-origins.https.html

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,52 @@
88
</head>
99
<body>
1010
<script>
11-
promise_test(async t => {
12-
const invalid_origins = [
13-
'/',
14-
'*',
15-
'https://example:bogus',
16-
'https://\ud800.com',
17-
'http://example.com',
18-
'ftp://example.com',
19-
'chrome-extension://foobar',
11+
test(t => {
12+
const test_cases = [
13+
{success: false, origin: '/'},
14+
{success: false, origin: '*'},
15+
{success: false, origin: 'https://example:bogus'},
16+
{success: false, origin: 'https://\ud800.com'},
17+
{success: false, origin: 'http://example.com'},
18+
{success: false, origin: 'ftp://example.com'},
19+
{success: false, origin: 'chrome-extension://foobar'},
20+
{success: false, origin: 'about:blank'},
21+
{success: false, origin: 'about:srcdoc'},
22+
23+
{success: true, origin: 'https://example.com'},
24+
{success: true, origin: 'http://localhost:3000'},
2025
];
2126

22-
for (const origin of invalid_origins) {
23-
assert_throws_dom('SecurityError', () => {
24-
navigator.modelContext.registerTool({
27+
for (const test of test_cases) {
28+
const register = () => {
29+
navigator.modelContext.registerTool({
2530
name: 'test_tool_' + Math.random(),
2631
description: 'Test tool',
2732
execute: async () => 'hello'
28-
}, { exposedTo: [origin] });
29-
}, `Should throw SecurityError for origin: ${origin}`);
33+
}, { exposedTo: [test.origin] });
34+
};
35+
36+
if (!test.success) {
37+
assert_throws_dom('SecurityError', register, `Should throw SecurityError for origin: ${origin}`);
38+
} else {
39+
try {
40+
register();
41+
} catch (e) {
42+
throw new Error(`Should not have thrown for ${test.origin}`);
43+
}
44+
}
3045
}
31-
}, 'registerTool() throws SecurityError for invalid or non-HTTPS origins in exposedTo');
46+
}, 'registerTool() throws SecurityError for invalid or ' +
47+
'non-potentially-trustworthy origins in exposedTo');
48+
49+
test(t => {
50+
// Does not throw, because the given abort signal is processed before `exposedTo` is.
51+
navigator.modelContext.registerTool({
52+
name: "name",
53+
description: "description",
54+
execute: () => {}
55+
}, {signal: AbortSignal.abort('already-aborted'), exposedTo: ['about:blank#invalidOrigin']})
56+
}, "registerTool() does not throw for invalid exposedTo, if aborted signal is passed in");
3257
</script>
3358
</body>
3459
</html>

webmcp/imperative/permissions-policy.https.html

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
}, '*');
5555
const failure_msg = await failure_promise;
5656

57-
assert_true(failure_msg.includes('SecurityError'), `Expected SecurityError in navigated iframe, got: ${failure_msg}`);
57+
assert_true(failure_msg.includes('NotAllowedError'), `Expected NotAllowedError in navigated iframe, got: ${failure_msg}`);
5858
}, 'registerTool() throws in iframe, after navigation to an origin not covered by permissions policy');
5959

6060
promise_test(async t => {
@@ -80,8 +80,8 @@
8080
iframe.contentWindow.postMessage('getTools', '*');
8181
const failure_msg = await failure_promise;
8282

83-
assert_true(failure_msg.includes('SecurityError'), `Expected SecurityError in iframe, got: ${failure_msg}`);
84-
}, 'getTools() throws SecurityError in iframe when permissions policy is disabled');
83+
assert_true(failure_msg.includes('NotAllowedError'), `Expected NotAllowedError in iframe, got: ${failure_msg}`);
84+
}, 'getTools() throws NotAllowedError in iframe when permissions policy is disabled');
8585

8686
promise_test(async t => {
8787
const iframe = document.createElement('iframe');
@@ -108,8 +108,8 @@
108108
iframe.contentWindow.postMessage({ action: 'execute_fake_tool', name: 'dummy_tool' }, '*');
109109
const response = await failure_promise;
110110

111-
assert_equals(response.error_name, 'SecurityError', 'Should reject with SecurityError');
112-
}, 'executeTool() throws SecurityError in iframe when permissions policy is disabled');
111+
assert_equals(response.error_name, 'NotAllowedError', 'Should reject with NotAllowedError');
112+
}, 'executeTool() throws NotAllowedError in iframe when permissions policy is disabled');
113113
</script>
114114
</body>
115115
</html>

0 commit comments

Comments
 (0)