chore: lint rule banning disabled TLS verification in tests (#104)

.agents/memory/INBOX.md

@@ -23,3 +23,7 @@ One note per PR that hit friction, four lines:
   Would have prevented it: at the start, check for a simulator (xcrun, or an Android emulator with KVM) and, if there is none, ask the human up front to run e2e-device locally.
   Cost: blocked
   Seen: 2026-09-24
+- 2026-09-24 #108 skill: implement-issue
+  What went wrong: the new no-tls-bypass lint rule's red tests covered only the literal spellings in the issue, so `vi.stubEnv(...)` and `globalAgent.options.rejectUnauthorized = false` got through.
+  Would have prevented it: when an issue asks a lint rule to catch "equivalent" forms, write a red test for each way the pattern can be spelled (assignment, call argument, member assignment, object property) before implementing.
+  Cost: review round

eslint.config.mjs

@@ -68,6 +68,18 @@ export const LEGACY_MODULE_BOUNDARY = [
   "packages/appduct/src/__tests__/link-open.integration.test.ts",
 ];
 
+// Tests that switched off TLS verification before the ban. Each talks to a daemon running as a
+// CLI subprocess, whose certificate it never sees, so it has no `ca` to pass. Same deal as
+// above: remove when converted, never add.
+export const LEGACY_TLS_BYPASS = [
+  "packages/appduct/src/__tests__/cli-v2.integration.test.ts",
+  "packages/appduct/src/__tests__/e2e/app-client.ts",
+  "packages/appduct/src/__tests__/e2e/harness.ts",
+  "packages/appduct/src/__tests__/e2e/hostility.e2e.test.ts",
+  "packages/appduct/src/__tests__/events.integration.test.ts",
+  "packages/appduct/src/__tests__/exit-codes.integration.test.ts",
+];
+
 // Rule 1, modules. A module is a directory under a package's src that has an index.ts; that
 // file is its only import surface. Modules are discovered at lint time, so a directory becomes
 // one the moment it gains an index.ts and there is no list to keep in step.
@@ -113,6 +125,35 @@ const noNodeIoDynamicImport = {
   },
 };
 
+// A test that disables TLS verification passes against any certificate, so it cannot catch a
+// daemon serving the wrong one. Only tests are linted: on a TLS server, rejectUnauthorized is
+// about client certificates and false is the normal setting.
+const TLS_MESSAGE = "Trust the daemon's own certificate with `ca: daemon.tls.current().certPem` instead of switching off TLS verification.";
+const keyName = (node) => (node.type === "Identifier" ? node.name : node.type === "Literal" ? node.value : undefined);
+const noTlsBypass = {
+  meta: { type: "problem", docs: { description: "no disabling TLS verification in tests" }, schema: [], messages: { bypass: TLS_MESSAGE } },
+  create(context) {
+    const report = (node) => context.report({ node, messageId: "bypass" });
+    const isFalse = (node) => node.type === "Literal" && node.value === false;
+    return {
+      Property(node) {
+        const key = node.computed ? undefined : keyName(node.key);
+        if (key === "NODE_TLS_REJECT_UNAUTHORIZED") report(node);
+        if (key === "rejectUnauthorized" && isFalse(node.value)) report(node);
+      },
+      AssignmentExpression(node) {
+        const key = node.left.type === "MemberExpression" ? keyName(node.left.property) : undefined;
+        if (key === "NODE_TLS_REJECT_UNAUTHORIZED") report(node);
+        if (key === "rejectUnauthorized" && isFalse(node.right)) report(node);
+      },
+      // vi.stubEnv("NODE_TLS_REJECT_UNAUTHORIZED", "0"), Reflect.set(process.env, "NODE_TLS_REJECT_UNAUTHORIZED", ...)
+      CallExpression(node) {
+        if (node.arguments.some((arg) => arg.type === "Literal" && arg.value === "NODE_TLS_REJECT_UNAUTHORIZED")) report(node);
+      },
+    };
+  },
+};
+
 const moduleBoundary = {
   meta: {
     type: "problem",
@@ -148,7 +189,7 @@ export default [
   {
     files: SOURCE,
     languageOptions: { parser: tseslint.parser, ecmaVersion: 2024, sourceType: "module" },
-    plugins: { appduct: { rules: { "module-boundary": moduleBoundary, "no-node-io-dynamic-import": noNodeIoDynamicImport } } },
+    plugins: { appduct: { rules: { "module-boundary": moduleBoundary, "no-node-io-dynamic-import": noNodeIoDynamicImport, "no-tls-bypass": noTlsBypass } } },
   },
   {
     files: SOURCE,
@@ -165,4 +206,9 @@ export default [
     ignores: LEGACY_VI_MOCK,
     rules: { "no-restricted-properties": VI_MOCK_RESTRICTION },
   },
+  {
+    files: TESTS,
+    ignores: LEGACY_TLS_BYPASS,
+    rules: { "appduct/no-tls-bypass": "error" },
+  },
 ];

packages/appduct/src/__tests__/lint-boundaries.test.ts

@@ -2,7 +2,8 @@
  * The architecture rules in AGENTS.md are enforced by the root ESLint config. These tests lint
  * small snippets under hypothetical paths, so they pin the rules themselves rather than the
  * current state of the tree: a module's index.ts is its only import surface, Node I/O is reached
- * only from adapters and composition roots, and tests never module-mock.
+ * only from adapters and composition roots, tests never module-mock, and tests never switch off
+ * TLS verification.
  */
 import path from "node:path";
 import { fileURLToPath, pathToFileURL } from "node:url";
@@ -86,6 +87,64 @@ describe("lint: tests", () => {
   });
 });
 
+describe("lint: TLS verification in tests", () => {
+  test("a test switching off TLS verification process-wide fails", async () => {
+    const rules = await lint("packages/appduct/src/__tests__/probe.integration.test.ts", 'process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0";\n');
+    expect(rules).toContain("appduct/no-tls-bypass");
+  });
+
+  test("the bracketed form of the same assignment fails", async () => {
+    const rules = await lint("packages/appduct/src/__tests__/probe.integration.test.ts", 'process.env["NODE_TLS_REJECT_UNAUTHORIZED"] = "0";\n');
+    expect(rules).toContain("appduct/no-tls-bypass");
+  });
+
+  test("handing a child process an environment without TLS verification fails", async () => {
+    const rules = await lint("packages/appduct/src/__tests__/probe.e2e.test.ts", 'export const env = { ...process.env, NODE_TLS_REJECT_UNAUTHORIZED: "0" };\n');
+    expect(rules).toContain("appduct/no-tls-bypass");
+  });
+
+  test("stubbing the environment variable through vitest fails", async () => {
+    const rules = await lint("packages/appduct/src/__tests__/probe.integration.test.ts", 'import { vi } from "vitest";\nvi.stubEnv("NODE_TLS_REJECT_UNAUTHORIZED", "0");\n');
+    expect(rules).toContain("appduct/no-tls-bypass");
+  });
+
+  test("stubbing an unrelated environment variable passes", async () => {
+    const rules = await lint("packages/appduct/src/__tests__/probe.integration.test.ts", 'import { vi } from "vitest";\nvi.stubEnv("APPDUCT_HOME", "/tmp/probe");\n');
+    expect(rules).toEqual([]);
+  });
+
+  test("switching off verification on the global HTTPS agent fails", async () => {
+    const rules = await lint("packages/appduct/src/__tests__/probe.integration.test.ts", 'import https from "node:https";\nhttps.globalAgent.options.rejectUnauthorized = false;\n');
+    expect(rules).toContain("appduct/no-tls-bypass");
+  });
+
+  test("a test client passing rejectUnauthorized: false fails", async () => {
+    const rules = await lint(
+      "packages/appduct/src/__tests__/probe.integration.test.ts",
+      'import WebSocket from "ws";\nexport const ws = new WebSocket("wss://127.0.0.1:1", { rejectUnauthorized: false });\n',
+    );
+    expect(rules).toContain("appduct/no-tls-bypass");
+  });
+
+  test("a test helper outside a *.test.ts file is covered too", async () => {
+    const rules = await lint("packages/appduct/src/__tests__/e2e/probe-helper.ts", 'import { connect } from "node:tls";\nexport const s = connect({ port: 1, rejectUnauthorized: false });\n');
+    expect(rules).toContain("appduct/no-tls-bypass");
+  });
+
+  test("a test trusting the daemon's own certificate passes", async () => {
+    const rules = await lint(
+      "packages/appduct/src/__tests__/probe.integration.test.ts",
+      'import WebSocket from "ws";\ndeclare const daemon: { tls: { current(): { certPem: string } } };\nexport const ws = new WebSocket("wss://127.0.0.1:1", { ca: daemon.tls.current().certPem });\n',
+    );
+    expect(rules).toEqual([]);
+  });
+
+  test("source outside tests is not linted by it: on a TLS server the option is about client certificates", async () => {
+    const rules = await lint("packages/appduct/src/daemon/node-probe-server.ts", 'import { createServer } from "node:https";\nexport const s = createServer({ requestCert: false, rejectUnauthorized: false });\n');
+    expect(rules).toEqual([]);
+  });
+});
+
 /**
  * The burn-down lists exempt files that predate the rules. Each entry must still be a genuine
  * violator: a file converted to a port but left on the list would silently re-admit the exact
@@ -97,6 +156,7 @@ describe("lint: burn-down lists", () => {
     LEGACY_NODE_IO: string[];
     LEGACY_VI_MOCK: string[];
     LEGACY_MODULE_BOUNDARY: string[];
+    LEGACY_TLS_BYPASS: string[];
     NODE_IO_RESTRICTION: Linter.RuleEntry;
     VI_MOCK_RESTRICTION: Linter.RuleEntry;
   };
@@ -124,4 +184,9 @@ describe("lint: burn-down lists", () => {
     const { LEGACY_MODULE_BOUNDARY } = await loadConfig();
     expect(await violators(LEGACY_MODULE_BOUNDARY, { "appduct/module-boundary": "error" }, "appduct/module-boundary")).toEqual(LEGACY_MODULE_BOUNDARY);
   });
+
+  test("every LEGACY_TLS_BYPASS entry still switches off TLS verification", async () => {
+    const { LEGACY_TLS_BYPASS } = await loadConfig();
+    expect(await violators(LEGACY_TLS_BYPASS, { "appduct/no-tls-bypass": "error" }, "appduct/no-tls-bypass")).toEqual(LEGACY_TLS_BYPASS);
+  });
 });