From 331d79ec9e2040d0defe0cc54f0f1262eeafde0e Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Thu, 10 Sep 2026 16:40:42 -0700 Subject: [PATCH] key run coordinator binding on principals in the orchestration db --- .../orchestration/db/principal-match.test.ts | 37 ++++++ .../orchestration/db/principal-match.ts | 27 +++++ .../orchestration/db/runs/run-binding.ts | 37 +++--- .../db/runs/run-coordinator-binding.ts | 23 ++++ .../orchestration/db/runs/run-create.ts | 22 ++-- .../orchestration/db/runs/run-lookup.ts | 36 +++++- .../db/runs/run-principal-binding.test.ts | 112 ++++++++++++++++++ src/main/runtime/orchestration/types.ts | 7 ++ 8 files changed, 267 insertions(+), 34 deletions(-) create mode 100644 src/main/runtime/orchestration/db/principal-match.test.ts create mode 100644 src/main/runtime/orchestration/db/principal-match.ts create mode 100644 src/main/runtime/orchestration/db/runs/run-coordinator-binding.ts create mode 100644 src/main/runtime/orchestration/db/runs/run-principal-binding.test.ts diff --git a/src/main/runtime/orchestration/db/principal-match.test.ts b/src/main/runtime/orchestration/db/principal-match.test.ts new file mode 100644 index 00000000000..6bcbbc43436 --- /dev/null +++ b/src/main/runtime/orchestration/db/principal-match.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from 'vitest' +import { isEquivalentPrincipal } from './principal-match' + +describe('principal-match', () => { + const leaf = '11111111-1111-4111-8111-111111111111' + + it('treats pane principals with the same leaf as equivalent across a tab-half remint', () => { + expect(isEquivalentPrincipal(`pane:tab-a:${leaf}`, `pane:tab-a:${leaf}`)).toBe(true) + expect(isEquivalentPrincipal(`pane:tab-a:${leaf}`, `pane:tab-b:${leaf}`)).toBe(true) + expect( + isEquivalentPrincipal(`pane:tab-a:${leaf}`, 'pane:tab-a:22222222-2222-4222-8222-222222222222') + ).toBe(false) + }) + + it('matches session principals exactly and only exactly', () => { + expect(isEquivalentPrincipal('session:s-1', 'session:s-1')).toBe(true) + expect(isEquivalentPrincipal('session:s-1', 'session:s-2')).toBe(false) + }) + + it('requires an exact match for unparseable values', () => { + expect(isEquivalentPrincipal('legacy-value', 'legacy-value')).toBe(true) + expect(isEquivalentPrincipal('legacy-value', 'other-value')).toBe(false) + expect(isEquivalentPrincipal('unknown:payload', 'unknown:payload')).toBe(true) + expect(isEquivalentPrincipal('unknown:payload', 'unknown:other')).toBe(false) + }) + + it('NEVER bridges pane and session principals, in either direction', () => { + // The invariant, not an edge case: a structured pane key's tab half embeds the session id in + // plain text, so a caller who learns a session id can fabricate this pane key. Only the + // random leaf is a credential; matching it to the session principal would hand the attacker + // the coordinator's Run binding. + const realSessionId = 'session-alpha-1' + const fabricated = `pane:structured-agent-session-${realSessionId}:${leaf}` + expect(isEquivalentPrincipal(fabricated, `session:${realSessionId}`)).toBe(false) + expect(isEquivalentPrincipal(`session:${realSessionId}`, fabricated)).toBe(false) + }) +}) diff --git a/src/main/runtime/orchestration/db/principal-match.ts b/src/main/runtime/orchestration/db/principal-match.ts new file mode 100644 index 00000000000..24078d8e467 --- /dev/null +++ b/src/main/runtime/orchestration/db/principal-match.ts @@ -0,0 +1,27 @@ +import { parseOrchestrationPrincipal } from '../../../../shared/orchestration-principal' +import { isEquivalentPaneKey } from './pane-key-match' + +/** + * Equivalence over serialized `OrchestrationPrincipal` strings. + * + * INVARIANT: cross-kind is NEVER equivalent, in either direction. A structured pane key's tab half + * embeds the session id in plain text (`structured-agent-session-:`), so deriving + * `session:` from `pane:` here would let anyone who learns a session id fabricate a "matching" + * pane key and reach the coordinator's Run binding through caller-supplied-pane-key paths. That + * derivation is legal exactly once — PR 1's one-time server-side backfill/dual-write over pane + * keys the host itself wrote (`principalFromPaneKey`) — and never at request/match time: the + * random leaf is the only real credential. The backfill rule does NOT generalize to matching. + */ +export function isEquivalentPrincipal(a: string, b: string): boolean { + if (a === b) { + return true + } + const aParsed = parseOrchestrationPrincipal(a) + const bParsed = parseOrchestrationPrincipal(b) + // Session principals match exactly (handled above); unparseable or cross-kind never match. + if (aParsed?.kind !== 'pane' || bParsed?.kind !== 'pane') { + return false + } + // Leaf-UUID rule preserved so break-out remints keep matching. + return isEquivalentPaneKey(aParsed.paneKey, bParsed.paneKey) +} diff --git a/src/main/runtime/orchestration/db/runs/run-binding.ts b/src/main/runtime/orchestration/db/runs/run-binding.ts index 63697981309..c0a1691247b 100644 --- a/src/main/runtime/orchestration/db/runs/run-binding.ts +++ b/src/main/runtime/orchestration/db/runs/run-binding.ts @@ -1,16 +1,15 @@ -import { principalFromPaneKey } from '../../../../../shared/orchestration-principal' import type { RunRow } from '../../types' import { OrchestrationError } from '../../orchestration-error' import { LEGACY_CONTRACT_VERSION } from '../contract-constants' import { isEquivalentPaneKey } from '../pane-key-match' +import { isEquivalentPrincipal } from '../principal-match' import type { OrchestrationDb } from '../orchestration-db' +import { runCoordinatorBinding, type RunCoordinatorParam } from './run-coordinator-binding' export function bindRun( this: OrchestrationDb, params: { runId: string - coordinatorHandle: string - coordinatorPaneKey: string takeoverLegacy?: boolean legacyCoordinatorAuthority?: { runId: string @@ -19,8 +18,9 @@ export function bindRun( paneKey: string consumerGeneration: number } - } + } & RunCoordinatorParam ): RunRow | undefined { + const coordinator = runCoordinatorBinding(params) this.db.exec('BEGIN IMMEDIATE') try { const run = this.getRunRaw(params.runId) @@ -29,8 +29,8 @@ export function bindRun( return undefined } const sameBinding = - run.coordinator_pane_key !== null && - isEquivalentPaneKey(run.coordinator_pane_key, params.coordinatorPaneKey) + run.coordinator_principal !== null && + isEquivalentPrincipal(run.coordinator_principal, coordinator.principalId) const adoption = this.getLegacyAdoption() const adoptedRun = adoption?.adopted_run_id === params.runId const legacyAuthority = params.legacyCoordinatorAuthority @@ -38,6 +38,7 @@ export function bindRun( const legacyPrincipal = legacyPrincipalId ? this.getLegacyCompatibilityPrincipal(legacyPrincipalId) : undefined + // Why: a null handle or pane key never proves — a session binding yields false, matching today. const provenLegacyBinding = Boolean( adoptedRun && legacyAuthority && @@ -49,8 +50,9 @@ export function bindRun( legacyPrincipal.status === 'committed' && legacyPrincipal.terminal_handle === legacyAuthority.terminalHandle && isEquivalentPaneKey(legacyPrincipal.pane_key, legacyAuthority.paneKey) && - params.coordinatorHandle === legacyAuthority.terminalHandle && - isEquivalentPaneKey(params.coordinatorPaneKey, legacyAuthority.paneKey) + coordinator.terminalHandle === legacyAuthority.terminalHandle && + coordinator.paneKey !== null && + isEquivalentPaneKey(coordinator.paneKey, legacyAuthority.paneKey) ) if (legacyAuthority && !provenLegacyBinding) { throw new OrchestrationError( @@ -81,7 +83,8 @@ export function bindRun( const takeoverAlreadyApplied = Boolean( params.takeoverLegacy && sameBinding && - run.coordinator_handle === params.coordinatorHandle && + coordinator.terminalHandle !== null && + run.coordinator_handle === coordinator.terminalHandle && coordinatorPrincipal?.status !== 'committed' ) const replacesLegacyCoordinator = Boolean( @@ -89,7 +92,7 @@ export function bindRun( !provenLegacyBinding && retainedCoordinatorHandle && (params.takeoverLegacy || - retainedCoordinatorHandle !== params.coordinatorHandle || + retainedCoordinatorHandle !== coordinator.terminalHandle || !sameBinding) ) if (params.takeoverLegacy && !adoptedRun) { @@ -110,10 +113,9 @@ export function bindRun( } ) } - const incomingPrincipal = principalFromPaneKey(params.coordinatorPaneKey) - this.unbindOtherRunsForPane(params.coordinatorPaneKey, params.runId) + this.unbindOtherRunsForPrincipal(coordinator.principalId, params.runId) for (const handle of new Set( - [run.coordinator_handle, params.coordinatorHandle].filter((value): value is string => + [run.coordinator_handle, coordinator.terminalHandle].filter((value): value is string => Boolean(value) ) )) { @@ -123,14 +125,15 @@ export function bindRun( if ( (params.takeoverLegacy && !takeoverAlreadyApplied) || !sameBinding || - run.coordinator_handle !== params.coordinatorHandle + run.coordinator_handle !== coordinator.terminalHandle ) { if (adoptedRun && (params.takeoverLegacy || !activeLegacyAssignment)) { if ( coordinatorPrincipal?.status === 'committed' && (params.takeoverLegacy || - coordinatorPrincipal.terminal_handle !== params.coordinatorHandle || - !isEquivalentPaneKey(coordinatorPrincipal.pane_key, params.coordinatorPaneKey)) + coordinatorPrincipal.terminal_handle !== coordinator.terminalHandle || + coordinator.paneKey === null || + !isEquivalentPaneKey(coordinatorPrincipal.pane_key, coordinator.paneKey)) ) { this.setLegacyCompatibilityPrincipalStatus(coordinatorPrincipal.id, 'revoked') } @@ -143,7 +146,7 @@ export function bindRun( updated_at = datetime('now') WHERE id = ?` ) - .run(params.coordinatorHandle, params.coordinatorPaneKey, incomingPrincipal, params.runId) + .run(coordinator.terminalHandle, coordinator.paneKey, coordinator.principalId, params.runId) this.fenceOutstandingDelivery(params.runId) if (params.takeoverLegacy || replacesLegacyCoordinator) { this.promoteLegacyCoordinatorMailForTakeover(params.runId, retainedCoordinatorHandle) diff --git a/src/main/runtime/orchestration/db/runs/run-coordinator-binding.ts b/src/main/runtime/orchestration/db/runs/run-coordinator-binding.ts new file mode 100644 index 00000000000..caffbea1b6e --- /dev/null +++ b/src/main/runtime/orchestration/db/runs/run-coordinator-binding.ts @@ -0,0 +1,23 @@ +import { principalFromPaneKey } from '../../../../../shared/orchestration-principal' +import type { RunCoordinatorBinding } from '../../types' + +/** Either the resolver's opaque binding or the legacy handle+pane shape existing callers pass. */ +export type RunCoordinatorParam = + | { coordinator: RunCoordinatorBinding } + | { coordinatorHandle: string; coordinatorPaneKey: string } + +/** Legacy shape normalizes through the same classification as PR 1's dual-write derivation. */ +export function runCoordinatorBinding(params: RunCoordinatorParam): RunCoordinatorBinding { + if ('coordinator' in params) { + return params.coordinator + } + const principalId = principalFromPaneKey(params.coordinatorPaneKey) + if (!principalId) { + throw new Error('A run coordinator binding requires a non-empty pane key.') + } + return { + principalId, + terminalHandle: params.coordinatorHandle, + paneKey: params.coordinatorPaneKey + } +} diff --git a/src/main/runtime/orchestration/db/runs/run-create.ts b/src/main/runtime/orchestration/db/runs/run-create.ts index e8ea7fd130e..3703b1ef483 100644 --- a/src/main/runtime/orchestration/db/runs/run-create.ts +++ b/src/main/runtime/orchestration/db/runs/run-create.ts @@ -1,23 +1,19 @@ -import { principalFromPaneKey } from '../../../../../shared/orchestration-principal' import type { RunRow } from '../../types' import { generateId } from '../generated-id' import type { OrchestrationDb } from '../orchestration-db' +import { runCoordinatorBinding, type RunCoordinatorParam } from './run-coordinator-binding' // ── Runs ── export function createRun( this: OrchestrationDb, - params: { - objective: string - coordinatorHandle: string - coordinatorPaneKey: string - } + params: { objective: string } & RunCoordinatorParam ): RunRow { const id = generateId('run') - const coordinatorPrincipal = principalFromPaneKey(params.coordinatorPaneKey) + const coordinator = runCoordinatorBinding(params) this.db.exec('BEGIN IMMEDIATE') try { - this.unbindOtherRunsForPane(params.coordinatorPaneKey) + this.unbindOtherRunsForPrincipal(coordinator.principalId) this.db .prepare( `INSERT INTO runs ( @@ -28,11 +24,13 @@ export function createRun( .run( id, params.objective, - params.coordinatorHandle, - params.coordinatorPaneKey, - coordinatorPrincipal + coordinator.terminalHandle, + coordinator.paneKey, + coordinator.principalId ) - this.rememberRunCoordinatorHandle(id, params.coordinatorHandle) + if (coordinator.terminalHandle !== null) { + this.rememberRunCoordinatorHandle(id, coordinator.terminalHandle) + } this.db.exec('COMMIT') } catch (error) { this.db.exec('ROLLBACK') diff --git a/src/main/runtime/orchestration/db/runs/run-lookup.ts b/src/main/runtime/orchestration/db/runs/run-lookup.ts index 291bbf79b29..d7d845fbc80 100644 --- a/src/main/runtime/orchestration/db/runs/run-lookup.ts +++ b/src/main/runtime/orchestration/db/runs/run-lookup.ts @@ -5,6 +5,7 @@ import { RUN_PANE_KEY_MATCH_SUFFIX_SQL, paneKeyMatchSuffix } from '../pane-key-match' +import { isEquivalentPrincipal } from '../principal-match' import { exposeRunTimestamps } from '../utc-timestamp' import { encodeRunListCursor, decodeRunListCursor } from '../run-list-cursor' import type { RunListPage } from '../run-list-page' @@ -22,6 +23,11 @@ const RUNS_BOUND_TO_PANE_SQL = `SELECT ${RUN_COLUMN_LIST} FROM runs WHERE coordinator_pane_key IS NOT NULL AND legacy = 0 AND ${RUN_PANE_KEY_MATCH_SUFFIX_SQL} = ? ORDER BY rowid` +// Why: NO suffix pre-filter here — the after-first-':' shape would exclude reminted tab halves +// for `pane:` principals. runs is small; the hoisted statement still hits the SyncDatabase cache. +const RUNS_BOUND_TO_PRINCIPAL_SQL = `SELECT ${RUN_COLUMN_LIST} FROM runs + WHERE coordinator_principal IS NOT NULL AND legacy = 0 + ORDER BY rowid` export function getRun(this: OrchestrationDb, id: string): RunRow | undefined { const run = this.getRunRaw(id) @@ -118,16 +124,32 @@ export function runsBoundToPane(this: OrchestrationDb, paneKey: string): RunRow[ ) } +export function getCurrentRunForPrincipal( + this: OrchestrationDb, + principalId: string +): RunRow | undefined { + const run = this.runsBoundToPrincipal(principalId)[0] + return run ? exposeRunTimestamps(run) : undefined +} + +export function runsBoundToPrincipal(this: OrchestrationDb, principalId: string): RunRow[] { + return (this.db.prepare(RUNS_BOUND_TO_PRINCIPAL_SQL).all() as RunRow[]).filter( + (run) => + run.coordinator_principal !== null && + isEquivalentPrincipal(run.coordinator_principal, principalId) + ) +} + export function getRunRaw(this: OrchestrationDb, id: string): RunRow | undefined { return this.db.prepare(RUN_BY_ID_SQL).get(id) as RunRow | undefined } -export function unbindOtherRunsForPane( +export function unbindOtherRunsForPrincipal( this: OrchestrationDb, - paneKey: string, + principalId: string, exceptRunId?: string ): void { - for (const run of this.runsBoundToPane(paneKey)) { + for (const run of this.runsBoundToPrincipal(principalId)) { if (run.id !== exceptRunId) { if (run.coordinator_handle) { this.routeAllUnreadDirectMessagesToRunMailbox(run.id, run.coordinator_handle) @@ -164,8 +186,10 @@ export type RunLookupMethods = { listRuns: typeof listRuns getCurrentRunForPane: typeof getCurrentRunForPane runsBoundToPane: typeof runsBoundToPane + getCurrentRunForPrincipal: typeof getCurrentRunForPrincipal + runsBoundToPrincipal: typeof runsBoundToPrincipal getRunRaw: typeof getRunRaw - unbindOtherRunsForPane: typeof unbindOtherRunsForPane + unbindOtherRunsForPrincipal: typeof unbindOtherRunsForPrincipal requireRun: typeof requireRun fenceOutstandingDelivery: typeof fenceOutstandingDelivery } @@ -178,8 +202,10 @@ export function attachRunLookup(ctor: { prototype: object }): void { listRuns, getCurrentRunForPane, runsBoundToPane, + getCurrentRunForPrincipal, + runsBoundToPrincipal, getRunRaw, - unbindOtherRunsForPane, + unbindOtherRunsForPrincipal, requireRun, fenceOutstandingDelivery }) diff --git a/src/main/runtime/orchestration/db/runs/run-principal-binding.test.ts b/src/main/runtime/orchestration/db/runs/run-principal-binding.test.ts new file mode 100644 index 00000000000..961fc895bf7 --- /dev/null +++ b/src/main/runtime/orchestration/db/runs/run-principal-binding.test.ts @@ -0,0 +1,112 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { OrchestrationDb } from '../../db' + +const LEAF = '11111111-1111-4111-8111-111111111111' +const PANE_KEY = `tab_coord:${LEAF}` +const SESSION_ID = 'session-alpha-1' +const SESSION_BINDING = { + principalId: `session:${SESSION_ID}`, + terminalHandle: 'structworker_11111111-2222-4333-8444-555555555555', + paneKey: `structured-agent-session-${SESSION_ID}:${LEAF}` +} + +describe('run coordinator principal binding', () => { + let db: OrchestrationDb | undefined + + afterEach(() => { + db?.close() + }) + + function createDb(): OrchestrationDb { + db = new OrchestrationDb(':memory:') + return db + } + + it('derives the principal from the legacy handle+pane createRun shape', () => { + const d = createDb() + const run = d.createRun({ + objective: 'Legacy shape', + coordinatorHandle: 'term_coord', + coordinatorPaneKey: PANE_KEY + }) + const raw = d.getRunRaw(run.id)! + expect(raw.coordinator_principal).toBe(`pane:${PANE_KEY}`) + expect(raw.coordinator_handle).toBe('term_coord') + expect(raw.coordinator_pane_key).toBe(PANE_KEY) + }) + + it('writes all three coordinator columns from a resolver binding', () => { + const d = createDb() + const run = d.createRun({ objective: 'Session binding', coordinator: SESSION_BINDING }) + const raw = d.getRunRaw(run.id)! + expect(raw.coordinator_principal).toBe(SESSION_BINDING.principalId) + expect(raw.coordinator_handle).toBe(SESSION_BINDING.terminalHandle) + expect(raw.coordinator_pane_key).toBe(SESSION_BINDING.paneKey) + }) + + it('unbinds other runs for the principal: nulls all three columns, bumps the generation, fences delivery', () => { + const d = createDb() + const run = d.createRun({ objective: 'To unbind', coordinator: SESSION_BINDING }) + d.insertMessage({ from: 'a', to: `run:${run.id}`, subject: 'pending', runId: run.id }) + const delivery = d.getOrCreateRunDelivery({ + runId: run.id, + consumerGeneration: run.consumer_generation + })! + + d.unbindOtherRunsForPrincipal(SESSION_BINDING.principalId) + + const raw = d.getRunRaw(run.id)! + expect(raw.coordinator_principal).toBeNull() + expect(raw.coordinator_handle).toBeNull() + expect(raw.coordinator_pane_key).toBeNull() + expect(raw.consumer_generation).toBe(run.consumer_generation + 1) + const status = d.db + .prepare('SELECT status FROM deliveries WHERE id = ?') + .get(delivery.delivery.id) as { status: string } + expect(status.status).toBe('fenced') + }) + + it('keeps the exempted run bound while unbinding its siblings', () => { + const d = createDb() + const kept = d.createRun({ objective: 'Kept', coordinator: SESSION_BINDING }) + d.unbindOtherRunsForPrincipal(SESSION_BINDING.principalId, kept.id) + expect(d.getRunRaw(kept.id)!.coordinator_principal).toBe(SESSION_BINDING.principalId) + }) + + it('recognizes the same pane binding across a tab-half remint and does not rebump it', () => { + const d = createDb() + const run = d.createRun({ + objective: 'Remint', + coordinatorHandle: 'term_coord', + coordinatorPaneKey: PANE_KEY + }) + const rebound = d.bindRun({ + runId: run.id, + coordinatorHandle: 'term_coord', + coordinatorPaneKey: `tab_reminted:${LEAF}` + })! + expect(rebound.consumer_generation).toBe(run.consumer_generation) + expect(d.getRunRaw(run.id)!.coordinator_pane_key).toBe(PANE_KEY) + }) + + it('recognizes the same session binding and does not rebump it', () => { + const d = createDb() + const run = d.createRun({ objective: 'Session stable', coordinator: SESSION_BINDING }) + const rebound = d.bindRun({ runId: run.id, coordinator: SESSION_BINDING })! + expect(rebound.consumer_generation).toBe(run.consumer_generation) + }) + + it('bumps the generation when the coordinator principal actually changes kind', () => { + const d = createDb() + const run = d.createRun({ + objective: 'Handover', + coordinatorHandle: 'term_coord', + coordinatorPaneKey: PANE_KEY + }) + const rebound = d.bindRun({ runId: run.id, coordinator: SESSION_BINDING })! + expect(rebound.consumer_generation).toBe(run.consumer_generation + 1) + const raw = d.getRunRaw(run.id)! + expect(raw.coordinator_principal).toBe(SESSION_BINDING.principalId) + expect(raw.coordinator_handle).toBe(SESSION_BINDING.terminalHandle) + }) +}) diff --git a/src/main/runtime/orchestration/types.ts b/src/main/runtime/orchestration/types.ts index 6227488f219..6b742057390 100644 --- a/src/main/runtime/orchestration/types.ts +++ b/src/main/runtime/orchestration/types.ts @@ -40,6 +40,13 @@ export type GateStatus = 'pending' | 'resolved' | 'timeout' export type CoordinatorStatus = 'idle' | 'running' | 'completed' | 'failed' +/** Opaque coordinator identity carrier: DB writers persist all three fields; methods pass it through whole. */ +export type RunCoordinatorBinding = { + principalId: string + terminalHandle: string | null + paneKey: string | null +} + export type RunRow = { id: string objective: string