perf(test): move temp path guard to check

2026-05-06 05:50:43 +00:00 · 2026-04-20 19:07:18 +01:00
parent 8ea7866356
commit 305d04b758
4 changed files with 147 additions and 205 deletions
--- a/package.json
+++ b/package.json
@@ -1261,6 +1261,7 @@
    "check:madge-import-cycles": "node --import tsx scripts/check-madge-import-cycles.ts",
    "check:no-conflict-markers": "node scripts/check-no-conflict-markers.mjs",
    "check:static-import-sccs": "pnpm check:madge-import-cycles",
+    "check:temp-path-guardrails": "node --import tsx scripts/check-temp-path-guardrails.ts",
    "check:test-types": "pnpm tsgo:test",
    "check:timed": "node scripts/check-timed.mjs",
    "check:timed:all-types": "node scripts/check-timed.mjs --include-test-types",
--- a/scripts/check-temp-path-guardrails.ts
+++ b/scripts/check-temp-path-guardrails.ts
@@ -1,8 +1,6 @@
-import { describe, expect, it } from "vitest";
-import {
-  loadRuntimeSourceFilesForGuardrails,
-  shouldSkipGuardrailRuntimeSource,
-} from "../test-utils/runtime-source-guardrail-scan.js";
+import { execFileSync } from "node:child_process";
+import fs from "node:fs/promises";
+import path from "node:path";

 type QuoteChar = "'" | '"' | "`";

@@ -10,19 +8,67 @@ type QuoteScanState = {
  quote: QuoteChar | null;
  escaped: boolean;
 };
+
+type RuntimeSourceGuardrailFile = {
+  relativePath: string;
+  source: string;
+};
+
 const WEAK_RANDOM_SAME_LINE_PATTERN =
  /(?:Date\.now[^\r\n]*Math\.random|Math\.random[^\r\n]*Date\.now)/u;
 const PATH_JOIN_CALL_PATTERN = /path\s*\.\s*join\s*\(/u;
 const OS_TMPDIR_CALL_PATTERN = /os\s*\.\s*tmpdir\s*\(/u;
+const FILE_READ_CONCURRENCY = 24;
+const DEFAULT_GUARDRAIL_SKIP_PATTERNS = [
+  /\.test\.tsx?$/,
+  /\.test-helpers\.tsx?$/,
+  /\.test-utils\.tsx?$/,
+  /\.test-harness\.tsx?$/,
+  /\.test-support\.tsx?$/,
+  /\.suite\.tsx?$/,
+  /\.e2e\.tsx?$/,
+  /\.d\.ts$/,
+  /[\\/](?:__tests__|tests|test-helpers|test-utils|test-support)[\\/]/,
+  /[\\/][^\\/]*test-helpers(?:\.[^\\/]+)?\.ts$/,
+  /[\\/][^\\/]*test-utils(?:\.[^\\/]+)?\.ts$/,
+  /[\\/][^\\/]*test-harness(?:\.[^\\/]+)?\.ts$/,
+  /[\\/][^\\/]*test-support(?:\.[^\\/]+)?\.ts$/,
+];

-function shouldSkip(relativePath: string): boolean {
-  return shouldSkipGuardrailRuntimeSource(relativePath);
+function shouldSkipGuardrailRuntimeSource(relativePath: string): boolean {
+  return DEFAULT_GUARDRAIL_SKIP_PATTERNS.some((pattern) => pattern.test(relativePath));
 }

 function stripCommentsForScan(input: string): string {
  return input.replace(/\/\*[\s\S]*?\*\//g, "").replace(/(^|[^:])\/\/.*$/gm, "$1");
 }

+function beginQuotedSection(state: QuoteScanState, ch: string): boolean {
+  if (ch !== "'" && ch !== '"' && ch !== "`") {
+    return false;
+  }
+  state.quote = ch;
+  return true;
+}
+
+function consumeQuotedChar(state: QuoteScanState, ch: string): boolean {
+  if (!state.quote) {
+    return false;
+  }
+  if (state.escaped) {
+    state.escaped = false;
+    return true;
+  }
+  if (ch === "\\") {
+    state.escaped = true;
+    return true;
+  }
+  if (ch === state.quote) {
+    state.quote = null;
+  }
+  return true;
+}
+
 function findMatchingParen(source: string, openIndex: number): number {
  let depth = 1;
  const quoteState: QuoteScanState = { quote: null, escaped: false };
@@ -115,32 +161,6 @@ function splitTopLevelArguments(source: string): string[] {
  return out;
 }

-function beginQuotedSection(state: QuoteScanState, ch: string): boolean {
-  if (ch !== "'" && ch !== '"' && ch !== "`") {
-    return false;
-  }
-  state.quote = ch;
-  return true;
-}
-
-function consumeQuotedChar(state: QuoteScanState, ch: string): boolean {
-  if (!state.quote) {
-    return false;
-  }
-  if (state.escaped) {
-    state.escaped = false;
-    return true;
-  }
-  if (ch === "\\") {
-    state.escaped = true;
-    return true;
-  }
-  if (ch === state.quote) {
-    state.quote = null;
-  }
-  return true;
-}
-
 function isOsTmpdirExpression(argument: string): boolean {
  return /^os\s*\.\s*tmpdir\s*\(\s*\)$/u.test(argument.trim());
 }
@@ -187,60 +207,104 @@ function hasDynamicTmpdirJoin(source: string): boolean {
  return false;
 }

-describe("temp path guard", () => {
-  it("skips test helper filename variants", () => {
-    expect(shouldSkip("src/commands/test-helpers.ts")).toBe(true);
-    expect(shouldSkip("src/commands/sessions.test-helpers.ts")).toBe(true);
-    expect(shouldSkip("src\\commands\\sessions.test-helpers.ts")).toBe(true);
-    expect(shouldSkip("src/plugins/test-helpers/fs-fixtures.ts")).toBe(true);
+function listTrackedRuntimeSourceFiles(repoRoot: string): string[] {
+  const stdout = execFileSync("git", ["-C", repoRoot, "ls-files", "--", "src", "extensions"], {
+    encoding: "utf8",
+    stdio: ["ignore", "pipe", "inherit"],
  });
+  return stdout
+    .split(/\r?\n/u)
+    .filter(Boolean)
+    .filter((relativePath) => relativePath.endsWith(".ts") || relativePath.endsWith(".tsx"))
+    .filter((relativePath) => !shouldSkipGuardrailRuntimeSource(relativePath))
+    .map((relativePath) => path.join(repoRoot, relativePath));
+}

-  it("detects dynamic and ignores static fixtures", () => {
-    const dynamicFixtures = [
-      "const p = path.join(os.tmpdir(), `openclaw-${id}`);",
-      "const p = path.join(os.tmpdir(), 'safe', `${token}`);",
-    ];
-    const staticFixtures = [
-      "const p = path.join(os.tmpdir(), 'openclaw-fixed');",
-      "const p = path.join(os.tmpdir(), `openclaw-fixed`);",
-      "const p = path.join(os.tmpdir(), prefix + '-x');",
-      "const p = path.join(os.tmpdir(), segment);",
-      "const p = path.join('/tmp', `openclaw-${id}`);",
-      "// path.join(os.tmpdir(), `openclaw-${id}`)",
-      "const p = path.join(os.tmpdir());",
-    ];
-
-    expect(dynamicFixtures.every((fixture) => hasDynamicTmpdirJoin(fixture))).toBe(true);
-    expect(staticFixtures.every((fixture) => !hasDynamicTmpdirJoin(fixture))).toBe(true);
+async function readRuntimeSourceFiles(
+  repoRoot: string,
+  absolutePaths: string[],
+): Promise<RuntimeSourceGuardrailFile[]> {
+  const output: Array<RuntimeSourceGuardrailFile | undefined> = Array.from({
+    length: absolutePaths.length,
  });
+  let nextIndex = 0;

-  it("enforces runtime guardrails for tmpdir joins and weak randomness", async () => {
-    const files = await loadRuntimeSourceFilesForGuardrails(process.cwd());
-    const offenders: string[] = [];
-    const weakRandomMatches: string[] = [];
-
-    for (const file of files) {
-      const relativePath = file.relativePath;
-      const source = file.source;
-      const mightContainTmpdirJoin =
-        source.includes("tmpdir") &&
-        source.includes("path") &&
-        source.includes("join") &&
-        source.includes("`");
-      const mightContainWeakRandom = source.includes("Date.now") && source.includes("Math.random");
-
-      if (!mightContainTmpdirJoin && !mightContainWeakRandom) {
+  const worker = async () => {
+    for (;;) {
+      const index = nextIndex;
+      nextIndex += 1;
+      if (index >= absolutePaths.length) {
+        return;
+      }
+      const absolutePath = absolutePaths[index];
+      if (!absolutePath) {
        continue;
      }
-      if (mightContainTmpdirJoin && hasDynamicTmpdirJoin(source)) {
-        offenders.push(relativePath);
-      }
-      if (mightContainWeakRandom && WEAK_RANDOM_SAME_LINE_PATTERN.test(source)) {
-        weakRandomMatches.push(relativePath);
+      let source: string;
+      try {
+        source = await fs.readFile(absolutePath, "utf8");
+      } catch {
+        // File tracked by git but deleted on disk (e.g. pending deletion).
+        continue;
      }
+      output[index] = {
+        relativePath: path.relative(repoRoot, absolutePath),
+        source,
+      };
    }
+  };

-    expect(offenders).toEqual([]);
-    expect(weakRandomMatches).toEqual([]);
-  });
-});
+  const workers = Array.from(
+    { length: Math.min(FILE_READ_CONCURRENCY, Math.max(1, absolutePaths.length)) },
+    () => worker(),
+  );
+  await Promise.all(workers);
+  return output.filter((entry): entry is RuntimeSourceGuardrailFile => entry !== undefined);
+}
+
+async function main() {
+  const repoRoot = process.cwd();
+  const files = await readRuntimeSourceFiles(repoRoot, listTrackedRuntimeSourceFiles(repoRoot));
+  const offenders: string[] = [];
+  const weakRandomMatches: string[] = [];
+
+  for (const file of files) {
+    const source = file.source;
+    const mightContainTmpdirJoin =
+      source.includes("tmpdir") &&
+      source.includes("path") &&
+      source.includes("join") &&
+      source.includes("`");
+    const mightContainWeakRandom = source.includes("Date.now") && source.includes("Math.random");
+
+    if (!mightContainTmpdirJoin && !mightContainWeakRandom) {
+      continue;
+    }
+    if (mightContainTmpdirJoin && hasDynamicTmpdirJoin(source)) {
+      offenders.push(file.relativePath);
+    }
+    if (mightContainWeakRandom && WEAK_RANDOM_SAME_LINE_PATTERN.test(source)) {
+      weakRandomMatches.push(file.relativePath);
+    }
+  }
+
+  if (offenders.length === 0 && weakRandomMatches.length === 0) {
+    return;
+  }
+
+  if (offenders.length > 0) {
+    console.error("Dynamic os.tmpdir()/path.join() template paths found:");
+    for (const offender of offenders) {
+      console.error(`- ${offender}`);
+    }
+  }
+  if (weakRandomMatches.length > 0) {
+    console.error("Weak Date.now()+Math.random() same-line IDs found:");
+    for (const offender of weakRandomMatches) {
+      console.error(`- ${offender}`);
+    }
+  }
+  process.exitCode = 1;
+}
+
+await main();
--- a/scripts/check.mjs
+++ b/scripts/check.mjs
@@ -9,6 +9,7 @@ export async function main(argv = process.argv.slice(2)) {

  const tailChecks = [
    { name: "webhook body guard", args: ["lint:webhook:no-low-level-body-read"] },
+    { name: "temp path guard", args: ["check:temp-path-guardrails"] },
    { name: "pairing store guard", args: ["lint:auth:no-pairing-store-group"] },
    { name: "pairing account guard", args: ["lint:auth:pairing-account-scope"] },
    includeArchitecture
--- a/src/test-utils/runtime-source-guardrail-scan.ts
+++ b/src/test-utils/runtime-source-guardrail-scan.ts
@@ -1,124 +0,0 @@
-import { execFileSync } from "node:child_process";
-import fs from "node:fs/promises";
-import path from "node:path";
-import { listRuntimeSourceFiles } from "./repo-scan.js";
-
-export type RuntimeSourceGuardrailFile = {
-  relativePath: string;
-  source: string;
-};
-
-const DEFAULT_GUARDRAIL_SKIP_PATTERNS = [
-  /\.test\.tsx?$/,
-  /\.test-helpers\.tsx?$/,
-  /\.test-utils\.tsx?$/,
-  /\.test-harness\.tsx?$/,
-  /\.test-support\.tsx?$/,
-  /\.suite\.tsx?$/,
-  /\.e2e\.tsx?$/,
-  /\.d\.ts$/,
-  /[\\/](?:__tests__|tests|test-helpers|test-utils|test-support)[\\/]/,
-  /[\\/][^\\/]*test-helpers(?:\.[^\\/]+)?\.ts$/,
-  /[\\/][^\\/]*test-utils(?:\.[^\\/]+)?\.ts$/,
-  /[\\/][^\\/]*test-harness(?:\.[^\\/]+)?\.ts$/,
-  /[\\/][^\\/]*test-support(?:\.[^\\/]+)?\.ts$/,
-];
-
-const runtimeSourceGuardrailCache = new Map<string, Promise<RuntimeSourceGuardrailFile[]>>();
-const trackedRuntimeSourceListCache = new Map<string, string[]>();
-const FILE_READ_CONCURRENCY = 24;
-
-export function shouldSkipGuardrailRuntimeSource(relativePath: string): boolean {
-  return DEFAULT_GUARDRAIL_SKIP_PATTERNS.some((pattern) => pattern.test(relativePath));
-}
-
-async function readRuntimeSourceFiles(
-  repoRoot: string,
-  absolutePaths: string[],
-): Promise<RuntimeSourceGuardrailFile[]> {
-  const output: Array<RuntimeSourceGuardrailFile | undefined> = Array.from({
-    length: absolutePaths.length,
-  });
-  let nextIndex = 0;
-
-  const worker = async () => {
-    for (;;) {
-      const index = nextIndex;
-      nextIndex += 1;
-      if (index >= absolutePaths.length) {
-        return;
-      }
-      const absolutePath = absolutePaths[index];
-      if (!absolutePath) {
-        continue;
-      }
-      let source: string;
-      try {
-        source = await fs.readFile(absolutePath, "utf8");
-      } catch {
-        // File tracked by git but deleted on disk (e.g. pending deletion).
-        continue;
-      }
-      output[index] = {
-        relativePath: path.relative(repoRoot, absolutePath),
-        source,
-      };
-    }
-  };
-
-  const workers = Array.from(
-    { length: Math.min(FILE_READ_CONCURRENCY, Math.max(1, absolutePaths.length)) },
-    () => worker(),
-  );
-  await Promise.all(workers);
-  return output.filter((entry): entry is RuntimeSourceGuardrailFile => entry !== undefined);
-}
-
-function tryListTrackedRuntimeSourceFiles(repoRoot: string): string[] | null {
-  const cached = trackedRuntimeSourceListCache.get(repoRoot);
-  if (cached) {
-    return cached.slice();
-  }
-
-  try {
-    const stdout = execFileSync("git", ["-C", repoRoot, "ls-files", "--", "src", "extensions"], {
-      encoding: "utf8",
-      stdio: ["ignore", "pipe", "ignore"],
-    });
-    const files = stdout
-      .split(/\r?\n/u)
-      .filter(Boolean)
-      .filter((relativePath) => relativePath.endsWith(".ts") || relativePath.endsWith(".tsx"))
-      .filter((relativePath) => !shouldSkipGuardrailRuntimeSource(relativePath))
-      .map((relativePath) => path.join(repoRoot, relativePath));
-    trackedRuntimeSourceListCache.set(repoRoot, files);
-    return files.slice();
-  } catch {
-    return null;
-  }
-}
-
-export async function loadRuntimeSourceFilesForGuardrails(
-  repoRoot: string,
-): Promise<RuntimeSourceGuardrailFile[]> {
-  let pending = runtimeSourceGuardrailCache.get(repoRoot);
-  if (!pending) {
-    pending = (async () => {
-      const trackedFiles = tryListTrackedRuntimeSourceFiles(repoRoot);
-      const sourceFiles =
-        trackedFiles ??
-        (
-          await listRuntimeSourceFiles(repoRoot, {
-            roots: ["src", "extensions"],
-            extensions: [".ts", ".tsx"],
-          })
-        ).filter((absolutePath) => {
-          const relativePath = path.relative(repoRoot, absolutePath);
-          return !shouldSkipGuardrailRuntimeSource(relativePath);
-        });
-      return await readRuntimeSourceFiles(repoRoot, sourceFiles);
-    })();
-    runtimeSourceGuardrailCache.set(repoRoot, pending);
-  }
-  return await pending;
-}