From 9770258558f547d4baba5d882fa27ea52e5f1e41 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 9 Sep 2026 21:55:51 -0400 Subject: [PATCH] chore(#4590): add no-rendered-text-length-assert ESLint rule (#4595) * test(#4590): add no-rendered-text-length-assert ESLint rule Enforces ADR-456's typed-surface mandate for one specific bug shape: a test assertion whose pass/fail depends on the length/substring content of a template literal that interpolates an OS-derived path (os.tmpdir(), os.homedir(), path.join/resolve/..., or a PATH_RETURNING_FNS resolver). Because macOS's default tmpdir prefix is longer than Linux's, such an assertion can pass on one runner and fail on another -- the defect class behind #4421's incident (git show 4e75b836e9), already fixed there by pinning to a typed field per ADR-456 Sec(c) before this rule existed to catch a recurrence. Two repo-wide sweeps against the real tests/ tree narrowed the rule to a sound scope: an initial design that traced call arguments (to approximate the historical incident's cross-file render-function shape) produced false positives on ordinary fs.readFileSync(path.join(...)) + assert.match patterns; a second design that matched any bare direct path-returning call produced 45 false positives on path suffix/prefix/non-emptiness checks. The shipped rule matches only a path-returning expression interpolated into a template literal, directly or via one identifier hop -- disclosed in the rule's own "Known boundaries" as not covering the literal cross-file incident shape, which would require tracing into a callee's body. Phase 1 of epic #4589 (CI test-matrix Linux-primary migration) -- Phase 2's safety argument depends on this class of OS-dependent test assertion being enforced going forward, not merely fixed once. Co-Authored-By: Claude Sonnet 5 * fix(#4590): address code-review findings on no-rendered-text-length-assert Reletter the "Known boundaries" doc-comment list (a)-(e), fixing a gap left by an earlier edit pass and every stale cross-reference to it. Collapse isDirectPathTaint/isTaintedInterpolation's duplicated TemplateLiteral-walk into one recursive relationship (isTaintedInterpolation now delegates a nested-template-literal case back to isDirectPathTaint instead of re-implementing the .some() traversal) -- behavior unchanged, confirmed by re-running the repo-wide sweep (still zero false positives). Found by the Standards-axis /code-review pass on this PR. Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: sim Co-authored-by: Claude Sonnet 5 --- ...03-portability-enforcement-architecture.md | 38 +++ .../no-rendered-text-length-assert.cjs | 271 +++++++++++++++++ eslint.config.mjs | 9 + ...-rendered-text-length-assert.rule.test.cjs | 279 ++++++++++++++++++ tests/portability-rule-disable-ban.test.cjs | 2 + 5 files changed, 599 insertions(+) create mode 100644 eslint-rules/no-rendered-text-length-assert.cjs create mode 100644 tests/no-rendered-text-length-assert.rule.test.cjs diff --git a/docs/adr/1703-portability-enforcement-architecture.md b/docs/adr/1703-portability-enforcement-architecture.md index 1e51fd80d..d8481147c 100644 --- a/docs/adr/1703-portability-enforcement-architecture.md +++ b/docs/adr/1703-portability-enforcement-architecture.md @@ -80,6 +80,7 @@ Replace all three with a single coherent mechanism: **AST-based ESLint rules in | `no-exact-case-env-access` | `DEFECT.WINDOWS-EXACT-CASE-ENV-ACCESS` | `src/**/*.cts`, `gsd-core/bin/**`, `scripts/**`, `hooks/**` | | `require-full-tmpdir-triad` | `DEFECT.WINDOWS-TEST-PORTABILITY` (#4220) | tests | | `no-unbounded-dirname-walk` | `DEFECT.WINDOWS-TEST-PORTABILITY` (#4020 / #4220) | tests, scripts | +| `no-rendered-text-length-assert` | ADR-456 §(c) typed-surface mandate (not a `DEFECT.WINDOWS-*` class — see #4590 amendment below) | tests | **Amendment (2026-08-18, epic #3411 Phase 3 / #3619).** `no-private-binary-resolution` is the first catalog entry added after the original seven, and it extends this architecture to a @@ -186,6 +187,43 @@ A repo-wide sweep for other instances of either pattern (beyond the incident sit none: `require-full-tmpdir-triad` and `no-unbounded-dirname-walk` both ran clean against the rest of the tree once the three live sites were fixed. +**Amendment (2026-09-09, epic #4589 Phase 1 / #4590).** `no-rendered-text-length-assert` extends +this catalog's zero-escape-hatch discipline to a bug class outside the `DEFECT.WINDOWS-*` +taxonomy: a test assertion whose pass/fail depends on the length or substring content of a +TEMPLATE LITERAL that INTERPOLATES an OS-derived path-returning expression (`os.tmpdir()`, +`os.homedir()`, or a `PATH_RETURNING_FNS` resolver) alongside other rendered content. Because +macOS's default tmpdir prefix (`/private/var/folders/…`) is longer than Linux's, such an assertion +can pass on one runner and fail on another — the root cause behind #4421's incident +(`git show 4e75b836e9`, `tests/state-todos-render.test.cjs`), which had already been fixed by +pinning the assertion to a typed field (`json.todos[0].needs`) per ADR-456 §(c) before this rule +existed to catch a recurrence. The rule catches the same DEFECT CLASS written directly in a test +file — an inline template literal that itself embeds a path-returning expression and is then +length/substring-probed — not the literal cross-file production-render-function incident shape +itself (a call whose return value happens to embed one of its own arguments); detecting the latter +would require tracing into the callee's own function body, which is out of scope for a +single-file AST rule. A bare path-returning expression probed directly, with no surrounding +template literal (e.g. `p.endsWith('.md')`, `resolved.startsWith(root)`, `dir.length > 0`), is +never flagged: it is a direct, deterministic check on the path value itself, not an assertion +about other content that happens to share a rendered string with a variable-length path. + +Reaching this sound scope took two successive repo-wide sweeps. The first targeted an initial +broader design that traced one hop into a resolved call's own arguments to approximate the +cross-file production-render-function shape; that design proved unsound, producing dozens of +false positives on ordinary `fs.readFileSync(path.join(...))` + `assert.match` patterns (correct +code, not instances of the defect), because a call's return value cannot be soundly assumed to +embed one of its own arguments just because that argument is a path. The call-argument tracing was +removed in favor of a one-hop receiver model that flagged ANY direct path-returning call as taint, +with or without a template literal. The second sweep, run against that narrower rule, found the +one-hop-receiver model was itself still too broad: it produced 45 false positives across `tests/` +of exactly one shape — a bare path value probed for a structural property of its own (suffix, +prefix, or non-emptiness), e.g. `.endsWith('.md')` file-extension checks, `.startsWith(root)` +path-confinement checks, and `.length > 0` non-emptiness checks — none of which are instances of +the #4421 OS-tmpdir-length hazard. Bare-direct-path-call matching was removed, restricting the +rule to fire ONLY when the asserted-on value is a template literal interpolating a path-returning +expression. The known miss (the literal cross-file-render-function shape, a receiver-side +two-hop chain, or a path value arriving as a function parameter) is disclosed in the rule's own +header rather than attempted unsoundly. + **Taxonomy coverage.** This catalog addresses every `DEFECT.WINDOWS-*` class plus `DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE` in `CONTEXT.md`, to the extent each is *statically* detectable. `DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE` has two parts: (a) the CRLF / literal-`\n` diff --git a/eslint-rules/no-rendered-text-length-assert.cjs b/eslint-rules/no-rendered-text-length-assert.cjs new file mode 100644 index 000000000..e04b3f3d6 --- /dev/null +++ b/eslint-rules/no-rendered-text-length-assert.cjs @@ -0,0 +1,271 @@ +'use strict'; + +/** + * no-rendered-text-length-assert + * + * Enforces ADR-456's typed-surface mandate for one specific bug shape: a test + * assertion whose pass/fail depends on the LENGTH (or a substring match sensitive + * to length) of a rendered/templated string that embeds an OS-derived path + * (os.tmpdir(), os.homedir(), path.join/resolve/…, or a project resolver from + * PATH_RETURNING_FNS). Because macOS's default tmpdir prefix + * (`/private/var/folders/…`) is longer than Linux's, such an assertion can pass + * on one OS and fail on another — the exact shape behind #4421's incident + * (`git show 4e75b836e9`, `tests/state-todos-render.test.cjs`). + * + * Triggers on: + * .length (op: > < >= <= === !== == !=) + * .includes|startsWith|endsWith() + * assert.match(, /regex/) + * where is (directly, or via one identifier hop) a TEMPLATE + * LITERAL that interpolates a path-returning expression. A bare path-returning + * call used as the receiver on its own — e.g. `path.join(a, b).length > 240` + * or `const p = path.resolve(a, b); p.endsWith('.md')` — is NEVER flagged: the + * hazard this rule targets only exists when the path's OS-dependent length is + * embedded alongside OTHER rendered content, not when a path value is probed + * directly about its own shape. + * + * DEFECT category: this rule is not itself a `DEFECT.WINDOWS-*` class (the + * failure axis is tmpdir LENGTH across POSIX runners, not a Windows-vs-POSIX + * separator/behavior difference) — it is an ADR-456 typed-surface-mandate + * enforcement extension. See docs/adr/1703-portability-enforcement-architecture.md's + * rule catalog for how it is cataloged alongside the `DEFECT.WINDOWS-*` rules. + * + * ── Taint model ───────────────────────────────────────────────────────────── + * + * ONE identifier hop: if the asserted-on expression is a bare Identifier, + * resolve it to its `const`/`let` declarator initializer in the enclosing + * scope, then check whether THAT resolved expression is a TEMPLATE LITERAL + * with at least one interpolated expression that is directly a path-returning + * call. A resolved expression that is itself directly a path-returning call + * (no template literal involved) is NOT taint — only interpolation into a + * larger rendered string counts. No further + * inference through call arguments — a call's return value is not assumed to + * embed its own arguments' content just because one argument happens to be a + * path (a function receiving a path argument does not necessarily embed that + * path in its output; distinguishing genuinely path-embedding renderers from + * ordinary path-consuming functions like fs.readFileSync would require + * tracing into the callee's body, which is out of scope for a single-file AST + * rule — see Known boundaries (d) below, added after a repo-wide sweep with + * the ORIGINAL two-hop design found this exact false-positive shape live in + * the existing suite: `assert.match(md, /regex/)` where + * `md = fs.readFileSync(path.join(...))` was flagged purely because + * readFileSync's argument was a path, not because its return text embeds one. + * + * A template literal is checked by inspecting its interpolated expressions + * directly (no extra hop spent) — `` `foo ${path.join(a,b)} bar` `` is 0-hop + * taint at the point it is written. + * + * ── Known boundaries ──────────────────────────────────────────────────────── + * + * (a) Function-parameter provenance is not traced. If the path-tainted value + * arrives as a parameter to the enclosing function/callback rather than a + * local `const`/`let` declaration, there is no declarator initializer to + * resolve to and the rule does not flag it. + * + * (b) Name-based matching only, inherited from portability-vocab.cjs's + * PATH_RETURNING_FNS: a shadowed local `path`/`os` identifier is STILL + * treated as the real module (matching sibling rule no-path-literal-in-assert's + * own documented boundary) — this can rarely over-fire on a shadowed name, + * an accepted precedent already shipped in this catalog. + * + * (c) Does not re-derive the production truncation threshold. This rule flags + * the test-side anti-pattern (asserting on rendered text that embeds a + * path), not the specific numeric boundary that made any one incident + * OS-specific — that would require analyzing the render function's own + * body, which is out of scope for a single-file test-lint rule. + * + * (d) Does not trace through any function call's arguments to infer that the + * call's return value embeds them. This means the LITERAL historical + * #4421/#2618 incident shape (a call to a cross-file production render + * function, e.g. `renderPendingTodoBullet({ filePath: tmpFile })`, whose + * return value happens to embed the argument) is NOT detected by this + * rule — that would require tracing into the callee's body (a different, + * heavier kind of analysis, evaluated and rejected — see + * .gsd/phase/chore-4590-rendered-text-length-assert/40-design.md + * "Rejected" #2). What IS detected is the same defect class written + * directly in the test file: a template literal that itself interpolates + * a path-returning expression, then has its length/substring probed. A + * repo-wide sweep with an earlier, broader version of this rule (tracing + * into call arguments) found this was the only way to keep the rule + * sound: it produced dozens of false positives on ordinary + * `fs.readFileSync(path.join(...))` + `assert.match` patterns, which are + * correct code, not instances of this defect. + * + * (e) Does not flag a bare path-returning call used directly as the receiver + * (with no surrounding template literal) — e.g. `path.join(a, b).length + * > 240`, `full.endsWith('.md')`, `resolved.startsWith(root)`, or + * `dir.length > 0`. Asserting a property of a path value itself (its own + * suffix, prefix, non-emptiness, or exact length) is a direct, + * deterministic check on that path — not an assertion about SOME OTHER + * rendered content that happens to share a string with a variable-length + * path — so it is not the target defect class. A repo-wide sweep with an + * earlier version of this rule that matched ANY direct path-returning + * call (with or without a template literal) found 45 false positives of + * exactly this shape across `tests/` — file-extension checks, path- + * confinement checks, and non-emptiness checks — none of which are + * instances of #4421's OS-tmpdir-length hazard. + */ + +const { + isPathReturningCall, + isPosixNormalizerCall, + unwrapString, +} = require('./lib/portability-vocab.cjs'); + +/** @type {import('eslint').Rule.RuleModule} */ +const rule = { + meta: { + type: 'problem', + docs: { + description: + 'Disallow length/substring assertions on rendered text that embeds an OS-derived path (ADR-456 typed-surface mandate)', + category: 'Portability', + }, + schema: [], + messages: { + renderedTextLength: + 'Assertion depends on the length/content of rendered text that embeds an OS-derived path ' + + '(ADR-456 typed-surface mandate): this can pass on one runner and fail on another because ' + + "tmpdir/homedir path length differs by OS (e.g. macOS's /private/var/folders/… prefix). " + + 'Extract and assert on the underlying typed/structured field instead.', + }, + }, + + create(context) { + const LENGTH_COMPARISON_OPERATORS = new Set([ + '>', '<', '>=', '<=', '===', '!==', '==', '!=', + ]); + const MEMBERSHIP_METHODS = new Set(['includes', 'startsWith', 'endsWith']); + + function isLengthMember(node) { + return ( + node && + node.type === 'MemberExpression' && + !node.computed && + node.property.type === 'Identifier' && + node.property.name === 'length' + ); + } + + function isNumericLiteral(node) { + return node && node.type === 'Literal' && typeof node.value === 'number'; + } + + // Resolves `node` ONE hop if it is a bare Identifier bound by a simple + // `const`/`let` declarator in an enclosing scope; otherwise returns `node` + // unchanged (safe no-op passthrough for non-Identifier nodes). + function resolveOneHop(node) { + if (!node || node.type !== 'Identifier') return node; + let scope = context.sourceCode + ? context.sourceCode.getScope(node) + : context.getScope(); + while (scope) { + const variable = scope.variables.find((v) => v.name === node.name); + if (variable) { + const def = variable.defs.find((d) => d.type === 'Variable'); + if ( + def && + def.node.type === 'VariableDeclarator' && + def.node.init && + (def.node.parent.kind === 'const' || def.node.parent.kind === 'let') + ) { + return def.node.init; + } + return node; // found the binding but not a simple const/let init — stop + } + scope = scope.upper; + } + return node; + } + + // True when `exprNode` is (after POSIX-normalizer suppression) a + // TemplateLiteral with at least one interpolated expression that is + // path-tainted. A bare path-returning call with no surrounding template + // literal is NOT taint on its own — see "Known boundaries" (e) in the file + // header: asserting on a path value itself (its own suffix/prefix/ + // non-emptiness/length) is not the target defect class. + function isDirectPathTaint(exprNode) { + if (!exprNode) return false; + if (isPosixNormalizerCall(exprNode)) return false; + if (exprNode.type === 'TemplateLiteral') { + return exprNode.expressions.some((e) => isTaintedInterpolation(e)); + } + return false; + } + + // An interpolated expression inside a template literal counts as taint if + // it is (after unwrapping a String() cast) directly a path-returning call, + // or is itself a nested template literal with tainted interpolation + // (delegates back to isDirectPathTaint for that case rather than + // re-implementing the walk). + function isTaintedInterpolation(exprNode) { + if (!exprNode) return false; + const unwrapped = unwrapString(exprNode); + if (isPathReturningCall(unwrapped)) return true; + return isDirectPathTaint(exprNode); + } + + // True when `receiverNode` — after ONE identifier hop — resolves to a + // template literal with path-tainted interpolation. A bare path-returning + // call on its own never qualifies (see "Known boundaries" (e)). + function isPathTaintedReceiver(receiverNode) { + if (!receiverNode) return false; + const resolved = resolveOneHop(receiverNode); + return isDirectPathTaint(resolved); + } + + return { + BinaryExpression(node) { + if (!LENGTH_COMPARISON_OPERATORS.has(node.operator)) return; + + let receiver = null; + if (isLengthMember(node.left) && isNumericLiteral(node.right)) { + receiver = node.left.object; + } else if (isLengthMember(node.right) && isNumericLiteral(node.left)) { + receiver = node.right.object; + } else { + return; // both sides non-literal (a.length === b.length) — no fixed threshold + } + + if (isPathTaintedReceiver(receiver)) { + context.report({ node, messageId: 'renderedTextLength' }); + } + }, + + CallExpression(node) { + const callee = node.callee; + + // .includes|startsWith|endsWith() + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.property.type === 'Identifier' && + MEMBERSHIP_METHODS.has(callee.property.name) && + node.arguments.length >= 1 + ) { + if (isPathTaintedReceiver(callee.object)) { + context.report({ node, messageId: 'renderedTextLength' }); + } + return; + } + + // assert.match(, /regex/) + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.object.type === 'Identifier' && + callee.object.name === 'assert' && + callee.property.type === 'Identifier' && + callee.property.name === 'match' && + node.arguments.length >= 2 + ) { + if (isPathTaintedReceiver(node.arguments[0])) { + context.report({ node, messageId: 'renderedTextLength' }); + } + } + }, + }; + }, +}; + +module.exports = rule; diff --git a/eslint.config.mjs b/eslint.config.mjs index 17e61870f..f63a69fcf 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -39,6 +39,7 @@ import requireRegisteredExit from './eslint-rules/require-registered-exit.cjs'; import noSwallowedPrecondition from './eslint-rules/no-swallowed-precondition.cjs'; import noExactCaseEnvAccess from './eslint-rules/no-exact-case-env-access.cjs'; import noAdhocTimeoutLiteral from './eslint-rules/no-adhoc-timeout-literal.cjs'; +import noRenderedTextLengthAssert from './eslint-rules/no-rendered-text-length-assert.cjs'; const adhocTimeoutLiteralAllowlist = require('./eslint-rules/no-adhoc-timeout-literal.allowlist.json'); @@ -72,6 +73,7 @@ const localPlugin = { 'no-swallowed-precondition': noSwallowedPrecondition, 'no-exact-case-env-access': noExactCaseEnvAccess, 'no-adhoc-timeout-literal': noAdhocTimeoutLiteral, + 'no-rendered-text-length-assert': noRenderedTextLengthAssert, }, }; @@ -717,6 +719,13 @@ export default tseslint.config( // Require a fixed-point termination guard on any dirname() ancestor walk — // a length/equality-only bound spins forever at a Windows drive root (#4020 / #4220). 'local/no-unbounded-dirname-walk': 'error', + // #4590 (epic #4589 Phase 1): ban length/substring assertions on a + // template literal that interpolates an OS-derived path (tmpdir/homedir + // length differs by OS — the #4421 incident shape). Does NOT flag a bare + // path-returning call probed directly (e.g. `.endsWith('.md')`, + // `.length > 0`) — only path-in-rendered-text embedding. See ADR-456 + // §(c) typed-surface mandate. + 'local/no-rendered-text-length-assert': 'error', // Ban unbounded sync child_process spawns in tests (DEFECT.UNBOUNDED-SUBPROCESS). // No allowlist: the epic (#3064) migrated every site; the rule runs with no // exemption surface. The only sanctioned escapes are an explicit `timeout` on diff --git a/tests/no-rendered-text-length-assert.rule.test.cjs b/tests/no-rendered-text-length-assert.rule.test.cjs new file mode 100644 index 000000000..3894647ca --- /dev/null +++ b/tests/no-rendered-text-length-assert.rule.test.cjs @@ -0,0 +1,279 @@ +'use strict'; + +/** + * no-rendered-text-length-assert.rule.test.cjs + * + * RuleTester unit tests for the local/no-rendered-text-length-assert ESLint rule. + * Mirrors the style of tests/no-path-literal-in-assert.rule.test.cjs. + * + * Rule: report when a length/substring assertion is made against rendered text + * that embeds an OS-derived path (os.tmpdir(), os.homedir(), path.join/resolve/…, + * or a PATH_RETURNING_FNS resolver) — the #4421 incident shape. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const { RuleTester } = require('eslint'); + +const noRenderedTextLengthAssert = require('../eslint-rules/no-rendered-text-length-assert.cjs'); + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: 'commonjs', + }, +}); + +// ─── module shape ───────────────────────────────────────────────────────────── + +describe('no-rendered-text-length-assert rule module', () => { + test('exports meta and create', () => { + assert.strictEqual(typeof noRenderedTextLengthAssert.meta, 'object'); + assert.strictEqual(typeof noRenderedTextLengthAssert.create, 'function'); + assert.strictEqual(noRenderedTextLengthAssert.meta.type, 'problem'); + assert.ok(noRenderedTextLengthAssert.meta.messages.renderedTextLength); + }); +}); + +// ─── INVALID cases (violation expected) ─────────────────────────────────────── + +describe('no-rendered-text-length-assert invalid cases', () => { + test('invalid: matrix row 1 — template literal directly embeds path.join, .length > numeric', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [], + invalid: [ + { + code: '`foo ${path.join(a, b)} bar`.length > 240;', + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'renderedTextLength' }], + }, + ], + }); + }); + + test('invalid: matrix row 2 — identifier resolved one hop to a template literal embedding os.tmpdir()', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [], + invalid: [ + { + code: `const bullet = \`x \${os.tmpdir()} y\`; bullet.length <= 240;`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'renderedTextLength' }], + }, + ], + }); + }); + + test("invalid: matrix row 3 — KNOWN BOUNDARY: a locally shadowed `path` identifier is still name-matched as the real module (inherited from portability-vocab.cjs; same accepted boundary as sibling rule no-path-literal-in-assert). This case IS flagged, not a miss.", () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [], + invalid: [ + { + code: `const path = { join: () => 'safe' }; \`x \${path.join(a, b)} y\`.length > 240;`, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'renderedTextLength' }], + }, + ], + }); + }); +}); + +// ─── VALID cases (no violation expected) ───────────────────────────────────── + +describe('no-rendered-text-length-assert valid cases', () => { + test('valid: matrix row 1 — fixed literal, no path taint', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + code: `'literal string'.length === 5;`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 2 — no path-derived field in scope', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + code: `const obj = { name: 'foo' }; JSON.stringify(obj).length === 42;`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 3 — typed field access (the ADR-456-compliant fix shape); also not even the flagged AST shape', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + code: `json.todos[0].needs === 'yes';`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 4 — path-returning call already wrapped in a POSIX normalizer before being passed as the argument; must NOT flag', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + code: `const tmpFile = String(path.join(tmpDir(), 'x')).replace(/\\\\/g, '/'); renderX({ filePath: tmpFile }).length > 240;`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 5 — length-vs-length, no fixed numeric threshold, no truncation-boundary defect is plausible', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + code: `a.length === b.length;`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 6 — KNOWN BOUNDARY: receiver-side two-hop chain (b→a, then a is a bare Identifier not a direct path call or template literal) exceeds the one-hop budget (see rule header "Known boundaries"); documented, deliberate miss', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + code: `const a = path.join(x, y); const b = a; b.length > 240;`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 7 — KNOWN BOUNDARY: tmpFile arrives as a function PARAMETER, no declarator initializer to resolve (see rule header "Known boundaries" (b)); a different resolveOneHop code path than row 6 (binding found but not a Variable def, vs. binding found and resolved to another bare Identifier); documented, deliberate miss', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + code: `function makeAssertion(tmpFile) { tmpFile.length > 240; }`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 8 — hostile/malformed: an identifier with no resolvable declaration anywhere in scope must not crash the rule and must not be flagged', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + code: `undeclaredIdentifier.length > 240;`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 9 — KNOWN NON-GOAL (not a known boundary miss, a deliberate scope decision): the literal historical #4421/#2618 incident shape (call-argument taint) is out of scope for this rule', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + // KNOWN NON-GOAL (not a known boundary miss, a deliberate scope + // decision): this is the literal historical #4421/#2618 incident + // shape, but detecting it would require tracing into renderX's own + // function body to know its return embeds the filePath argument — + // out of scope for a single-file AST rule (see rule file "Known + // boundaries" (e) and 40-design.md "Rejected" #2). An earlier + // broader version of this rule that DID trace call arguments + // produced false positives on ordinary + // fs.readFileSync(path.join(...)) + assert.match patterns across + // dozens of real files in this repo — proving the heuristic + // unsound, not merely theoretically risky. + code: `const tmpFile = path.join(tmpDir(), 'x'); const bullet = renderX({ filePath: tmpFile }); bullet.includes('Needs:');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 10 — regression: earlier rule version flagged this — fs.readFileSync\'s return does not embed its path argument', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + // regression: earlier rule version flagged this — fs.readFileSync's + // return does not embed its path argument + code: `const md = fs.readFileSync(path.join(__dirname, 'x.md'), 'utf8'); assert.match(md, /some pattern/);`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 11 — bare path value, not embedded in a template literal — not the target defect class (see rule\'s "Known boundaries")', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + // bare path value, not embedded in a template literal — not the + // target defect class (see rule's "Known boundaries") + code: `path.join(a, b).length > 240;`, + filename: 'tests/foo.test.cjs', + }, + { + // bare path value, not embedded in a template literal — not the + // target defect class (see rule's "Known boundaries") + code: `const p = path.resolve(a, b); p.length <= 240;`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 12 — regression (repo sweep, 45 false positives): file-extension check on a bare path value, not a rendered-text embedding', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + // real false-positive shape found by a repo-wide sweep: a + // file-extension check on a path value, now correctly excluded + code: `const full = path.join(dir, 'notes.md'); full.endsWith('.md');`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 13 — regression (repo sweep, 45 false positives): path-confinement check on a bare path value, not a rendered-text embedding', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + // real false-positive shape found by a repo-wide sweep: a + // path-confinement check on a resolved path value, now correctly + // excluded + code: `const resolved = path.resolve(root, sub); resolved.startsWith(root);`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); + + test('valid: matrix row 14 — regression (repo sweep, 45 false positives): non-emptiness check on a bare path value, not a rendered-text embedding', () => { + ruleTester.run('no-rendered-text-length-assert', noRenderedTextLengthAssert, { + valid: [ + { + // real false-positive shape found by a repo-wide sweep: a + // non-emptiness check on a path value, now correctly excluded + code: `const dir = path.dirname(x); dir.length > 0;`, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + }); +}); diff --git a/tests/portability-rule-disable-ban.test.cjs b/tests/portability-rule-disable-ban.test.cjs index 9640ac052..5b55b541d 100644 --- a/tests/portability-rule-disable-ban.test.cjs +++ b/tests/portability-rule-disable-ban.test.cjs @@ -47,6 +47,8 @@ const PROTECTED_RULES = [ // #4244 hardening (origin #4020 / #4220 Windows CI hang) — applies to tests/**/*.test.cjs 'require-full-tmpdir-triad', 'no-unbounded-dirname-walk', + // #4590 (epic #4589 Phase 1) + 'no-rendered-text-length-assert', ]; // ── Detect disable directives via the comment text ───────────────────────────