diff --git a/.changeset/generator-payload-alignment.md b/.changeset/generator-payload-alignment.md index 55140bcb..40b85d01 100644 --- a/.changeset/generator-payload-alignment.md +++ b/.changeset/generator-payload-alignment.md @@ -58,3 +58,19 @@ and "the run silently skipped that capture" cannot come apart. A capture declaring a `metadata.taskless.version` this build does not implement is now refused rather than read as if it were version 1. + +A runtime rule now has exactly one executable file, enforced rather than +assumed. + +Only `check.ts` is signed; the capture `*.yml` are inert data the reconcile +gate neither signs nor reports. A second module beside `check.ts` would be +code reachable from a blessed entry point without being blessed itself — one +relative import away — so tampering with it bypasses the gate while `check.ts` +still matches its blessed digest. Such a rule is refused, and `verify` names +the file. + +Every extension a check could import is covered, not only `.ts`, and the +search is recursive: "one import away" is not "one directory away", so a +nested directory would otherwise carry unsigned code straight past the check. +A rule's `.tests/` fixtures are excluded at any depth, since a check reads +real files under a root and a fixture that is itself TypeScript is ordinary. diff --git a/openspec/changes/generator-payload-alignment/specs/cli-runtime-rule-execution/spec.md b/openspec/changes/generator-payload-alignment/specs/cli-runtime-rule-execution/spec.md index b0797d83..7aef6822 100644 --- a/openspec/changes/generator-payload-alignment/specs/cli-runtime-rule-execution/spec.md +++ b/openspec/changes/generator-payload-alignment/specs/cli-runtime-rule-execution/spec.md @@ -46,18 +46,38 @@ executed by `check`. ### Requirement: A runtime rule has exactly one executable file -The CLI SHALL refuse a runtime rule whose directory contains any `.ts` file other than +The CLI SHALL refuse a runtime rule whose directory contains any module file other than `check.ts`. `check.ts` is the only executable surface of a runtime rule and the only artifact carrying a signature, so any other module would be code reachable from a blessed entry point -without itself being blessed. The generator commits to emitting exactly one; this requirement -makes the guarantee enforced rather than trusted. +without itself being blessed — one relative import away, while tampering with it leaves +`check.ts` still matching its blessed digest. The generator commits to emitting exactly one; +this requirement makes the guarantee enforced rather than trusted. + +"Module file" SHALL cover every extension a check could import, not only `.ts`: the loader +transpiles TypeScript, but a `.js` sibling resolves just as readily. + +The search SHALL be recursive. "One import away" is not "one directory away" — a single +`import "./lib/helper.ts"` reaches an arbitrarily deep relative path in one hop — so a search +bounded to the rule root would let any nested directory carry unsigned code past the check while +`check.ts` still matched its blessed signature. The search SHALL also consider only files, so a +directory whose name ends in a module extension is not reported as one. + +The rule's `.tests/` directory SHALL be excluded from this search. A runtime check reads real +files under a root, so a fixture that is itself TypeScript is the normal case rather than a +smuggled helper, and refusing it would break correct rules — a worse failure than the one being +prevented. #### Scenario: A helper module beside check.ts is refused -- **WHEN** a runtime rule directory contains `check.ts` and any other `.ts` file -- **THEN** the CLI SHALL refuse the rule and state why +- **WHEN** a runtime rule directory contains `check.ts` and any other module file, at any depth +- **THEN** the CLI SHALL refuse the rule and name the offending file - **AND** SHALL NOT execute its `check.ts` +#### Scenario: A TypeScript test fixture is not a stray module + +- **WHEN** a runtime rule carries a `.ts` file under its `.tests/` directory +- **THEN** the CLI SHALL still discover and run the rule + ### Requirement: Declared versions are read The CLI SHALL read `RUNTIME_CHECK_PROTOCOL_VERSION` against a delivered check's declared protocol diff --git a/openspec/changes/generator-payload-alignment/tasks.md b/openspec/changes/generator-payload-alignment/tasks.md index 2df0f36f..00bd7dfa 100644 --- a/openspec/changes/generator-payload-alignment/tasks.md +++ b/openspec/changes/generator-payload-alignment/tasks.md @@ -30,8 +30,8 @@ green on its own; none depends on a later one to be correct. ## 4. One executable per runtime rule (slice 4) -- [ ] 4.1 Refuse a runtime rule directory containing any `.ts` other than `check.ts` -- [ ] 4.2 Test that a helper module beside `check.ts` is refused and the check is not invoked +- [x] 4.1 Refuse a runtime rule directory containing any `.ts` other than `check.ts` +- [x] 4.2 Test that a helper module beside `check.ts` is refused and the check is not invoked ## 5. The file-set writer (slice 5) diff --git a/packages/cli/src/rules/inspect.ts b/packages/cli/src/rules/inspect.ts index 81dfdcb7..e7b2109e 100644 --- a/packages/cli/src/rules/inspect.ts +++ b/packages/cli/src/rules/inspect.ts @@ -2,9 +2,17 @@ import { readFile } from "node:fs/promises"; import { parse } from "yaml"; -import { ruleCapturesDirectory, ruleConfigPath, ruleFilePath } from "./engines"; +import { + ruleCapturesDirectory, + ruleConfigPath, + ruleDirectory, + ruleFilePath, +} from "./engines"; import { type EngineName } from "./layout"; -import { assessCaptureDirectory } from "./runtime/discover"; +import { + assessCaptureDirectory, + strayModules, +} from "./runtime/discover"; import { validateValeRule } from "../schemas/vale-rule"; import { verifyRule, type VerifyResult } from "./verify"; import { verifyValeRule } from "./vale/verify"; @@ -173,6 +181,18 @@ export async function verifyOneRule( } catch { errors.push(`Missing check.ts at ${checkFile}`); } + // `check.ts` is the only executable surface and the only signed artifact, so + // a second module beside it is code reachable from a blessed entry point + // without being blessed itself. The generator commits to emitting one; this + // is what makes that a property of our side rather than a promise held on + // the other side of a wire. + const strays = await strayModules(ruleDirectory(cwd, engine, ruleId)); + if (strays.length > 0) { + errors.push( + `${ruleId} contains ${strays.join(", ")} beside check.ts. A runtime rule has exactly one executable file, because only check.ts is signed — anything else it imports would run unverified. The rule is skipped.` + ); + } + const captures = ruleCapturesDirectory(cwd, engine, ruleId); if (captures !== undefined) { const assessed = await assessCaptureDirectory(captures); diff --git a/packages/cli/src/rules/runtime/discover.ts b/packages/cli/src/rules/runtime/discover.ts index 1de012ce..c8cc229f 100644 --- a/packages/cli/src/rules/runtime/discover.ts +++ b/packages/cli/src/rules/runtime/discover.ts @@ -3,12 +3,14 @@ import { join } from "node:path"; import { parse } from "yaml"; +import { isMissingDirectory } from "../errno"; + import { MATCH_MODES, SUPPORTED_METADATA_VERSIONS, } from "../../types/runtime-rule"; import type { CaptureRule, MatchMode } from "../../types/runtime-rule"; -import { RULES_DIRECTORY } from "../layout"; +import { RULES_DIRECTORY, RULE_TESTS_DIRECTORY } from "../layout"; /** * Directory (relative to `.taskless/`) that holds runtime rules — the @@ -18,6 +20,12 @@ import { RULES_DIRECTORY } from "../layout"; */ export const RUNTIME_RULES_DIR = join(RULES_DIRECTORY, "runtime"); +/** The one executable file of a runtime rule, per `ENGINE_LAYOUTS.runtime`. */ +const CHECK_FILE = "check.ts"; + +/** Where a runtime rule's ast-grep capture rules live, per `ENGINE_LAYOUTS.runtime`. */ +const CAPTURES_DIRECTORY = "captures"; + /** A parsed capture `*.yml` of a runtime rule, with the fields the harness needs. */ export interface LoadedCaptureRule { /** Absolute path to the capture `*.yml`. */ @@ -48,6 +56,77 @@ export interface RuntimeRule { checkFile: string; } +/** + * Module files in a runtime rule that are not its `check.ts`. + * + * `check.ts` is the only executable surface of a runtime rule and the only + * artifact carrying a signature; the capture `*.yml` are inert data the + * reconcile gate neither signs nor reports. A second module beside `check.ts` + * would therefore be code reachable from a blessed entry point — one `import + * "./helper.ts"` away — while itself unsigned and unverified, so tampering with + * it bypasses the gate entirely and `check.ts` still matches its blessed digest. + * + * The generator commits to emitting exactly one. THIS MAKES THE GUARANTEE + * CHECKED RATHER THAN TRUSTED, which is the whole reason it is here: a promise + * held on the other side of a wire is not a property of this side. + * + * Every extension a check could import is covered, not just `.ts` — the loader + * transpiles TypeScript, but `import "./helper.js"` resolves to a `.js` file + * sitting there just as happily. + * + * `.tests/` IS DELIBERATELY NOT SEARCHED. A runtime check reads real files + * under a root, so its fixtures are a tree of ordinary source, and a fixture + * that is legitimately TypeScript is the normal case rather than a smuggled + * helper. Searching it would refuse correct rules, which is a worse failure + * than the one being prevented. + */ +const MODULE_EXTENSIONS = [".ts", ".mts", ".cts", ".js", ".mjs", ".cjs"]; + +export async function strayModules(ruleDirectory: string): Promise { + const stray: string[] = []; + + // RECURSIVE, because "one import away" is not "one directory away". A single + // `import "./lib/helper.ts"` reaches an arbitrarily deep relative path in one + // hop, so scanning only the rule root and `captures/` would let any nested + // directory carry unsigned code straight past this check while `check.ts` + // still matched its blessed signature. + async function walk(directory: string, prefix: string): Promise { + let entries; + try { + entries = await readdir(directory, { withFileTypes: true }); + } catch (error) { + // A directory that is genuinely absent contributes nothing. Anything + // else is a real IO problem and must not be read as "nothing here", + // because that is indistinguishable from a clean rule. + if (isMissingDirectory(error)) return; + throw error; + } + for (const entry of entries) { + const relative = prefix === "" ? entry.name : `${prefix}/${entry.name}`; + if (entry.isDirectory()) { + // Fixtures are data the check reads, not modules it imports; see the + // note above on why searching them would refuse correct rules. + if (relative === RULE_TESTS_DIRECTORY) continue; + await walk(join(directory, entry.name), relative); + continue; + } + // `isFile()` rather than a name test: a DIRECTORY named `helper.ts` + // is not an executable file, and reporting it would refuse a fine rule + // with an error naming something that cannot run. + if (!entry.isFile()) continue; + if (relative === CHECK_FILE) continue; + if ( + MODULE_EXTENSIONS.some((extension) => entry.name.endsWith(extension)) + ) { + stray.push(relative); + } + } + } + + await walk(ruleDirectory, ""); + return stray.toSorted(); +} + /** * Narrow an unknown `match` value to a {@link MatchMode}. An absent value is * `anchor`, the documented default; an unrecognized one is `undefined`. @@ -263,9 +342,18 @@ export async function discoverRuntimeRulesIn( // Capture rules live in `captures/`, not at the rule root. The name avoids // "matcher", which denotes a Vale `[]` config section elsewhere in // this same tree. - const captureRules = await loadCaptureRules(join(directory, "captures")); + const captureRules = await loadCaptureRules( + join(directory, CAPTURES_DIRECTORY) + ); if (captureRules.length === 0) continue; // not a runtime rule + // Fails closed, and refuses the WHOLE rule rather than the stray file: + // the danger is what `check.ts` can reach, so removing the extra module + // from the listing would change nothing about what executes. `verify` + // names the files (see `inspect.ts`, which calls the same helper). + const stray = await strayModules(directory); + if (stray.length > 0) continue; + // The check file is always `check.ts` inside the rule directory (per spec). // We deliberately do NOT resolve `metadata.taskless.check` as a path — an // arbitrary value (e.g. `../../evil.ts`) must not be able to point execution @@ -274,7 +362,7 @@ export async function discoverRuntimeRulesIn( name: entry.name, dir: directory, captureRules, - checkFile: join(directory, "check.ts"), + checkFile: join(directory, CHECK_FILE), }); } return rules; diff --git a/packages/cli/test/engine-dispatch.test.ts b/packages/cli/test/engine-dispatch.test.ts index 378a304c..1522e8ec 100644 --- a/packages/cli/test/engine-dispatch.test.ts +++ b/packages/cli/test/engine-dispatch.test.ts @@ -432,6 +432,96 @@ describe("engine dispatch by directory", () => { } ); + it.each([ + ["helper.ts", "helper.ts"], + // The loader transpiles TypeScript, but `import "./helper.js"` resolves to + // a .js file sitting there just as happily. + ["helper.js", "helper.js"], + ["helper.mjs", "helper.mjs"], + // Reachable from check.ts by a relative import just the same. + ["captures/helper.ts", "captures/helper.ts"], + // "One import away" is not "one directory away": a single + // `import "./lib/helper.ts"` reaches any depth in one hop. + ["lib/helper.ts", "lib/helper.ts"], + ["captures/nested/helper.js", "captures/nested/helper.js"], + ["a/b/c/deep.mjs", "a/b/c/deep.mjs"], + ])( + "refuses a runtime rule carrying %s beside check.ts", + async (path, named) => { + const rule = ruleDirectory( + temporaryDirectory, + "runtime", + "logs-abc12345" + ); + await mkdir(join(rule, "captures"), { recursive: true }); + await writeFile( + join(rule, "captures", "logs.yml"), + RUNTIME_CAPTURE, + "utf8" + ); + await writeFile(join(rule, "check.ts"), RUNTIME_CHECK, "utf8"); + // Parent computed from the delivered path rather than via `dirname`, to + // keep this test off the shared import line. + const slash = path.lastIndexOf("/"); + if (slash > 0) { + await mkdir(join(rule, path.slice(0, slash)), { recursive: true }); + } + await writeFile(join(rule, path), "export const x = 1;\n", "utf8"); + + // Only check.ts is signed, so a second module is code reachable from a + // blessed entry point without being blessed itself. + expect(await discoverRuntimeRules(temporaryDirectory)).toHaveLength(0); + + const result = await verifyOneRule(temporaryDirectory, { + engine: "runtime", + ruleId: "logs-abc12345", + } as Parameters[1]); + expect(result.ok).toBe(false); + expect(result.errors.join("\n")).toContain(named); + } + ); + + it("does not report a directory whose name ends in a module extension", async () => { + // `readdir` returns directories too. A directory named `helper.ts` cannot + // execute, so refusing the rule would be a false positive naming something + // that is not a file. + const rule = ruleDirectory(temporaryDirectory, "runtime", "logs-abc12345"); + await mkdir(join(rule, "captures"), { recursive: true }); + await mkdir(join(rule, "helper.ts"), { recursive: true }); + await writeFile( + join(rule, "captures", "logs.yml"), + RUNTIME_CAPTURE, + "utf8" + ); + await writeFile(join(rule, "check.ts"), RUNTIME_CHECK, "utf8"); + + expect(await discoverRuntimeRules(temporaryDirectory)).toHaveLength(1); + }); + + it("allows a TypeScript fixture under .tests/", async () => { + // A runtime check reads real files under a root, so a fixture that is + // legitimately TypeScript is the normal case rather than a smuggled + // helper. Refusing it would break correct rules — a worse failure than the + // one the stray-module check prevents. + const rule = ruleDirectory(temporaryDirectory, "runtime", "logs-abc12345"); + await mkdir(join(rule, "captures"), { recursive: true }); + await mkdir(join(rule, ".tests", "valid"), { recursive: true }); + await writeFile( + join(rule, "captures", "logs.yml"), + RUNTIME_CAPTURE, + "utf8" + ); + await writeFile(join(rule, "check.ts"), RUNTIME_CHECK, "utf8"); + await mkdir(join(rule, ".tests", "valid", "deep"), { recursive: true }); + await writeFile( + join(rule, ".tests", "valid", "deep", "sample.ts"), + "console.log('x');\n", + "utf8" + ); + + expect(await discoverRuntimeRules(temporaryDirectory)).toHaveLength(1); + }); + it("does not discover runtime rules left at the pre-migration path", async () => { // 0004 moves this tree; a leftover here is not a second runtime source. const legacy = join(tasklessDirectory, "runtime-rules", "logs-abc12345");