-
Notifications
You must be signed in to change notification settings - Fork 0
browser-automation: attach-lane primitives (Phase 1) #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
c6f5fd4
46f7a3a
61b25e9
6a9fab4
4ed5774
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,160 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { describe, it, expect } from 'vitest'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { mkdtempSync } from 'node:fs'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { tmpdir } from 'node:os'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { join } from 'node:path'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { parseDevToolsActivePort } from '../devtools-port.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { pidFromSingletonLock } from '../singleton-lock.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { isNavigationAllowed, revalidateRedirect } from '../url-policy.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AttachError, redactDiagnostic, toStructuredError } from '../errors.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AttachStateMachine, withDeadline } from '../state.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { acquireLease, releaseLease, requireLease } from '../lease.js'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe('parseDevToolsActivePort', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('parses a well-formed file', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const r = parseDevToolsActivePort('9222\n/devtools/browser/07797f92-4fc3-432a-91dd-0e982e1a3bc0\n'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(r.port).toBe(9222); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(r.wsUrl).toBe('ws://127.0.0.1:9222/devtools/browser/07797f92-4fc3-432a-91dd-0e982e1a3bc0'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('rejects a single-line file', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(() => parseDevToolsActivePort('9222\n')).toThrow(/malformed/i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('rejects a non-numeric port', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(() => parseDevToolsActivePort('nope\n/devtools/browser/x\n')).toThrow(/invalid port/i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('rejects a non-browser-target ws path', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(() => parseDevToolsActivePort('9222\n/devtools/page/abc\n')).toThrow(/unexpected ws path/i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe('pidFromSingletonLock', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('extracts the pid', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(pidFromSingletonLock('MacBook-Air.local-666')).toBe(666); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('handles hostnames containing dashes', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(pidFromSingletonLock('my-host-name.local-12345')).toBe(12345); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('rejects a target without a pid suffix', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(() => pidFromSingletonLock('just-a-hostname.local')).toThrow(/malformed/i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Add tests for trimming behavior and large-PID parsing in singleton-lock PID extraction Since
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe('url-policy', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('allows https and about:blank', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(isNavigationAllowed('https://founderscard.com/home').allowed).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(isNavigationAllowed('about:blank').allowed).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it.each([ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['file:///etc/passwd', 'SCHEME_BLOCKED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['javascript:alert(1)', 'SCHEME_BLOCKED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['data:text/html,<b>x</b>', 'SCHEME_BLOCKED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['devtools://devtools/bundled/inspector.html', 'SCHEME_BLOCKED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Extend URL policy tests to cover IPv6 localhost and allowlist corner cases Since the policy explicitly blocks Suggested implementation: describe('url-policy', () => {
it('allows https and about:blank', () => {
expect(isNavigationAllowed('https://founderscard.com/home').allowed).toBe(true);
expect(isNavigationAllowed('about:blank').allowed).toBe(true);
});
it('rejects IPv6 localhost with PRIVATE_HOST', () => {
const result = isNavigationAllowed('https://[::1]/');
expect(result.allowed).toBe(false);
expect(result.reason).toBe('PRIVATE_HOST');
});
it('matches allowHosts case-insensitively and does not allow similar-looking domains', () => {
const allowed = isNavigationAllowed('https://FoundersCard.com/path', {
allowHosts: ['founderscard.com'],
});
const blocked = isNavigationAllowed('https://evilfounderscard.com/', {
allowHosts: ['founderscard.com'],
});
expect(allowed.allowed).toBe(true);
expect(blocked.allowed).toBe(false);
});
it('treats an empty allowHosts array the same as the default allowlist', () => {
const url = 'https://founderscard.com/home';
const defaultResult = isNavigationAllowed(url);
const explicitEmptyAllowlist = isNavigationAllowed(url, { allowHosts: [] });
expect(defaultResult.allowed).toBe(true);
expect(explicitEmptyAllowlist.allowed).toBe(true);
});
it.each([
['file:///etc/passwd', 'SCHEME_BLOCKED'],If
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['chrome://settings', 'SCHEME_BLOCKED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['http://example.com', 'SCHEME_BLOCKED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['https://user:pass@example.com/x', 'CREDENTIALS_IN_URL'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['https://localhost/admin', 'PRIVATE_HOST'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['https://127.0.0.1:8080/', 'PRIVATE_HOST'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['https://192.168.1.10/router', 'PRIVATE_HOST'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['https://10.1.2.3/internal', 'PRIVATE_HOST'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['https://172.16.0.1/', 'PRIVATE_HOST'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['https://printer.local/', 'PRIVATE_HOST'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ['not a url', 'MALFORMED_URL'], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ])('rejects %s with %s', (url, reason) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const r = isNavigationAllowed(url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(r.allowed).toBe(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(r.reason).toBe(reason); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('enforces the host allowlist including subdomains', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const opts = { allowHosts: ['founderscard.com', 'hotels.com'] }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(isNavigationAllowed('https://founderscard.com/x', opts).allowed).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(isNavigationAllowed('https://www.hotels.com/', opts).allowed).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(isNavigationAllowed('https://evil.com/', opts).reason).toBe('HOST_NOT_ALLOWLISTED'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('revalidates redirects with the same policy', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(revalidateRedirect('https://ok.com/landing').allowed).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(revalidateRedirect('http://downgraded.com/').reason).toBe('SCHEME_BLOCKED'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe('errors', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('redacts credential-class substrings', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const out = redactDiagnostic('failed with token=abc123 and api_key: xyz987 at https://x.com?access_token=tt'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(out).not.toContain('abc123'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(out).not.toContain('xyz987'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(out).not.toContain('tt'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('keeps explicit AttachError codes', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(toStructuredError(new AttachError('LEASE_DENIED', 'nope')).code).toBe('LEASE_DENIED'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('classifies known failure shapes', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(toStructuredError(new Error('Executing JavaScript through AppleScript is turned off')).code).toBe('JS_DISABLED'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(toStructuredError(new Error('browserType.connectOverCDP: Timeout 12000ms exceeded')).code).toBe('CDP_TIMEOUT'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(toStructuredError(new Error('tab 999 not found in window 1')).code).toBe('TAB_CLOSED'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('defaults to UNKNOWN', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(toStructuredError(new Error('weird')).code).toBe('UNKNOWN'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe('AttachStateMachine', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('walks the happy path', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const m = new AttachStateMachine(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m.to('connecting'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m.to('ready'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m.to('navigating'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m.to('ready'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(m.state).toBe('ready'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('rejects illegal transitions', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const m = new AttachStateMachine(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(() => m.to('navigating')).toThrow(/illegal/i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('failed only resets through disconnected (no silent replay)', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const m = new AttachStateMachine(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m.to('connecting'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m.to('ready'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m.to('navigating'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m.to('failed'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(() => m.to('navigating')).toThrow(/illegal/i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| m.to('disconnected'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(m.state).toBe('disconnected'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('withDeadline rejects with NAV_TIMEOUT after expiry', async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await expect(withDeadline(new Promise(() => {}), 20, 'goto')).rejects.toMatchObject({ code: 'NAV_TIMEOUT' }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe('lease', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const dir = mkdtempSync(join(tmpdir(), 'attach-lease-')); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const file = join(dir, 'attach.lease'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('acquires, requires, and releases', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const lease = acquireLease({ file, isPidAlive: () => true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(requireLease(file, lease.nonce).pid).toBe(process.pid); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| releaseLease(file, lease.nonce); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(() => requireLease(file, lease.nonce)).toThrow(/nonce/i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('denies while a live lease exists, allows after the holder dies', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const lease = acquireLease({ file, isPidAlive: () => true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(() => acquireLease({ file, isPidAlive: () => true })).toThrow(/lease held/i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const takenOver = acquireLease({ file, isPidAlive: () => false }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(takenOver.nonce).not.toBe(lease.nonce); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| releaseLease(file, takenOver.nonce); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('allows reclaim after TTL expiry even if the holder is alive', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let t = 1_000_000; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const now = () => t; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| acquireLease({ file, ttlMs: 100, now, isPidAlive: () => true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t += 101; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const second = acquireLease({ file, ttlMs: 100, now, isPidAlive: () => true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(second.acquiredAt).toBe(t); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| releaseLease(file, second.nonce); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('refuses to release with a foreign nonce', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const lease = acquireLease({ file, isPidAlive: () => true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expect(() => releaseLease(file, 'not-the-nonce')).toThrow(/another session/i); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| releaseLease(file, lease.nonce); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| /** | ||
| * @fileoverview Parser for Chrome's `DevToolsActivePort` discovery file. | ||
| * | ||
| * When Chrome runs with remote debugging enabled it writes a two-line file at | ||
| * the root of its user-data directory: | ||
| * | ||
| * ``` | ||
| * 9222 | ||
| * /devtools/browser/07797f92-4fc3-432a-91dd-0e982e1a3bc0 | ||
| * ``` | ||
| * | ||
| * Line 1 is the listening port, line 2 the browser-target WebSocket path. The | ||
| * GUID rotates every browser boot, so callers MUST re-read this file per | ||
| * connection attempt — a cached ws URL goes stale the moment Chrome restarts. | ||
| * | ||
| * @module browser-automation/attach/devtools-port | ||
| */ | ||
|
|
||
| /** Parsed contents of a `DevToolsActivePort` file. */ | ||
| export interface DevToolsEndpoint { | ||
| /** TCP port the DevTools server listens on (loopback only). */ | ||
| port: number; | ||
| /** Browser-target WebSocket path, e.g. `/devtools/browser/<guid>`. */ | ||
| wsPath: string; | ||
| /** Full loopback WebSocket URL for `connectOverCDP`. */ | ||
| wsUrl: string; | ||
| } | ||
|
|
||
| /** | ||
| * Parse the raw text of a `DevToolsActivePort` file. | ||
| * | ||
| * @param text Raw file contents (two newline-separated lines). | ||
| * @returns The parsed endpoint. | ||
| * @throws {Error} When the file is malformed: fewer than two non-empty lines, | ||
| * a non-numeric/out-of-range port, or a ws path that is not a | ||
| * `/devtools/browser/…` browser-target path. | ||
| */ | ||
| export function parseDevToolsActivePort(text: string): DevToolsEndpoint { | ||
| const lines = String(text) | ||
| .split('\n') | ||
| .map((l) => l.trim()) | ||
| .filter(Boolean); | ||
| if (lines.length < 2) { | ||
| throw new Error('DevToolsActivePort malformed: expected "<port>\\n<wsPath>"'); | ||
| } | ||
| const port = Number(lines[0]); | ||
| if (!Number.isInteger(port) || port <= 0 || port > 65535) { | ||
| throw new Error(`DevToolsActivePort malformed: invalid port "${lines[0]}"`); | ||
| } | ||
| const wsPath = lines[1]; | ||
| if (!wsPath.startsWith('/devtools/browser/')) { | ||
| throw new Error(`DevToolsActivePort malformed: unexpected ws path "${wsPath}"`); | ||
| } | ||
| return { port, wsPath, wsUrl: `ws://127.0.0.1:${port}${wsPath}` }; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,90 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @fileoverview Structured errors + diagnostic redaction for the attach lane. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Attach operations run inside the user's AUTHENTICATED browser, so raw error | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * text can carry session URLs, tokens, or account identifiers. Everything that | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * leaves the attach layer is (a) mapped to a stable machine-readable code and | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * (b) redacted of credential-class substrings before it can reach a report or | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * a model prompt (Codex spec review F13/F14). | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @module browser-automation/attach/errors | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Stable failure codes for every attach-lane operation. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export type AttachErrorCode = | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'CDP_TIMEOUT' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'CDP_UNAVAILABLE' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'JS_DISABLED' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'ACCESSIBILITY_DENIED' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'PROFILE_MISMATCH' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'TAB_CLOSED' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'NAV_TIMEOUT' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'POLICY_BLOCKED' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'LEASE_DENIED' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'STALE_PORT_FILE' | ||||||||||||||||||||||||||||||||||||||||||||||||||
| | 'UNKNOWN'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Structured attach error surfaced to tools/callers. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface StructuredAttachError { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| code: AttachErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Redacted, human-readable diagnostic. Never verbatim page/session text. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| message: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Error subclass carrying a structured code through throw sites. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export class AttachError extends Error { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| readonly code: AttachErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor(code: AttachErrorCode, message: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| super(message); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| this.name = 'AttachError'; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| this.code = code; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const CREDENTIAL_PATTERNS: Array<[RegExp, string]> = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/(token|secret|password|passwd|apikey|api_key|auth|bearer|session|cookie)(["']?\s*[:=]\s*["']?)[^\s"'&;]+/gi, '$1$2[redacted]'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/eyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{4,}/g, '[redacted-jwt]'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/\b(sk|pk|rk|ak)-[A-Za-z0-9_-]{12,}\b/g, '[redacted-key]'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/[?&](code|state|id_token|access_token|refresh_token)=[^\s&"']+/gi, '?[redacted-param]'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+44
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Credential redaction misses space-separated The first pattern requires a literal 🔒 Proposed fix: allow whitespace-only separators for select keywords const CREDENTIAL_PATTERNS: Array<[RegExp, string]> = [
[/(token|secret|password|passwd|apikey|api_key|auth|bearer|session|cookie)(["']?\s*[:=]\s*["']?)[^\s"'&;]+/gi, '$1$2[redacted]'],
+ [/\bbearer\s+[^\s"'&;]+/gi, 'bearer [redacted]'],
[/eyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{4,}/g, '[redacted-jwt]'],📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Redact credential-class substrings from a diagnostic string. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param text Raw diagnostic text. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * @returns The text with tokens/keys/secrets masked. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export function redactDiagnostic(text: string): string { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| let out = String(text ?? ''); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const [re, rep] of CREDENTIAL_PATTERNS) out = out.replace(re, rep); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return out; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const CODE_MATCHERS: Array<[RegExp, AttachErrorCode]> = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/timeout.*connectovercdp|connectovercdp.*timeout|cdp.*timeout/i, 'CDP_TIMEOUT'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/econnrefused|ws error|socket hang up|cdp.*unavailable/i, 'CDP_UNAVAILABLE'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/javascript through applescript is turned off/i, 'JS_DISABLED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/not authorized to send apple events|accessibility/i, 'ACCESSIBILITY_DENIED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/profile.*mismatch|wrong profile/i, 'PROFILE_MISMATCH'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/tab .* not found|window .* not found|target closed/i, 'TAB_CLOSED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/navigation.*timeout|nav.*timeout|net::err_timed_out/i, 'NAV_TIMEOUT'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/policy|blocked|not allowed/i, 'POLICY_BLOCKED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/lease/i, 'LEASE_DENIED'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| [/devtoolsactiveport.*malformed|stale.*port/i, 'STALE_PORT_FILE'], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+63
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Line 72's 🩹 Proposed fix- [/lease/i, 'LEASE_DENIED'],
+ [/\blease\b/i, 'LEASE_DENIED'],📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * Map any thrown value to a {@link StructuredAttachError} with a redacted | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * message. An {@link AttachError} keeps its explicit code; everything else is | ||||||||||||||||||||||||||||||||||||||||||||||||||
| * classified by message pattern, defaulting to `UNKNOWN`. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| export function toStructuredError(err: unknown): StructuredAttachError { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (err instanceof AttachError) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { code: err.code, message: redactDiagnostic(err.message) }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const msg = err instanceof Error ? err.message : String(err); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const [re, code] of CODE_MATCHERS) { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if (re.test(msg)) return { code, message: redactDiagnostic(msg) }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return { code: 'UNKNOWN', message: redactDiagnostic(msg) }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (testing): Add edge-case tests for whitespace, extra lines, and out-of-range ports in DevToolsActivePort parsing
Since the parser trims lines, skips empties, and enforces 1–65535, please add tests that: (1) confirm files with trailing blank lines/extra whitespace and additional lines still parse correctly; and (2) assert that ports like
0and65536are rejected, covering the<= 0 || > 65535logic and clarifying behavior on slightly malformed inputs.