(7/9) Guard clients against bypassing the public sync API

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",

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.

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");
+}

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 });",
+    ),
+    [],
+  );
+});