diff --git a/.env.example b/.env.example index 406ed4c259..fa6df90b3d 100644 --- a/.env.example +++ b/.env.example @@ -25,9 +25,17 @@ SESSION_SECRET=insecure-dev-only-session-secret-0000 # Directory where the hub keeps durable repo and asset state. Created # on boot if absent; relative paths resolve against the hub's working -# directory. +# directory. The default below lives inside this git work tree, so local +# `bun run dev` also needs HUB_ALLOW_GIT_INSIDE_WORK_TREE. Tests must +# not use this path — they install test/disposable-hub-data-dir.ts. HUB_DATA_DIR=.data/hub +# Opt-in to initialize hub git-on-disk state inside an existing git work +# tree. Required for the default HUB_DATA_DIR above. Never set this in +# tests; a missed nested init walks up to this repo's .git and a genesis +# commit can land on the working branch and delete the checkout. +HUB_ALLOW_GIT_INSIDE_WORK_TREE=1 + # Directory of built user-interface files the hub serves at BASE_URL. # Relative paths resolve against the hub's working directory (apps/hub # under `bun run dev`), so ../web/dist is the web app's build output; diff --git a/AGENTS.md b/AGENTS.md index 838dd8a012..28550bab67 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -90,6 +90,17 @@ behind human approval. To check whether a workspace package exists, look in `packages/`, not `node_modules` — an absent `node_modules` entry means "not installed yet," not "doesn't exist." +- Bun loads repo-root `.env`, so an unset test inherits + `HUB_DATA_DIR=.data/hub` inside this work tree. The hub's git-on-disk + init then walks up to the enclosing `.git`, and a genesis commit can + land on the working branch and delete the checkout. Tests that boot + the hub or exercise seed/deploy git paths must call + `installDisposableHubDataDir()` from `test/disposable-hub-data-dir.ts` + (it mktemps outside any work tree and clears + `HUB_ALLOW_GIT_INSIDE_WORK_TREE`). Local `bun run dev` may set that + opt-in; tests must not. Run those suites with + `HUB_DATA_DIR=$(mktemp -d)` as a belt-and-suspenders override. Never + delete the `HUB_DATA_DIR` env key itself. - `scripts/checks/*` are heuristics over source text, not proof — they can and do false-positive (install artifacts read as vendored trees, comments read as imports, class names read as user copy). A check failure is a diff --git a/apps/hub/src/asset-service-factory.test.ts b/apps/hub/src/asset-service-factory.test.ts new file mode 100644 index 0000000000..97a2f2ea36 --- /dev/null +++ b/apps/hub/src/asset-service-factory.test.ts @@ -0,0 +1,66 @@ +import { afterAll, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { createBootAssetWiring } from "./asset-service-factory"; +import { HubDataDirInsideGitWorkTreeError } from "./hub-data-dir-git-safety"; + +const scratch: string[] = []; + +afterAll(() => { + for (const dir of scratch) rmSync(dir, { recursive: true, force: true }); +}); + +function scratchDir(prefix: string): string { + const dir = mkdtempSync(path.join(tmpdir(), prefix)); + scratch.push(dir); + return dir; +} + +function git(cwd: string, args: readonly string[]): void { + const env = { ...process.env }; + delete env["GIT_DIR"]; + delete env["GIT_WORK_TREE"]; + delete env["GIT_INDEX_FILE"]; + env["GIT_AUTHOR_NAME"] = "safety-test"; + env["GIT_AUTHOR_EMAIL"] = "safety@test"; + env["GIT_COMMITTER_NAME"] = "safety-test"; + env["GIT_COMMITTER_EMAIL"] = "safety@test"; + const result = Bun.spawnSync(["git", "-c", "core.hooksPath=", ...args], { + cwd, + stdout: "pipe", + stderr: "pipe", + env, + }); + if (result.exitCode !== 0) { + throw new Error( + `git ${args.join(" ")} failed: ${result.stderr.toString()}`, + ); + } +} + +test("createBootAssetWiring refuses a data dir inside an existing git work tree", async () => { + const workTree = scratchDir("boot-asset-inside-"); + git(workTree, ["init", "-b", "main"]); + const dataDir = path.join(workTree, ".data", "hub"); + mkdirSync(dataDir, { recursive: true }); + await expect( + createBootAssetWiring({ db: {} as never, dataDir }), + ).rejects.toBeInstanceOf(HubDataDirInsideGitWorkTreeError); +}); + +test("createBootAssetWiring opt-in skips the work-tree refusal", async () => { + const workTree = scratchDir("boot-asset-opt-in-"); + git(workTree, ["init", "-b", "main"]); + const dataDir = path.join(workTree, ".data", "hub"); + mkdirSync(dataDir, { recursive: true }); + try { + await createBootAssetWiring({ + db: {} as never, + dataDir, + allowGitInsideWorkTree: true, + }); + } catch (error) { + expect(error).not.toBeInstanceOf(HubDataDirInsideGitWorkTreeError); + } +}); diff --git a/apps/hub/src/asset-service-factory.ts b/apps/hub/src/asset-service-factory.ts index b28724780a..00e8857f54 100644 --- a/apps/hub/src/asset-service-factory.ts +++ b/apps/hub/src/asset-service-factory.ts @@ -12,6 +12,7 @@ import { } from "@intx/hub-sessions"; import { generateKeyPair } from "@intx/crypto"; import type { DB } from "@intx/db"; +import { assertHubDataDirGitSafety } from "./hub-data-dir-git-safety"; // Host policy constant, not configuration — shared by boot (tool-package // registry routing) and asset-service construction. @@ -35,7 +36,12 @@ export interface BootAssetWiring { export async function createBootAssetWiring(args: { db: DB["db"]; dataDir: string; + allowGitInsideWorkTree?: boolean; }): Promise { + assertHubDataDirGitSafety( + args.dataDir, + args.allowGitInsideWorkTree === true ? { allowInsideWorkTree: true } : {}, + ); const signingKey = await generateKeyPair(); const agentRepoStore = createAgentRepoStore({ dataDir: args.dataDir, diff --git a/apps/hub/src/config.ts b/apps/hub/src/config.ts index adb73fc8e9..dff805f42f 100644 --- a/apps/hub/src/config.ts +++ b/apps/hub/src/config.ts @@ -188,6 +188,9 @@ const HubEnv = type({ "ALLOW_UNVERIFIED_EMAILS?": type("'1' | 'true'").describe( "dev/test-only opt-in to let @workbench/access-policy trust an email that better-auth has not verified — self-signup domain checks and pending-invite redemption normally require emailVerified; never set this for a real deployment", ), + "HUB_ALLOW_GIT_INSIDE_WORK_TREE?": type("'1' | 'true'").describe( + "opt-in to initialize hub git-on-disk state inside a directory that is already a git work tree; refused by default because a nested init that misses its own .git walks up and commits onto the enclosing working branch", + ), "SIDECAR_PROVISIONERS?": type("string").describe( "comma-separated sidecar-allocation backend ids to register for workbenches placed on their own exclusive sidecar, e.g. 'docker'; unset or empty (default) keeps the hub on its current single shared sidecar with no exclusive-placement backend available", ), @@ -357,6 +360,8 @@ export type HubConfig = { /** Dev/test-only opt-in to skip @workbench/access-policy's email- * verification requirement. */ readonly allowUnverifiedEmails: boolean; + /** Opt-in to initialize git-on-disk state inside an existing git work tree. */ + readonly allowGitInsideWorkTree?: boolean; /** Dev/test-only override for the routine scheduler's poll interval — * see `ROUTINE_SCHEDULER_POLL_INTERVAL_MS` above. Unset runs the real * production cadence (`routine-scheduler.ts`'s own default). */ @@ -666,6 +671,8 @@ export function readHubConfig( }; if (parsed.OPERATOR_TENANT_ID !== undefined) hubConfig.operatorTenantId = parsed.OPERATOR_TENANT_ID; + if (parsed.HUB_ALLOW_GIT_INSIDE_WORK_TREE !== undefined) + hubConfig.allowGitInsideWorkTree = true; if (parsed.PORT !== undefined) hubConfig.listenPort = Number(parsed.PORT); if (parsed.ROUTINE_SCHEDULER_POLL_INTERVAL_MS !== undefined) hubConfig.routineSchedulerPollIntervalMs = Number( diff --git a/apps/hub/src/hub-data-dir-git-safety.test.ts b/apps/hub/src/hub-data-dir-git-safety.test.ts new file mode 100644 index 0000000000..904ddadd3a --- /dev/null +++ b/apps/hub/src/hub-data-dir-git-safety.test.ts @@ -0,0 +1,90 @@ +import { afterAll, expect, test } from "bun:test"; +import { + mkdirSync, + mkdtempSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { + assertHubDataDirGitSafety, + enclosingGitWorkTree, + HubDataDirInsideGitWorkTreeError, +} from "./hub-data-dir-git-safety"; + +const scratch: string[] = []; + +afterAll(() => { + for (const dir of scratch) rmSync(dir, { recursive: true, force: true }); +}); + +function scratchDir(prefix: string): string { + const dir = mkdtempSync(path.join(tmpdir(), prefix)); + scratch.push(dir); + return dir; +} + +function git(cwd: string, args: readonly string[]): void { + const env = { ...process.env }; + delete env["GIT_DIR"]; + delete env["GIT_WORK_TREE"]; + delete env["GIT_INDEX_FILE"]; + env["GIT_AUTHOR_NAME"] = "safety-test"; + env["GIT_AUTHOR_EMAIL"] = "safety@test"; + env["GIT_COMMITTER_NAME"] = "safety-test"; + env["GIT_COMMITTER_EMAIL"] = "safety@test"; + const result = Bun.spawnSync(["git", "-c", "core.hooksPath=", ...args], { + cwd, + stdout: "pipe", + stderr: "pipe", + env, + }); + if (result.exitCode !== 0) { + throw new Error( + `git ${args.join(" ")} failed: ${result.stderr.toString()}`, + ); + } +} + +test("a directory with no enclosing git work tree is allowed", () => { + const dir = scratchDir("hub-data-outside-"); + expect(enclosingGitWorkTree(dir)).toBeNull(); + expect(() => assertHubDataDirGitSafety(dir)).not.toThrow(); +}); + +test("a directory inside an existing git work tree fails loud", () => { + const workTree = scratchDir("hub-data-inside-wt-"); + git(workTree, ["init", "-b", "main"]); + writeFileSync(path.join(workTree, "keep.txt"), "keep\n"); + git(workTree, ["add", "keep.txt"]); + git(workTree, ["commit", "-m", "keep"]); + const dataDir = path.join(workTree, ".data", "hub"); + mkdirSync(dataDir, { recursive: true }); + + expect(enclosingGitWorkTree(dataDir)).toBe(realpathSync(workTree)); + expect(() => assertHubDataDirGitSafety(dataDir)).toThrow( + HubDataDirInsideGitWorkTreeError, + ); + try { + assertHubDataDirGitSafety(dataDir); + throw new Error("expected assertHubDataDirGitSafety to throw"); + } catch (error) { + expect(error).toBeInstanceOf(HubDataDirInsideGitWorkTreeError); + const message = (error as Error).message; + expect(message).toContain(realpathSync(dataDir)); + expect(message).toContain(realpathSync(workTree)); + expect(message).toContain("HUB_ALLOW_GIT_INSIDE_WORK_TREE=1"); + expect(message).toContain("refuses"); + } +}); + +test("opt-in allows initializing inside an existing git work tree", () => { + const workTree = scratchDir("hub-data-opt-in-"); + git(workTree, ["init", "-b", "main"]); + const dataDir = path.join(workTree, ".data", "hub"); + expect(() => + assertHubDataDirGitSafety(dataDir, { allowInsideWorkTree: true }), + ).not.toThrow(); +}); diff --git a/apps/hub/src/hub-data-dir-git-safety.ts b/apps/hub/src/hub-data-dir-git-safety.ts new file mode 100644 index 0000000000..d4f530c127 --- /dev/null +++ b/apps/hub/src/hub-data-dir-git-safety.ts @@ -0,0 +1,82 @@ +// Refuse to initialize hub git-on-disk state inside an existing git work +// tree. isomorphic-git walks up from `dataDir` looking for `.git`; when +// HUB_DATA_DIR lives inside this repo (the `.env` default `.data/hub`), +// a missed nested init authors a genesis commit onto the working branch +// and a checkout of that tree can delete the checkout. +import { spawnSync } from "node:child_process"; +import { mkdirSync, realpathSync } from "node:fs"; +import path from "node:path"; + +const GIT_DIR_VARS = new Set([ + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", +]); + +export function envWithoutGitDir( + env: NodeJS.ProcessEnv = process.env, +): NodeJS.ProcessEnv { + const next: NodeJS.ProcessEnv = {}; + for (const [key, value] of Object.entries(env)) { + if (value === undefined) continue; + if (GIT_DIR_VARS.has(key)) continue; + next[key] = value; + } + return next; +} + +export function enclosingGitWorkTree( + dir: string, + env: NodeJS.ProcessEnv = process.env, +): string | null { + const absolute = path.resolve(dir); + mkdirSync(absolute, { recursive: true }); + const canonical = realpathSync(absolute); + const result = spawnSync( + "git", + ["-c", "safe.directory=*", "-C", canonical, "rev-parse", "--show-toplevel"], + { encoding: "utf8", env: envWithoutGitDir(env) }, + ); + if (result.status !== 0) return null; + const toplevel = result.stdout.trim(); + return toplevel === "" ? null : realpathSync(toplevel); +} + +export class HubDataDirInsideGitWorkTreeError extends Error { + readonly hubDataDir: string; + readonly enclosingWorkTree: string; + + constructor(hubDataDir: string, enclosingWorkTree: string) { + super( + [ + `HUB_DATA_DIR (${hubDataDir}) is inside the git work tree at ${enclosingWorkTree}.`, + "The hub refuses to initialize git-on-disk state there: a nested", + "init that misses its own .git walks up and commits onto the working", + "branch, which can delete the checkout.", + "Point HUB_DATA_DIR at a directory outside any git work tree, or set", + "HUB_ALLOW_GIT_INSIDE_WORK_TREE=1 to opt in.", + ].join(" "), + ); + this.name = "HubDataDirInsideGitWorkTreeError"; + this.hubDataDir = hubDataDir; + this.enclosingWorkTree = enclosingWorkTree; + } +} + +export function assertHubDataDirGitSafety( + dir: string, + opts?: { + allowInsideWorkTree?: boolean; + env?: NodeJS.ProcessEnv; + }, +): void { + if (opts?.allowInsideWorkTree === true) return; + const enclosing = enclosingGitWorkTree(dir, opts?.env ?? process.env); + if (enclosing === null) return; + throw new HubDataDirInsideGitWorkTreeError( + realpathSync(path.resolve(dir)), + enclosing, + ); +} diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index fe6c3ff89b..bf17db671a 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -632,7 +632,13 @@ export async function createHub(config: HubConfig) { config.signInRateLimit.max, ); const { signingKey, agentRepoStore, assetService } = - await createBootAssetWiring({ db, dataDir: config.hubDataDir }); + await createBootAssetWiring({ + db, + dataDir: config.hubDataDir, + ...(config.allowGitInsideWorkTree === true + ? { allowGitInsideWorkTree: true } + : {}), + }); const baseLookups = createHubSessionLookups({ db, agentRepoStore }); // Shared with `createRunKeyHistoryListener` below: one store instance // for the process, read here ahead of `workflow_run` and written to diff --git a/apps/hub/test/config.test.ts b/apps/hub/test/config.test.ts index 72052fe537..da9655a19f 100644 --- a/apps/hub/test/config.test.ts +++ b/apps/hub/test/config.test.ts @@ -304,6 +304,29 @@ describe("readHubConfig", () => { expect(readHubConfig(validEnv).allowUnverifiedEmails).toBe(false); }); + test("allowGitInsideWorkTree is omitted by default", () => { + expect(readHubConfig(validEnv).allowGitInsideWorkTree).toBeUndefined(); + }); + + test("HUB_ALLOW_GIT_INSIDE_WORK_TREE='1' or 'true' opts in", () => { + expect( + readHubConfig({ ...validEnv, HUB_ALLOW_GIT_INSIDE_WORK_TREE: "1" }) + .allowGitInsideWorkTree, + ).toBe(true); + expect( + readHubConfig({ ...validEnv, HUB_ALLOW_GIT_INSIDE_WORK_TREE: "true" }) + .allowGitInsideWorkTree, + ).toBe(true); + }); + + test("HUB_ALLOW_GIT_INSIDE_WORK_TREE rejects any other value", () => { + const message = readExpectingError({ + ...validEnv, + HUB_ALLOW_GIT_INSIDE_WORK_TREE: "yes", + }); + expect(message).toContain("HUB_ALLOW_GIT_INSIDE_WORK_TREE"); + }); + test("ALLOW_UNVERIFIED_EMAILS='1' or 'true' opts in", () => { expect( readHubConfig({ ...validEnv, ALLOW_UNVERIFIED_EMAILS: "1" }) diff --git a/package.json b/package.json index 8d3d02595e..021c1c6ae1 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "setup:memory": "bun run scripts/setup-memory.ts", "seed": "bun packages/cli/src/index.ts seed", "reset": "bun packages/cli/src/index.ts reset", - "check:structural": "bun test scripts/checks/test && bun run check:deletion && bun run check:killdates && bun run check:licenses && bun run check:db-gate && bun run check:no-product-tenancy && bun run check:browser-safe-subpaths && bun run check:web-utilities && bun run check:tailwind-source && bun run check:ui-vocabulary && bun run check:react-ui-drift && bun run check:react-ui-pin && bun run check:tool-package-pins && bun run check:tool-package-freshness && bun run check:report-error && bun run check:tsconfig-references", + "check:structural": "bun test scripts/checks/test && bun run check:deletion && bun run check:killdates && bun run check:licenses && bun run check:db-gate && bun run check:no-product-tenancy && bun run check:browser-safe-subpaths && bun run check:web-utilities && bun run check:tailwind-source && bun run check:ui-vocabulary && bun run check:react-ui-drift && bun run check:react-ui-pin && bun run check:tool-package-pins && bun run check:tool-package-freshness && bun run check:hub-git-safety && bun run check:report-error && bun run check:tsconfig-references", "check:deletion": "bun run scripts/checks/deletion.ts", "check:report-error": "bun run scripts/checks/report-error.ts", "check:killdates": "bun run scripts/checks/killdates.ts", @@ -42,6 +42,7 @@ "check:react-ui-pin": "bun run scripts/checks/react-ui-pin.ts", "check:tool-package-pins": "bun run scripts/checks/tool-package-pins.ts", "check:tool-package-freshness": "bun run scripts/checks/tool-package-freshness.ts", + "check:hub-git-safety": "bun run scripts/checks/hub-git-safety.ts", "check:tsconfig-references": "bun run scripts/generate-tsconfig-references.ts --check", "build:sidecar-image": "docker build -f apps/sidecar/Dockerfile -t corbits-sidecar:dev .", "eval": "bun run scripts/evals-run.ts" diff --git a/packages/cli/test/seed.test.ts b/packages/cli/test/seed.test.ts index 0afc3930f0..27cea6b8f5 100644 --- a/packages/cli/test/seed.test.ts +++ b/packages/cli/test/seed.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { installDisposableHubDataDir } from "../../../test/disposable-hub-data-dir"; import { CliError } from "@workbench/hub-client"; import type { SeedConfig } from "../src/config"; import { resolveSeedWorkflows, runSeed, type SeedDeps } from "../src/seed"; @@ -13,6 +14,8 @@ import { type FakeHandler, } from "./helpers"; +installDisposableHubDataDir(); + const CONFIG: SeedConfig = { hubUrl: "http://localhost:3000", adminDefaulted: false, diff --git a/packages/hub-client/src/workflow-push.test.ts b/packages/hub-client/src/workflow-push.test.ts index 7b73246b67..d6db1f2820 100644 --- a/packages/hub-client/src/workflow-push.test.ts +++ b/packages/hub-client/src/workflow-push.test.ts @@ -17,11 +17,19 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { installDisposableHubDataDir } from "../../../test/disposable-hub-data-dir"; import { createGitWorkflowPusher } from "./workflow-push"; +installDisposableHubDataDir(); + async function git(args: string[], cwd: string): Promise { + const env = { ...process.env }; + delete env["GIT_DIR"]; + delete env["GIT_WORK_TREE"]; + delete env["GIT_INDEX_FILE"]; const child = Bun.spawn(["git", "-c", "core.hooksPath=", ...args], { cwd, + env, stdout: "pipe", stderr: "pipe", }); @@ -176,4 +184,53 @@ describe("createGitWorkflowPusher", () => { await rm(work, { recursive: true, force: true }); } }); + + test("does not commit onto an inherited GIT_DIR pointing at another work tree", async () => { + const work = await mkdtemp(join(tmpdir(), "workflow-push-git-dir-")); + const previousGitDir = process.env.GIT_DIR; + const previousWorkTree = process.env.GIT_WORK_TREE; + try { + const victim = join(work, "victim"); + await git(["init", "-b", "main", victim], work); + await Bun.write(join(victim, "keep.txt"), "keep\n"); + await git(["add", "keep.txt"], victim); + await git( + [ + "-c", + "user.email=keep@test", + "-c", + "user.name=keep", + "commit", + "-m", + "keep", + ], + victim, + ); + + const remoteDir = join(work, "remote.git"); + await git(["init", "--bare", "--initial-branch=main", remoteDir], work); + + process.env.GIT_DIR = join(victim, ".git"); + process.env.GIT_WORK_TREE = victim; + + const pusher = createGitWorkflowPusher(); + const outcome = await pusher({ + remoteUrl: `file://${remoteDir}`, + tokenSecret: "unused-for-file-transport", + workflowJson: '{"v":1}', + packageName: "@workbench-seed/test", + }); + expect(outcome.outcome).toBe("pushed"); + + const log = await git(["log", "--format=%s"], victim); + expect(log).not.toContain("Deploy the default workflow definition"); + expect(await readFile(join(victim, "keep.txt"), "utf-8")).toBe("keep\n"); + } finally { + if (previousGitDir === undefined) delete process.env.GIT_DIR; + else process.env.GIT_DIR = previousGitDir; + if (previousWorkTree === undefined) delete process.env.GIT_WORK_TREE; + else process.env.GIT_WORK_TREE = previousWorkTree; + await rm(work, { recursive: true, force: true }); + } + }); }); diff --git a/packages/hub-client/src/workflow-push.ts b/packages/hub-client/src/workflow-push.ts index 4f4b458b9a..c170ab4e4c 100644 --- a/packages/hub-client/src/workflow-push.ts +++ b/packages/hub-client/src/workflow-push.ts @@ -26,6 +26,24 @@ function requireGit(): void { } } +const GIT_DIR_VARS = new Set([ + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", +]); + +function gitProcessEnv(extra: Record): Record { + const env: Record = {}; + for (const [key, value] of Object.entries(process.env)) { + if (value === undefined) continue; + if (GIT_DIR_VARS.has(key)) continue; + env[key] = value; + } + return { ...env, ...extra }; +} + async function runGit( args: string[], cwd: string, @@ -33,7 +51,7 @@ async function runGit( ): Promise<{ code: number; output: string }> { const child = Bun.spawn(["git", "-c", "core.hooksPath=", ...args], { cwd, - env: { ...process.env, ...env }, + env: gitProcessEnv(env), stdout: "pipe", stderr: "pipe", }); diff --git a/packages/hub-client/test/seed.test.ts b/packages/hub-client/test/seed.test.ts index 260b15e3af..47fc1d1e51 100644 --- a/packages/hub-client/test/seed.test.ts +++ b/packages/hub-client/test/seed.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { installDisposableHubDataDir } from "../../../test/disposable-hub-data-dir"; import { CliError, isSidecarUnavailableError } from "../src/errors"; import { CATALOG_TEST_WORKFLOWS, @@ -27,6 +28,8 @@ import { type FakeHandler, } from "./helpers"; +installDisposableHubDataDir(); + const MODEL = { provider: "anthropic", model: "claude-sonnet-4-5", diff --git a/scripts/checks/hub-git-safety.ts b/scripts/checks/hub-git-safety.ts new file mode 100644 index 0000000000..5877d42de6 --- /dev/null +++ b/scripts/checks/hub-git-safety.ts @@ -0,0 +1,150 @@ +// check:hub-git-safety — fail when this branch carries a commit authored +// by the hub's own git-on-disk machinery (or a seed/deploy test that +// inherited the working tree's GIT_DIR / HUB_DATA_DIR). Those commits +// have landed on the working branch and wiped checkouts; catching them +// in CI is cheaper than reconstructing a deleted worktree. +import { spawnSync } from "node:child_process"; +import { + emptyReport, + reportAndExit, + rootFromArgs, + type CheckReport, +} from "./lib/repo"; + +export const JUNK_COMMIT_SUBJECTS = [ + "initial @corbits/fake-tools@0.0.1", + "seed v1", + "Deploy the default workflow definition", +] as const; + +export const JUNK_AUTHOR_EMAILS = [ + "hub@interchange.local", + "seed@workbench.localhost", +] as const; + +/** A commit touching this many paths with no source-tree file is hub dump. */ +export const LARGE_COMMIT_FILE_THRESHOLD = 50; + +const SOURCE_PATH = + /^(apps|packages|scripts|workflows|docs|test|tools|\.github)\//; + +export type InspectedCommit = { + readonly hash: string; + readonly subject: string; + readonly authorEmail: string; + readonly files: readonly string[]; +}; + +export function isSourcePath(file: string): boolean { + return SOURCE_PATH.test(file) && !file.includes(".data/"); +} + +export function auditHubJunkCommits( + commits: readonly InspectedCommit[], +): CheckReport { + const report = emptyReport(); + for (const commit of commits) { + const short = commit.hash.slice(0, 8); + if ((JUNK_COMMIT_SUBJECTS as readonly string[]).includes(commit.subject)) { + report.violations.push( + `${short}: subject ${JSON.stringify(commit.subject)} is hub/seed ` + + `git-on-disk machinery, not a workbench change. Reset that commit ` + + `off the branch; tests must use a disposable HUB_DATA_DIR outside ` + + `this work tree (test/disposable-hub-data-dir.ts).`, + ); + } + if ( + (JUNK_AUTHOR_EMAILS as readonly string[]).includes(commit.authorEmail) + ) { + report.violations.push( + `${short}: author ${commit.authorEmail} is hub/seed git identity ` + + `(subject ${JSON.stringify(commit.subject)}). That commit is ` + + `on-disk hub state, not a workbench change.`, + ); + } + const dataFiles = commit.files.filter( + (file) => file === ".data" || file.startsWith(".data/"), + ); + if (dataFiles.length > 0) { + report.violations.push( + `${short}: commits hub data paths (${dataFiles.slice(0, 3).join(", ")}` + + `${dataFiles.length > 3 ? `, … +${dataFiles.length - 3}` : ""}). ` + + `HUB_DATA_DIR must not live inside this work tree.`, + ); + } + if ( + commit.files.length > LARGE_COMMIT_FILE_THRESHOLD && + !commit.files.some(isSourcePath) + ) { + report.violations.push( + `${short}: touches ${commit.files.length} paths with no corresponding ` + + `source-tree change — the shape of a hub git-on-disk dump onto the ` + + `working branch.`, + ); + } + } + return report; +} + +function git(root: string, args: readonly string[]): string | undefined { + const result = spawnSync("git", [...args], { cwd: root, encoding: "utf8" }); + return result.status === 0 ? result.stdout.trim() : undefined; +} + +export function resolveBaseRef( + root: string, + explicit: string | undefined, +): string | undefined { + if (explicit !== undefined && explicit.length > 0) return explicit; + return git(root, ["merge-base", "HEAD", "origin/main"]); +} + +export function readCommitsSince( + root: string, + baseRef: string, +): InspectedCommit[] { + const hashes = git(root, ["log", "--format=%H", `${baseRef}..HEAD`]); + if (hashes === undefined || hashes === "") return []; + const commits: InspectedCommit[] = []; + for (const hash of hashes.split("\n")) { + if (hash === "") continue; + const meta = git(root, ["log", "-1", "--format=%s%n%ae", hash]); + if (meta === undefined) continue; + const newline = meta.indexOf("\n"); + const subject = newline === -1 ? meta : meta.slice(0, newline); + const authorEmail = newline === -1 ? "" : meta.slice(newline + 1); + const names = git(root, [ + "diff-tree", + "--no-commit-id", + "--name-only", + "-r", + hash, + ]); + commits.push({ + hash, + subject, + authorEmail, + files: names === undefined || names === "" ? [] : names.split("\n"), + }); + } + return commits; +} + +function main(): void { + const root = rootFromArgs(Bun.argv.slice(2)); + const baseRef = resolveBaseRef(root, process.env["CHECK_BASE_REF"]); + if (baseRef === undefined) { + const report = emptyReport(); + report.notes.push( + "no base ref (no origin/main, no CHECK_BASE_REF); skipping — CI " + + "supplies the base ref for the authoritative run", + ); + reportAndExit("check:hub-git-safety", report); + } + const commits = readCommitsSince(root, baseRef); + const report = auditHubJunkCommits(commits); + report.notes.push(`${commits.length} commit(s) since ${baseRef.slice(0, 8)}`); + reportAndExit("check:hub-git-safety", report); +} + +if (import.meta.main) main(); diff --git a/scripts/checks/test/hub-git-safety.test.ts b/scripts/checks/test/hub-git-safety.test.ts new file mode 100644 index 0000000000..36ec5225c3 --- /dev/null +++ b/scripts/checks/test/hub-git-safety.test.ts @@ -0,0 +1,90 @@ +import { expect, test } from "bun:test"; +import { + auditHubJunkCommits, + isSourcePath, + JUNK_COMMIT_SUBJECTS, + LARGE_COMMIT_FILE_THRESHOLD, + type InspectedCommit, +} from "../hub-git-safety"; + +function commit(overrides: Partial): InspectedCommit { + return { + hash: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + subject: "Bound the workflow-artifact rate limiter's per-run map", + authorEmail: "dev@example.com", + files: ["apps/hub/src/index.ts"], + ...overrides, + }; +} + +test("a normal source commit passes", () => { + expect(auditHubJunkCommits([commit({})]).violations).toEqual([]); +}); + +test("each known hub/seed subject is a violation naming the subject", () => { + for (const subject of JUNK_COMMIT_SUBJECTS) { + const report = auditHubJunkCommits([commit({ subject })]); + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain(subject); + expect(report.violations[0]).toContain("disposable HUB_DATA_DIR"); + } +}); + +test("a hub-authored commit is a violation even with a plausible subject", () => { + const report = auditHubJunkCommits([ + commit({ + subject: "genesis", + authorEmail: "hub@interchange.local", + }), + ]); + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain("hub@interchange.local"); +}); + +test("a seed-authored commit is a violation", () => { + const report = auditHubJunkCommits([ + commit({ + subject: "push workflow", + authorEmail: "seed@workbench.localhost", + }), + ]); + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain("seed@workbench.localhost"); +}); + +test("committing .data/hub paths is a violation", () => { + const report = auditHubJunkCommits([ + commit({ + files: [".data/hub/assets/workflow/assistant/.git/HEAD"], + }), + ]); + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain(".data/hub"); +}); + +test("a large commit with no source-tree file is a violation", () => { + const files = Array.from( + { length: LARGE_COMMIT_FILE_THRESHOLD + 1 }, + (_, i) => `object-${i}`, + ); + const report = auditHubJunkCommits([commit({ files })]); + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain("no corresponding source-tree change"); +}); + +test("a large commit that also changes source passes the size heuristic", () => { + const files = [ + "apps/hub/src/index.ts", + ...Array.from( + { length: LARGE_COMMIT_FILE_THRESHOLD }, + (_, i) => `extra-${i}`, + ), + ]; + expect(auditHubJunkCommits([commit({ files })]).violations).toEqual([]); +}); + +test("isSourcePath rejects hub data even under apps/", () => { + expect(isSourcePath("apps/hub/src/index.ts")).toBe(true); + expect(isSourcePath("apps/hub/.data/hub/HEAD")).toBe(false); + expect(isSourcePath(".data/hub/HEAD")).toBe(false); +}); diff --git a/scripts/typecheck.ts b/scripts/typecheck.ts index eccead8602..390aa37b8e 100644 --- a/scripts/typecheck.ts +++ b/scripts/typecheck.ts @@ -88,10 +88,16 @@ const projectCommands = projectPaths.map((path) => [ const projectsOk = await runInParallel(projectCommands); -const [rootCode, isolationCode] = await Promise.all([ +const [rootCode, isolationCode, hubDataDirCode] = await Promise.all([ run(["bunx", "tsc", "-p", "tsconfig.json", "--noEmit"]), run(["bunx", "tsc", "-p", "test/isolation/tsconfig.json", "--noEmit"]), + run(["bunx", "tsc", "-p", "test/tsconfig.json", "--noEmit"]), ]); -if (!projectsOk || rootCode !== 0 || isolationCode !== 0) { +if ( + !projectsOk || + rootCode !== 0 || + isolationCode !== 0 || + hubDataDirCode !== 0 +) { process.exit(1); } diff --git a/test/disposable-hub-data-dir.test.ts b/test/disposable-hub-data-dir.test.ts new file mode 100644 index 0000000000..14a70dab6a --- /dev/null +++ b/test/disposable-hub-data-dir.test.ts @@ -0,0 +1,37 @@ +import { expect, test } from "bun:test"; +import { existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { createDisposableHubDataDir } from "./disposable-hub-data-dir"; + +test("points HUB_DATA_DIR at a fresh directory under the OS temp dir", () => { + const previous = process.env["HUB_DATA_DIR"]; + const { hubDataDir, restore } = createDisposableHubDataDir(); + try { + expect(hubDataDir.startsWith(tmpdir())).toBe(true); + expect(existsSync(hubDataDir)).toBe(true); + expect(process.env["HUB_DATA_DIR"]).toBe(hubDataDir); + } finally { + restore(); + } + expect(existsSync(hubDataDir)).toBe(false); + if (previous === undefined) + expect(process.env["HUB_DATA_DIR"]).toBeUndefined(); + else expect(process.env["HUB_DATA_DIR"]).toBe(previous); +}); + +test("clears HUB_ALLOW_GIT_INSIDE_WORK_TREE so tests cannot inherit local-dev opt-in", () => { + const previous = process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"]; + process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"] = "1"; + const { restore } = createDisposableHubDataDir(); + try { + expect(process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"]).toBeUndefined(); + } finally { + restore(); + } + expect(process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"]).toBe("1"); + if (previous === undefined) { + delete process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"]; + } else { + process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"] = previous; + } +}); diff --git a/test/disposable-hub-data-dir.ts b/test/disposable-hub-data-dir.ts new file mode 100644 index 0000000000..584335e494 --- /dev/null +++ b/test/disposable-hub-data-dir.ts @@ -0,0 +1,43 @@ +// Disposable HUB_DATA_DIR for tests that boot the hub or exercise +// seed/deploy git paths. Bun loads repo-root `.env`, so an unset test +// inherits `HUB_DATA_DIR=.data/hub` inside this work tree; the hub's +// git-on-disk init then walks up to the enclosing `.git` and a genesis +// commit can land on the working branch. This fixture mktemps outside +// any work tree, points HUB_DATA_DIR at it, and clears the local-dev +// opt-in so tests cannot inherit `HUB_ALLOW_GIT_INSIDE_WORK_TREE`. +import { afterAll } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +export function createDisposableHubDataDir(): { + hubDataDir: string; + restore: () => void; +} { + const previousDataDir = process.env["HUB_DATA_DIR"]; + const previousAllow = process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"]; + const hubDataDir = mkdtempSync(join(tmpdir(), "hub-data-")); + process.env["HUB_DATA_DIR"] = hubDataDir; + delete process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"]; + let restored = false; + const restore = () => { + if (restored) return; + restored = true; + if (previousDataDir === undefined) delete process.env["HUB_DATA_DIR"]; + else process.env["HUB_DATA_DIR"] = previousDataDir; + if (previousAllow === undefined) { + delete process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"]; + } else { + process.env["HUB_ALLOW_GIT_INSIDE_WORK_TREE"] = previousAllow; + } + rmSync(hubDataDir, { recursive: true, force: true }); + }; + return { hubDataDir, restore }; +} + +/** Installs a disposable HUB_DATA_DIR for the rest of this test file. */ +export function installDisposableHubDataDir(): string { + const { hubDataDir, restore } = createDisposableHubDataDir(); + afterAll(restore); + return hubDataDir; +} diff --git a/test/tsconfig.json b/test/tsconfig.json new file mode 100644 index 0000000000..3ec7530c9f --- /dev/null +++ b/test/tsconfig.json @@ -0,0 +1,8 @@ +{ + "extends": "../tsconfig.base.json", + "compilerOptions": { + "types": ["bun"], + "noEmit": true + }, + "include": ["*.ts"] +}