From 7359bc6643dc34a3650af4697324e41ab5a5008d Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Fri, 30 Oct 2026 10:57:50 -0700 Subject: [PATCH] fix(reference): match eval with any number of arguments in no-eval-call The prompt asks for any call to eval(), but the pattern eval($ARG) matched exactly one argument, so downstream evals graded generated fail cases for eval() and eval(a, b) as wrong. Match any arity, add a fail case for zero and two arguments, name the arities in the prompt, and verify the published sg reference against its own cases. --- .changeset/reference-eval-any-arity.md | 5 +++ .../cli/assets/reference-sg/no-eval-call.yml | 2 +- packages/cli/assets/reference-sg/prompt.md | 4 +- .../tests/fail/eval-multiple-args/payload.ts | 1 + .../tests/fail/eval-no-args/payload.ts | 1 + packages/cli/assets/reference.json | 24 +++++++++++- packages/cli/src/rules/corpus/manifest.ts | 2 + packages/cli/test/reference-verify.test.ts | 37 +++++++++++++++---- packages/cli/test/reference.test.ts | 2 + 9 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 .changeset/reference-eval-any-arity.md create mode 100644 packages/cli/assets/reference-sg/tests/fail/eval-multiple-args/payload.ts create mode 100644 packages/cli/assets/reference-sg/tests/fail/eval-no-args/payload.ts diff --git a/.changeset/reference-eval-any-arity.md b/.changeset/reference-eval-any-arity.md new file mode 100644 index 00000000..eb8d5c96 --- /dev/null +++ b/.changeset/reference-eval-any-arity.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +The `no-eval-call` entry in `@taskless/cli/reference.json` now flags a call to `eval` with any number of arguments, as its prompt asks, and carries a fail case for `eval()` and for `eval(a, b)`. Previously the reference rule matched exactly one argument, so a rule generated correctly from the prompt, with fixtures on each side of the arity boundary as `create-sg-rule` recommends, was graded against a reference that did not fire on them. The prompt now names the arities it expects. The corpus `version` is unchanged: the shape is the same, only the content of one entry moved. diff --git a/packages/cli/assets/reference-sg/no-eval-call.yml b/packages/cli/assets/reference-sg/no-eval-call.yml index 3e04575a..f61d7c6b 100644 --- a/packages/cli/assets/reference-sg/no-eval-call.yml +++ b/packages/cli/assets/reference-sg/no-eval-call.yml @@ -9,4 +9,4 @@ note: |- The evidence is one expression in one file, which is what makes this an `sg` rule rather than a runtime one: no other file has to be read to decide it. rule: - pattern: eval($ARG) + pattern: eval($$$ARGS) diff --git a/packages/cli/assets/reference-sg/prompt.md b/packages/cli/assets/reference-sg/prompt.md index 3f121a3d..79d7e03a 100644 --- a/packages/cli/assets/reference-sg/prompt.md +++ b/packages/cli/assets/reference-sg/prompt.md @@ -1,4 +1,6 @@ -Create a rule that flags any call to `eval()` in TypeScript. +Create a rule that flags any call to `eval()` in TypeScript, whatever the +number of arguments: `eval()`, `eval(code)` and `eval(code, scope)` are all +calls to `eval`. Executing a string at runtime is an injection risk, and it defeats every static analysis the project runs: nothing downstream can see what the code will do. diff --git a/packages/cli/assets/reference-sg/tests/fail/eval-multiple-args/payload.ts b/packages/cli/assets/reference-sg/tests/fail/eval-multiple-args/payload.ts new file mode 100644 index 00000000..5c437bec --- /dev/null +++ b/packages/cli/assets/reference-sg/tests/fail/eval-multiple-args/payload.ts @@ -0,0 +1 @@ +const value = eval(source, scope); diff --git a/packages/cli/assets/reference-sg/tests/fail/eval-no-args/payload.ts b/packages/cli/assets/reference-sg/tests/fail/eval-no-args/payload.ts new file mode 100644 index 00000000..947db09b --- /dev/null +++ b/packages/cli/assets/reference-sg/tests/fail/eval-no-args/payload.ts @@ -0,0 +1 @@ +const value = eval(); diff --git a/packages/cli/assets/reference.json b/packages/cli/assets/reference.json index 86b5c902..5b94be9e 100644 --- a/packages/cli/assets/reference.json +++ b/packages/cli/assets/reference.json @@ -145,11 +145,11 @@ "id": "no-eval-call", "directory": ".taskless/rules/sg/no-eval-call", "ruleFile": "no-eval-call.yml", - "prompt": "Create a rule that flags any call to `eval()` in TypeScript.\n\nExecuting a string at runtime is an injection risk, and it defeats every static\nanalysis the project runs: nothing downstream can see what the code will do.\nPrefer a parser, a lookup table, or an explicit dispatch.\n\nDeciding this needs only the expression itself. No other file has to be read,\nand no state has to be resolved.\n\nCode that should NOT be flagged:\n\n```ts\nconst parsed = JSON.parse(payload);\n```\n\nCode that SHOULD be flagged:\n\n```ts\nconst result = eval(payload);\n```\n", + "prompt": "Create a rule that flags any call to `eval()` in TypeScript, whatever the\nnumber of arguments: `eval()`, `eval(code)` and `eval(code, scope)` are all\ncalls to `eval`.\n\nExecuting a string at runtime is an injection risk, and it defeats every static\nanalysis the project runs: nothing downstream can see what the code will do.\nPrefer a parser, a lookup table, or an explicit dispatch.\n\nDeciding this needs only the expression itself. No other file has to be read,\nand no state has to be resolved.\n\nCode that should NOT be flagged:\n\n```ts\nconst parsed = JSON.parse(payload);\n```\n\nCode that SHOULD be flagged:\n\n```ts\nconst result = eval(payload);\n```\n", "rule": [ { "path": "no-eval-call.yml", - "content": "id: no-eval-call\nlanguage: TypeScript\nseverity: error\nmessage: \"`eval` executes arbitrary code at runtime; use a parser or a lookup table instead.\"\nnote: |-\n This is an example rule, shipped with the Taskless CLI to demonstrate the\n `sg` tier. It was not written for this repository.\n\n The evidence is one expression in one file, which is what makes this an `sg`\n rule rather than a runtime one: no other file has to be read to decide it.\nrule:\n pattern: eval($ARG)\n" + "content": "id: no-eval-call\nlanguage: TypeScript\nseverity: error\nmessage: \"`eval` executes arbitrary code at runtime; use a parser or a lookup table instead.\"\nnote: |-\n This is an example rule, shipped with the Taskless CLI to demonstrate the\n `sg` tier. It was not written for this repository.\n\n The evidence is one expression in one file, which is what makes this an `sg`\n rule rather than a runtime one: no other file has to be read to decide it.\nrule:\n pattern: eval($$$ARGS)\n" } ], "tests": { @@ -162,6 +162,14 @@ { "path": "tests/fail/eval-call/payload.ts", "content": "const result = eval(payload);\n" + }, + { + "path": "tests/fail/eval-no-args/payload.ts", + "content": "const value = eval();\n" + }, + { + "path": "tests/fail/eval-multiple-args/payload.ts", + "content": "const value = eval(source, scope);\n" } ], "cases": [ @@ -176,6 +184,18 @@ "name": "eval-call", "path": "tests/fail/eval-call", "files": ["tests/fail/eval-call/payload.ts"] + }, + { + "bucket": "fail", + "name": "eval-no-args", + "path": "tests/fail/eval-no-args", + "files": ["tests/fail/eval-no-args/payload.ts"] + }, + { + "bucket": "fail", + "name": "eval-multiple-args", + "path": "tests/fail/eval-multiple-args", + "files": ["tests/fail/eval-multiple-args/payload.ts"] } ] } diff --git a/packages/cli/src/rules/corpus/manifest.ts b/packages/cli/src/rules/corpus/manifest.ts index 0a4b6a52..66d55cc7 100644 --- a/packages/cli/src/rules/corpus/manifest.ts +++ b/packages/cli/src/rules/corpus/manifest.ts @@ -82,6 +82,8 @@ export const CORPUS_MANIFESTS: readonly CorpusManifest[] = [ testPaths: [ "tests/pass/json-parse/payload.ts", "tests/fail/eval-call/payload.ts", + "tests/fail/eval-no-args/payload.ts", + "tests/fail/eval-multiple-args/payload.ts", ], }, { diff --git a/packages/cli/test/reference-verify.test.ts b/packages/cli/test/reference-verify.test.ts index 029ead53..cd3686fe 100644 --- a/packages/cli/test/reference-verify.test.ts +++ b/packages/cli/test/reference-verify.test.ts @@ -7,6 +7,7 @@ import { afterEach, describe, expect, it } from "vitest"; import type { Reference } from "../src/rules/reference"; import { findValeBinary } from "../src/rules/vale/binary"; +import { verifyRule } from "../src/rules/verify"; import { verifyValeRule } from "../src/rules/vale/verify"; /** @@ -15,9 +16,10 @@ import { verifyValeRule } from "../src/rules/vale/verify"; * Other teams grade a generated rule by running OUR rule against THEIR cases, * so a reference narrower than its prompt marks a correct generation wrong. * `prefer-use-over-utilize` shipped with a prompt asking for the inflected - * forms and a swap that lacked `utilized` (#591). Nothing ran the reference - * against its own cases, so nothing noticed. This stages the published entry - * exactly as a consumer would and verifies it. + * forms and a swap that lacked `utilized`, and `no-eval-call` with a prompt + * asking for any call and a pattern that matched exactly one argument (#591). + * Nothing ran the reference against its own cases, so nothing noticed. This + * stages each published entry exactly as a consumer would and verifies it. */ const referencePath = join( @@ -48,13 +50,32 @@ function stage(rule: Reference["rules"][number]): string { return cwd; } +/** Read the published corpus and return its rule for one engine. */ +async function referenceRule( + engine: Reference["rules"][number]["engine"] +): Promise { + const reference = JSON.parse( + await readFile(referencePath, "utf8") + ) as Reference; + const rule = reference.rules.find((entry) => entry.engine === engine); + if (rule === undefined) throw new Error(`reference has no ${engine} rule`); + return rule; +} + +describe("the sg reference rule", () => { + it("fires on every fail case and stays quiet on every pass case", async () => { + const rule = await referenceRule("sg"); + + const result = await verifyRule(stage(rule), rule.id); + + expect(result.tests.errors).toEqual([]); + expect(result.success).toBe(true); + }); +}); + withVale("the vale reference rule", () => { it("fires on every fail case and stays quiet on every pass case", async () => { - const reference = JSON.parse( - await readFile(referencePath, "utf8") - ) as Reference; - const rule = reference.rules.find((entry) => entry.engine === "vale"); - if (rule === undefined) throw new Error("reference has no vale rule"); + const rule = await referenceRule("vale"); const result = await verifyValeRule(stage(rule), rule.id); if ("outcome" in result) { diff --git a/packages/cli/test/reference.test.ts b/packages/cli/test/reference.test.ts index 69905454..976b188d 100644 --- a/packages/cli/test/reference.test.ts +++ b/packages/cli/test/reference.test.ts @@ -248,6 +248,8 @@ describe("the reference corpus payload", () => { sg.tests.cases.map((one) => [one.bucket, one.name]).toSorted() ).toEqual([ ["fail", "eval-call"], + ["fail", "eval-multiple-args"], + ["fail", "eval-no-args"], ["pass", "json-parse"], ]); for (const file of sg.tests.files) {