Phase 4 of #3464, following #3465, #3466 and #3502. Those cut the ceiling 305 -> 278 and made the rule accurate. This closes the remaining structural weakness: suppression was FILE-WIDE, so a single justified exemption silently absolved every other source-grep in that file, forever, including ones added later by someone else. hasAllowAnnotation did comments.some(...) over the whole file and returned {} early. A marker is now checked per report: a violation is suppressed only by a marker on its own line, or on a line above it with nothing but blank lines and other comments in between, bounded by MAX_MARKER_LOOKAHEAD_LINES = 8. The bound is comment-purity rather than raw distance, and that distinction is load-bearing: an intervening line of real code (a `test(...)` opener, say) ends the window even when the marker is physically close. Chosen from the actual placements in the affected files rather than picked a priori -- the repo's convention puts several lines of prose rationale between the marker and the code, so a tighter rule would have invalidated legitimate existing markers and forced churn for no correctness gain. Measured before writing any code, by running the real rule with the suppression check neutralized across all 1194 files its globs match: 14 violation sites in 8 files, and ZERO in files carrying no marker -- so the green build was legitimate, and the entire migration surface was those 14. 11 sites were mechanical: an existing marker already stated the right reason, it just sat too far away. Those were relocated to their call sites with the original #NNN citations preserved. Three were orphans -- the file's markers were about an entirely different concern and nobody had ever justified these reads. All three are fixed BEHAVIORALLY, with no new markers: install-minimal-hooks.test.cjs:975 asserted src.includes('gsd-update-check') && src.includes('replace(') against bin/install.js. It now calls the exported stripStaleGsdHookBlocks() on a legacy TOML fixture and asserts the actual stripped output. This is the case this phase was opened around: it could be added with no review friction and stay invisible indefinitely under file-wide amnesty. config.test.cjs:1917 regex-tested src/init.cts for detectGitCreateTag. It now drives `init complete-milestone` and asserts the git_create_tag field. config-schema.property.test.cjs:1107 did the same for detectFallowConfig; it now drives `init code-review` and asserts fallow_enabled. Each was proven RED against a broken production file and GREEN against the real one, with src/init.cts and bin/install.js confirmed byte-identical afterwards. Marker lines in the 8 files went 20 -> 24, against a filed expectation of "must not increase" (projected 14). That projection was wrong and is corrected on #3508 rather than met by deletion. It assumed every existing marker was a distant blanket that site-scoping would consolidate. Some are already site-adjacent and guard real source-greps the rule CANNOT detect -- verified in install-minimal-hooks.test.cjs:2686-2757, where seven markers each sit directly above a readFileSync(reloadScript) + .includes() pair reading hooks/gsd-config-reload.js. Removing them to hit a number would have repeated the Phase 1 mistake: deleting markers on "the rule doesn't fire" evidence when the rule provably cannot see the violation. An earlier revision of this commit message attributed that invisibility to the #3502 dynamic-path blind spot, on the grounds that reloadScript is a variable. Adversarial review caught that as a false causal claim and it is corrected here. looksLikeSourcePath's hasSourceDir regex is /['"](?:bin|lib|gsd-core|src)['"]/i, and those reads target hooks/ -- so a fully literal path.join(ROOT,'hooks','gsd-config-reload.js') is equally invisible. The variable indirection is irrelevant. This is a FIFTH, distinct blind spot: the source-dir allowlist omits hooks/, which is a real shipped production directory (eslint.config.mjs registers its own rule block for hooks/**/*.js). Recorded in 40-design.md Known limits and left for a follow-on phase -- widening the allowlist is unmeasured, and measuring before widening is the discipline #3502 established. The conclusion was right; the stated mechanism was not, and asserting an unverified cause is the error being corrected. The honest metric is not fewer markers. It is that every marker now sits adjacent to the specific read it justifies instead of absolving a whole file. Site-scoping turns one blanket marker covering N sites into N site markers by design; the count rising is the mechanism working. A second review finding is fixed here too. Suppression originally keyed only off the text-search line, so a marker placed directly above the readFileSync() call -- the intuitive place to annotate "this read is fine" -- did NOT suppress when the search sat on the following line, because the read's own assignment line breaks comment-purity. It failed safe (a loud error, never silent suppression), but it was a trap contributors would hit, and it contradicted this change's own claim that the placement rule would not force churn. A violation is now suppressed by a marker adjacent to EITHER the search site or the originating read. The violation is fundamentally the read+search pair, so annotating either half is legitimate, and it stays strictly site-scoped -- the decisive isolation row still holds. 17 RuleTester rows cover the new semantics. The decisive one asserts that a marker adjacent to one violation does NOT suppress an unrelated violation elsewhere in the same file -- exactly 1 error, reported at the second site. Teeth-checked by reverting the predicate to file-wide, confirming that row and two others flip pass->fail, then restoring. Two pre-existing RuleTester cases that asserted the old file-wide semantics were corrected. Compatibility held where it matters: 277 marker-bearing files have no detectable violation at all, and site-scoping makes their markers no-ops rather than errors. All stay green, untouched. Ceiling unchanged at 278; lint-allow-test-rule-refs reports 278/278. Deliberately not done here, and recorded for the follow-on phase: the same measurement found only 8 of 285 marker-bearing files contain a detectable violation. That suggests a large honest ceiling drop, but "the rule doesn't fire" is the unsound oracle that forced the Phase 1 revert of 295 files, and the rule still has documented blind spots -- as install-minimal-hooks itself demonstrates above. It needs two independent signals agreeing, which is only credible now that the rule is accurate. With suppression site-scoped, "an effective exemption" is finally well-defined, which is what makes re-pointing the ratchet at effective exemptions -- rather than at marker-text presence -- the natural next step. Closes #3508 Co-authored-by: sim <sim@local> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
599 lines
24 KiB
JavaScript
599 lines
24 KiB
JavaScript
'use strict';
|
|
|
|
/**
|
|
* no-source-grep
|
|
*
|
|
* 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 a SITE-SCOPED escape comment: // allow-test-rule: <reason> (#NNN)
|
|
* A marker suppresses only the violation(s) it sits next to (same line, or
|
|
* above with nothing but blank/comment lines between), not the whole file
|
|
* (#3508 / epic #3464 phase 4). "Next to" is checked against EITHER half of
|
|
* the read+search pair -- the text-search call site, or the readFileSync()
|
|
* call that originated the tracked value -- so annotating the read directly
|
|
* (the intuitive placement) suppresses the violation just as well as
|
|
* annotating the search call (adversarial-review fix, epic #3464 phase 4).
|
|
*/
|
|
|
|
// 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;
|
|
|
|
// How many source lines a `// allow-test-rule: <reason>` marker is allowed
|
|
// to sit above the violation it suppresses (0 = same line as the marker's
|
|
// own line, i.e. the line directly below it). The repo's real placement
|
|
// style is a marker followed by a short run of CONTINUATION PROSE (more
|
|
// `//` comment lines expanding on the reason) immediately before the flagged
|
|
// call -- observed spans across the 8 real #3508 violation sites run 0-4
|
|
// comment lines (e.g. the #3502 marker in tests/adr-index-gate.test.cjs, the
|
|
// #770 markers in tests/install-minimal-hooks.test.cjs). 8 gives that a
|
|
// comfortable margin without being effectively unbounded -- large enough to
|
|
// never force churn on a legitimately-placed marker, small enough that a
|
|
// marker meant for one call site cannot drift into covering an unrelated
|
|
// site 40+ lines later (test-matrix.md row 4, the defect this closes).
|
|
const MAX_MARKER_LOOKAHEAD_LINES = 8;
|
|
|
|
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/.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/.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: <reason> (#NNN) directly above (or trailing) the flagged line to suppress just that site.',
|
|
},
|
|
},
|
|
create(context) {
|
|
const sourceCode = context.getSourceCode
|
|
? context.getSourceCode()
|
|
: context.sourceCode;
|
|
|
|
// All comments in the file (used both to find markers and to know which
|
|
// lines are "just a comment" for the lookahead purity check below).
|
|
const allComments = sourceCode.getAllComments();
|
|
|
|
// Line numbers of every `// allow-test-rule: <reason>` marker comment in
|
|
// the file. A marker may span one line (the normal `//` form) or several
|
|
// (a block comment) -- record every line it occupies so a violation on
|
|
// any of those lines counts as "same line" (trailing-marker form, row 2
|
|
// of the test matrix).
|
|
const markerLines = [];
|
|
for (const c of allComments) {
|
|
if (/allow-test-rule:\s*\S/.test(c.value)) {
|
|
for (let l = c.loc.start.line; l <= c.loc.end.line; l++) {
|
|
markerLines.push(l);
|
|
}
|
|
}
|
|
}
|
|
|
|
// Line numbers fully occupied by ANY comment (marker or not) -- a marker
|
|
// followed by ordinary prose lines before the flagged call is the repo's
|
|
// real style (test-matrix.md row 3), so those in-between lines must not
|
|
// disqualify the marker.
|
|
const commentLineSet = new Set();
|
|
for (const c of allComments) {
|
|
for (let l = c.loc.start.line; l <= c.loc.end.line; l++) {
|
|
commentLineSet.add(l);
|
|
}
|
|
}
|
|
|
|
function isBlankLine(line) {
|
|
const text = sourceCode.lines[line - 1];
|
|
return text !== undefined && text.trim() === '';
|
|
}
|
|
|
|
// A violation at `violationLine` is suppressed if some marker sits on
|
|
// that exact line (trailing form) or on an earlier line within
|
|
// MAX_MARKER_LOOKAHEAD_LINES, with every line strictly between the
|
|
// marker and the violation being blank and/or itself a comment line --
|
|
// i.e. no live code (not even the readFileSync() call the marker is
|
|
// ostensibly about) sits between the marker and the call it suppresses.
|
|
// This is what makes suppression SITE-scoped rather than file-wide: a
|
|
// marker parked far above an unrelated later violation (test-matrix.md
|
|
// row 4) no longer reaches it.
|
|
function isSuppressed(violationLine) {
|
|
for (const markerLine of markerLines) {
|
|
if (markerLine > violationLine) continue;
|
|
if (violationLine - markerLine > MAX_MARKER_LOOKAHEAD_LINES) continue;
|
|
let pure = true;
|
|
for (let l = markerLine + 1; l < violationLine; l++) {
|
|
if (!isBlankLine(l) && !commentLineSet.has(l)) {
|
|
pure = false;
|
|
break;
|
|
}
|
|
}
|
|
if (pure) return true;
|
|
}
|
|
return false;
|
|
}
|
|
|
|
// 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;
|
|
|
|
// 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 || node.type !== 'CallExpression') return false;
|
|
|
|
const callee = node.callee;
|
|
const isFsRead =
|
|
(callee.type === 'Identifier' && callee.name === 'readFileSync') ||
|
|
(callee.type === 'MemberExpression' &&
|
|
callee.property.type === 'Identifier' &&
|
|
callee.property.name === 'readFileSync');
|
|
|
|
if (!isFsRead) return false;
|
|
|
|
const args = node.arguments;
|
|
if (!args || args.length === 0) return false;
|
|
|
|
const firstArg = args[0];
|
|
const fullSrc = sourceCode.getText(firstArg);
|
|
|
|
return looksLikeSourcePath(fullSrc);
|
|
}
|
|
|
|
// Given the source text of a path expression, determine if it references
|
|
// a .cjs/.cts/.js/.mjs/.mts/.ts source file in a source directory.
|
|
function looksLikeSourcePath(src) {
|
|
// 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;
|
|
}
|
|
|
|
// 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();
|
|
|
|
// Variable -> line number of the readFileSync() call that originated the
|
|
// value tracked at that variable (same line as the hop=1 seed for a
|
|
// direct binding; propagated unchanged through every derivation hop,
|
|
// since a transitive chain is still fundamentally about the SAME
|
|
// original read+search pair). Populated in lockstep with hopOf below so
|
|
// a report can consult "where was this text actually read from" and
|
|
// honor a marker placed at either half of the pair (adversarial-review
|
|
// fix: marker adjacent to the read alone must suppress too, not just a
|
|
// marker adjacent to the search call).
|
|
const readLineOf = new Map();
|
|
|
|
// Generic conservative fallback: walk every Identifier under `node` and
|
|
// return the {hop, line} of the identifier with 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 walkForTrackedInfo(node) {
|
|
let best = 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 (best === null || h < best.hop) {
|
|
best = { hop: h, line: readLineOf.get(v) };
|
|
}
|
|
}
|
|
}
|
|
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 best;
|
|
}
|
|
|
|
// Determine whether tracking should propagate through `node`'s value
|
|
// into whatever it is assigned/bound to, and if so, at what (minimum)
|
|
// hop -- and from which original read line -- it draws from. Returns
|
|
// null when the value shape is one we know for certain cannot still
|
|
// carry the tracked file's text.
|
|
function trackedInfo(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)
|
|
? { hop: hopOf.get(v), line: readLineOf.get(v) }
|
|
: null;
|
|
}
|
|
|
|
case 'AwaitExpression':
|
|
return trackedInfo(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 = trackedInfo(node.consequent);
|
|
const a = trackedInfo(node.alternate);
|
|
if (c === null) return a;
|
|
if (a === null) return c;
|
|
return c.hop <= a.hop ? c : a;
|
|
}
|
|
|
|
case 'TemplateLiteral': {
|
|
// `` `${a}` `` -- a template embedding a tracked value still
|
|
// carries its text.
|
|
let best = null;
|
|
for (const expr of node.expressions) {
|
|
const info = trackedInfo(expr);
|
|
if (info !== null && (best === null || info.hop < best.hop)) {
|
|
best = info;
|
|
}
|
|
}
|
|
return best;
|
|
}
|
|
|
|
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 = trackedInfo(node.left);
|
|
const r = trackedInfo(node.right);
|
|
if (l === null) return r;
|
|
if (r === null) return l;
|
|
return l.hop <= r.hop ? 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 objInfo = trackedInfo(callee.object);
|
|
if (objInfo !== null) {
|
|
const propName = callee.property.name;
|
|
if (NON_PROPAGATING_METHODS.has(propName)) return null;
|
|
if (PROPAGATING_STRING_METHODS.has(propName)) return objInfo;
|
|
// 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 objInfo;
|
|
}
|
|
}
|
|
|
|
// 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 walkForTrackedInfo(node);
|
|
}
|
|
|
|
default:
|
|
// Any other expression shape (LogicalExpression, parenthesized
|
|
// expressions -- which are not a distinct AST node -- etc.):
|
|
// conservative default, see walkForTrackedInfo doc comment.
|
|
return walkForTrackedInfo(node);
|
|
}
|
|
}
|
|
|
|
const pendingDeclarators = [];
|
|
const pendingAssignments = [];
|
|
const pendingCalls = [];
|
|
|
|
return {
|
|
VariableDeclarator(node) {
|
|
if (node.id.type === 'Identifier' && node.init) {
|
|
pendingDeclarators.push({ id: node.id, init: node.init });
|
|
}
|
|
},
|
|
AssignmentExpression(node) {
|
|
if (node.left.type === 'Identifier' && node.right) {
|
|
pendingAssignments.push({ left: node.left, right: node.right });
|
|
}
|
|
},
|
|
CallExpression(node) {
|
|
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(),
|
|
// recording the readFileSync() call's own line as the "origin read
|
|
// line" for that variable.
|
|
for (const { id, init } of pendingDeclarators) {
|
|
if (isSourceReadFileSync(init)) {
|
|
const v = resolveVariable(id);
|
|
if (v && !hopOf.has(v)) {
|
|
hopOf.set(v, 1);
|
|
readLineOf.set(v, init.loc.start.line);
|
|
}
|
|
}
|
|
}
|
|
for (const { left, right } of pendingAssignments) {
|
|
if (isSourceReadFileSync(right)) {
|
|
const v = resolveVariable(left);
|
|
if (v && !hopOf.has(v)) {
|
|
hopOf.set(v, 1);
|
|
readLineOf.set(v, right.loc.start.line);
|
|
}
|
|
}
|
|
}
|
|
|
|
// Fixpoint over derived bindings, bounded by MAX_TRANSITIVE_HOPS.
|
|
// Each variable is added at most once, so this always terminates.
|
|
// The origin read line is carried through unchanged from whichever
|
|
// parent variable the hop was derived from -- a transitive chain is
|
|
// still fundamentally about the same original read+search pair.
|
|
let changed = true;
|
|
while (changed) {
|
|
changed = false;
|
|
for (const { id, init } of pendingDeclarators) {
|
|
const v = resolveVariable(id);
|
|
if (!v || hopOf.has(v)) continue;
|
|
const parentInfo = trackedInfo(init);
|
|
if (parentInfo !== null && parentInfo.hop + 1 <= MAX_TRANSITIVE_HOPS) {
|
|
hopOf.set(v, parentInfo.hop + 1);
|
|
readLineOf.set(v, parentInfo.line);
|
|
changed = true;
|
|
}
|
|
}
|
|
for (const { left, right } of pendingAssignments) {
|
|
const v = resolveVariable(left);
|
|
if (!v || hopOf.has(v)) continue;
|
|
const parentInfo = trackedInfo(right);
|
|
if (parentInfo !== null && parentInfo.hop + 1 <= MAX_TRANSITIVE_HOPS) {
|
|
hopOf.set(v, parentInfo.hop + 1);
|
|
readLineOf.set(v, parentInfo.line);
|
|
changed = true;
|
|
}
|
|
}
|
|
}
|
|
|
|
// Report a violation at `node` unless a marker's site-scoped
|
|
// suppression (see isSuppressed above) covers either the search
|
|
// call's own line OR the line of the readFileSync() call that
|
|
// originated the tracked value (adversarial-review fix: the
|
|
// violation is fundamentally about the read+search PAIR, so a
|
|
// marker adjacent to either half is a legitimate, still strictly
|
|
// site-scoped, way to annotate it). `readLine` is optional -- pass
|
|
// it whenever the call site can determine one.
|
|
function reportUnlessSuppressed(node, readLine) {
|
|
if (isSuppressed(node.loc.start.line)) return;
|
|
if (readLine !== undefined && readLine !== null && isSuppressed(readLine)) {
|
|
return;
|
|
}
|
|
context.report({ node, messageId: 'noSourceGrep' });
|
|
}
|
|
|
|
// 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)) {
|
|
reportUnlessSuppressed(node, readLineOf.get(v));
|
|
continue;
|
|
}
|
|
}
|
|
// Inline: readFileSync(...).includes(...) -- read and search are
|
|
// the same line, so no separate read line to pass.
|
|
if (isSourceReadFileSync(obj)) {
|
|
reportUnlessSuppressed(node);
|
|
}
|
|
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;
|
|
|
|
const argInfo = trackedInfo(args[0]);
|
|
if (argInfo !== null) {
|
|
reportUnlessSuppressed(node, argInfo.line);
|
|
}
|
|
}
|
|
},
|
|
};
|
|
},
|
|
};
|
|
|
|
module.exports = rule;
|