mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
fix(orchestration): authenticate the caller pane on send, reply, ask, and check
These four were the only orchestration methods that took a caller-supplied terminal handle without checking it against the request's attested pane, so any pane could act as another terminal. They now call the same assertCallerHandleMatchesEvidence the Run-scoped methods already call, in the same shape, guarded on the handle actually being declared. check matters as much as the three mail methods: it consumes the named handle's mail, so naming a worker drained that worker's dispatch mailbox. reply checks before marking the original read, so a rejected impersonation applies no effects. ask was only partly covered before: db.verifyDispatchCapability runs solely when the dispatch carries a capability_hash, which non-inject dispatches do not, leaving getActiveDispatchForIdentity keyed off the spoofed handle. This is a consistency fence, not a security boundary. The evidence is caller-supplied environment, so a caller that presents none keeps its declared handle -- plain shells, hook-disabled SSH panes, restored panes, and older clients all have none, and failing closed on them would be a remote-wire-compatibility break. The SSH bridge's runtime double omitted verifyOrchestrationCompatibilityCaller and threw once the rule applied; it now models attestation and covers the reject case.
This commit is contained in:
@@ -135,7 +135,7 @@ orca orchestration inbox [--limit <n>] [--json]
|
||||
|
||||
Rules:
|
||||
|
||||
- Omit `--from` unless impersonating another terminal; Orca auto-resolves it from the current terminal.
|
||||
- Omit `--from`; Orca resolves it from the current terminal. A managed agent pane may only name itself — `--from` is routing metadata and cannot grant another terminal's identity.
|
||||
- A coordinator `check` returns the bound Run's oldest FIFO Delivery (up to 50 messages) and replays that exact batch until `--ack <delivery_id>`. Process every message before acknowledging; `check --ack <id> --wait` acknowledges, checks, and waits in one operation.
|
||||
- Use `--peek` and `--all` only for read-only history/debugging. Type filters decide when a waiter wakes; the returned actionable Delivery is still the oldest full batch.
|
||||
- Use `dispatch:<id>` for coordinator guidance to one supervised worker. Orca routes that stable address locally or through the connected-server relay; do not substitute a remote terminal handle.
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -14,7 +14,9 @@ export type RunScopeParams = {
|
||||
callerEvidence?: OrchestrationCompatibilityEvidence
|
||||
}
|
||||
|
||||
// Why: declared handles select mutable Run bindings, so attested callers may only name themselves.
|
||||
// Why: declared handles select mutable Run bindings and mailboxes, so attested callers may only name themselves.
|
||||
// Why: evidence is caller-supplied env, so a caller that presents none keeps its declared handle — plain shells,
|
||||
// hook-disabled SSH panes, restored panes, and older clients have none. This fences mistakes, not a hostile pane.
|
||||
export function assertCallerHandleMatchesEvidence(
|
||||
runtime: OrcaRuntimeService,
|
||||
callerTerminalHandle: string,
|
||||
|
||||
@@ -3141,4 +3141,112 @@ describe('orchestration RPC methods', () => {
|
||||
expect(db.listTasks()).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('attested sender identity', () => {
|
||||
const workerPaneKey = 'tab_worker:bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb'
|
||||
|
||||
function attestAs(terminalHandle: string, paneKey: string): void {
|
||||
ctx = {
|
||||
runtime,
|
||||
orchestrationCompatibilityEvidence: {
|
||||
terminalHandle,
|
||||
paneKey,
|
||||
launchToken: 'launch-secret'
|
||||
}
|
||||
}
|
||||
vi.spyOn(runtime, 'verifyOrchestrationCompatibilityCaller').mockReturnValue({
|
||||
hostScope: { kind: 'local', hostId: 'local' },
|
||||
paneKey,
|
||||
terminalHandle,
|
||||
processIncarnation: `runtime_test:${terminalHandle}:1`,
|
||||
launchTokenHash: 'hash'
|
||||
})
|
||||
}
|
||||
|
||||
it('rejects a send whose --from names another terminal', async () => {
|
||||
setup()
|
||||
attestAs('term_worker', workerPaneKey)
|
||||
|
||||
await expect(
|
||||
call('orchestration.send', {
|
||||
from: 'term_coord',
|
||||
to: `run:${activeRunId}`,
|
||||
subject: 'spoofed'
|
||||
})
|
||||
).rejects.toMatchObject({ code: 'consumer_fenced' })
|
||||
expect(db.getInbox()).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('rejects a reply whose --from names another terminal without consuming the original', async () => {
|
||||
setup()
|
||||
const original = db.insertMessage({
|
||||
from: 'term_coord',
|
||||
to: 'term_worker',
|
||||
subject: 'ping'
|
||||
})
|
||||
attestAs('term_intruder', 'tab_x:cccccccc-cccc-4ccc-8ccc-cccccccccccc')
|
||||
|
||||
await expect(
|
||||
call('orchestration.reply', { id: original.id, body: 'pong', from: 'term_worker' })
|
||||
).rejects.toMatchObject({ code: 'consumer_fenced' })
|
||||
// Why: rejecting after markAsRead would silently consume another pane's mail.
|
||||
expect(db.getMessageById(original.id)?.read).toBe(0)
|
||||
})
|
||||
|
||||
it('rejects an ask whose --from names another terminal', async () => {
|
||||
setup()
|
||||
attestAs('term_worker', workerPaneKey)
|
||||
|
||||
await expect(
|
||||
call('orchestration.ask', {
|
||||
to: 'term_coord',
|
||||
question: 'ready?',
|
||||
from: 'term_coord'
|
||||
})
|
||||
).rejects.toMatchObject({ code: 'consumer_fenced' })
|
||||
})
|
||||
|
||||
it('rejects a check that names another terminal without consuming its mail', async () => {
|
||||
setup()
|
||||
db.insertMessage({ from: 'term_coord', to: 'term_worker', subject: 'work' })
|
||||
attestAs('term_intruder', 'tab_x:cccccccc-cccc-4ccc-8ccc-cccccccccccc')
|
||||
|
||||
await expect(call('orchestration.check', { terminal: 'term_worker' })).rejects.toMatchObject({
|
||||
code: 'consumer_fenced'
|
||||
})
|
||||
expect(db.getUnreadMessages('term_worker')).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('keeps the declared handle when evidence is present but cannot be attested', async () => {
|
||||
setup()
|
||||
ctx = {
|
||||
runtime,
|
||||
orchestrationCompatibilityEvidence: { terminalHandle: 'term_worker' }
|
||||
}
|
||||
// Why: the fail-open branch — a pane that drops ORCA_AGENT_LAUNCH_TOKEN is unattestable, not rejected.
|
||||
vi.spyOn(runtime, 'verifyOrchestrationCompatibilityCaller').mockReturnValue(null)
|
||||
vi.spyOn(runtime, 'deliverPendingMessagesForHandle').mockImplementation(() => {})
|
||||
|
||||
const result = (await call('orchestration.send', {
|
||||
from: 'term_coord',
|
||||
to: `run:${activeRunId}`,
|
||||
subject: 'unattestable'
|
||||
})) as { message: { from_handle: string } }
|
||||
|
||||
expect(result.message.from_handle).toBe('term_coord')
|
||||
})
|
||||
|
||||
it('leaves callers without attestation evidence on their declared handle', async () => {
|
||||
setup()
|
||||
vi.spyOn(runtime, 'deliverPendingMessagesForHandle').mockImplementation(() => {})
|
||||
|
||||
const result = (await call('orchestration.send', {
|
||||
from: 'term_coord',
|
||||
to: `run:${activeRunId}`,
|
||||
subject: 'legacy shell'
|
||||
})) as { message: { from_handle: string } }
|
||||
|
||||
expect(result.message.from_handle).toBe('term_coord')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -20,7 +20,7 @@ import {
|
||||
} from '../../../../shared/orchestration-rpc-contract'
|
||||
import { clampOrchestrationAskTimeoutMs } from '../../../../shared/orchestration-ask-timeout'
|
||||
import { ORCHESTRATION_GATE_METHODS } from './orchestration-gates'
|
||||
import { resolveRunScope } from './orchestration-run-scope'
|
||||
import { assertCallerHandleMatchesEvidence, resolveRunScope } from './orchestration-run-scope'
|
||||
import { ORCHESTRATION_RUN_METHODS } from './orchestration-runs'
|
||||
import { ORCHESTRATION_WORKER_METHODS } from './orchestration-worker-methods'
|
||||
import { ORCHESTRATION_FEDERATION_METHODS } from './orchestration-federation-methods'
|
||||
@@ -392,10 +392,14 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
orchestrationCapability,
|
||||
legacyCoordinatorRunId,
|
||||
revalidateLegacyCoordinator,
|
||||
orchestrationCompatibilityCallerAuthority
|
||||
orchestrationCompatibilityCallerAuthority,
|
||||
orchestrationCompatibilityEvidence
|
||||
}
|
||||
) => {
|
||||
const db = runtime.getOrchestrationDb()
|
||||
if (params.from) {
|
||||
assertCallerHandleMatchesEvidence(runtime, params.from, orchestrationCompatibilityEvidence)
|
||||
}
|
||||
const from = params.from ?? 'unknown'
|
||||
const attestedCaller =
|
||||
orchestrationCompatibilityCallerAuthority?.terminalHandle === from
|
||||
@@ -689,6 +693,14 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
}
|
||||
) => {
|
||||
const db = runtime.getOrchestrationDb()
|
||||
// Why: check consumes the named handle's mail, so naming someone else is mail theft, not just a mislabel.
|
||||
if (params.terminal) {
|
||||
assertCallerHandleMatchesEvidence(
|
||||
runtime,
|
||||
params.terminal,
|
||||
orchestrationCompatibilityEvidence
|
||||
)
|
||||
}
|
||||
const handle = params.terminal ?? 'unknown'
|
||||
const typeFilter = parseMessageTypes(params.types)
|
||||
|
||||
@@ -991,6 +1003,10 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
{ orchestrationCompatibilityEvidence, runtime, legacyCoordinatorRunId }
|
||||
) => {
|
||||
const db = runtime.getOrchestrationDb()
|
||||
// Why: check before any effect so a rejected impersonation cannot consume the original message first.
|
||||
if (params.from) {
|
||||
assertCallerHandleMatchesEvidence(runtime, params.from, orchestrationCompatibilityEvidence)
|
||||
}
|
||||
const original = db.getMessageById(params.id)
|
||||
if (!original) {
|
||||
throw new Error(`Message not found: ${params.id}`)
|
||||
@@ -1365,7 +1381,13 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
params: AskParams,
|
||||
handler: async (
|
||||
params,
|
||||
{ runtime, signal, orchestrationCapability, recordMutationReceipt }
|
||||
{
|
||||
runtime,
|
||||
signal,
|
||||
orchestrationCapability,
|
||||
recordMutationReceipt,
|
||||
orchestrationCompatibilityEvidence
|
||||
}
|
||||
) => {
|
||||
// Why: group addresses have no unambiguous first-answer authority.
|
||||
if (params.to && isGroupAddress(params.to)) {
|
||||
@@ -1375,6 +1397,9 @@ export const ORCHESTRATION_METHODS: RpcMethod[] = [
|
||||
}
|
||||
|
||||
const db = runtime.getOrchestrationDb()
|
||||
if (params.from) {
|
||||
assertCallerHandleMatchesEvidence(runtime, params.from, orchestrationCompatibilityEvidence)
|
||||
}
|
||||
const from = params.from ?? 'unknown'
|
||||
// Why: echoed on every return so a clamped caller reports the budget actually waited, not the one it asked for.
|
||||
const timeoutMs = clampOrchestrationAskTimeoutMs(params.timeoutMs)
|
||||
|
||||
@@ -41,7 +41,7 @@ function createFakeChild(): FakeChild {
|
||||
}
|
||||
|
||||
describe('runRemoteOrcaCli', () => {
|
||||
function createRuntime() {
|
||||
function createRuntime(attestedCaller: { terminalHandle: string } | null = null) {
|
||||
const messages: {
|
||||
id: string
|
||||
from_handle: string
|
||||
@@ -96,6 +96,8 @@ describe('runRemoteOrcaCli', () => {
|
||||
}),
|
||||
getOrchestrationDb: () => db,
|
||||
getTerminalPaneKey: () => null,
|
||||
// Why: the bridge forwards remote pane evidence on every orchestration call; a double that omits this would authenticate by accident.
|
||||
verifyOrchestrationCompatibilityCaller: vi.fn(() => attestedCaller),
|
||||
deliverPendingMessagesForHandle: vi.fn(),
|
||||
notifyMessageArrived: vi.fn(),
|
||||
linearIssueContext: vi.fn(async (request: unknown) => ({
|
||||
@@ -194,6 +196,29 @@ describe('runRemoteOrcaCli', () => {
|
||||
expect(db.getUnreadMessages('term_windows')[0]?.from_handle).toBe('term_ssh')
|
||||
})
|
||||
|
||||
it('rejects a remote sender whose --from is not the attested pane', async () => {
|
||||
const { runtime, db } = createRuntime({ terminalHandle: 'term_ssh_other' })
|
||||
|
||||
const result = await runRemoteOrcaCli(
|
||||
runtime,
|
||||
{
|
||||
argv: ['orchestration', 'send', '--to', 'term_windows', '--subject', 'ping', '--json'],
|
||||
cwd: '/home/alice/repo',
|
||||
env: {
|
||||
ORCA_TERMINAL_HANDLE: 'term_ssh',
|
||||
ORCA_PANE_KEY: 'tab_ssh:11111111-1111-4111-8111-111111111111',
|
||||
ORCA_AGENT_LAUNCH_TOKEN: 'launch-secret'
|
||||
}
|
||||
},
|
||||
LEGACY_FALLBACK_OPTIONS
|
||||
)
|
||||
|
||||
const payload = JSON.parse(result.stdout) as { ok: boolean; error?: { code: string } }
|
||||
expect(payload.ok).toBe(false)
|
||||
expect(payload.error?.code).toBe('consumer_fenced')
|
||||
expect(db.getUnreadMessages('term_windows')).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('does not trust caller-supplied remote pane identity in the legacy fallback', async () => {
|
||||
const { runtime, db } = createRuntime()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user