fix(webview): refuse a backslash or tab-led recovery path, which the URL parser reads as an origin

The lost-frame handler in webview-tabs.js remounts a web-tab frame at the path
the frame reports it lost. It promised "path only, never an origin" and collapsed
a leading run of slashes so `//host/x` could not jump the frame off the proxy,
but it left two spellings through that the WHATWG URL parser treats the same way:
a backslash, which is read as `/` for http(s) schemes, and an ASCII tab or
newline, which the parser deletes before it looks at anything, so `/\host/x` and
`/<tab>/host/x` both resolve to `https://host/x`. That mattered only in
direct-mode tabs, where `POST /api/webviews/:id/open` returns no embedUrl and the
recovered path is resolved with `new URL(path, src)` straight into the frame's
src; a page in such a tab could remount its own frame on a foreign origin.

Not an escalation (the page can already navigate itself anywhere, and the remount
carries no Codeman-origin access), but the comment did not hold and the existing
test only covered the form that already worked. The handler now strips tab, CR
and LF, collapses any leading run of `/` or `\` to one `/`, and refuses whatever
still opens a second separator. The new test drives the reachable direct-mode
branch with all four spellings and fails without the fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-09-14 23:38:54 +02:00
parent b0dddc9c57
commit d9364f52e1
2 changed files with 38 additions and 4 deletions
+11 -4
View File
@@ -264,10 +264,17 @@ Object.assign(CodemanApp.prototype, {
if (recent.length >= 5) return; if (recent.length >= 5) return;
recent.push(now); recent.push(now);
this._webviewRecoveries.set(id, recent); this._webviewRecoveries.set(id, recent);
// Path only, never an origin: a `//host/x` here would jump the frame off // Path only, never an origin. Three spellings would resolve to a foreign
// the proxy (resolveUpstreamUrl refuses it server-side as well). // origin (in direct mode `new URL(path, src)` is the frame's src, so the
const path = data.path.replace(/^\/+/, '/'); // frame would remount there): the protocol-relative `//host/x`; a
void this.openWebview(id, { path: path.startsWith('/') && !path.startsWith('//') ? path : '/' }); // backslash, which the WHATWG parser treats as `/` for http(s), so
// `/\host/x` too; and an ASCII tab or newline, which the parser deletes
// before it looks at anything, so `/<tab>/host/x` IS `//host/x` by the time
// it resolves. Drop the invisible ones, collapse the leading separators to
// one `/`, and refuse whatever still opens a second one. The proxied form
// is refused server-side as well (resolveUpstreamUrl).
const path = data.path.replace(/[\t\n\r]/g, '').replace(/^[/\\]+/, '/');
void this.openWebview(id, { path: path.startsWith('/') && !/^\/[/\\]/.test(path) ? path : '/' });
return; return;
} }
}; };
+27
View File
@@ -385,4 +385,31 @@ describe('lost-frame recovery', () => {
expect(opens).toBeLessThanOrEqual(5); expect(opens).toBeLessThanOrEqual(5);
expect(opens).toBeGreaterThan(0); expect(opens).toBeGreaterThan(0);
}); });
/**
* The proxied form above cannot leave the prefix whatever the path says. A
* DIRECT-mode tab is the reachable case: `POST /open` returns no embedUrl, so
* the recovered path is resolved with `new URL(path, src)` and becomes the
* frame's src. The WHATWG parser reads a backslash as `/` for http(s), and
* deletes ASCII tab and newline before parsing, so `/\host/x` and `/<tab>/host/x`
* both mean `//host/x` there: a page in such a tab could remount its own frame
* on a foreign origin. Not an escalation (the page can already navigate itself
* anywhere), but the handler promises "path only, never an origin".
*/
it('keeps a direct-mode frame on its own origin whatever separator the path opens with', async () => {
const { win, app } = boot();
app._installWebviewLostListener();
await app.openWebview('direct');
expect(frameSrc(win, 'direct')).toBe('https://localhost:9443/');
const attempts = ['/\\evil.example/x', '\\\\evil.example/x', '/\t/evil.example/x', '/\n/evil.example/x'];
for (const path of attempts) {
lost(win, frameOf(win, 'direct').contentWindow, path);
await vi.waitFor(() => expect(frameSrc(win, 'direct')).toBe('https://localhost:9443/evil.example/x'));
// Reset for the next spelling so the assertion above cannot pass on a stale
// src; through openWebview rather than another lost message, which is
// capped at five per minute per frame.
await app.openWebview('direct', { path: '/' });
expect(frameSrc(win, 'direct')).toBe('https://localhost:9443/');
}
});
}); });