mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 16:02:50 +00:00
fix(mobile-web): strip unknown keys on the shell to page envelope too
Follow-up to 4c7fd1aec6. The result and event payloads were made tolerant, but the envelope around them was still strict, so an additive field on a shell frame from a newer APK made an older page drop the whole frame. For init that is every grant lost at once, which is a worse brick than the payload case.
parseMobileWebBridgeShellMessage and parseMobileWebBridgeInitialMessage now parse through tolerantMobileWebShellPayload. parseMobileWebBridgePageMessage stays strict - the shell is the authority in that direction, and the census ratchet now asserts the page envelope keeps its strict nodes.
Lead policy decision: stripping preserves the PII fence, because an undeclared resumeRoute.hostPath or a raw error message is removed before the page can read it. Three test groups flip from 'rejected' to 'parsed with the field absent': the six privileged init fields, the host-shaped resume route, and the raw error message. Bounds checks are unaffected - an over-long workspaceName still fails the frame - and the adversarial shell mutation corpus keeps every other case rejecting.
Known residual, now written into the Compatibility Policy: an unknown resumeRoute.kind still fails init. A closed variant is not a field; the page cannot invent a meaning for it, so a new route kind must negotiate.
bridge-contract.ts crossed the 300-line oxlint ceiling, so the bridge route schemas move to bridge-route-contract.ts and are re-exported. No max-lines disable.
Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<string, unknown> = {}): Record<string, unknown> {
|
||||
@@ -184,6 +197,5 @@ function shellMutationCorpus(): { label: string; value: Record<string, unknown>
|
||||
})
|
||||
}
|
||||
}
|
||||
cases.push({ label: 'shell unknown field', value: shellEvent({ hostPath: '/private/repo' }) })
|
||||
return cases
|
||||
}
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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<typeof MobileWebBridgeErrorCodeSchema>
|
||||
export type MobileWebBridgePageMessage = z.infer<typeof MobileWebBridgePageMessageSchema>
|
||||
export type MobileWebBridgeShellMessage = z.infer<typeof MobileWebBridgeShellMessageSchema>
|
||||
export type MobileWebNavigationRoute = z.infer<typeof MobileWebNavigationRouteSchema>
|
||||
export type MobileWebResumeRoute = z.infer<typeof MobileWebResumeRouteSchema>
|
||||
|
||||
export type { MobileWebBridgeMessageContext } from './bridge-message-parser'
|
||||
export type MobileWebBridgeParseResult<T> = MobileWebBridgeMessageParseResult<T>
|
||||
@@ -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<MobileWebBridgeShellMessage> {
|
||||
return parseMobileWebBridgeMessage(raw, expected, MobileWebBridgeShellMessageSchema)
|
||||
return parseMobileWebBridgeMessage(raw, expected, TolerantShellMessageSchema)
|
||||
}
|
||||
|
||||
export function parseMobileWebBridgeInitialMessage(
|
||||
raw: string
|
||||
): MobileWebBridgeParseResult<z.infer<typeof ShellInitSchema>> {
|
||||
return parseMobileWebBridgeMessageDocument(raw, ShellInitSchema)
|
||||
return parseMobileWebBridgeMessageDocument(raw, TolerantShellInitSchema)
|
||||
}
|
||||
|
||||
function validateRequestOperation(
|
||||
|
||||
@@ -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<typeof MobileWebNavigationRouteSchema>
|
||||
export type MobileWebResumeRoute = z.infer<typeof MobileWebResumeRouteSchema>
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user