diff --git a/docs/reference/mobile-hybrid-webview-architecture.md b/docs/reference/mobile-hybrid-webview-architecture.md index 964c87f4fb7..544565b064f 100644 --- a/docs/reference/mobile-hybrid-webview-architecture.md +++ b/docs/reference/mobile-hybrid-webview-architecture.md @@ -230,8 +230,18 @@ depends on its direction and on whether it adds a field or an operation: `unsupported_capability`, so a newer page against an older shell degrades at the call site instead of hanging. - **Additive frame type, either direction** is safe at the same version: both - receivers drop a frame they cannot parse. Frame *fields* do not share that - property — the envelope schemas are strict in both directions. + receivers drop a frame they cannot parse. +- **Additive envelope field, shell to page** is safe for the same reason as a + payload field: `parseMobileWebBridgeShellMessage` and + `parseMobileWebBridgeInitialMessage` parse through the tolerant view, so an + undeclared key is stripped rather than dropping the frame. That matters most + for `init`, where dropping the frame costs the page every grant at once. + Stripping keeps the leak fence intact — an undeclared `resumeRoute.hostPath` + or a raw error `message` still never reaches the page. The page->shell + envelope stays strict. +- **Additive route kind or other closed variant** is not covered by any of the + above and must negotiate. An unknown `resumeRoute.kind` still fails `init`, + because the page cannot invent a meaning for a variant it does not have. Desktop must retain support for the existing bridge floor until a replacement has shipped in at least two stable mobile releases and the supported shell diff --git a/src/shared/mobile-web/bridge-contract-adversarial.test.ts b/src/shared/mobile-web/bridge-contract-adversarial.test.ts index 8d1bc613124..1c1a582fbb3 100644 --- a/src/shared/mobile-web/bridge-contract-adversarial.test.ts +++ b/src/shared/mobile-web/bridge-contract-adversarial.test.ts @@ -79,6 +79,19 @@ describe('mobile web bridge adversarial corpus', () => { ok: false }) }) + + // An undeclared key on a shell frame is stripped, not fatal: the shell can be a newer release + // than the page, and dropping the frame costs the page the whole message. The key still never + // reaches the page, so the leak fence is unchanged. + it('strips an undeclared shell field instead of dropping the frame', () => { + const parsed = parseMobileWebBridgeShellMessage( + JSON.stringify(shellEvent({ hostPath: '/private/repo' })), + CONTEXT + ) + + expect(parsed).toMatchObject({ ok: true }) + expect(parsed.ok && parsed.value).not.toHaveProperty('hostPath') + }) }) function pageRequest(overrides: Record = {}): Record { @@ -184,6 +197,5 @@ function shellMutationCorpus(): { label: string; value: Record }) } } - cases.push({ label: 'shell unknown field', value: shellEvent({ hostPath: '/private/repo' }) }) return cases } diff --git a/src/shared/mobile-web/bridge-contract.test.ts b/src/shared/mobile-web/bridge-contract.test.ts index fddd0c886b1..df5bb8d17ed 100644 --- a/src/shared/mobile-web/bridge-contract.test.ts +++ b/src/shared/mobile-web/bridge-contract.test.ts @@ -371,26 +371,30 @@ describe('mobile web bridge shell contract', () => { ).toEqual({ ok: false, error: 'too_large' }) }) + // Stripped rather than rejected: `init` carries every grant, so dropping the frame over one + // undeclared key from a newer shell costs the page every capability. The key is still never + // readable by the page, which is the whole point of the fence. it.each(['hostId', 'hostIdentity', 'publicKeyB64', 'deviceToken', 'endpoint', 'credential'])( - 'rejects privileged %s state from the initial page message', + 'strips privileged %s state from the initial page message', (field) => { - expect( - parseMobileWebBridgeInitialMessage( - JSON.stringify({ - version: MOBILE_WEB_BRIDGE_PROTOCOL_VERSION, - type: 'init', - shellSessionId: SHELL_SESSION_ID, - buildId: BUILD_ID, - connection: 'connected', - grants: [operationGrant()], - [field]: 'credential-secret' - }) - ) - ).toEqual({ ok: false, error: 'invalid_message' }) + const parsed = parseMobileWebBridgeInitialMessage( + JSON.stringify({ + version: MOBILE_WEB_BRIDGE_PROTOCOL_VERSION, + type: 'init', + shellSessionId: SHELL_SESSION_ID, + buildId: BUILD_ID, + connection: 'connected', + grants: [operationGrant()], + [field]: 'credential-secret' + }) + ) + + expect(parsed).toMatchObject({ ok: true }) + expect(parsed.ok && parsed.value).not.toHaveProperty(field) } ) - it('rejects unbounded or host-shaped resume routes', () => { + it('rejects unbounded resume routes and strips host-shaped ones', () => { const base = { version: MOBILE_WEB_BRIDGE_PROTOCOL_VERSION, type: 'init', @@ -410,7 +414,21 @@ describe('mobile web bridge shell contract', () => { }).success ).toBe(false) expect( - MobileWebBridgeShellMessageSchema.safeParse({ + parseMobileWebBridgeShellMessage( + JSON.stringify({ + ...base, + resumeRoute: { + kind: 'session', + workspaceId: 'opaque-workspace', + workspaceName: 'x'.repeat(241) + } + }), + CONTEXT + ) + ).toEqual({ ok: false, error: 'invalid_message' }) + + const parsed = parseMobileWebBridgeShellMessage( + JSON.stringify({ ...base, resumeRoute: { kind: 'session', @@ -418,8 +436,16 @@ describe('mobile web bridge shell contract', () => { workspaceName: 'Feature', hostPath: '/private/worktree' } - }).success - ).toBe(false) + }), + CONTEXT + ) + expect(parsed).toMatchObject({ ok: true }) + expect(parsed.ok && parsed.value).toMatchObject({ + resumeRoute: { kind: 'session', workspaceName: 'Feature' } + }) + expect(parsed.ok && (parsed.value as { resumeRoute: object }).resumeRoute).not.toHaveProperty( + 'hostPath' + ) }) it('bounds the optional local host display name', () => { @@ -599,12 +625,21 @@ describe('mobile web bridge shell contract', () => { error: { code: 'host_error', retryable: true } } expect(MobileWebBridgeShellMessageSchema.safeParse(response).success).toBe(true) - expect( - MobileWebBridgeShellMessageSchema.safeParse({ + + // The message is stripped rather than fatal, so the page keeps the error code it can act on + // and still cannot read the host path inside the message. + const parsed = parseMobileWebBridgeShellMessage( + JSON.stringify({ ...response, error: { ...response.error, message: '/private/path: permission denied' } - }).success - ).toBe(false) + }), + CONTEXT + ) + expect(parsed).toMatchObject({ ok: true }) + expect(parsed.ok && parsed.value).toMatchObject({ + error: { code: 'host_error', retryable: true } + }) + expect(parsed.ok && (parsed.value as { error: object }).error).not.toHaveProperty('message') }) it('parses matching shell events and rejects stale subscription events', () => { diff --git a/src/shared/mobile-web/bridge-contract.ts b/src/shared/mobile-web/bridge-contract.ts index f1d6eb7775b..d1e05dd38f3 100644 --- a/src/shared/mobile-web/bridge-contract.ts +++ b/src/shared/mobile-web/bridge-contract.ts @@ -17,7 +17,8 @@ import { type MobileWebBridgeMessageContext, type MobileWebBridgeMessageParseResult } from './bridge-message-parser' -import { MobileWebWorkspaceIdSchema } from './workspace-operation-contract' +import { MobileWebNavigationRouteSchema, MobileWebResumeRouteSchema } from './bridge-route-contract' +import { tolerantMobileWebShellPayload } from './shell-payload-tolerance' export { isMobileWebBridgeOperation, @@ -36,6 +37,8 @@ export { MOBILE_WEB_BRIDGE_MAX_SUBSCRIPTIONS } from './bridge-limits' export { MOBILE_WEB_BRIDGE_PROTOCOL_VERSION } from './bridge-protocol-version' +export { MobileWebNavigationRouteSchema, MobileWebResumeRouteSchema } from './bridge-route-contract' +export type { MobileWebNavigationRoute, MobileWebResumeRoute } from './bridge-route-contract' const MobileWebBridgeOperationShape = { capability: MobileWebBridgeCapabilitySchema, @@ -92,38 +95,6 @@ const PageHardwareBackResultSchema = PageEnvelopeSchema.extend({ handled: z.boolean() }).strict() -const MobileWebWorkspaceListRouteSchema = z - .object({ - kind: z.literal('workspaceList'), - notice: z.literal('worktree-missing').optional() - }) - .strict() -const MobileWebSessionRouteSchema = z - .object({ - kind: z.literal('session'), - workspaceId: MobileWebWorkspaceIdSchema, - workspaceName: z.string().max(240) - }) - .strict() - -export const MobileWebResumeRouteSchema = z.discriminatedUnion('kind', [ - MobileWebWorkspaceListRouteSchema, - MobileWebSessionRouteSchema -]) - -export const MobileWebNavigationRouteSchema = z.discriminatedUnion('kind', [ - MobileWebWorkspaceListRouteSchema, - MobileWebSessionRouteSchema, - z - .object({ - kind: z.literal('tasks'), - taskSource: z.enum(['github', 'gitlab', 'linear']).optional() - }) - .strict(), - z.object({ kind: z.literal('accounts') }).strict(), - z.object({ kind: z.literal('newWorkspace') }).strict() -]) - const PageRouteStateSchema = PageEnvelopeSchema.extend({ type: z.literal('routeState'), route: MobileWebResumeRouteSchema @@ -271,8 +242,6 @@ export const MobileWebBridgeShellMessageSchema = z.union([ export type MobileWebBridgeErrorCode = z.infer export type MobileWebBridgePageMessage = z.infer export type MobileWebBridgeShellMessage = z.infer -export type MobileWebNavigationRoute = z.infer -export type MobileWebResumeRoute = z.infer export type { MobileWebBridgeMessageContext } from './bridge-message-parser' export type MobileWebBridgeParseResult = MobileWebBridgeMessageParseResult @@ -284,17 +253,28 @@ export function parseMobileWebBridgePageMessage( return parseMobileWebBridgeMessage(raw, expected, MobileWebBridgePageMessageSchema) } +/** + * Shell->page frames are authored by an APK that can be newer than the page reading them, and a + * frame the page cannot parse is dropped whole — for `init` that is every capability lost, not one + * field. Parsing through the tolerant view strips a key the page does not declare instead of + * failing the frame, which also keeps the PII fence: an undeclared `hostPath` or raw error + * `message` never reaches the page either way. Page->shell stays strict; the shell is the + * authority there. + */ +const TolerantShellMessageSchema = tolerantMobileWebShellPayload(MobileWebBridgeShellMessageSchema) +const TolerantShellInitSchema = tolerantMobileWebShellPayload(ShellInitSchema) + export function parseMobileWebBridgeShellMessage( raw: string, expected: MobileWebBridgeMessageContext ): MobileWebBridgeParseResult { - return parseMobileWebBridgeMessage(raw, expected, MobileWebBridgeShellMessageSchema) + return parseMobileWebBridgeMessage(raw, expected, TolerantShellMessageSchema) } export function parseMobileWebBridgeInitialMessage( raw: string ): MobileWebBridgeParseResult> { - return parseMobileWebBridgeMessageDocument(raw, ShellInitSchema) + return parseMobileWebBridgeMessageDocument(raw, TolerantShellInitSchema) } function validateRequestOperation( diff --git a/src/shared/mobile-web/bridge-route-contract.ts b/src/shared/mobile-web/bridge-route-contract.ts new file mode 100644 index 00000000000..d217de7d6a4 --- /dev/null +++ b/src/shared/mobile-web/bridge-route-contract.ts @@ -0,0 +1,41 @@ +import { z } from 'zod' +import { MobileWebWorkspaceIdSchema } from './workspace-operation-contract' + +/** Routes the bridge carries in both directions: the shell restores one on `init`, drives one on a + * `navigation` frame, and the page reports the one it settled on. Opaque workspace handles only — a + * host path must never appear here. */ +const MobileWebWorkspaceListRouteSchema = z + .object({ + kind: z.literal('workspaceList'), + notice: z.literal('worktree-missing').optional() + }) + .strict() + +const MobileWebSessionRouteSchema = z + .object({ + kind: z.literal('session'), + workspaceId: MobileWebWorkspaceIdSchema, + workspaceName: z.string().max(240) + }) + .strict() + +export const MobileWebResumeRouteSchema = z.discriminatedUnion('kind', [ + MobileWebWorkspaceListRouteSchema, + MobileWebSessionRouteSchema +]) + +export const MobileWebNavigationRouteSchema = z.discriminatedUnion('kind', [ + MobileWebWorkspaceListRouteSchema, + MobileWebSessionRouteSchema, + z + .object({ + kind: z.literal('tasks'), + taskSource: z.enum(['github', 'gitlab', 'linear']).optional() + }) + .strict(), + z.object({ kind: z.literal('accounts') }).strict(), + z.object({ kind: z.literal('newWorkspace') }).strict() +]) + +export type MobileWebNavigationRoute = z.infer +export type MobileWebResumeRoute = z.infer diff --git a/src/shared/mobile-web/shell-payload-tolerance-census.test.ts b/src/shared/mobile-web/shell-payload-tolerance-census.test.ts index 21d72ac0962..a68236c69ad 100644 --- a/src/shared/mobile-web/shell-payload-tolerance-census.test.ts +++ b/src/shared/mobile-web/shell-payload-tolerance-census.test.ts @@ -2,6 +2,10 @@ import { readFileSync, readdirSync } from 'node:fs' import { join, resolve } from 'node:path' import { beforeAll, describe, expect, it } from 'vitest' import type { z } from 'zod' +import { + MobileWebBridgePageMessageSchema, + MobileWebBridgeShellMessageSchema +} from './bridge-contract' import { tolerantMobileWebShellPayload } from './shell-payload-tolerance' const PAGE_DIR = resolve(__dirname, '..', '..', 'mobile-web', 'src') @@ -134,11 +138,21 @@ describe('mobile web shell payload tolerance census', () => { expect(offenders).toEqual({}) }) + // The envelope is the same hazard one level up: an additive field on `init` from a newer APK + // used to fail the union, and a dropped `init` costs the page every grant at once. + it('leaves no strict object under the shell->page envelope', () => { + expect(strictPaths(MobileWebBridgeShellMessageSchema).length).toBeGreaterThan(8) + expect(strictPaths(tolerantMobileWebShellPayload(MobileWebBridgeShellMessageSchema))).toEqual( + [] + ) + }) + it('keeps the page->shell request schemas strict', () => { const payloads = [...schemas.keys()].filter((name) => name.endsWith('PayloadSchema')) const open = payloads.filter((name) => strictPaths(schemas.get(name)!).length === 0) expect(payloads.length).toBeGreaterThanOrEqual(50) expect(open.length).toBeLessThan(payloads.length / 2) + expect(strictPaths(MobileWebBridgePageMessageSchema).length).toBeGreaterThan(4) }) })