diff --git a/package.json b/package.json index 9e56ab5..b808f3c 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "demo:web": "DEMO=1 pnpm dev:web", "build": "pnpm --filter sync build && pnpm --filter cli build && pnpm --filter server build && pnpm --filter web build", "build:desktop": "pnpm build && pnpm --filter desktop build && pnpm --filter desktop package", - "lint": "oxlint", + "lint": "oxlint && pnpm check:boundaries", "fmt": "oxfmt .", "fmt:check": "oxfmt --check .", "typecheck": "pnpm --filter sync build && pnpm -r typecheck", @@ -18,7 +18,8 @@ "cli": "node packages/cli/dist/index.js", "dev:cli": "pnpm --filter cli dev", "scenario": "pnpm --filter web build && pnpm --filter harness scenario", - "test:e2e": "pnpm --filter sync build && pnpm --filter web build && pnpm --filter harness test:e2e" + "test:e2e": "pnpm --filter sync build && pnpm --filter web build && pnpm --filter harness test:e2e", + "check:boundaries": "node --test scripts/check-boundaries.test.mjs && node scripts/check-boundaries.mjs" }, "devDependencies": { "@typescript/native-preview": "7.0.0-dev.20260420.1", diff --git a/packages/harness/README.md b/packages/harness/README.md index 60610ff..fec1069 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -17,3 +17,7 @@ The browser check runs the built renderer and real API/engine on disposable port Failures retain a Playwright trace and screenshot in `packages/harness/test-results/`. The HTML report includes API responses, upstream requests, and server logs; open it with `pnpm --filter harness exec playwright show-report`. The same check runs in CI and uploads its report on failure. Additional phases: `conflicts`, `needs-approval`, `changes-requested`, `empty`, `assigned`, `review-requested`, `unassigned`, and `rate-limited` (20-second reset). Assignment-only and review-request-only fixtures are distinct: the current engine still uses review requests; changing the product inbox query belongs to #84. Tests can use `startScenario({ instances: 2 })` and target the second upstream with `setPhase(phase, 1)` / `syncPrs(1)` to check partial outages. + +## GitHub access boundary + +`pnpm check:boundaries` parses production TypeScript and checks client dependencies. GitHub/network clients and direct server/CLI requests must stay in `packages/sync`; renderer requests are centralized in `api.ts` and use relative `/api/` URLs. Consumers may import only the public `sync` entry point. Electron's loopback readiness probe and port allocation are explicit exceptions; its updater remains separate from dashboard data access. Tests and the simulator may implement an HTTP server. This is a regression guard, not a security sandbox. It runs as part of lint and CI. diff --git a/scripts/check-boundaries.mjs b/scripts/check-boundaries.mjs new file mode 100644 index 0000000..2e30318 --- /dev/null +++ b/scripts/check-boundaries.mjs @@ -0,0 +1,174 @@ +import { readFileSync, readdirSync } from "node:fs"; +import { resolve, relative, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; +import ts from "typescript"; + +const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const networkModule = + /^(?:@octokit(?:\/|$)|axios$|undici$|got$|node-fetch$|(?:node:)?(?:https?|net|tls)$)/; + +export function violations(path, source) { + const errors = []; + const file = ts.createSourceFile( + path, + source, + ts.ScriptTarget.Latest, + true, + path.endsWith("tsx") ? ts.ScriptKind.TSX : ts.ScriptKind.TS, + ); + const host = ts.createCompilerHost({ noLib: true, noResolve: true }); + host.getSourceFile = (name) => (name === path ? file : undefined); + const checker = ts + .createProgram([path], { noLib: true, noResolve: true }, host) + .getTypeChecker(); + const webApi = path === "packages/web/src/api.ts"; + const desktop = path.startsWith("packages/desktop/"); + const report = (node, message) => { + const { line } = file.getLineAndCharacterOfPosition(node.getStart(file)); + errors.push(`${path}:${line + 1}: ${message}`); + }; + const moduleCheck = (node, name) => { + if ( + networkModule.test(name) && + !(path === "packages/desktop/src/main.ts" && name === "node:net") + ) + report(node, `GitHub/network clients belong in packages/sync: ${name}`); + if (/sync[\\/]src[\\/]/.test(name) || name.startsWith("sync/")) + report(node, "Use the public sync export, not its internals"); + }; + const visit = (node) => { + if ( + (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) && + node.moduleSpecifier && + ts.isStringLiteral(node.moduleSpecifier) + ) + moduleCheck(node, node.moduleSpecifier.text); + if (ts.isCallExpression(node)) { + const name = node.expression.getText(file); + if ( + (name === "require" || + node.expression.kind === ts.SyntaxKind.ImportKeyword) && + node.arguments[0] && + ts.isStringLiteral(node.arguments[0]) + ) + moduleCheck(node, node.arguments[0].text); + if (webApi && (name === "fetchJson" || name === "fetch")) { + const arg = node.arguments[0]; + const text = + arg && + (ts.isStringLiteral(arg) || ts.isNoSubstitutionTemplateLiteral(arg) + ? arg.text + : ts.isTemplateExpression(arg) + ? arg.head.text + : null); + if ( + !( + text?.startsWith("/api/") || + (name === "fetch" && arg?.getText(file) === "url") + ) + ) + report(node, "Renderer requests must use a relative /api/ URL"); + } + } + if ( + ts.isIdentifier(node) && + ["fetch", "XMLHttpRequest", "WebSocket", "EventSource"].includes( + node.text, + ) + ) { + const parentNode = node.parent; + if ( + (ts.isPropertyAssignment(parentNode) && parentNode.name === node) || + (ts.isPropertyAccessExpression(parentNode) && + parentNode.name === node && + !["globalThis", "window", "self"].includes( + parentNode.expression.getText(file), + )) + ) + return; + if ( + checker + .getSymbolAtLocation(node) + ?.declarations?.some( + (declaration) => + ts.isVariableDeclaration(declaration) || + ts.isParameter(declaration) || + ts.isBindingElement(declaration), + ) + ) + return; + // The packaged app's only fetch is its loopback readiness probe. + let parent = node.parent; + while (parent && !ts.isFunctionDeclaration(parent)) + parent = parent.parent; + const readiness = desktop && parent?.name?.text === "waitForServer"; + if (!webApi && !readiness) + report( + node, + "Network access must go through sync (renderer transport belongs in api.ts)", + ); + } + if ( + ts.isElementAccessExpression(node) && + node.argumentExpression && + ts.isStringLiteral(node.argumentExpression) && + ["fetch", "XMLHttpRequest", "WebSocket", "EventSource"].includes( + node.argumentExpression.text, + ) + ) + report(node, "Computed network access bypasses the sync boundary"); + ts.forEachChild(node, visit); + }; + visit(file); + return errors; +} + +function* sourceFiles(dir) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const path = resolve(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name !== "test-utils") yield* sourceFiles(path); + } else if ( + /\.tsx?$/.test(entry.name) && + !/\.(?:test|spec)\./.test(entry.name) && + !entry.name.endsWith(".d.ts") + ) + yield path; + } +} + +export function checkRepository() { + const errors = []; + for (const pkg of ["server", "web", "desktop", "cli"]) { + for (const file of sourceFiles(resolve(root, "packages", pkg, "src"))) + errors.push( + ...violations( + relative(root, file).replaceAll("\\", "/"), + readFileSync(file, "utf8"), + ), + ); + const manifest = JSON.parse( + readFileSync(resolve(root, "packages", pkg, "package.json"), "utf8"), + ); + for (const name of Object.keys(manifest.dependencies ?? {})) + if ( + networkModule.test(name) && + !(path === "packages/desktop/src/main.ts" && name === "node:net") + ) + errors.push( + `packages/${pkg}/package.json: network dependency ${name} belongs in sync`, + ); + } + return errors; +} + +if ( + process.argv[1] && + resolve(process.argv[1]) === fileURLToPath(import.meta.url) +) { + const errors = checkRepository(); + if (errors.length) { + console.error(errors.join("\n")); + process.exitCode = 1; + } else console.log("GitHub access boundaries passed"); +} diff --git a/scripts/check-boundaries.test.mjs b/scripts/check-boundaries.test.mjs new file mode 100644 index 0000000..437a704 --- /dev/null +++ b/scripts/check-boundaries.test.mjs @@ -0,0 +1,48 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { violations, checkRepository } from "./check-boundaries.mjs"; + +test("production clients use the public sync boundary", () => + assert.deepEqual(checkRepository(), [])); +test("rejects direct clients, aliases, computed fetch, and internal imports", () => { + for (const source of [ + 'import { Octokit } from "@octokit/rest";', + 'const upstream = fetch; upstream("https://api.github.com");', + 'globalThis["fetch"]("https://api.github.com");', + 'import("node:https");', + 'import { getClient } from "../../sync/src/providers/github/client";', + 'export * from "sync/providers/github";', + ]) + assert.ok( + violations("packages/server/src/routes.ts", source).length, + source, + ); +}); +test("allows local renderer transport and rejects absolute requests", () => { + assert.deepEqual( + violations( + "packages/web/src/api.ts", + 'fetchJson(`/api/${id}/prs`); fetch("/api/config");', + ), + [], + ); + assert.ok( + violations( + "packages/web/src/api.ts", + 'fetch("https://api.github.com/user");', + ).length, + ); + assert.ok( + violations("packages/web/src/App.tsx", 'fetch("/api/config");').length, + ); +}); + +test("does not confuse payload properties or local outcome variables with global fetch", () => { + assert.deepEqual( + violations( + "packages/cli/src/tui.tsx", + "for (const fetch of result.fetches) console.log(fetch.status); serve({ fetch: app.fetch });", + ), + [], + ); +});