From e57e3b5d6e7eae9640f52b25559d28da5100f650 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 14 Aug 2026 17:19:24 -0400 Subject: [PATCH] chore(#3502): widen no-source-grep, close four measured blind spots (#3505) Phase 3 of #3464, following #3465 and #3466. Those phases cut the exemption ceiling 305 -> 278 by removing vestigial markers and rewriting real source-greps behaviorally. This one addresses why the ratchet was weak in the first place: it counts markers, and markers correlated only loosely with violations, because the rule's implementation was far narrower than its intent. Four gaps, each measured against 833 files under tests/ before any code was written: A TEXT_METHODS omitted matchAll, split and replace, and never handled the regex-side form re.test(tracked) / /lit/.test(tracked) where the tracked value is the ARGUMENT rather than the callee object. B The extension test was /\.(?:cjs|js|ts)/, which does not match .cts, .mts or .mjs. Under ADR-457 this repo's production modules live in src/**/*.cts, so the rule has been structurally blind to the entire TypeScript source surface since that migration. Highest-value fix here. C Tracking stopped at one hop, so an intermediate transform (const b = strip(a); b.match(...)) escaped. D Variables were tracked by NAME in a flat Set, with no scope resolution, so a name reused across describe/test blocks was conflated. D removes one verified false positive where an outer `const src = readFileSync(...)` was cross-attributed to a shadowed arrow-function parameter of the same name. Deliberately NOT implemented: flagging reads whose path cannot be statically resolved. Measured at 4255 sites across 255 files, 66 of them newly red, with a 6-of-6 false-positive rate in spot-checking -- every sampled site read a markdown workflow or fixture doc through a path variable, not JS source. The heuristic "no JS extension literal present" inverts to "not a source file" in this codebase. Shipping it would have manufactured exactly the marker-spam dynamic this epic exists to stop. The principled version needs real static resolution (constant-folding path.join and template literals) and is left to a future phase. The widening surfaced two genuinely-invisible violations, handled on their merits rather than uniformly: tests/verifier-behavior-unverified.test.cjs read src/verification.cts and regexed it for the VERIFIER_STATUSES array. Fixed BEHAVIORALLY with no marker: that constant is already exported, so the test now asserts the real runtime value -- strictly stronger, and immune to source formatting. Mutation-checked: injecting present_behavior_unverified into the exported array turns it red, restoring turns it green. tests/adr-index-gate.test.cjs scans src/plan-drift-guard.cts for docs/adr/.md citations and asserts each cited ADR exists. Those citations live in COMMENTS, erased at compile time: no exported value, no runtime observable, and making it behavioral would mean contorting production code into exporting its own documentation citations. Irreducible, so it takes one marker, cited to #3502, stating exactly why. Documenting a real exemption beats leaving the violation invisible, which was the status quo. Two defects in this branch's own work, both found by review and fixed here rather than shipped: FALSE POSITIVE (adversarial review). Hop propagation walked every Identifier in a declarator init and treated any reference to a tracked variable as derivation, regardless of whether the derived VALUE still carried source text. So `const len = raw.length; /^\d+$/.test(len)` was reported as a source-grep. Propagation is now value-shape aware: it follows identity, string-returning string methods, split/join, template embedding, string concatenation, conditional branches and call arguments; it stops at .length, numeric methods (indexOf/search/charCodeAt), boolean methods (includes/startsWith/test), comparisons, negation, typeof, and Number/parseInt/Boolean coercions. Unrecognized shapes still propagate -- the conservative default for a linter is a rarer false positive over a silent false negative, and that choice is documented inline. Seven RuleTester rows now cover this axis, which was previously untested. SUPER-QUADRATIC SCAN (security review). resolveVariable() resolved each identifier with two linear Array.find passes over scope.references and scope.variables, once per identifier walked -- O(vars-in-scope) per lookup. On a synthetic single-scope file of N consts each referencing ~20 priors: 4.76s at N=3000 and 24.32s at N=6000 (~5.1x for 2x N). Replaced with a Map built once per file, lazily, from the scope manager: 0.19s and 0.37s for the same inputs (~1.95x for 2x N, linear). Semantics unchanged. No measurable effect on the real repo either way, but this is exactly the bug class ADR-3212 / local/no-unbounded-quantifier exists to catch, and this repo has a prior incident where a rule written to catch complexity bugs shipped with one of its own. The per-widening measurement had a flaw worth recording: each widening was measured in isolation, so a site needing TWO at once appeared in neither column, and the UNION column was dominated by the rejected dynamic-path noise and never inspected for interactions. adr-index-gate needs both B (.cts) and A (.matchAll) and was missed for exactly that reason. Real newly-red count was 2, not the 1 predicted. Corrected on #3502 rather than quietly amended. Marker-bearing files 277 -> 278 against an unchanged ceiling of 278. The ceiling is NOT raised: the sole addition is the cited irreducible exemption. 27 RuleTester rows cover the change. The valid rows carry as much weight as the invalid ones -- shadowed same-name bindings, sibling block scopes, .md and .json literal reads, dynamic path variables, non-textual derivations, reads never text-searched, and require() of a .cjs must all stay valid. A widening that flagged those would be worse than the status quo, because it would push contributors toward adding markers to silence noise. Known limit, documented rather than papered over: a tracked value round-tripped through an array or object literal and read back via destructuring is still not tracked. Pre-existing, not introduced here, and deliberately not widened for -- closing it means tracking member identity, with its own false-positive surface. Closes #3502 Co-authored-by: sim Co-authored-by: Claude Opus 5 --- eslint-rules/no-source-grep.cjs | 428 +++++++++++++-- tests/adr-index-gate.test.cjs | 5 + tests/eslint-rules.test.cjs | 551 ++++++++++++++++++++ tests/verifier-behavior-unverified.test.cjs | 17 +- 4 files changed, 961 insertions(+), 40 deletions(-) diff --git a/eslint-rules/no-source-grep.cjs b/eslint-rules/no-source-grep.cjs index f51734ac4..8be9da7ef 100644 --- a/eslint-rules/no-source-grep.cjs +++ b/eslint-rules/no-source-grep.cjs @@ -3,25 +3,106 @@ /** * no-source-grep * - * Flags variables bound to readFileSync() of a .cjs/.js/.ts source path that - * later have .includes/.match/.startsWith/.indexOf called on them. + * Flags variables bound to readFileSync() of a .cjs/.cts/.js/.mjs/.mts/.ts + * source path that later have a text-search method called on them, whether + * directly, via a bounded chain of derived bindings (`const b = f(a)`, + * `b = a`, ...), or via `regex.test(tracked)` / `/lit/.test(tracked)`. + * + * Variable identity is resolved through real lexical scope (ESLint + * `Variable` objects via `sourceCode.scopeManager`/`getScope`), not by name + * string, so a same-named binding in an unrelated or shadowing scope is + * never conflated with a tracked one. * * Honor file-level escape comment: // allow-test-rule: */ +// How many derivation hops from the original readFileSync() binding to +// follow before giving up on a transitive chain. hop=1 is the variable +// bound directly to the readFileSync() result; hop=2 is a variable derived +// one step from it; etc. Depth-bounded on purpose (test-matrix.md rows +// 9-11): a chain longer than this is a documented, accepted blind spot, not +// a bug — see 40-design.md "Known limits". +const MAX_TRANSITIVE_HOPS = 3; + +const TEXT_METHODS = new Set([ + 'includes', + 'match', + 'matchAll', + 'startsWith', + 'endsWith', + 'indexOf', + 'search', + 'split', + 'replace', +]); + +// Method names that, called ON an already-tracked value, still return a +// value that may carry the source file's TEXT (string in, string/array +// out) -- so tracking continues to propagate through the derived result. +const PROPAGATING_STRING_METHODS = new Set([ + 'replace', + 'replaceAll', + 'slice', + 'substring', + 'substr', + 'trim', + 'trimStart', + 'trimEnd', + 'toLowerCase', + 'toUpperCase', + 'normalize', + 'padStart', + 'padEnd', + 'concat', + 'repeat', + 'at', + 'toString', + 'valueOf', + 'split', + 'join', +]); + +// Method names that, called ON an already-tracked value, definitively +// return a non-text (number/boolean) result. Calling one of these directly +// on a tracked value IS itself the violation TEXT_METHODS exists to catch +// (see `includes`/`startsWith`/etc. above) -- but the RETURN VALUE of the +// call must not stay tracked, or `const ok = raw.includes('x'); ok.foo()` +// would go on being treated as if `ok` were still source text. +const NON_PROPAGATING_METHODS = new Set([ + 'indexOf', + 'lastIndexOf', + 'search', + 'charCodeAt', + 'codePointAt', + 'localeCompare', + 'includes', + 'startsWith', + 'endsWith', + 'test', +]); + +// Global "shape-narrowing" functions whose return value is definitively +// not text, regardless of what is passed in. +const NON_PROPAGATING_CALLEE_NAMES = new Set([ + 'Number', + 'parseInt', + 'parseFloat', + 'Boolean', +]); + /** @type {import('eslint').Rule.RuleModule} */ const rule = { meta: { type: 'problem', docs: { description: - 'Disallow reading source .cjs/.js/.ts files with readFileSync and then doing text search on the result', + 'Disallow reading source .cjs/.cts/.js/.mjs/.mts/.ts files with readFileSync and then doing text search on the result', category: 'Best Practices', }, schema: [], messages: { noSourceGrep: - 'Source-grep test: do not read source .cjs/.js/.ts files with readFileSync and call .includes/.match/.startsWith/.indexOf on the result. Use require() to run the module instead. Add // allow-test-rule: at the top of the file to suppress.', + 'Source-grep test: do not read source .cjs/.cts/.js/.mjs/.mts/.ts files with readFileSync and call .includes/.match/.matchAll/.startsWith/.indexOf/.split/.replace/.search (or regex.test()) on the result. Use require() to run the module instead. Add // allow-test-rule: at the top of the file to suppress.', }, }, create(context) { @@ -36,15 +117,57 @@ const rule = { ); if (hasAllowAnnotation) return {}; - // Track variable names bound to readFileSync of a source path - const sourceGrepVars = new Set(); + // Map from Identifier AST node -> resolved ESLint `Variable`, built once + // per file (see buildIdentifierVariableMap) so that resolveVariable() is + // an O(1) lookup instead of a per-call linear scan over scope.references + // / scope.variables. Populated lazily on first use from Program:exit, + // after the scope manager has finished analyzing the whole file, and + // rebuilt fresh for every file since `create(context)` runs per file + // (nothing here is module-level state). + let identifierVariableMap = null; - // Detect if a node represents a readFileSync call on a source file (.cjs/.js/.ts) - // that lives in a source directory (bin, lib, gsd-core, src). + // Walk every scope exactly once and record, for each Identifier node + // that is either a resolved reference or a declaration site, the + // `Variable` it resolves to. References are indexed first and + // declarations only fill in gaps, mirroring the precedence of the + // original per-call algorithm (which checked scope.references before + // falling back to scope.variables) -- though in practice an Identifier + // node can only ever be one or the other, never both. + function buildIdentifierVariableMap() { + const map = new Map(); + const scopeManager = sourceCode.scopeManager; + for (const scope of scopeManager.scopes) { + for (const ref of scope.references) { + if (ref.resolved) map.set(ref.identifier, ref.resolved); + } + } + for (const scope of scopeManager.scopes) { + for (const variable of scope.variables) { + for (const def of variable.defs) { + if (def.name && !map.has(def.name)) map.set(def.name, variable); + } + } + } + return map; + } + + // Resolve an Identifier node to the ESLint `Variable` it names, via real + // scope analysis rather than name-string matching. Handles both uses + // (references, resolved through reference.resolved) and declaration + // sites (the `id` of a VariableDeclarator, a parameter, etc.). + function resolveVariable(identifierNode) { + if (!identifierVariableMap) { + identifierVariableMap = buildIdentifierVariableMap(); + } + return identifierVariableMap.get(identifierNode) || null; + } + + // Detect if a node represents a readFileSync call on a source file + // (.cjs/.cts/.js/.mjs/.mts/.ts) that lives in a source directory + // (bin, lib, gsd-core, src). function isSourceReadFileSync(node) { - if (node.type !== 'CallExpression') return false; + if (!node || node.type !== 'CallExpression') return false; - // Match: readFileSync(...) or fs.readFileSync(...) or require('fs').readFileSync(...) const callee = node.callee; const isFsRead = (callee.type === 'Identifier' && callee.name === 'readFileSync') || @@ -64,47 +187,284 @@ const rule = { } // Given the source text of a path expression, determine if it references - // a .cjs/.js/.ts source file in a source directory. + // a .cjs/.cts/.js/.mjs/.mts/.ts source file in a source directory. function looksLikeSourcePath(src) { - // Must end with a .cjs, .js, or .ts extension (in a string) - const hasCjsExt = /['"`.][^'"`.]*\.(?:cjs|js|ts)['"`)]/i.test(src); - if (!hasCjsExt) return false; + // Must end with a source extension (in a string). Longer extensions + // are listed first in the alternation so `.cts`/`.mts`/`.mjs` are + // never partially matched by the shorter `.js`/`.ts`/`.cjs` arms. + const hasSourceExt = /['"`.][^'"`.]*\.(?:cts|mts|mjs|cjs|js|ts)['"`)]/i.test(src); + if (!hasSourceExt) return false; // Must reference a source directory indicator somewhere in the expression const hasSourceDir = /['"](?:bin|lib|gsd-core|src)['"]/i.test(src); return hasSourceDir; } - const TEXT_METHODS = new Set(['includes', 'match', 'startsWith', 'endsWith', 'indexOf', 'search']); + // Variable -> hop number. hop=1 is a variable bound directly to a + // source readFileSync() result; each additional derivation hop + // increments by 1, capped at MAX_TRANSITIVE_HOPS. + const hopOf = new Map(); + + // Generic conservative fallback: walk every Identifier under `node` and + // return the smallest hop number among identifiers that resolve to an + // already-tracked variable, or null if none do. This is the DEFAULT for + // any expression shape not explicitly recognized below (arguments to an + // unknown function call, logical expressions, etc.) -- for an + // unrecognized shape we choose to PROPAGATE (risking a rarer false + // positive) rather than silently drop a true positive, because the + // callee/operator may still be returning text derived from the tracked + // value. Clearly-scalar shapes (member access, comparisons, numeric/ + // boolean methods, Number()/parseInt()/etc.) are special-cased below to + // explicitly NOT propagate instead, since for those we know for certain + // the result cannot carry text. + function walkForTrackedHop(node) { + let min = null; + (function walk(n) { + if (!n || typeof n.type !== 'string') return; + if (n.type === 'Identifier') { + const v = resolveVariable(n); + if (v && hopOf.has(v)) { + const h = hopOf.get(v); + if (min === null || h < min) min = h; + } + } + for (const key of Object.keys(n)) { + if (key === 'parent') continue; + const val = n[key]; + if (Array.isArray(val)) { + for (const child of val) { + if (child && typeof child.type === 'string') walk(child); + } + } else if (val && typeof val.type === 'string') { + walk(val); + } + } + })(node); + return min; + } + + // Determine whether tracking should propagate through `node`'s value + // into whatever it is assigned/bound to, and if so, at what (minimum) + // hop it draws from. Returns null when the value shape is one we know + // for certain cannot still carry the tracked file's text. + function minTrackedHop(node) { + if (!node || typeof node.type !== 'string') return null; + + switch (node.type) { + case 'Identifier': { + // Identity: `const b = a;` + const v = resolveVariable(node); + return v && hopOf.has(v) ? hopOf.get(v) : null; + } + + case 'AwaitExpression': + return minTrackedHop(node.argument); + + case 'ConditionalExpression': { + // `cond ? a : other` -- only the branches can carry the tracked + // value; the test itself is a boolean and does not propagate. + const c = minTrackedHop(node.consequent); + const a = minTrackedHop(node.alternate); + if (c === null) return a; + if (a === null) return c; + return Math.min(c, a); + } + + case 'TemplateLiteral': { + // `` `${a}` `` -- a template embedding a tracked value still + // carries its text. + let min = null; + for (const expr of node.expressions) { + const h = minTrackedHop(expr); + if (h !== null && (min === null || h < min)) min = h; + } + return min; + } + + case 'BinaryExpression': { + // String concatenation (`a + 'x'` / `'x' + a`) may still carry + // text; every OTHER binary operator (===, !==, ==, !=, <, >, <=, + // >=, arithmetic, etc.) produces a boolean/number and must not + // propagate. + if (node.operator !== '+') return null; + const l = minTrackedHop(node.left); + const r = minTrackedHop(node.right); + if (l === null) return r; + if (r === null) return l; + return Math.min(l, r); + } + + case 'UnaryExpression': + // `!x`, `typeof x`, `void x`, `-x`, `+x`, `~x` all produce a + // non-text primitive. + return null; + + case 'MemberExpression': + // Bare property/element access that is NOT itself a call (e.g. + // `.length`, `.size`, or any other property read). This is the + // reported false-positive shape (`const len = raw.length;`): + // none of these definitively still carry the original text, so + // do not propagate. + return null; + + case 'ArrayExpression': + case 'ObjectExpression': + // Do not widen into array/object literals or the destructuring + // that would be needed to read a value back out of them. This is + // a documented, deliberate blind spot (not a new bug) -- see + // 40-design.md "Known limits". + return null; + + case 'CallExpression': { + const callee = node.callee; + + // Number(...), parseInt(...), parseFloat(...), Boolean(...): + // the result is definitively not text, regardless of the arg. + if ( + callee.type === 'Identifier' && + NON_PROPAGATING_CALLEE_NAMES.has(callee.name) + ) { + return null; + } + + // Array.isArray(...): definitively boolean. + if ( + callee.type === 'MemberExpression' && + callee.object.type === 'Identifier' && + callee.object.name === 'Array' && + callee.property.type === 'Identifier' && + callee.property.name === 'isArray' + ) { + return null; + } + + // Method call on a tracked receiver: `obj.method(...)`. Whether + // the result stays tracked depends on what the method returns. + if ( + callee.type === 'MemberExpression' && + callee.property.type === 'Identifier' + ) { + const objHop = minTrackedHop(callee.object); + if (objHop !== null) { + const propName = callee.property.name; + if (NON_PROPAGATING_METHODS.has(propName)) return null; + if (PROPAGATING_STRING_METHODS.has(propName)) return objHop; + // Unrecognized method name on a known-tracked receiver: + // conservative default for an unrecognized call result (see + // fallback rationale above) -- propagate rather than risk + // silently dropping a true positive. + return objHop; + } + } + + // Not a recognized narrowing/receiver call shape: fall through + // to the generic conservative walk (covers "tracked value passed + // as an argument to any call", e.g. `const b = strip(a);`). + return walkForTrackedHop(node); + } + + default: + // Any other expression shape (LogicalExpression, parenthesized + // expressions -- which are not a distinct AST node -- etc.): + // conservative default, see walkForTrackedHop doc comment. + return walkForTrackedHop(node); + } + } + + const pendingDeclarators = []; + const pendingAssignments = []; + const pendingCalls = []; return { VariableDeclarator(node) { - // const varName = readFileSync(...) OR const varName = fs.readFileSync(...) - if (node.init && isSourceReadFileSync(node.init)) { - if (node.id.type === 'Identifier') { - sourceGrepVars.add(node.id.name); - } + if (node.id.type === 'Identifier' && node.init) { + pendingDeclarators.push({ id: node.id, init: node.init }); } }, AssignmentExpression(node) { - if (node.right && isSourceReadFileSync(node.right)) { - if (node.left.type === 'Identifier') { - sourceGrepVars.add(node.left.name); - } + if (node.left.type === 'Identifier' && node.right) { + pendingAssignments.push({ left: node.left, right: node.right }); } }, CallExpression(node) { - // varName.includes(...), varName.match(...), etc. - if ( - node.callee.type === 'MemberExpression' && - TEXT_METHODS.has(node.callee.property.name) - ) { - const obj = node.callee.object; - if (obj.type === 'Identifier' && sourceGrepVars.has(obj.name)) { - context.report({ node, messageId: 'noSourceGrep' }); + if (node.callee.type !== 'MemberExpression') return; + const propName = node.callee.property.name; + if (TEXT_METHODS.has(propName)) { + pendingCalls.push({ node, kind: 'textMethod' }); + } else if (propName === 'test') { + pendingCalls.push({ node, kind: 'regexTest' }); + } + }, + 'Program:exit'() { + // Seed hop=1 for variables bound directly to a source readFileSync(). + for (const { id, init } of pendingDeclarators) { + if (isSourceReadFileSync(init)) { + const v = resolveVariable(id); + if (v && !hopOf.has(v)) hopOf.set(v, 1); } - // Inline: readFileSync(...).includes(...) - if (isSourceReadFileSync(obj)) { + } + for (const { left, right } of pendingAssignments) { + if (isSourceReadFileSync(right)) { + const v = resolveVariable(left); + if (v && !hopOf.has(v)) hopOf.set(v, 1); + } + } + + // Fixpoint over derived bindings, bounded by MAX_TRANSITIVE_HOPS. + // Each variable is added at most once, so this always terminates. + let changed = true; + while (changed) { + changed = false; + for (const { id, init } of pendingDeclarators) { + const v = resolveVariable(id); + if (!v || hopOf.has(v)) continue; + const parentHop = minTrackedHop(init); + if (parentHop !== null && parentHop + 1 <= MAX_TRANSITIVE_HOPS) { + hopOf.set(v, parentHop + 1); + changed = true; + } + } + for (const { left, right } of pendingAssignments) { + const v = resolveVariable(left); + if (!v || hopOf.has(v)) continue; + const parentHop = minTrackedHop(right); + if (parentHop !== null && parentHop + 1 <= MAX_TRANSITIVE_HOPS) { + hopOf.set(v, parentHop + 1); + changed = true; + } + } + } + + // Now that hopOf is stable, evaluate every candidate call site. + for (const { node, kind } of pendingCalls) { + const obj = node.callee.object; + + if (kind === 'textMethod') { + // varName.includes(...), varName.match(...), etc. + if (obj.type === 'Identifier') { + const v = resolveVariable(obj); + if (v && hopOf.has(v)) { + context.report({ node, messageId: 'noSourceGrep' }); + continue; + } + } + // Inline: readFileSync(...).includes(...) + if (isSourceReadFileSync(obj)) { + context.report({ node, messageId: 'noSourceGrep' }); + } + continue; + } + + // kind === 'regexTest': re.test(tracked) or /lit/.test(tracked). + // The tracked variable is the ARGUMENT here, not the callee object. + const looksLikeRegexReceiver = + obj.type === 'Identifier' || (obj.type === 'Literal' && !!obj.regex); + if (!looksLikeRegexReceiver) continue; + + const args = node.arguments; + if (!args || args.length === 0) continue; + + if (minTrackedHop(args[0]) !== null) { context.report({ node, messageId: 'noSourceGrep' }); } } diff --git a/tests/adr-index-gate.test.cjs b/tests/adr-index-gate.test.cjs index 077a68304..d34c91a47 100644 --- a/tests/adr-index-gate.test.cjs +++ b/tests/adr-index-gate.test.cjs @@ -683,6 +683,11 @@ const ADR_DIR = path.join(REPO_ROOT, 'docs', 'adr'); // the real gate treats fenced (and inline) code as code — markdown does not // render a link there, so masking it out is correct, not a regression. +// allow-test-rule: source-text-is-the-product — the ADR citation is a comment in src/plan-drift-guard.cts, erased at compile time, so no runtime observation can reach it (#3502) +// This site surfaced only after no-source-grep was widened to recognize .cts +// reads and .matchAll() (#3502); it is irreducible, not unconverted — there is +// no exported value to require() in its place, since a comment leaves no +// runtime trace to assert against. test('the ADR path cited by src/plan-drift-guard.cts exists', () => { // This module is compiled into the published payload, so a wrong citation // here ships to users. diff --git a/tests/eslint-rules.test.cjs b/tests/eslint-rules.test.cjs index 8a024bcd3..205b495ea 100644 --- a/tests/eslint-rules.test.cjs +++ b/tests/eslint-rules.test.cjs @@ -138,6 +138,557 @@ describe('no-source-grep rule', () => { }); }); +// ─── no-source-grep widening (#3502 / Phase 3 of #3464) ───────────────────── +// +// One RuleTester case per row of .gsd/phase/chore-3464-widen-source-grep/ +// 50-test-matrix.md. Row numbers in test names refer to that matrix. + +describe('no-source-grep rule — widening (#3502)', () => { + test('row 1: baseline literal .cjs read + .includes() (happy regression)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + src.includes('x'); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 2: .cts source read + .match() (gap B)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const ROOT = '/repo'; + const src = fs.readFileSync(path.join(ROOT, 'src', 'verification.cts'), 'utf-8'); + src.match(/x/); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 3: .mts source read + .match() (gap B)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const ROOT = '/repo'; + const src = fs.readFileSync(path.join(ROOT, 'src', 'x.mts'), 'utf-8'); + src.match(/x/); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 4: .mjs source read + .match() (gap B)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const ROOT = '/repo'; + const src = fs.readFileSync(path.join(ROOT, 'src', 'x.mjs'), 'utf-8'); + src.match(/x/); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 5: .matchAll() on a tracked read (gap A)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + src.matchAll(/x/g); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 6: regex.test(tracked) (gap A, argument-side detection)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const re = /x/; + re.test(src); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 7: /lit/.test(tracked) (gap A, argument-side detection)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + /x/.test(src); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 8: .split() / .replace() probes (gap A)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + src.split('\\n'); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + { + code: ` + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + src.replace(/x/, ''); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 9: two-hop derived variable (gap C)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + function strip(x) { return x; } + const a = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const b = strip(a); + b.match(/x/); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 10: three-hop derived variable — at the depth bound (gap C, boundary)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const a = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const b = a; + const c = b; + c.includes('x'); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('row 11: hop chain beyond the configured depth is a documented limit (gap C, boundary)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const a = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const b = a; + const c = b; + const d = c; + d.includes('x'); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('row 12: shadowed same-name param — false-positive guard (gap D)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const fn = (src) => src.replace(/x/, 'y'); + fn('unrelated'); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('row 13: same name, sibling block scopes — false-positive guard (gap D)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + { + const c = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + } + { + const c = 'x'; + c.includes('y'); + } + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('row 14: .md literal read + .includes() (negative space, unchanged)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const content = fs.readFileSync(path.join(__dirname, '..', 'workflows', 'a.md'), 'utf-8'); + content.includes('x'); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('row 15: .json literal read + .match() (negative space, unchanged)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const content = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.json'), 'utf-8'); + content.match(/x/); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('row 16: dynamic path variable → .includes() — deliberately not flagged (rejected widening)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const fs = require('fs'); + function readIt(p) { + const content = fs.readFileSync(p, 'utf-8'); + content.includes('x'); + } + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('row 17: tracked read, no text search (negative space, unchanged)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const data = JSON.parse(fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8')); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('row 18: require() of a .cjs (negative space, unchanged)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const mod = require('../lib/a.cjs'); + mod.someMethod(); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('row 19: file carrying the file-level suppression annotation suppresses an otherwise-invalid fixture', () => { + // The raw marker text is assembled via string concatenation so this + // FILE's own bytes never contain a contiguous "allow" + "-test-rule:" + // token (scripts/lint-allow-test-rule-refs.cjs does a raw whole-file + // substring scan). At RuleTester-run time the concatenation resolves to + // a real single-line comment, which the rule under test honors normally. + const marker = '// ' + 'allow' + '-test-rule: split marker for row 19, see #3502'; + const code = [ + "const fs = require('fs');", + "const path = require('path');", + marker, + "const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8');", + "src.includes('x');", + ].join('\n'); + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('row 20: marker text inside a string literal (not a comment) does not suppress', () => { + // Same split-marker technique as row 19, applied to a STRING literal + // (not a comment) — this row exists to prove the rule's suppression + // check only honors an actual comment, per the #3465 discriminator. + const stringMarkerLine = "const note = '" + 'allow' + "-test-rule: this is just data, not a directive';"; + const code = [ + "const fs = require('fs');", + "const path = require('path');", + stringMarkerLine, + "const src = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8');", + 'src.includes(note);', + ].join('\n'); + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); +}); + +// ─── no-source-grep hop-propagation value-shape (adversarial-review fix) ──── +// +// minTrackedHop() used to walk EVERY Identifier under a derivation's RHS +// and treat any bare reference to a tracked variable as propagating, +// regardless of whether the derived VALUE could still carry text (e.g. +// `.length`). These rows cover the value-shape gate that replaced that +// blind walk: propagate only through derivations that plausibly still +// carry the source file's text; do not propagate through scalar-producing +// shapes (member access, numeric/boolean methods, comparisons, Number() +// et al). + +describe('no-source-grep rule — hop-propagation value-shape (adversarial-review fix)', () => { + test('valid: .length derivation does not propagate (reported false-positive repro)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const raw = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const len = raw.length; + if (/^\\d+$/.test(len)) {} + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('invalid: numeric-returning method derivation does not cascade to a second error', () => { + // raw.indexOf('x') is itself already flagged directly (indexOf is one + // of the TEXT_METHODS this rule flags on a tracked receiver, unrelated + // to hop propagation). The important assertion here is that there is + // exactly ONE error, not two: the numeric result of .indexOf() must + // NOT stay tracked, so String(n).includes('1') is not a second finding. + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const raw = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const n = raw.indexOf('x'); + String(n).includes('1'); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('invalid: boolean-returning method derivation does not cascade to a second error', () => { + // Same shape as above with a boolean-returning method: raw.includes('x') + // is itself already flagged directly. The boolean result must NOT stay + // tracked, so String(ok).includes('true') is not a second finding. + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const raw = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const ok = raw.includes('x'); + String(ok).includes('true'); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('valid: comparison of a tracked derivation does not propagate', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const raw = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const same = raw.length === 0; + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('invalid: string-returning method derivation still propagates and is caught', () => { + // raw.replace(...) is flagged directly (replace is a TEXT_METHOD, same + // as the indexOf/includes rows above) AND the string-returning result + // (b) correctly stays tracked, so b.includes('y') is a second, distinct + // finding. Two errors total, both real. + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const raw = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const b = raw.replace(/x/, ''); + b.includes('y'); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }, { messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('invalid: template-literal derivation still propagates and is caught', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const raw = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + const b = \`\${raw}\`; + b.match(/y/); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); + + test('invalid: direct .includes() on the tracked source read is unchanged (no regression)', () => { + ruleTester.run('no-source-grep', noSourceGrep, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const path = require('path'); + const raw = fs.readFileSync(path.join(__dirname, '..', 'lib', 'a.cjs'), 'utf-8'); + raw.includes('x'); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noSourceGrep' }], + }, + ], + }); + }); +}); + // ─── no-magic-sleep-in-tests ───────────────────────────────────────────────── describe('no-magic-sleep-in-tests rule', () => { diff --git a/tests/verifier-behavior-unverified.test.cjs b/tests/verifier-behavior-unverified.test.cjs index 300960df3..9bddf4333 100644 --- a/tests/verifier-behavior-unverified.test.cjs +++ b/tests/verifier-behavior-unverified.test.cjs @@ -81,13 +81,18 @@ test('PARITY: per-truth state never leaks into the overall-status vocabulary', ( }); test('overall-status enum in verification.cts is unchanged (no per-truth leak)', () => { - const cts = fs.readFileSync(path.join(ROOT, 'src', 'verification.cts'), 'utf-8'); - // eslint-disable-next-line local/no-unbounded-quantifier -- parses this repo's own bounded src/verification.cts source, not adversarial input - const m = cts.match(/VERIFIER_STATUSES[^=]*=\s*\[([^\]]*)\]/); - assert.ok(m, 'VERIFIER_STATUSES array must be present'); - assert.doesNotMatch(m[1], /present_behavior_unverified/i); + // Assert on the real exported runtime value rather than regexing the + // source text — strictly stronger (exercises the built module) and + // immune to source formatting changes. + const verificationLib = require(path.join(ROOT, 'gsd-core', 'bin', 'lib', 'verification.cjs')); + const { VERIFIER_STATUSES } = verificationLib; + assert.ok(Array.isArray(VERIFIER_STATUSES), 'VERIFIER_STATUSES array must be present'); + assert.ok( + !VERIFIER_STATUSES.includes('present_behavior_unverified'), + 'VERIFIER_STATUSES must not leak the per-truth present_behavior_unverified state', + ); for (const s of ['passed', 'gaps_found', 'human_needed']) { - assert.match(m[1], new RegExp(`'${s}'`)); + assert.ok(VERIFIER_STATUSES.includes(s), `VERIFIER_STATUSES must contain ${s}`); } });