mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
fix(plugins): close four trust-boundary holes in the plugin system (#11232)
* fix(plugins): close trust-boundary holes in the plugin system
Move five security decisions to their chokepoints rather than leaving them
enumerated at individual call sites.
- Kill-list revocation reaches content packs: PluginContentPackRegistry now
takes an isKilled predicate and intersects it with any caller-supplied
approval, so a killed plugin's VM recipes can no longer reach
spawn(..., { shell: true }) through either reconcile() call site.
- Bound kill-list generatedAt to a 24h future skew at the parse chokepoint.
A far-future timestamp previously made every genuine later list look
"older" and disabled revocation permanently, persisted across restarts.
- Protect the whole auto.components.settings.Plugin* translation subtree
instead of an enumerated prefix list, so language packs cannot forge the
consent provenance badge or rewrite install-error security copy.
- Resolve manifest panel icons by own-key only; "constructor"/"__proto__"
previously yielded non-component prototype members that crashed the
right sidebar to its error boundary.
- Give panel liveness frames a reserved control budget so a panel that
saturates its action budget can still answer the watchdog.
Co-authored-by: Orca <help@stably.ai>
* fix(plugins): keep the kill-list future bound off the cache read path
The schema-level generatedAt bound re-judged the on-disk cache against the
device clock at every launch, so a client whose clock ran behind the last
genuine publication discarded its whole cached kill list and started with
zero revocations. Move the bound to the two fetch chokepoints instead.
Co-authored-by: Orca <help@stably.ai>
* fix(plugins): remove the reserved-lane starvation window and the revocation TOCTOU
Review follow-ups on the trust-boundary fixes:
- The reserved liveness lane had a per-window count equal to the ping
interval, so a panel's own pong-shaped traffic could spend it and drop
the next genuine reply — reintroducing the starvation the lane exists to
prevent. The lane is now size-bounded only; rate stays bounded because
every pong is also charged to the data budget.
- Only schema-valid pongs take the lane now, so near-miss pong-shaped junk
cannot drain it. readPanelPongId replaces the zod parse on this
guest-controlled path (a rejected safeParse allocates an issue list, ~90x
the accepted-path cost) and is pinned to the schema by a parity test.
- Re-read the kill list inside approveAtomically: approvedKeys is snapshotted
before an awaited verification phase, so a plugin killed during that wait
could still publish VM recipes and language packs.
- Assert the curated icon resolves to FileText; the old equality also passed
when both sides fell back to Plug.
Co-authored-by: Orca <help@stably.ai>
* fix(plugins): match zod's safe-integer bound in the pong reader
readPanelPongId used Number.isInteger, but zod's .int() rejects anything
above 2**53-1, so pingIds like 1e100 took the reserved lane the schema
would have refused. The parity test never probed that boundary.
Co-authored-by: Orca <help@stably.ai>
---------
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -1,9 +1,12 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
PLUGIN_KILL_LIST_ENTRY_LIMIT,
|
||||
PLUGIN_KILL_LIST_FUTURE_SKEW_MS,
|
||||
findKilledPlugin,
|
||||
isPluginKillListTooFarInFuture,
|
||||
killedPluginKeys,
|
||||
pluginKillListSchema
|
||||
pluginKillListSchema,
|
||||
type PluginKillList
|
||||
} from './plugin-kill-list'
|
||||
|
||||
function entry(pluginKey = 'community.unsafe'): Record<string, unknown> {
|
||||
@@ -87,3 +90,19 @@ describe('pluginKillListSchema', () => {
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isPluginKillListTooFarInFuture', () => {
|
||||
const list = (generatedAt: string): PluginKillList =>
|
||||
pluginKillListSchema.parse({ version: 1, generatedAt, plugins: [entry()] })
|
||||
|
||||
it('flags a snapshot that would freeze out every later revocation', () => {
|
||||
expect(isPluginKillListTooFarInFuture(list('9999-12-31T23:59:59Z'))).toBe(true)
|
||||
})
|
||||
|
||||
it('allows a snapshot inside the clock-skew window', () => {
|
||||
const generatedAt = new Date(
|
||||
Date.now() + PLUGIN_KILL_LIST_FUTURE_SKEW_MS - 60_000
|
||||
).toISOString()
|
||||
expect(isPluginKillListTooFarInFuture(list(generatedAt))).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -2,6 +2,8 @@ import { z } from 'zod'
|
||||
import { isQualifiedPluginKey } from './plugin-manifest'
|
||||
|
||||
export const PLUGIN_KILL_LIST_ENTRY_LIMIT = 4_096
|
||||
/** Publisher clocks and client clocks disagree by minutes, never by days. */
|
||||
export const PLUGIN_KILL_LIST_FUTURE_SKEW_MS = 24 * 60 * 60 * 1000
|
||||
|
||||
const advisoryUrlSchema = z
|
||||
.string()
|
||||
@@ -38,6 +40,17 @@ export const pluginKillListSchema = z
|
||||
export type PluginKillList = z.infer<typeof pluginKillListSchema>
|
||||
export type PluginKillListEntry = z.infer<typeof pluginKillListEntrySchema>
|
||||
|
||||
/** A far-future generatedAt makes every genuine later list look "older" and
|
||||
* disables revocation permanently. Checked only on freshly fetched snapshots:
|
||||
* a cached list was already accepted once, and re-judging it against the
|
||||
* device clock would drop live revocations whenever that clock runs slow. */
|
||||
export function isPluginKillListTooFarInFuture(
|
||||
killList: PluginKillList,
|
||||
now = Date.now()
|
||||
): boolean {
|
||||
return Date.parse(killList.generatedAt) > now + PLUGIN_KILL_LIST_FUTURE_SKEW_MS
|
||||
}
|
||||
|
||||
export function killedPluginKeys(killList: PluginKillList): ReadonlySet<string> {
|
||||
return new Set(killList.plugins.map((plugin) => plugin.pluginKey))
|
||||
}
|
||||
|
||||
@@ -35,6 +35,9 @@ describe('plugin language-pack artifacts', () => {
|
||||
|
||||
it.each([
|
||||
'PluginConsentDialog',
|
||||
// The provenance badge and install-error copy carry the trust decision.
|
||||
'PluginConsentProvenance',
|
||||
'pluginError',
|
||||
'PluginKeybindingConsentPreview',
|
||||
'PluginMarketplaceListingRow',
|
||||
'PluginMarketplacePreviewDialog',
|
||||
@@ -54,6 +57,33 @@ describe('plugin language-pack artifacts', () => {
|
||||
).toMatchObject({ ok: false, error: expect.stringContaining('protected security copy') })
|
||||
})
|
||||
|
||||
it('forges no trust badge: the community→Official swap is refused', () => {
|
||||
expect(
|
||||
parsePluginLanguagePackArtifact(
|
||||
JSON.stringify({
|
||||
auto: {
|
||||
components: {
|
||||
settings: { PluginConsentProvenance: { community: 'Official' } }
|
||||
}
|
||||
}
|
||||
})
|
||||
)
|
||||
).toMatchObject({
|
||||
ok: false,
|
||||
error: expect.stringContaining('auto.components.settings.PluginConsentProvenance')
|
||||
})
|
||||
})
|
||||
|
||||
it('still lets language packs translate non-plugin settings copy', () => {
|
||||
expect(
|
||||
parsePluginLanguagePackArtifact(
|
||||
JSON.stringify({
|
||||
auto: { components: { settings: { AppearanceSection: { title: 'Apariencia' } } } }
|
||||
})
|
||||
).ok
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it.each([
|
||||
['array leaf', { settings: { choices: ['one'] } }],
|
||||
['numeric leaf', { settings: { count: 1 } }],
|
||||
|
||||
@@ -2,20 +2,11 @@ export const PLUGIN_LANGUAGE_CATALOG_MAX_ENTRIES = 20_000
|
||||
export const PLUGIN_LANGUAGE_CATALOG_MAX_DEPTH = 16
|
||||
|
||||
const DANGEROUS_CATALOG_KEYS = new Set(['__proto__', 'prototype', 'constructor'])
|
||||
const PROTECTED_TRANSLATION_PREFIXES = [
|
||||
'auto.components.settings.PluginConsentDialog',
|
||||
'auto.components.settings.PluginInstallDialog',
|
||||
'auto.components.settings.PluginKeybindingConsentPreview',
|
||||
'auto.components.settings.PluginMarketplaceBrowser',
|
||||
'auto.components.settings.PluginMarketplaceListingRow',
|
||||
'auto.components.settings.PluginMarketplacePreviewDialog',
|
||||
'auto.components.settings.PluginMarketplaceSourceDialog',
|
||||
'auto.components.settings.PluginRemoveDialog',
|
||||
'auto.components.settings.PluginRollbackDialog',
|
||||
'auto.components.settings.PluginSettingsRow',
|
||||
'auto.components.settings.PluginVmRecipeConsentPreview',
|
||||
'auto.components.settings.PluginsSettingsSection'
|
||||
]
|
||||
// Why: every plugin-facing security surface lives under this namespace, so
|
||||
// protecting the whole subtree keeps a new dialog from silently becoming
|
||||
// plugin-writable the way PluginConsentProvenance did when it was extracted.
|
||||
const PROTECTED_TRANSLATION_ROOT = 'auto.components.settings.'
|
||||
const PROTECTED_TRANSLATION_MODULE = /^plugin/i
|
||||
|
||||
export type PluginLanguagePackRegistration = {
|
||||
id: `plugin:${string}`
|
||||
@@ -51,9 +42,10 @@ function isCatalogObject(value: unknown): value is Record<string, unknown> {
|
||||
}
|
||||
|
||||
function protectedTranslation(path: string): boolean {
|
||||
return PROTECTED_TRANSLATION_PREFIXES.some(
|
||||
(prefix) => path === prefix || path.startsWith(`${prefix}.`)
|
||||
)
|
||||
if (!path.startsWith(PROTECTED_TRANSLATION_ROOT)) {
|
||||
return false
|
||||
}
|
||||
return PROTECTED_TRANSLATION_MODULE.test(path.slice(PROTECTED_TRANSLATION_ROOT.length))
|
||||
}
|
||||
|
||||
function hasUnsafeCatalogKeyCharacter(key: string): boolean {
|
||||
|
||||
@@ -22,6 +22,13 @@ export const PLUGIN_PANEL_FRAME_NAME_PREFIX = 'orca-plugin-panel:'
|
||||
export const PANEL_MESSAGE_MAX_BYTES = 64 * 1024
|
||||
export const PANEL_MESSAGE_RATE_LIMIT = { maxMessages: 30, perMs: 10_000 }
|
||||
|
||||
/** Size cap for the reserved liveness lane. Deliberately size-only: any
|
||||
* per-window count on this lane can be spent by the panel's own pongs and
|
||||
* would drop the next genuine reply, which is the starvation this reserved
|
||||
* lane exists to prevent. Aggregate cost stays bounded because pongs are also
|
||||
* charged to the data budget and cost O(1) plus a walk capped here. */
|
||||
export const PANEL_CONTROL_MESSAGE_MAX_BYTES = 1024
|
||||
|
||||
/** Watchdog cadence: a panel that misses a pong deadline is demoted to an
|
||||
* errored badge. Busy-loop detection is valid only while the runtime frame-
|
||||
* process gate confirms the sandbox stays outside the host renderer. */
|
||||
@@ -127,8 +134,21 @@ export function looksLikePanelActionRequest(data: unknown): boolean {
|
||||
)
|
||||
}
|
||||
|
||||
export function looksLikePanelPong(data: unknown): boolean {
|
||||
return panelPongSchema.safeParse(data).success
|
||||
/** Reads a valid pong's pingId, or null. Hand-rolled rather than
|
||||
* `panelPongSchema.safeParse` because a rejected parse allocates an issue
|
||||
* list, which is ~90x the accepted-path cost — free CPU for a panel spamming
|
||||
* near-miss pongs. The schema stays the contract; this mirrors it exactly. */
|
||||
export function readPanelPongId(data: unknown): number | null {
|
||||
if (typeof data !== 'object' || data === null) {
|
||||
return null
|
||||
}
|
||||
const frame = data as { type?: unknown; pingId?: unknown }
|
||||
if (frame.type !== PANEL_PONG_TYPE || typeof frame.pingId !== 'number') {
|
||||
return null
|
||||
}
|
||||
// isSafeInteger, not isInteger: zod's .int() rejects 2**53 and above, and a
|
||||
// wider reader would admit ids the watchdog can never have issued.
|
||||
return Number.isSafeInteger(frame.pingId) && frame.pingId >= 0 ? frame.pingId : null
|
||||
}
|
||||
|
||||
/** Validates action params against the host API spec (shared with workers). */
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import { PANEL_MESSAGE_MAX_BYTES, PANEL_MESSAGE_RATE_LIMIT } from './plugin-panel-bridge'
|
||||
import {
|
||||
PANEL_CONTROL_MESSAGE_MAX_BYTES,
|
||||
PANEL_MESSAGE_MAX_BYTES,
|
||||
PANEL_MESSAGE_RATE_LIMIT
|
||||
} from './plugin-panel-bridge'
|
||||
|
||||
/**
|
||||
* Per-plugin bridge budgets: message size cap and a sliding-window rate
|
||||
@@ -40,6 +44,20 @@ export function createPanelMessageBudget(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Reserved liveness lane, size-bounded only. A per-window count here would be
|
||||
* spent by the panel's own pongs and would then drop the next genuine reply —
|
||||
* the exact starvation this lane exists to prevent. Rate is still bounded
|
||||
* because the caller also charges every pong to the data budget.
|
||||
*/
|
||||
export function createPanelControlMessageBudget(): PanelMessageBudget {
|
||||
return {
|
||||
maxBytes: PANEL_CONTROL_MESSAGE_MAX_BYTES,
|
||||
admit: (_now, messageBytes) =>
|
||||
messageBytes > PANEL_CONTROL_MESSAGE_MAX_BYTES ? 'oversized' : null
|
||||
}
|
||||
}
|
||||
|
||||
const textEncoder = new TextEncoder()
|
||||
|
||||
function utf8Bytes(value: string, stopAfter: number): number {
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { PANEL_PONG_TYPE, panelPongSchema, readPanelPongId } from './plugin-panel-bridge'
|
||||
|
||||
/** `readPanelPongId` is hand-rolled to avoid zod's ~90x rejected-parse
|
||||
* allocation cost on the guest-controlled bridge path. It must therefore
|
||||
* accept exactly what `panelPongSchema` accepts, forever. */
|
||||
|
||||
const CASES: unknown[] = [
|
||||
{ type: PANEL_PONG_TYPE, pingId: 0 },
|
||||
{ type: PANEL_PONG_TYPE, pingId: 7 },
|
||||
{ type: PANEL_PONG_TYPE, pingId: Number.MAX_SAFE_INTEGER },
|
||||
// Above the safe range zod's .int() refuses, though Number.isInteger accepts.
|
||||
{ type: PANEL_PONG_TYPE, pingId: Number.MAX_SAFE_INTEGER + 1 },
|
||||
{ type: PANEL_PONG_TYPE, pingId: 2 ** 60 },
|
||||
{ type: PANEL_PONG_TYPE, pingId: 1e100 },
|
||||
{ type: PANEL_PONG_TYPE, pingId: Number.MAX_VALUE },
|
||||
{ type: PANEL_PONG_TYPE, pingId: 7, extra: 'ignored' },
|
||||
{ type: PANEL_PONG_TYPE, pingId: -1 },
|
||||
{ type: PANEL_PONG_TYPE, pingId: 1.5 },
|
||||
{ type: PANEL_PONG_TYPE, pingId: Number.NaN },
|
||||
{ type: PANEL_PONG_TYPE, pingId: Number.POSITIVE_INFINITY },
|
||||
{ type: PANEL_PONG_TYPE, pingId: '7' },
|
||||
{ type: PANEL_PONG_TYPE, pingId: null },
|
||||
{ type: PANEL_PONG_TYPE },
|
||||
{ type: 'orca-panel-action', pingId: 7 },
|
||||
{ pingId: 7 },
|
||||
'orca-panel-pong',
|
||||
null,
|
||||
undefined,
|
||||
42,
|
||||
[]
|
||||
]
|
||||
|
||||
describe('readPanelPongId', () => {
|
||||
it.each(CASES.map((data, index) => [index, data]))(
|
||||
'agrees with panelPongSchema on case %i',
|
||||
(_index, data) => {
|
||||
expect(readPanelPongId(data) !== null).toBe(panelPongSchema.safeParse(data).success)
|
||||
}
|
||||
)
|
||||
|
||||
it('returns the pingId the watchdog must correlate against', () => {
|
||||
expect(readPanelPongId({ type: PANEL_PONG_TYPE, pingId: 7 })).toBe(7)
|
||||
expect(readPanelPongId({ type: PANEL_PONG_TYPE, pingId: 0 })).toBe(0)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user