Skip to content

Commit 4db7d6d

Browse files
authored
fix(mobile): stop openLink echoing a malformed link's input (tester-army#793)
device.openLink built its INVALID_ARGUMENT message from the raw input, so a string that fails to parse as an absolute URL still echoed whatever it carried. A malformed magic link is exactly the case where a one-time token sits in that string, and the message reaches logs, the report, and CI output. Name no part of the input, the same rule assertAppId already follows in this file: nothing about a string that failed to parse says which part is safe to repeat, and a token can sit in the query, the path, or the userinfo. linkLabel stays for the step label and the report, where the link did parse. Covered by a unit test over malformed links whose token sits in the query, the path, and the userinfo.
1 parent f88233f commit 4db7d6d

4 files changed

Lines changed: 33 additions & 3 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@e2e-dev/mobile": patch
3+
---
4+
5+
`device.openLink` no longer echoes the string it refused in its `INVALID_ARGUMENT` message. A malformed link can carry a magic-link token in its query, path, or userinfo, so the message names no part of the input, matching what `device.openApp` already does.

‎docs/reference/mobile.mdx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,10 @@ needs an app: an open bound to none leaves no app session to observe, so it
464464
is `INVALID_ARGUMENT`. `file:`, `data:`, `javascript:`, `view-source:`,
465465
`blob:`, and `filesystem:` links are `POLICY_DENIED`. A device link may use
466466
any app's custom scheme, so the rule refuses these schemes rather than
467-
admitting http(s) alone as the web does. The report shows the link cut before its
467+
admitting http(s) alone as the web does. A string that does not parse as an
468+
absolute URL is `INVALID_ARGUMENT` too, and its message names no part of the
469+
input, which may carry a magic-link token in its query, path, or userinfo.
470+
The report shows the link cut before its
468471
query and fragment, where a magic link carries its token. The replay cache
469472
anchors on the location the link lands on: a step recorded after `openLink`
470473
replays when the link lands on the same screen again.

‎packages/mobile/src/links.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ function denyForbiddenScheme(protocol: string): void {
3838
* Parses a link a test asked to open. A string that is not an absolute URL
3939
* is `INVALID_ARGUMENT`; a `file:`, `data:`, `javascript:`, `view-source:`,
4040
* `blob:`, or `filesystem:` link is `POLICY_DENIED`, as it is for `app.open`
41-
* on the web.
41+
* on the web. The message never echoes the input: a malformed link can carry
42+
* a magic-link token in its query, path, or userinfo, and nothing about a
43+
* string that failed to parse says which part is safe to repeat.
4244
*/
4345
export function linkTarget(input: string): URL {
4446
let url: URL;
@@ -47,7 +49,7 @@ export function linkTarget(input: string): URL {
4749
} catch {
4850
throw new TestError(
4951
'INVALID_ARGUMENT',
50-
`openLink needs an absolute URL such as myapp://orders/42 or https://example.com/verify, got "${input}"`,
52+
'openLink needs an absolute URL such as myapp://orders/42 or https://example.com/verify',
5153
);
5254
}
5355
denyForbiddenScheme(url.protocol);

‎packages/mobile/tests/unit/links.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,26 @@ describe('links', () => {
3838
}
3939
});
4040

41+
it('refuses a malformed link without echoing it, wherever a token may sit', () => {
42+
for (const malformed of [
43+
'not-a-url?token=s3cret',
44+
'https://exa mple.com/?token=s3cret',
45+
'https://user:s3cret@exa mple.com/verify',
46+
'https://exa mple.com/token=s3cret',
47+
'://x?token=s3cret',
48+
]) {
49+
expect(() => linkTarget(malformed)).toThrowError(
50+
expect.objectContaining({
51+
code: 'INVALID_ARGUMENT',
52+
message: expect.stringContaining('openLink needs an absolute URL'),
53+
}),
54+
);
55+
expect(() => linkTarget(malformed)).toThrowError(
56+
expect.objectContaining({ message: expect.not.stringContaining('s3cret') }),
57+
);
58+
}
59+
});
60+
4161
it('labels a link without its query or fragment, so a magic-link token never enters the report', () => {
4262
expect(linkLabel('https://app.example.com/magic?token=s3cret#frag')).toBe('https://app.example.com/magic');
4363
expect(linkLabel('myapp://orders/42')).toBe('myapp://orders/42');

0 commit comments

Comments
 (0)