diff --git a/server.ts b/server.ts index 7c1cfa4..7bf3f72 100644 --- a/server.ts +++ b/server.ts @@ -21,11 +21,14 @@ import { Engine } from "./src/engine"; import { VERDICT_STATUSES, type Run } from "./src/model"; import { createRpcHandlers } from "./src/rpc"; import { ScopeSync } from "./src/scope-sync"; +import type { ThreadScope } from "./src/scopes"; import { pruneShimBundles, isShimInstalled } from "./src/shim"; import { safely, detach } from "./src/safe"; import { MIGRATIONS, Store, type Db } from "./src/store"; import { installSimulators, type SimulatorCliRun } from "./src/sim/wire"; import { CLI_COMMANDS as SIM_VERBS } from "./src/sim/cli"; +import { checkoutHostMismatch } from "./src/build-security"; +import { resolveServerHostId } from "./src/sim/hostcheck"; import { SETTINGS_DESCRIPTORS as SIMULATOR_SETTINGS } from "./src/sim/settings"; import { ThreadSync } from "./src/thread-sync"; import { AGENT_INSTRUCTIONS, createTools } from "./src/tools"; @@ -238,6 +241,49 @@ export default async function plugin(bb: BbPluginApi): Promise { onChanged: publishSoon, }); + /** + * Which machine is this plugin running on? + * + * Derived once (nonce file + `hosts.pathsExist`) and cached, exactly as the + * Stills path does. Only needed when a scope names a host, so it is resolved + * lazily rather than on every startup. + */ + let serverHostId: string | null = null; + const resolveHostId = async (): Promise => { + if (serverHostId !== null) return serverHostId; + const hosts = await bb.sdk.hosts.list(); + serverHostId = await resolveServerHostId({ + pluginDataDir: dataDir, + listHosts: async () => hosts.map((entry) => ({ id: entry.id, name: entry.name })), + pathsExist: async (id, paths) => + (await bb.sdk.hosts.pathsExist({ hostId: id, paths })).existence, + kvGet: async (key) => (await bb.storage.kv.get(key)) ?? null, + kvSet: async (key, value) => bb.storage.kv.set(key, value), + }); + return serverHostId; + }; + + /** + * A tracked build only works on the machine holding the checkout, because + * every path check and the process probe use `node:fs` here. Answer with the + * plugin's sentence instead of letting `realpath` throw ENOENT. + */ + const checkoutElsewhere = async (scope: ThreadScope): Promise => { + if (scope.hostId === null) return null; + try { + const hosts = await bb.sdk.hosts.list(); + return checkoutHostMismatch( + scope, + await resolveHostId(), + hosts.map((entry) => ({ id: entry.id, name: entry.name })), + ); + } catch (error) { + // Never block a build because host discovery failed. + log.debug(`host check skipped: ${String(error)}`); + return null; + } + }; + const engine: Engine = new Engine(store, { projectFor: (signals): string | null => collectorRef ? collectorRef.projectFor(signals) : null, @@ -446,6 +492,7 @@ export default async function plugin(bb: BbPluginApi): Promise { phaseFor, refreshProjectNames: () => dto.refreshProjectNames(), scopeFor: (threadId) => scopeSync.bounded(threadId), + checkoutElsewhere, wrapped, onShimStateKnown, confirmHostAction: async (threadId, consent) => { @@ -729,6 +776,7 @@ export default async function plugin(bb: BbPluginApi): Promise { projectName: (id) => dto.projectName(id), phaseFor, scopeFor: (threadId) => scopeSync.bounded(threadId), + checkoutElsewhere, showRun: (id) => cli.show(id), }); diff --git a/src/build-security.ts b/src/build-security.ts index f09e923..2b9019d 100644 --- a/src/build-security.ts +++ b/src/build-security.ts @@ -2,6 +2,7 @@ import { realpath, stat } from "node:fs/promises"; import { dirname, isAbsolute, relative, resolve } from "node:path"; import { pathIsUnder } from "./scopes"; +import { locateCheckout, type HostSummary } from "./sim/hostcheck"; const PATH_FLAGS = new Set([ "-project", @@ -48,6 +49,38 @@ const HOST_MUTATING_OPTIONS = new Set([ "-collect-test-diagnostics", ]); +/** + * Refuse a tracked build whose checkout is on a different machine. + * + * bb supports a server with enrolled Macs, so a thread's environment can live + * on a host that is not the one running this plugin. Everything below — + * `confinedBuildCwd`, `validateBuildArguments`, the process probe that + * attributes the run — resolves paths with `node:fs` on THIS machine. Given a + * checkout on another host, `realpath` threw first, so the user was told + * "ENOENT: no such file or directory" about a directory that plainly exists on + * the machine they were looking at. + * + * `src/sim/hostcheck.ts` already states the rule for Stills ("a real refusal + * with a real sentence rather than a mysterious 'no such file'"); this applies + * the same rule to tracked builds. + * + * Fails OPEN when either host is unknown: an unresolved identity must never + * refuse the single-machine setup that every existing user has. + */ +export function checkoutHostMismatch( + scope: { hostId: string | null }, + serverHostId: string | null, + hosts: readonly HostSummary[], +): string | null { + const location = locateCheckout(serverHostId, scope.hostId, hosts); + if (location.kind !== "other-host") return null; + return ( + `This thread's checkout lives on ${location.hostName}, but tracked xcodebuild runs ` + + `on the machine running bb. Run the build on ${location.hostName} instead — ` + + `xcodebuild there is not tracked by this plugin.` + ); +} + export async function confinedBuildCwd(root: string, requested?: string): Promise { const realRoot = await realpath(root); const candidate = requested === undefined ? realRoot : resolve(realRoot, requested); diff --git a/src/cli.ts b/src/cli.ts index 035bb96..f9bb16d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -55,6 +55,8 @@ export interface CliDeps { refreshProjectNames(): void; /** Resolve the invoking thread's checkout before host-side execution. */ scopeFor(threadId: string): Promise; + /** Refusal sentence when the checkout is on another machine, else null. */ + checkoutElsewhere(scope: ThreadScope): Promise; wrapped: WrappedDeps; onShimStateKnown(installed: boolean): void; confirmHostAction( @@ -240,6 +242,10 @@ export function createCli(deps: CliDeps) { stderr: "The invoking checkout does not match this thread, so xcodebuild was not started.\n", }; } + // Before any node:fs work: this thread's checkout may live on another + // enrolled Mac, where every path below resolves to nothing. + const elsewhere = await deps.checkoutElsewhere(scope); + if (elsewhere !== null) return { exitCode: 1, stderr: `${elsewhere}\n` }; root = scope.path; workingDir = await confinedBuildCwd(root, ctx.cwd); await validateBuildArguments(argv, root, workingDir); diff --git a/src/scope-sync.ts b/src/scope-sync.ts index 718493c..9cfffd4 100644 --- a/src/scope-sync.ts +++ b/src/scope-sync.ts @@ -47,6 +47,8 @@ export interface ScopeSyncDeps { path?: string | null; projectId?: string | null; branchName?: string | null; + /** Machine the checkout is on; `path` is only local when this is ours. */ + hostId?: string | null; }>; log(message: string): void; isDisposed(): boolean; @@ -116,6 +118,7 @@ export class ScopeSync { projectId: env.projectId ?? null, environmentId, path: env.path, + hostId: env.hostId ?? null, branch: env.branchName ?? null, active, }, diff --git a/src/scopes.ts b/src/scopes.ts index 18c8f38..0bc7370 100644 --- a/src/scopes.ts +++ b/src/scopes.ts @@ -17,6 +17,11 @@ export interface ThreadScope { environmentId: string | null; /** Absolute worktree/checkout path of the thread's environment. */ path: string; + /** + * Machine the checkout lives on. bb supports a server with enrolled Macs, so + * `path` is only meaningful on this host — see `checkoutHostMismatch`. + */ + hostId: string | null; branch: string | null; /** True while the thread is running a turn. */ active: boolean; diff --git a/src/tools.ts b/src/tools.ts index bc108ac..a358247 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -52,6 +52,8 @@ export interface ToolDeps { phaseFor(run: Run): BuildPhase | null; /** Cached scope, or one bounded resolve. Never blocks on a slow SDK call. */ scopeFor(threadId: string): Promise; + /** Refusal sentence when the checkout is on another machine, else null. */ + checkoutElsewhere(scope: ThreadScope): Promise; showRun(id: string): { stdout?: string; stderr?: string }; } @@ -192,6 +194,10 @@ export function createTools(deps: ToolDeps) { if (scope === null) { return "This thread has no resolvable checkout, so xcodebuild was not started."; } + // Before any node:fs work: this thread's checkout may live on another + // enrolled Mac, where every path below resolves to nothing. + const elsewhere = await deps.checkoutElsewhere(scope); + if (elsewhere !== null) return elsewhere; let workingDir: string; try { workingDir = await confinedBuildCwd(scope.path, cwd); diff --git a/test/agent-scope-security.test.ts b/test/agent-scope-security.test.ts index 90d4db8..998645a 100644 --- a/test/agent-scope-security.test.ts +++ b/test/agent-scope-security.test.ts @@ -15,6 +15,7 @@ const SCOPE: ThreadScope = { projectId: "proj_app", environmentId: "env_app", path: "/Users/me/.bb/worktrees/env_app/App", + hostId: null, branch: "feature/security", active: true, updatedAt: NOW, @@ -25,6 +26,7 @@ const OTHER_SCOPE: ThreadScope = { projectId: "proj_other", environmentId: "env_other", path: "/Users/me/Git/Other", + hostId: null, branch: "main", }; @@ -66,7 +68,7 @@ function run(id: string, overrides: Partial = {}): Run { }; } -function fixture() { +function fixture(checkoutRefusal: string | null = null) { const store = makeStore(); store.insertRun(run("r:mine")); store.insertRun( @@ -107,6 +109,7 @@ function fixture() { } as unknown as Collector; const confirmHostAction = vi.fn(async () => false); const common = { + checkoutElsewhere: async () => checkoutRefusal, store, collector, dataDir: "/tmp/xcode-security-test", @@ -133,6 +136,36 @@ function fixture() { return { cli, confirmHostAction, tools }; } +describe("a checkout on another machine is refused with a sentence", () => { + const REFUSAL = + "This thread's checkout lives on scw-mini, but tracked xcodebuild runs on the machine running bb."; + + it("refuses the agent build tool before touching the filesystem", async () => { + const { tools, confirmHostAction } = fixture(REFUSAL); + const result = await tools.build.execute( + { args: ["-scheme", "App"] }, + { threadId: SCOPE.threadId, signal: new AbortController().signal }, + ); + expect(result).toBe(REFUSAL); + // The old failure was an ENOENT thrown by realpath; nothing should have + // got as far as asking the user to approve a host action. + expect(result).not.toContain("ENOENT"); + expect(confirmHostAction).not.toHaveBeenCalled(); + }); + + it("lets a checkout on this machine through the host gate", async () => { + const { tools } = fixture(null); + const result = await tools.build.execute( + { args: ["-scheme", "App"] }, + { threadId: SCOPE.threadId, signal: new AbortController().signal }, + ); + // It gets past the host gate and on to the real build path; whatever it + // fails on next, it must not be the cross-machine refusal. + expect(result).not.toBe(REFUSAL); + expect(result).not.toContain("lives on"); + }); +}); + describe("agent Xcode surfaces fail closed to the invoking thread", () => { it("removes the machine-wide escape hatch from agent tool schemas", () => { const { tools } = fixture(); diff --git a/test/checkout-host.test.ts b/test/checkout-host.test.ts new file mode 100644 index 0000000..bd16aaa --- /dev/null +++ b/test/checkout-host.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; + +import { checkoutHostMismatch } from "../src/build-security"; + +const HOSTS = [ + { id: "host_server", name: "Vedrans-MacBook-Pro" }, + { id: "host_other", name: "scw-mini" }, +]; + +describe("checkoutHostMismatch", () => { + it("allows a checkout on the machine running the plugin", () => { + expect(checkoutHostMismatch({ hostId: "host_server" }, "host_server", HOSTS)).toBeNull(); + }); + + it("refuses a checkout on another host and names that host", () => { + const refusal = checkoutHostMismatch({ hostId: "host_other" }, "host_server", HOSTS); + expect(refusal).toContain("scw-mini"); + // The point of the fix: an actionable sentence, never a bare ENOENT. + expect(refusal).not.toContain("ENOENT"); + }); + + it("falls back to a generic name when the host is not in the list", () => { + const refusal = checkoutHostMismatch({ hostId: "host_ghost" }, "host_server", HOSTS); + expect(refusal).toContain("another machine"); + }); + + it("stays out of the way when either host is unknown", () => { + // Never refuse a working single-machine setup just because identity is + // unresolved — that would be a regression for every existing user. + expect(checkoutHostMismatch({ hostId: null }, "host_server", HOSTS)).toBeNull(); + expect(checkoutHostMismatch({ hostId: "host_other" }, null, HOSTS)).toBeNull(); + }); +}); diff --git a/test/scopes.test.ts b/test/scopes.test.ts index 9cdf034..5c615b5 100644 --- a/test/scopes.test.ts +++ b/test/scopes.test.ts @@ -21,6 +21,7 @@ function makeScopes(): ThreadScopes { projectId: "proj_1", environmentId: "env_1", path: "/Users/me/.bb/worktrees/env_app/App", + hostId: null, branch: "feature/login", active: true, }, @@ -32,6 +33,7 @@ function makeScopes(): ThreadScopes { projectId: "proj_1", environmentId: "env_2", path: "/Users/me/Git/App", + hostId: null, branch: "main", active: false, }, @@ -66,6 +68,7 @@ describe("ThreadScopes.threadFor", () => { projectId: "proj_1", environmentId: "env_3", path: "/Users/me/Git/App/Modules/Kit", + hostId: null, branch: "main", active: false, }, @@ -84,6 +87,7 @@ describe("ThreadScopes.threadFor", () => { projectId: "proj_1", environmentId: "env_1", path: "/Users/me/.bb/worktrees/env_app/App", + hostId: null, branch: "feature/login", active: false, }, @@ -110,6 +114,7 @@ describe("runMatchesScope", () => { const scope = { threadId: "th_app", path: "/Users/me/.bb/worktrees/env_app/App", + hostId: null, branch: "feature/login", }; const base = { @@ -205,6 +210,7 @@ describe("scopeFilter", () => { const scope = { threadId: "thr_mine", path: "/Users/v/.bb/worktrees/env_mine/indexed", + hostId: null, branch: "feature", }; const foreign = {