Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
/**
* @fileoverview AttachController — the one object allowed to touch an attached
* (user-owned, already-running) browser.
*
* Non-negotiable contract, enforced here rather than by convention:
*
* 1. ATTACH-ONLY. The controller can never launch a browser. When the target
* is missing or fails identity verification it ABORTS with a structured
* error — it must not "helpfully" spawn Chrome (Codex spec review F9).
* 2. NON-DESTRUCTIVE DETACH. `detach()` tears down only OUR transport
* connection. The backend interface deliberately has no close/quit surface,
* so no code path can call `Browser.close()`/`Page.close()` on the user's
* browser (Codex F10 — wunderland's `BrowserSession.disconnect()` calls
* `browser.close()` and is exactly the hazard this replaces).
* 3. MARKER-TAB BOUND. Every operation targets the single agent tab the
* controller claimed; other tabs/windows are out of reach by construction.
* 4. LEASED. All operations require the cross-process lease nonce acquired at
* `claim()` (F3), and run under the no-replay state machine with deadlines
* (F14) plus the deny-by-default URL policy (F12).
*
* @module browser-automation/attach/AttachController
*/
import { AttachError, toStructuredError, type StructuredAttachError } from './errors.js';
import { acquireLease, releaseLease, requireLease, type AttachLease } from './lease.js';
import { AttachStateMachine, withDeadline } from './state.js';
import { isNavigationAllowed, revalidateRedirect, type UrlPolicyOptions } from './url-policy.js';

/**
* Transport backend contract. INTENTIONALLY has no `closeBrowser`/`closeTab`
* surface: the strongest guarantee that a detach can never destroy user state
* is that the API cannot express it.
*/
export interface AttachBackend {
/** Transport id for status/reporting. */
readonly kind: 'cdp' | 'jxa';
/** Open the transport (e.g. CDP socket). Must NOT create or focus tabs. */
connect(): Promise<void>;
/** Close ONLY our transport connection. Must not touch browser state. */
disconnectTransport(): Promise<void>;
/** Claim (or create) the marker agent tab; returns an opaque tab handle. */
claimAgentTab(): Promise<string>;
/** Read a lightweight identity probe: current profile evidence (e.g. a known tab title). */
probeIdentity(): Promise<string>;
/** Navigate the agent tab. Returns the URL actually landed on. */
gotoTab(tab: string, url: string): Promise<string>;
/** Read innerText (optionally scoped by selector) from the agent tab. */
readTab(tab: string, selector?: string, maxChars?: number): Promise<string>;
}

/** Controller configuration. */
export interface AttachControllerOptions {
backend: AttachBackend;
/** Lease file path (one per machine/user). */
leaseFile: string;
/**
* Expected profile identity substring (e.g. the account email). Claim ABORTS
* with PROFILE_MISMATCH when the probe does not contain it. Required: an
* attach client must never guess which profile it is driving (F9).
*/
expectedIdentity: string;
/** URL policy options (host allowlist etc.). */
urlPolicy?: UrlPolicyOptions;
/** Per-operation deadline in ms (default 45s). */
deadlineMs?: number;
}

/** Status snapshot returned by {@link AttachController.status}. */
export interface AttachStatus {
transport: 'cdp' | 'jxa' | 'none';
state: string;
leaseHeld: boolean;
agentTab?: string;
lastError?: StructuredAttachError;
}

/**
* Orchestrates one attach session end to end. One controller instance = one
* lease = one agent tab.
*/
export class AttachController {
private readonly backend: AttachBackend;
private readonly opts: AttachControllerOptions;
private readonly machine = new AttachStateMachine();
private lease?: AttachLease;
private tab?: string;
private lastError?: StructuredAttachError;

constructor(opts: AttachControllerOptions) {
this.backend = opts.backend;
this.opts = opts;
}

private get deadline(): number {
return this.opts.deadlineMs ?? 45_000;
}

/** Non-throwing status snapshot. */
status(): AttachStatus {
return {
transport: this.lease ? this.backend.kind : 'none',
state: this.machine.state,
leaseHeld: !!this.lease,
agentTab: this.tab,
lastError: this.lastError,
};
}

/**
* Acquire the lease, open the transport, verify profile identity, and claim
* the marker agent tab. Aborts (never launches) on any verification failure.
*/
async claim(): Promise<AttachStatus> {
this.machine.to('connecting');
try {
this.lease = acquireLease({ file: this.opts.leaseFile });
await withDeadline(this.backend.connect(), this.deadline, 'connect');
const identity = await withDeadline(this.backend.probeIdentity(), this.deadline, 'probeIdentity');
if (!identity.includes(this.opts.expectedIdentity)) {
throw new AttachError(
'PROFILE_MISMATCH',
`attached browser did not present the expected profile identity (looked for "${this.opts.expectedIdentity}")`,
);
}
this.tab = await withDeadline(this.backend.claimAgentTab(), this.deadline, 'claimAgentTab');
this.machine.to('ready');
return this.status();
} catch (err) {
this.lastError = toStructuredError(err);
this.machine.to('failed');
await this.safeReleaseOnFailure();
throw err;
}
}

/** Navigate the agent tab (policy-checked, redirect-revalidated). */
async goto(url: string): Promise<string> {
this.requireReady();
const verdict = isNavigationAllowed(url, this.opts.urlPolicy);
if (!verdict.allowed) {
throw new AttachError('POLICY_BLOCKED', `navigation to "${url}" rejected: ${verdict.reason}`);
}
requireLease(this.opts.leaseFile, this.lease!.nonce);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

LEASE_DENIED on foreign takeover leaves the controller in ready with a stale lastError. requireLease throws before the try/state transition, so after a detected takeover status().state stays ready and lastError is not updated. Consider recording the error and moving to failed so subsequent ops fail fast rather than re-probing the lease each call.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@registry/curated/system/browser-automation/src/attach/AttachController.ts` at
line 142, Update the attach flow around AttachController and requireLease so
LEASE_DENIED errors from foreign takeover are caught before they escape,
recorded as lastError, and transition the controller state from ready to failed.
Ensure subsequent operations fail fast without re-probing the lease.

this.machine.to('navigating');
try {
const landed = await withDeadline(this.backend.gotoTab(this.tab!, url), this.deadline, 'goto');
const landedVerdict = revalidateRedirect(landed, this.opts.urlPolicy);
if (!landedVerdict.allowed) {
this.machine.to('ready');
throw new AttachError('POLICY_BLOCKED', `redirect landed on a blocked target: ${landedVerdict.reason}`);
}
this.machine.to('ready');
return landed;
} catch (err) {
if (this.machine.state === 'navigating') this.machine.to('failed');
this.lastError = toStructuredError(err);
throw err;
}
}

/** Read text from the agent tab (size-capped by the backend). */
async read(selector?: string, maxChars?: number): Promise<string> {
this.requireReady();
requireLease(this.opts.leaseFile, this.lease!.nonce);
this.machine.to('reading');
try {
const text = await withDeadline(this.backend.readTab(this.tab!, selector, maxChars), this.deadline, 'read');
this.machine.to('ready');
return text;
} catch (err) {
if (this.machine.state === 'reading') this.machine.to('failed');
this.lastError = toStructuredError(err);
throw err;
}
}

/**
* Release the session: park is the CALLER's responsibility (navigate to
* about:blank first if desired), then this closes only our transport and
* releases the lease. There is no code path to close browser/tabs.
*/
async detach(): Promise<void> {
try {
await this.backend.disconnectTransport();
} finally {
if (this.lease) releaseLease(this.opts.leaseFile, this.lease.nonce);
this.lease = undefined;
this.tab = undefined;
if (this.machine.state !== 'disconnected') {
if (this.machine.state !== 'failed') this.machine.to('failed');
this.machine.to('disconnected');
}
}
}

private requireReady(): void {
if (this.machine.state !== 'ready' || !this.tab || !this.lease) {
throw new AttachError('UNKNOWN', `attach session not ready (state=${this.machine.state}) — call claim() first`);
}
}

private async safeReleaseOnFailure(): Promise<void> {
try {
await this.backend.disconnectTransport();
} catch {
/* transport already dead */
}
if (this.lease) {
try {
releaseLease(this.opts.leaseFile, this.lease.nonce);
} catch {
/* foreign lease — leave it */
}
this.lease = undefined;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
import { describe, it, expect, beforeEach } from 'vitest';
import { mkdtempSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { AttachController, type AttachBackend } from '../AttachController.js';

/**
* Spy backend that records every method call. Its type deliberately mirrors
* AttachBackend, which has NO browser/tab close surface — the core F10 claim
* is proven structurally plus asserted behaviorally here.
*/
function makeBackend(overrides: Partial<AttachBackend> = {}): { backend: AttachBackend; calls: string[] } {
const calls: string[] = [];
const backend: AttachBackend = {
kind: 'jxa',
async connect() {
calls.push('connect');
},
async disconnectTransport() {
calls.push('disconnectTransport');
},
async claimAgentTab() {
calls.push('claimAgentTab');
return 'w1 t1';
},
async probeIdentity() {
calls.push('probeIdentity');
return 'Inbox - johnny@example.com - Gmail';
},
async gotoTab(_tab, url) {
calls.push(`goto:${url}`);
return url;
},
async readTab() {
calls.push('read');
return 'text';
},
...overrides,
};
return { backend, calls };
}

let leaseFile: string;
beforeEach(() => {
leaseFile = join(mkdtempSync(join(tmpdir(), 'attach-ctl-')), 'attach.lease');
});

function controller(backend: AttachBackend, extra: Partial<ConstructorParameters<typeof AttachController>[0]> = {}) {
return new AttachController({
backend,
leaseFile,
expectedIdentity: 'johnny@example.com',
...extra,
});
}

describe('AttachController', () => {
it('claims: lease → connect → identity probe → agent tab', async () => {
const { backend, calls } = makeBackend();
const c = controller(backend);
const st = await c.claim();
expect(st.state).toBe('ready');
expect(st.leaseHeld).toBe(true);
expect(st.agentTab).toBe('w1 t1');
expect(calls).toEqual(['connect', 'probeIdentity', 'claimAgentTab']);
await c.detach();
});

it('ABORTS with PROFILE_MISMATCH instead of proceeding on wrong identity (never launches)', async () => {
const { backend, calls } = makeBackend({
async probeIdentity() {
calls.push('probeIdentity');
return 'someone-else@example.com session';
},
});
const { backend: b2, calls: c2 } = { backend, calls };
const c = controller(b2);
await expect(c.claim()).rejects.toMatchObject({ code: 'PROFILE_MISMATCH' });
expect(c2).not.toContain('claimAgentTab'); // no tab was touched
expect(c.status().leaseHeld).toBe(false); // lease released on failure
});

it('policy-blocks bad navigation before touching the backend', async () => {
const { backend, calls } = makeBackend();
const c = controller(backend);
await c.claim();
await expect(c.goto('file:///etc/passwd')).rejects.toMatchObject({ code: 'POLICY_BLOCKED' });
expect(calls.filter((x) => x.startsWith('goto:'))).toHaveLength(0);
await c.detach();
});

it('revalidates redirects: an https→http downgrade fails the op', async () => {
const { backend } = makeBackend({
async gotoTab() {
return 'http://downgraded.example.com/';
},
});
const c = controller(backend);
await c.claim();
await expect(c.goto('https://ok.example.com/')).rejects.toMatchObject({ code: 'POLICY_BLOCKED' });
await c.detach();
});

it('detach closes ONLY the transport and releases the lease — no close surface exists', async () => {
const { backend, calls } = makeBackend();
const c = controller(backend);
await c.claim();
await c.detach();
expect(calls).toContain('disconnectTransport');
// Structural F10 guarantee: the backend contract exposes no browser/tab
// close method for the controller to call.
const forbidden = ['close', 'closeBrowser', 'closeTab', 'quit'];
for (const name of forbidden) {
expect((backend as unknown as Record<string, unknown>)[name]).toBeUndefined();
}
// Lease is released → a second controller can claim.
const c2 = controller(makeBackend().backend);
await expect(c2.claim()).resolves.toMatchObject({ state: 'ready' });
await c2.detach();
});

it('second concurrent claim is LEASE_DENIED while the first holds', async () => {
const { backend } = makeBackend();
const c1 = controller(backend);
await c1.claim();
const c2 = controller(makeBackend().backend);
await expect(c2.claim()).rejects.toMatchObject({ code: 'LEASE_DENIED' });
await c1.detach();
});

it('a failed navigation lands in failed state and refuses further ops (no silent replay)', async () => {
const { backend } = makeBackend({
async gotoTab() {
throw new Error('tab 999 not found in window 1');
},
});
const c = controller(backend);
await c.claim();
await expect(c.goto('https://ok.example.com/')).rejects.toBeTruthy();
expect(c.status().state).toBe('failed');
expect(c.status().lastError?.code).toBe('TAB_CLOSED');
await expect(c.read()).rejects.toMatchObject({ code: 'UNKNOWN' });
await c.detach();
expect(c.status().state).toBe('disconnected');
});

it('ops enforce the lease nonce at call time (foreign takeover is detected)', async () => {
let t = 1_000_000;
const { backend } = makeBackend();
const c = controller(backend, { deadlineMs: 500 });
await c.claim();
// Simulate a foreign session force-taking the lease file.
const { acquireLease } = await import('../lease.js');
acquireLease({ file: leaseFile, isPidAlive: () => false, now: () => (t += 1) });
await expect(c.goto('https://ok.example.com/')).rejects.toMatchObject({ code: 'LEASE_DENIED' });
});
});
Loading
Loading