diff --git a/README.ko.md b/README.ko.md index f4f4858..a48e31b 100644 --- a/README.ko.md +++ b/README.ko.md @@ -303,6 +303,15 @@ codemode --account u1 --host local --code-file task.js 알 수 없는 실행 옵션이나 잘못된 선택값은 게스트 코드를 실행하기 전에 거절합니다. 선택값을 생략하면 네이티브 Aside의 기본값을 따릅니다. 반환된 라우팅 정보는 지정한 선택값이지, 실제 로그인 계정이나 원격 기기를 검증했다는 뜻은 아닙니다. 원격 호스트 이름은 브라우저 작업 때 Aside가 확인하므로, `return 1` 같은 실행만으로 해당 기기의 존재를 검증할 수는 없습니다. +공유 캐시·영구 승인·탭 저널은 설정이나 호출별 선택값에 account와 host가 모두 있어야 +재사용할 수 있습니다. 빈 기본 컨텍스트를 포함해 하나라도 빠지면 재사용을 끕니다. 두 값을 +지정하고 관찰·승인을 새로 받으세요. 기존의 컨텍스트 없는 기록은 재사용하지 않습니다. +브라우저 읽기는 계속 Aside 기본값을 상속할 수 있습니다. `browse.captureMany`와 `report.build`의 +로컬 파일 저장에는 명시적인 `host: "local"`이 필요합니다. 호스트를 생략해도 원격 기기를 +상속할 수 있어 거절합니다. 검증된 아티팩트 전송 경로는 없습니다. 모든 CLI 모드는 실행이나 +설정 쓰기 전에 옵션을 검사합니다. `--enable-browse`는 `--json`만, `--install-mcp`는 +`--account`, `--json`, `--force`, `--no-discovery`를 받습니다. + ### 브라우징은 기본으로 켜져 있습니다 설치한 그대로 `browse`를 부를 수 있습니다. 끄고 싶은 기계는 설정에서 `browseCaps.enabled`를 diff --git a/README.md b/README.md index 8064707..3da618e 100644 --- a/README.md +++ b/README.md @@ -330,6 +330,15 @@ codemode --account u1 --host local --code-file task.js Unknown execution flags and malformed selectors fail before guest code runs. An omitted selector inherits the native Aside default; this is not evidence of which account or device that default currently resolves to. Returned routing metadata reports the selected context, not independently verified browser identity. Remote host names are validated by native Aside when a browser operation runs, not by a browser-free `return 1` probe. +Shared browser caches, persistent approvals and tab journals require both account and host, +from config or per-call selectors. An incomplete context, including the empty default, disables +their reuse. To use these features, specify both selectors and obtain fresh observations and +approvals; old unscoped records are not reused. Browser reads still inherit Aside defaults. +`browse.captureMany` and `report.build` require explicit `host: "local"` for local files. Omitted +hosts may inherit remote devices and are refused too, because no verified artifact transfer exists. +All CLI modes validate flags before execution or settings writes. `--enable-browse` accepts only +`--json`; `--install-mcp` accepts `--account`, `--json`, `--force`, and `--no-discovery`. + ### Browsing is on by default A fresh install can call `browse` with no extra step. A machine that would rather it could not diff --git a/src/browser-context.js b/src/browser-context.js index 0be6c05..fe8f657 100644 --- a/src/browser-context.js +++ b/src/browser-context.js @@ -1,6 +1,7 @@ // Browser execution routing context validation and argv generation. // Enforces per-execution routing for CLI (--account uN, --host host) and MCP execute_code. import { createHash } from 'node:crypto'; +import path from 'node:path'; export function normalizeAccount(id) { if (id === null || id === undefined) return null; @@ -32,11 +33,12 @@ export function validateHost(host) { if (typeof host !== 'string') { throw new TypeError('--host must be a string, got ' + typeof host); } + if (/[\u0000-\u001f\u007f]/.test(host)) throw new Error('--host cannot contain control characters'); const s = host.trim(); if (!s) { throw new Error('--host cannot be empty'); } - if (s.startsWith('--')) { + if (s.startsWith('-')) { throw new Error('--host requires a value, for example --host local'); } // Native Aside validates host identity ('local', remote ID, or device name). @@ -91,8 +93,34 @@ const OPT_WITH_VALUE = new Set([ '--host', ]); -export function parseExecutionArgv(argv = []) { +const MODE_FLAGS = { + '--enable-browse': ['--enable-browse', '--json'], + '--install-mcp': ['--install-mcp', '--json', '--force', '--no-discovery'], + '--doctor': ['--doctor', '--browse', '--json'], +}; +const MODE_VALUES = { + '--enable-browse': [], + '--install-mcp': ['--account'], + '--doctor': ['--config', '--cwd', '--account', '--host'], +}; + +// Detect modes without interpreting opaque --code values, then validate before dispatch. +export function parseCliArgv(argv = []) { + const modes = new Set(); + for (let i = 0; i < argv.length; i++) { + const token = argv[i]; + if (Object.hasOwn(MODE_FLAGS, token)) modes.add(token); + if (token === '--code' || token === '--code-file') modes.add('execution'); + if (OPT_WITH_VALUE.has(token)) i++; + } + if (modes.size > 1) throw new Error('choose one CLI mode: execution, --doctor, --enable-browse, or --install-mcp'); + return parseExecutionArgv(argv, [...modes][0] || 'execution'); +} + +export function parseExecutionArgv(argv = [], mode = 'execution') { const flags = new Map(); + const booleanFlags = new Set(MODE_FLAGS[mode] || []); + const valueFlags = new Set(MODE_VALUES[mode] || OPT_WITH_VALUE); let i = 0; while (i < argv.length) { @@ -106,12 +134,13 @@ export function parseExecutionArgv(argv = []) { } if (token.startsWith('--')) { - if (!OPT_WITH_VALUE.has(token)) { - throw new Error(`unknown execution flag: ${token}`); + if (!valueFlags.has(token) && !booleanFlags.has(token)) { + throw new Error(`unknown ${mode} flag: ${token}`); } if (flags.has(token)) { throw new Error(`duplicate flag: ${token}`); } + if (booleanFlags.has(token)) { flags.set(token, true); i++; continue; } if (i + 1 >= argv.length) { if (token === '--account') throw new Error('--account needs an account id, for example --account u1'); if (token === '--host') throw new Error('--host requires a value, for example --host local'); @@ -123,6 +152,7 @@ export function parseExecutionArgv(argv = []) { throw new Error(`${token} requires a value`); } const val = argv[i + 1]; + if (token !== '--code' && !val.trim()) throw new Error(`${token} requires a non-empty value`); // For flags other than --code, a value starting with -- that matches known flags is a missing value error if (token !== '--code' && val.startsWith('--') && (OPT_WITH_VALUE.has(val) || val.length > 2)) { if (token === '--account') throw new Error('--account needs an account id, for example --account u1'); @@ -139,6 +169,8 @@ export function parseExecutionArgv(argv = []) { } } + if (flags.has('--code') && flags.has('--code-file')) throw new Error('pass exactly one of --code or --code-file'); + if (flags.has('--account')) { validateAccount(flags.get('--account')); } @@ -260,6 +292,7 @@ export function resolveBrowserContext({ argv, parsedFlags, mcpArgs, config } = { } export function buildCacheIdentity(baseAccountRoot, browserContext) { + if (!hasCompleteBrowserContext(browserContext)) return null; const routing = routingReport(browserContext); const tuple = [ 'v1', @@ -272,3 +305,20 @@ export function buildCacheIdentity(baseAccountRoot, browserContext) { const hash = createHash('sha256').update(JSON.stringify(tuple)).digest('hex').slice(0, 32); return `${baseAccountRoot}#ctx=${hash}`; } + +export function hasCompleteBrowserContext(context) { + return Boolean(context?.account && context?.host); +} + +export function browserContextDirectory(base, context) { + if (!hasCompleteBrowserContext(context)) throw new Error('persistent browser state requires both account and host'); + const tag = createHash('sha256').update(JSON.stringify([normalizeAccount(context.account), context.host])).digest('hex').slice(0, 16); + return path.join(base, `ctx-${tag}`); +} + +export function requireLocalArtifacts(context, action) { + if (context?.host === 'local') return; + const error = new Error(`${action} requires explicit host: "local" for local artifacts; inherited or remote hosts have no verified transfer path`); + error.code = 'EREMOTEARTIFACT'; + throw error; +} diff --git a/src/cli.js b/src/cli.js index 30a6263..1ba2a8e 100644 --- a/src/cli.js +++ b/src/cli.js @@ -22,7 +22,7 @@ import { createDetailedRgResolver, getAsideBundledRgPath } from './rg.js'; import { createHostGlobals } from './host/globals.js'; import { runCode } from './sandbox.js'; import { requireInteger, fitEnvelope } from './execution-output.js'; -import { resolveBrowserContext, parseExecutionArgv, validateAccount, validateHost, routingReport } from './browser-context.js'; +import { resolveBrowserContext, parseCliArgv, parseExecutionArgv, validateAccount, validateHost, routingReport } from './browser-context.js'; const argv = process.argv.slice(2); // Mode detection must not inspect option VALUES (guest code may itself be '--doctor'). @@ -58,6 +58,9 @@ function fail(error, extra = {}) { process.exit(1); } +try { parseCliArgv(argv); } +catch (e) { fail(e.message, { code: 'EBADARGV' }); } + // Before the config is loaded, on purpose. The file this command exists to fix is one of the // files loadConfig reads, so a broken one would block the only easy way to repair it. if (has('--enable-browse')) { diff --git a/src/host/browse/browse.js b/src/host/browse/browse.js index 0747027..2414a57 100644 --- a/src/host/browse/browse.js +++ b/src/host/browse/browse.js @@ -17,9 +17,8 @@ import { createWatch, createRecipes, createPrefetch } from './watch.js'; import { createAttach } from './attach.js'; import os from 'node:os'; import path from 'node:path'; -import { createHash } from 'node:crypto'; import { listAccountRoots } from '../../register.js'; -import { routingReport, normalizeAccount, buildCacheIdentity } from '../../browser-context.js'; +import { routingReport, buildCacheIdentity, hasCompleteBrowserContext, browserContextDirectory, requireLocalArtifacts } from '../../browser-context.js'; import { APPROVAL_DIR } from './approvals.js'; import { JOURNAL_DIR } from './tab-journal.js'; @@ -42,23 +41,20 @@ export function createBrowse({ config = {}, spawnAside, resolveAside, signal, en // suite was writing every refusal, claim and rejection into the shared one and leaving // them there, which is litter in somebody's temp directory and a test that can see // another run's records. - const hasSelection = Boolean(browserContext?.account || browserContext?.host); - const contextTag = hasSelection - ? createHash('sha256').update(JSON.stringify([normalizeAccount(browserContext?.account), browserContext?.host || null])).digest('hex').slice(0, 16) - : null; + const completeContext = hasCompleteBrowserContext(browserContext); const baseApprovalDir = typeof caps.approvalDir === 'string' && caps.approvalDir ? caps.approvalDir : APPROVAL_DIR; - const approvalDir = contextTag ? path.join(baseApprovalDir, `ctx-${contextTag}`) : baseApprovalDir; + const approvalDir = completeContext ? browserContextDirectory(baseApprovalDir, browserContext) : null; const baseJournalDir = typeof caps.tabJournalDir === 'string' && caps.tabJournalDir ? caps.tabJournalDir : JOURNAL_DIR; - const journalDir = contextTag ? path.join(baseJournalDir, `ctx-${contextTag}`) : baseJournalDir; + const journalDir = completeContext ? browserContextDirectory(baseJournalDir, browserContext) : null; - const approvals = createApprovals({ + const approvals = completeContext ? createApprovals({ ttlMs: Number.isSafeInteger(caps.approvalTtlMs) ? caps.approvalTtlMs : undefined, dir: approvalDir, - }); - const tabJournal = createTabJournal({ + }) : null; + const tabJournal = completeContext ? createTabJournal({ dir: journalDir, - }); + }) : null; const session = createBrowseSession({ spawnAside: spawner, resolveAside: resolver, signal, breaker, approvals, tabJournal, browserContext }); const captureManyImpl = createCaptureMany({ session, assertInside }); // Not u/0. Aside runs as whichever profile accounts.json calls current, and on a machine @@ -66,7 +62,7 @@ export function createBrowse({ config = {}, spawnAside, resolveAside, signal, en const asideHome = path.join(env.USERPROFILE || env.HOME || os.homedir() || '', '.aside'); const baseAccountRoot = resolveAccountRoot(asideHome, browserContext?.account); const accountRoot = buildCacheIdentity(baseAccountRoot, browserContext); - const cache = createCache({ ttlMs: Number.isSafeInteger(caps.cacheTtlMs) ? caps.cacheTtlMs : undefined }); + const cache = completeContext ? createCache({ ttlMs: Number.isSafeInteger(caps.cacheTtlMs) ? caps.cacheTtlMs : undefined }) : null; async function probe() { let resolved = null; @@ -90,6 +86,7 @@ export function createBrowse({ config = {}, spawnAside, resolveAside, signal, en e.code = 'EDISABLED'; throw e; } + requireLocalArtifacts(browserContext, 'browse.captureMany'); return captureManyImpl(urls, { ...opts, browseCaps: caps }); } @@ -110,6 +107,7 @@ export function createBrowse({ config = {}, spawnAside, resolveAside, signal, en // user's own tabs out of this. async function leakedTabs() { if (caps.enabled !== true) throw disabledError(); + if (!tabJournal) return { ok: false, code: 'EUNRESOLVEDCONTEXT', tabs: [], error: 'tab ownership requires both account and host; inherited identity is unverified' }; const listed = await attachImpl.tabs(); if (!listed || listed.ok !== true) { return { ok: false, code: listed && listed.code ? listed.code : 'ENOTABS', error: 'could not read the open tabs, so nothing can be called abandoned', tabs: [] }; @@ -127,6 +125,7 @@ export function createBrowse({ config = {}, spawnAside, resolveAside, signal, en async function approve(opts = {}) { if (caps.enabled !== true) throw disabledError(); const id = requireApprovalId('browse.approve', opts); + if (!approvals) throw unresolvedApprovalError(); const existing = approvals.read(id); const selected = routingReport(browserContext); if (existing.record?.context && (existing.record.context.account !== selected.account || existing.record.context.host !== selected.host)) { @@ -157,6 +156,7 @@ export function createBrowse({ config = {}, spawnAside, resolveAside, signal, en async function reject(opts = {}) { if (caps.enabled !== true) throw disabledError(); const id = requireApprovalId('browse.reject', opts); + if (!approvals) throw unresolvedApprovalError(); const done = approvals.reject(id); // A claimed record is never reported as rejected. By then the steps may have run, and // saying otherwise is the one wrong answer this surface can give. @@ -192,6 +192,12 @@ export function createBrowse({ config = {}, spawnAside, resolveAside, signal, en export { CAPABILITY_MATRIX }; +function unresolvedApprovalError() { + const error = new Error('persistent approvals require both account and host; inherited identity is unverified'); + error.code = 'EUNRESOLVEDCONTEXT'; + return error; +} + function disabledError() { const e = new Error(`browse is turned off on this machine. Turn it back on with: ${ENABLE_BROWSE_COMMAND}`); e.code = 'EDISABLED'; diff --git a/src/host/browse/session.js b/src/host/browse/session.js index 304e11c..0b52203 100644 --- a/src/host/browse/session.js +++ b/src/host/browse/session.js @@ -13,7 +13,6 @@ import { compile, deadlineMath, WIRE_LIMIT } from './script.js'; import { attachDiff } from './diff.js'; import { helperStamp } from './helper-bundle.js'; import { DEAD_END } from './policy.js'; -import { createTabJournal } from './tab-journal.js'; import { lossMarkers } from './result-contract.js'; import { browserContextToArgv, routingReport } from '../../browser-context.js'; @@ -357,7 +356,7 @@ export function buildRunSource(job, { plan = null, runId = null, requested = nul return compile({ ...job, runId }, rows); } -export function createBrowseSession({ spawnAside, resolveAside, now = Date.now, signal, breaker = null, approvals = null, tabJournal = createTabJournal(), browserContext = null } = {}) { +export function createBrowseSession({ spawnAside, resolveAside, now = Date.now, signal, breaker = null, approvals = null, tabJournal = null, browserContext = null } = {}) { if (typeof spawnAside !== 'function') throw new TypeError('spawnAside is required'); if (typeof resolveAside !== 'function') throw new TypeError('resolveAside is required'); diff --git a/src/host/namespaces.js b/src/host/namespaces.js index 8a4b7a2..03c6373 100644 --- a/src/host/namespaces.js +++ b/src/host/namespaces.js @@ -5,12 +5,13 @@ import { createAsideSpawner } from './browse/spawn.js'; import { createReport as createReportCore } from './report/report.js'; import { createApi } from './browse/adapters.js'; import { ENABLE_BROWSE_COMMAND } from '../enable-browse.js'; +import { requireLocalArtifacts } from '../browser-context.js'; -export function createReport({ config = {}, signal, assertInside, env = process.env, browserContext = null } = {}) { +export function createReport({ config = {}, signal, assertInside, env = process.env, browserContext = null, spawnAside, resolveAside } = {}) { const caps = config.browseCaps || {}; const session = createBrowseSession({ - spawnAside: createAsideSpawner(), - resolveAside: createAsideResolver(config, env), + spawnAside: spawnAside || createAsideSpawner(), + resolveAside: resolveAside || createAsideResolver(config, env), signal, browserContext, }); @@ -22,6 +23,7 @@ export function createReport({ config = {}, signal, assertInside, env = process. e.code = 'EDISABLED'; throw e; } + requireLocalArtifacts(browserContext, 'report.build'); return core.build({ ...opts, browseCaps: caps }); }, }); diff --git a/structure/batch-contract.md b/structure/batch-contract.md index 7425325..dd589ca 100644 --- a/structure/batch-contract.md +++ b/structure/batch-contract.md @@ -37,7 +37,10 @@ reports per-item facts; it never decides whether the run succeeded. Both batch and raw native calls prepend the execution's account/host selectors before `repl`. Returned routing metadata describes those selections, not a live identity attestation. Persistent approvals, tab journals and browser caches are separated by routing context so selecting another -account or host cannot reuse the first context's approval or tab ownership. +account or host cannot reuse the first context's approval or tab ownership. Both selectors must be +specified, either per-call or by config. Incomplete contexts, including the empty default, disable +shared cache and persistent approvals/journals. Session construction does not create an unscoped +journal; the browse factory supplies one only for a complete context. `itemStatus` reads codes before `ok`, because a host-killed item and an ordinary failure both carry `ok: false` while an item whose action list half ran arrives with `ok: true`. Reading `ok` first diff --git a/structure/browse-surface.md b/structure/browse-surface.md index c80b14b..dbeb96c 100644 --- a/structure/browse-surface.md +++ b/structure/browse-surface.md @@ -4,6 +4,22 @@ Everything under `src/host/browse/` exists to turn a list of urls into rows with the caller cannot account for. One module spawns Aside, one compiles the script, one validates the job, and the rest are the checks that keep a batch honest. +## Routing state and local artifacts + +The execution's `browserContext` scopes the cache, approval store and tab journal. Both account +and host must be specified, either by config or per-call. Any incomplete context, including the +empty default, disables shared cache and persistent approvals/journals: inherited identity can +change between calls, and local accounts.json cannot prove remote identity. Browser reads still +inherit Aside defaults when selectors are omitted. `browse.context()` still reports requested +routing, not a verified identity. `approve`, `reject` and `leakedTabs` report `EUNRESOLVEDCONTEXT` +when persistent identity is unavailable. Configure both selectors and obtain fresh observations +and approvals; old unscoped records are not reused. + +`browse.captureMany` and `report.build` require explicit host `local` before spawning. An omitted +host can inherit a remote device even with an explicit account, so it is refused with +`EREMOTEARTIFACT` too. There is no verified transfer path for remote session files. Textual browser +operations remain available on remote hosts; their session paths never authorize local file reads. + ## The job, validated before anything spawns `schema.js` is the option source of truth. Unknown keys are rejected with the valid list, and a key diff --git a/structure/overview.md b/structure/overview.md index 20db812..2e075fe 100644 --- a/structure/overview.md +++ b/structure/overview.md @@ -49,7 +49,10 @@ optional `account` and `host` fields. The selection is scoped to one execution's not local file access. Omitted selectors inherit native Aside defaults, which are not mutated. `src/browser-context.js` validates selectors and builds native argv. Routing metadata describes the selection, not a verified account identity or resolved remote machine. Invalid selectors -and unknown execution arguments fail before guest execution. +and unknown execution arguments fail before guest execution. All CLI modes validate their allowed +options before dispatch, including before enable-browse or install-MCP writes. Mixed modes fail. +Enable-browse accepts only `--json`; install-MCP accepts `--account`, `--json`, `--force`, and +`--no-discovery`. Host selectors reject leading dashes and control characters on CLI, config and MCP. ## What the guest may reach diff --git a/test/browse-approvals.test.js b/test/browse-approvals.test.js index 3dc6f5e..15cab41 100644 --- a/test/browse-approvals.test.js +++ b/test/browse-approvals.test.js @@ -9,6 +9,7 @@ import os from 'node:os'; import path from 'node:path'; import { createBrowse } from '../src/host/browse/browse.js'; import { createApprovals, isApprovalId } from '../src/host/browse/approvals.js'; +import { browserContextDirectory, resolveBrowserContext } from '../src/browser-context.js'; import { readFileSync, writeFileSync, statSync } from 'node:fs'; const FINAL = JSON.stringify({ @@ -23,12 +24,14 @@ let harnessSeq = 0; function harness() { const spawns = []; const approvalDir = path.join(os.tmpdir(), 'codemode-approvals-test-' + process.pid + '-h' + (harnessSeq += 1)); + const browserContext = resolveBrowserContext({ mcpArgs: { account: 'u1', host: 'local' } }); const browse = createBrowse({ + browserContext, config: { browseCaps: { enabled: true, approvalDir } }, resolveAside: async () => 'C:/fake/aside.exe', spawnAside: async (bin, args) => { spawns.push(args); return { stdout: FINAL, killed: false }; }, }); - return { browse, spawns, approvalDir }; + return { browse, spawns, approvalDir: browserContextDirectory(approvalDir, browserContext) }; } const writingJob = (url = 'https://a.test/1') => ({ urls: [url], refsFingerprint: 'r1-test', actions: [{ ref: 'e1', click: true }] }); diff --git a/test/browser-context-hardening.test.js b/test/browser-context-hardening.test.js new file mode 100644 index 0000000..6ca50e0 --- /dev/null +++ b/test/browser-context-hardening.test.js @@ -0,0 +1,162 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, readdirSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createBrowse } from '../src/host/browse/browse.js'; +import { createReport } from '../src/host/namespaces.js'; +import { createApprovals } from '../src/host/browse/approvals.js'; +import { createTabJournal } from '../src/host/browse/tab-journal.js'; +import { buildCacheIdentity, browserContextDirectory, resolveBrowserContext, validateHost } from '../src/browser-context.js'; +import { createToolHandler } from '../src/tools.js'; +import { createHostGlobals } from '../src/host/globals.js'; +import { makeRootGuard } from '../src/paths.js'; + +const cli = fileURLToPath(new URL('../bin/codemode.mjs', import.meta.url)); +const writingJob = { urls: ['https://a.test'], actions: [{ selector: 'button', click: true }] }; +const response = (payload) => ({ stdout: JSON.stringify({ type: 'final', ...payload }) + '\n[ok | 1ms]' }); + +test('host validation refuses option injection and control characters through CLI and MCP', async () => { + let calls = 0; + const handler = createToolHandler({ + config: { maxTimeoutMs: 1000, maxResultBytes: 65536 }, + globals: () => { calls++; return {}; }, + }); + for (const host of ['-p', '-h', ' remote\n', 'remote\tname', 'remote\u007fname', '\u0000local']) { + assert.throws(() => validateHost(host), /--host/); + await assert.rejects(handler({ code: 'return 1', host }), error => error.invalidParams === true); + if (!host.includes('\u0000')) { + const out = spawnSync(process.execPath, [cli, '--host', host, '--code', 'return 1'], { encoding: 'utf8' }); + assert.equal(out.status, 1); + assert.match(out.stdout, /--host/); + } + } + assert.equal(calls, 0); + assert.equal(validateHost('remote device'), 'remote device'); +}); + +test('per-call MCP selectors still override config and reach browse.context', async () => { + const config = { maxTimeoutMs: 1000, maxResultBytes: 65536, browseContext: { account: 'u3', host: 'configured-host' } }; + const handler = createToolHandler({ + config, + globals: (signal, options) => createHostGlobals(config, makeRootGuard([tmpdir()]), signal, options), + }); + const selected = JSON.parse((await handler({ code: 'return browse.context()', account: 'u1', host: 'local' })).content[0].text); + assert.equal(selected.result.account, 'u1'); + assert.equal(selected.result.host, 'local'); + assert.equal(selected.browserContext.accountSource, 'explicit'); + const configured = JSON.parse((await handler({ code: 'return browse.context()' })).content[0].text); + assert.equal(configured.result.account, 'u3'); + assert.equal(configured.result.host, 'configured-host'); + assert.equal(configured.browserContext.hostSource, 'config'); +}); + +test('explicit local host still permits capture execution', async () => { + let spawns = 0; + const browserContext = resolveBrowserContext({ mcpArgs: { account: 'u1', host: 'local' } }); + const browse = createBrowse({ + browserContext, config: { browseCaps: { enabled: true } }, + resolveAside: async () => 'aside', + spawnAside: async () => { spawns++; return response({ items: [{ jobId: 'j000', url: 'https://a.test', ok: true }], partial: [], leakedUrls: [] }); }, + }); + assert.equal((await browse.captureMany(['https://a.test'])).ok, true); + assert.equal(spawns, 1); +}); + +for (const context of [{}, { account: 'u1' }, { host: 'remote-a' }]) { + test(`unresolved/remote host refuses local artifacts: ${JSON.stringify(context)}`, async () => { + let spawns = 0; + const config = { browseCaps: { enabled: true } }; + const browserContext = resolveBrowserContext({ mcpArgs: context }); + const browse = createBrowse({ config, browserContext, resolveAside: async () => 'aside', spawnAside: async () => { spawns++; return response({}); } }); + await assert.rejects(browse.captureMany(['https://a.test'], { outDir: tmpdir() }), error => error.code === 'EREMOTEARTIFACT' && /explicit.*local/.test(error.message)); + await assert.rejects(createReport({ config, browserContext }).build({ outFile: 'unused.pdf' }), error => error.code === 'EREMOTEARTIFACT'); + assert.equal(spawns, 0); + }); +} + +for (const context of [{}, { account: 'u1' }, { host: 'local' }]) { + test(`incomplete context never reuses browser state: ${JSON.stringify(context)}`, async (t) => { + const dir = mkdtempSync(path.join(tmpdir(), 'cm-unresolved-review-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const approvalDir = path.join(dir, 'approvals'); + const tabJournalDir = path.join(dir, 'tabs'); + // Legacy records are deliberately present. Neither matching the local account nor + // seeing the same target id establishes the identity of an inherited remote host. + const legacy = createApprovals({ dir: approvalDir }); + const held = legacy.open({ job: writingJob, wants: ['click'], urls: writingJob.urls }); + const journal = createTabJournal({ dir: tabJournalDir }); + journal.record({ runId: 'legacy-run', stdout: JSON.stringify({ type: 'tab', ev: 'open', targetId: 'same-id', url: 'https://a.test' }) }); + const before = readdirSync(tabJournalDir); + let selected = 'identity-a'; + let spawns = 0; + const make = () => createBrowse({ + config: { browseCaps: { enabled: true, approvalDir, tabJournalDir } }, + browserContext: resolveBrowserContext({ mcpArgs: context }), + resolveAside: async () => 'aside', + spawnAside: async () => { + spawns++; + return response({ rows: [{ url: 'https://a.test', title: selected }] }); + }, + }); + assert.equal(buildCacheIdentity('/local/u/0', context), null); + assert.throws(() => browserContextDirectory(dir, context), /requires both account and host/); + const browse = make(); + const query = 'isolated-' + path.basename(dir); + const first = await browse.searchMany([query], { engine: 'youtube' }); + assert.equal(first.items[0].rows[0].title, 'identity-a'); + selected = 'identity-b'; + const second = await browse.searchMany([query], { engine: 'youtube' }); + const third = await make().searchMany([query], { engine: 'youtube' }); + assert.equal(second.items[0].rows[0].title, 'identity-b'); + assert.equal(third.items[0].rows[0].title, 'identity-b'); + assert.equal(spawns, 3, 'no cache reuse even within the same host-globals instance'); + const refusal = await browse.exec(writingJob); + assert.equal(refusal.approvalId, null); + for (const method of ['approve', 'reject']) { + await assert.rejects(browse[method]({ approvalId: held.approvalId }), error => error.code === 'EUNRESOLVEDCONTEXT'); + } + assert.equal(legacy.read(held.approvalId).state, 'pending'); + assert.equal((await browse.leakedTabs()).code, 'EUNRESOLVEDCONTEXT'); + assert.equal(spawns, 3, 'approval and journal lookups start no browser work'); + assert.deepEqual(readdirSync(tabJournalDir), before); + assert.equal(readdirSync(path.join(approvalDir, 'pending')).length, 1); + }); +} + +for (const mode of ['--enable-browse', '--install-mcp']) { + test(`${mode} validates all flags before changing settings`, (t) => { + const dir = mkdtempSync(path.join(tmpdir(), 'cm-argv-review-')); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const configDir = path.join(dir, 'codemode'); + const accountDir = path.join(dir, 'u', '1'); + mkdirSync(configDir, { recursive: true }); + mkdirSync(accountDir, { recursive: true }); + const config = path.join(configDir, 'config.json'); + const settings = path.join(accountDir, 'settings.json'); + const configBefore = '{"browseCaps":{"enabled":false},"untouched":true}\n'; + const settingsBefore = '{"mcp":{"servers":{}},"untouched":true}\n'; + writeFileSync(config, configBefore); + writeFileSync(settings, settingsBefore); + writeFileSync(path.join(dir, 'accounts.json'), '{"currentAccountId":1,"accounts":[{"id":1}]}'); + const invalid = [ + ['--typo'], ['--host', 'remote-a'], ['--config', 'ignored.json'], + ['--code', 'return 1'], ['--doctor'], ['--account'], ['--host', ''], + [mode === '--enable-browse' ? '--install-mcp' : '--enable-browse'], + ]; + if (mode === '--enable-browse') invalid.push(['--account', 'u1'], ['--force']); + for (const flags of invalid) { + const out = spawnSync(process.execPath, [cli, mode, ...flags], { + encoding: 'utf8', timeout: 5000, + env: { ...process.env, XDG_CONFIG_HOME: dir, ASIDE_HOME: dir, CODEMODE_ASIDE_CLI: path.join(dir, 'must-not-run'), CODEMODE_IGNORE_REPO_CONFIG: '1' }, + }); + assert.equal(out.status, 1, JSON.stringify(flags) + out.stdout + out.stderr); + assert.equal(JSON.parse(out.stdout).code, 'EBADARGV'); + assert.equal(readFileSync(config, 'utf8'), configBefore); + assert.equal(readFileSync(settings, 'utf8'), settingsBefore); + assert.deepEqual(readdirSync(accountDir), ['settings.json'], 'no backup or new settings file'); + } + }); +} diff --git a/test/browser-routing.test.js b/test/browser-routing.test.js index 767b492..c3c81b3 100644 --- a/test/browser-routing.test.js +++ b/test/browser-routing.test.js @@ -2,7 +2,7 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { existsSync, mkdtempSync, writeFileSync, readFileSync } from 'node:fs'; +import { existsSync, mkdtempSync, rmSync, writeFileSync, readFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; import http from 'node:http'; @@ -449,46 +449,28 @@ test('cache identity is partitioned per account and host context via readText', } }); -test('report.build session receives routing context and passes context to spawn', async () => { +test('report.build session receives routing context and passes context to spawn', async (t) => { const dir = mkdtempSync(path.join(tmpdir(), 'codemode-report-route-')); - const logFile = path.join(dir, 'argv.json'); - const fakeBin = path.join(dir, 'fake-aside'); - - const script = `#!/usr/bin/env node -import { writeFileSync } from 'node:fs'; -if (process.argv.includes('--version')) { - console.log('1.26.0'); - process.exit(0); -} -writeFileSync('${logFile}', JSON.stringify(process.argv.slice(2))); -console.log(JSON.stringify({ - type: 'final', - items: [{ jobId: 'j000', url: 'http://loopback', ok: true }], - partial: [], - leakedUrls: [] -})); -console.log('[ok | 5ms]'); -`; - - writeFileSync(fakeBin, script, { mode: 0o755 }); - + t.after(() => rmSync(dir, { recursive: true, force: true })); + let recordedArgs; + // Exercise the real report -> session -> transport path without an extensionless + // ESM/shebang executable, which is not portable to Node 18 or Windows. const report = createReport({ - config: { - browseCaps: { enabled: true }, - asidePath: fakeBin, - }, + config: { browseCaps: { enabled: true } }, assertInside: (p) => p, - browserContext: { account: 'u9', host: 'remote-box' }, + browserContext: { account: 'u9', host: 'local' }, + resolveAside: async () => 'fixture-aside', + spawnAside: async (_bin, args) => { + recordedArgs = args; + return { stdout: JSON.stringify({ type: 'final', items: [], partial: [], leakedUrls: [] }) + '\n[ok | 5ms]' }; + }, }); - try { await report.build({ items: [], outFile: path.join(dir, 'out.pdf') }); - } catch (_) { - // Expected post-spawn verification error since fakeBin produces mock output + } catch (error) { + assert.ok(recordedArgs, 'failure occurred before report transport: ' + error.message); } - - assert.ok(existsSync(logFile), 'report.build must spawn the Aside binary'); - const recordedArgs = JSON.parse(readFileSync(logFile, 'utf8')); + assert.ok(recordedArgs, 'report.build must spawn the Aside binary'); const replIdx = recordedArgs.indexOf('repl'); assert.notEqual(replIdx, -1, 'argv must contain "repl"'); @@ -501,7 +483,7 @@ console.log('[ok | 5ms]'); assert.ok(acctIdx < replIdx, '--account must appear before "repl"'); assert.notEqual(hostIdx, -1, 'report session spawn argv must contain "--host"'); - assert.equal(recordedArgs[hostIdx + 1], 'remote-box', '--host value must be remote-box'); + assert.equal(recordedArgs[hostIdx + 1], 'local', '--host value must be local for artifact materialization'); assert.ok(hostIdx < replIdx, '--host must appear before "repl"'); }); diff --git a/test/completeness-invariant.test.js b/test/completeness-invariant.test.js index 55298d0..e43a31a 100644 --- a/test/completeness-invariant.test.js +++ b/test/completeness-invariant.test.js @@ -119,6 +119,7 @@ function childOutput(item) { function browseWith(item, extraCaps = {}) { return createBrowse({ + browserContext: { account: 'u1', host: 'local', accountSource: 'explicit', hostSource: 'explicit' }, config: { browseCaps: { enabled: true, ...extraCaps } }, resolveAside, spawnAside: async () => ({ stdout: childOutput(item), killed: false }),