From 6b2da468e9e3d6e2bcc3d11ce907176af24004e9 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Tue, 29 Sep 2026 08:53:34 -0700 Subject: [PATCH 1/3] feat(check): reconcile every rule of every engine from a snapshot, and fail on an edited static rule --- openspec/changes/cli-v2-rule-api/design.md | 35 +- .../cli-v2-rule-api/specs/cli-check/spec.md | 2 +- .../specs/cli-rule-format/spec.md | 13 + .../specs/cli-rule-reconciliation/spec.md | 6 +- .../specs/cli-runtime-rule-execution/spec.md | 4 +- openspec/changes/cli-v2-rule-api/tasks.md | 48 +- packages/cli/src/agent/check.md | 110 ++- packages/cli/src/agent/ci.md | 30 +- packages/cli/src/agent/create-runtime-rule.md | 12 +- packages/cli/src/api/reconcile.ts | 145 --- packages/cli/src/api/restore.ts | 112 --- packages/cli/src/commands/check.ts | 67 +- packages/cli/src/rules/plan-check.ts | 279 ++++++ packages/cli/src/rules/report.ts | 124 +++ packages/cli/src/rules/runtime/plan.ts | 478 ---------- packages/cli/src/rules/runtime/repair.ts | 193 ---- packages/cli/src/rules/runtime/run-set.ts | 118 --- packages/cli/src/rules/snapshot.ts | 171 ++++ packages/cli/src/rules/verdicts.ts | 284 ++++++ packages/cli/src/schemas/check.ts | 44 +- packages/cli/test/check-snapshot.test.ts | 262 ++++++ packages/cli/test/engine-dispatch.test.ts | 61 +- packages/cli/test/repair-integration.test.ts | 835 ------------------ packages/cli/test/repair.test.ts | 265 ------ packages/cli/test/runtime-check.test.ts | 827 +++++++++-------- .../cli/test/runtime-dropped-rules.test.ts | 57 -- packages/cli/test/verdicts.test.ts | 251 ++++++ 27 files changed, 2080 insertions(+), 2753 deletions(-) create mode 100644 openspec/changes/cli-v2-rule-api/specs/cli-rule-format/spec.md delete mode 100644 packages/cli/src/api/reconcile.ts delete mode 100644 packages/cli/src/api/restore.ts create mode 100644 packages/cli/src/rules/plan-check.ts create mode 100644 packages/cli/src/rules/report.ts delete mode 100644 packages/cli/src/rules/runtime/plan.ts delete mode 100644 packages/cli/src/rules/runtime/repair.ts delete mode 100644 packages/cli/src/rules/runtime/run-set.ts create mode 100644 packages/cli/src/rules/snapshot.ts create mode 100644 packages/cli/src/rules/verdicts.ts create mode 100644 packages/cli/test/check-snapshot.test.ts delete mode 100644 packages/cli/test/repair-integration.test.ts delete mode 100644 packages/cli/test/repair.test.ts delete mode 100644 packages/cli/test/runtime-dropped-rules.test.ts create mode 100644 packages/cli/test/verdicts.test.ts diff --git a/openspec/changes/cli-v2-rule-api/design.md b/openspec/changes/cli-v2-rule-api/design.md index 5d6f6c8b..f97299fb 100644 --- a/openspec/changes/cli-v2-rule-api/design.md +++ b/openspec/changes/cli-v2-rule-api/design.md @@ -72,13 +72,29 @@ place, not by staying on raw `fetch`. ### 2. One snapshot per `check`, taken before anything is signed -`check` copies `.taskless/rules/` to `.taskless/.run/rules/` first (replacing -any previous snapshot). Everything after reads only the snapshot: signing, -reporting, config assembly, and all three engines. The assembled configs for a -`check` are written beside it as `.taskless/.run/.vale.ini` and -`.taskless/.run/.sgconfig.yml`, so their root-relative `StylesPath` and -`ruleDirs` resolve into the snapshot unchanged. Runtime rules execute from -`.taskless/.run/rules/runtime/`, replacing `.run/runtime-rules/`. +`check` copies `.taskless/rules/` to `.taskless/.run/snapshot/.taskless/rules/` +first (replacing any previous snapshot). The snapshot **mirrors the project's +layout** under a base directory, `.taskless/.run/snapshot/`, so every existing +path helper and both assemblers work unchanged when handed that base in place of +the project root: the assembled configs land at +`.taskless/.run/snapshot/.taskless/.vale.ini` and `.sgconfig.yml`, and their +root-relative `StylesPath` and `ruleDirs` resolve into the snapshot. The engines +still run from the project root, with only the config path changed. Everything +after the copy reads only the snapshot: signing, reporting, config assembly, and +all three engines. Runtime rules execute from the snapshot, replacing +`.run/runtime-rules/`. + +Measured before building on it (task 4.1): the mixed-engine fixture plus a Vale +rule scoped to `[docs/**/*.md]` produced the same seven findings across five +rules from both config locations, and the subdirectory-scoped rule fired on +`docs/deep/a.md` and not on a top-level file in both. + +The run directory ignores itself (`.taskless/.run/.gitignore` holds `*`), rather +than `check` adding `.run/` to the tracked `.taskless/.gitignore`: a `check` that +rewrote a tracked file would contradict "check writes only under `.taskless/.run/`", +and the first lint run on this repository after the change did exactly that. git, +ast-grep, and Vale all honor the nested file; a test runs both engines with no +outer ignore entry and gets identical findings. The snapshot is taken on every path, including unauthenticated and `--anonymous`, so there is one execution path rather than a verified one and an @@ -260,9 +276,8 @@ Free organization is never refused for a just-generated rule. ## Risks / Trade-offs - **[Risk] Vale section globs might resolve relative to the config file.** The - assembled config moves from `.taskless/` to `.taskless/.run/`. → A test runs - one Vale rule scoped to a subdirectory glob from both locations and asserts - identical findings before the snapshot is wired into `check`. + assembled config moves into the snapshot. → Measured identical before building + (Decision 2), and a test pins it. - **[Risk] 0.11.x stops working when the floor is set.** Remote generation fails and reconcile returns `400` once 0.12.0 ships. → Accepted server-side (#229); 0.11.x degrades to "service unavailable" and skips runtime rules without diff --git a/openspec/changes/cli-v2-rule-api/specs/cli-check/spec.md b/openspec/changes/cli-v2-rule-api/specs/cli-check/spec.md index e3cd1d5e..0a111490 100644 --- a/openspec/changes/cli-v2-rule-api/specs/cli-check/spec.md +++ b/openspec/changes/cli-v2-rule-api/specs/cli-check/spec.md @@ -8,7 +8,7 @@ The CLI SHALL exit with code 0 when no error-severity matches are found (includi - returned an `unsafe` verdict for an `sg` or `vale` rule; or - left a reported rule unaccounted for (in none, or more than one, of `rules`, `unknown`, and `entitlement.withheld`). -The CLI SHALL also exit with code 1 when two rule directories under different engines share an id. Under `--json`, `success` SHALL be `false` whenever the exit code is non-zero. +On an authenticated run that would reconcile, the CLI SHALL also exit with code 1 when two rule directories under different engines share an id. A logged-out run verifies nothing and does not fail on it. Under `--json`, `success` SHALL be `false` whenever the exit code is non-zero. #### Scenario: Exit 0 when clean diff --git a/openspec/changes/cli-v2-rule-api/specs/cli-rule-format/spec.md b/openspec/changes/cli-v2-rule-api/specs/cli-rule-format/spec.md new file mode 100644 index 00000000..dfefefad --- /dev/null +++ b/openspec/changes/cli-v2-rule-api/specs/cli-rule-format/spec.md @@ -0,0 +1,13 @@ +## MODIFIED Requirements + +### Requirement: Reconciliation survives the relayout + +The CLI SHALL report each rule to the reconcile endpoint by its `ruleId` (the rule's directory +name) with file paths relative to the rule's own directory, so where the engine-partitioned +layout places a rule's directory is not part of what is reported. Moving a rule directory +without renaming or editing it SHALL NOT change its reconciled state. + +#### Scenario: Moved rules reconcile unchanged + +- **WHEN** `check` reconciles after the migration has moved rules into `.taskless/rules///` +- **THEN** each rule is reported under the same `ruleId` with the same relative paths and signatures, the server resolves it to the same rule, and no rule is reported as new or missing diff --git a/openspec/changes/cli-v2-rule-api/specs/cli-rule-reconciliation/spec.md b/openspec/changes/cli-v2-rule-api/specs/cli-rule-reconciliation/spec.md index 3b0384c7..dd2474f7 100644 --- a/openspec/changes/cli-v2-rule-api/specs/cli-rule-reconciliation/spec.md +++ b/openspec/changes/cli-v2-rule-api/specs/cli-rule-reconciliation/spec.md @@ -230,8 +230,8 @@ outside this check. ### Requirement: The CLI runs the bytes it reported -Before signing anything, `check` SHALL copy `.taskless/rules/` into a snapshot at -`.taskless/.run/rules/`, replacing any previous snapshot and dereferencing symbolic links. +Before signing anything, `check` SHALL copy `.taskless/rules/` into a snapshot under +`.taskless/.run/`, replacing any previous snapshot and dereferencing symbolic links. It SHALL compute every reported signature from the snapshot and SHALL run every engine from the snapshot, with the assembled configs written under `.taskless/.run/`. A rule the verdict excludes SHALL be removed from the snapshot before any engine configuration is @@ -246,7 +246,7 @@ assembled. The snapshot SHALL be taken on every path, including unauthenticated #### Scenario: Static rules run from the snapshot - **WHEN** `check` runs sg and vale rules -- **THEN** the ast-grep and Vale configs SHALL point into `.taskless/.run/rules/` +- **THEN** the ast-grep and Vale configs SHALL point into the snapshot under `.taskless/.run/` - **AND** SHALL NOT point into `.taskless/rules/` #### Scenario: An excluded rule is absent from what runs diff --git a/openspec/changes/cli-v2-rule-api/specs/cli-runtime-rule-execution/spec.md b/openspec/changes/cli-v2-rule-api/specs/cli-runtime-rule-execution/spec.md index 37d2d6c4..ee0dfb47 100644 --- a/openspec/changes/cli-v2-rule-api/specs/cli-runtime-rule-execution/spec.md +++ b/openspec/changes/cli-v2-rule-api/specs/cli-runtime-rule-execution/spec.md @@ -3,14 +3,14 @@ ### Requirement: Blessed runtime rules execute from the materialized run directory When a runtime rule is executed on a validated path, the CLI SHALL execute it from the -snapshot `check` took at `.taskless/.run/rules/runtime/` **before** signing, not from the live +snapshot `check` took under `.taskless/.run/` **before** signing, not from the live `.taskless/rules/runtime/` tree and not from a copy made after reconciliation, so the bytes executed are exactly the bytes that were reported and judged (copy, sign, report, execute). #### Scenario: Execution uses the blessed bytes - **WHEN** a runtime rule is blessed and executed -- **THEN** the CLI SHALL invoke the `check.ts` in the snapshot under `.taskless/.run/rules/runtime/` +- **THEN** the CLI SHALL invoke the `check.ts` in the snapshot under `.taskless/.run/` - **AND** SHALL NOT execute a copy modified in `.taskless/rules/runtime/` after reconciliation #### Scenario: No copy is made between the verdict and execution diff --git a/openspec/changes/cli-v2-rule-api/tasks.md b/openspec/changes/cli-v2-rule-api/tasks.md index be01330f..f0950c9a 100644 --- a/openspec/changes/cli-v2-rule-api/tasks.md +++ b/openspec/changes/cli-v2-rule-api/tasks.md @@ -77,58 +77,57 @@ upgradeUrl }`, strip C0/C1 control characters except newline from ## 4. Snapshot (slice 3) -- [ ] 4.1 Measure first: run one Vale rule whose `.vale.ini` scopes a - subdirectory glob with its assembled config at `.taskless/.vale.ini` and - at `.taskless/.run/.vale.ini`, and assert identical findings. Do the same - for an ast-grep rule with `.sgconfig.yml`. If either differs, stop and - revise design Decision 2 before continuing. -- [ ] 4.2 Add `rules/snapshot.ts`: replace `.taskless/.run/rules/` with a +- [x] 4.1 Measure first: the mixed-engine fixture plus a Vale rule scoped to + `[docs/**/*.md]`, run with configs assembled at `.taskless/` and in the + mirrored snapshot, gives identical findings (7 across 5 rules) and the + same subdirectory scoping. Design Decision 2 records the layout. +- [x] 4.2 Add `rules/snapshot.ts`: replace `.taskless/.run/snapshot/` with a dereferencing copy of `.taskless/rules/`, skipping `.DS_Store`, `Thumbs.db`, `desktop.ini`; a dangling link drops the file. Tests cover a symlinked capture, a dangling link, and an OS metadata file. -- [ ] 4.3 Parameterize `assembleValeConfig` / `assembleSgConfig` (and what they - read through `engines.ts`) by a root, so `check` assembles into - `.taskless/.run/` from the snapshot while `verify` / `test` keep today's - paths. `assemble.test.ts` covers both roots. -- [ ] 4.4 Run runtime rules from `.taskless/.run/rules/runtime/`; delete +- [x] 4.3 Run `check`'s assembly against the snapshot base, so its configs land + inside the snapshot while `verify` / `test` keep today's paths. A test + pins identical findings from both locations, including a + subdirectory-scoped Vale rule. +- [x] 4.4 Run runtime rules from the snapshot; delete `materializeRuntimeRules` and `RUNTIME_RUN_DIR`. A test edits a live `check.ts` after signing and asserts the snapshot's bytes executed. ## 5. Per-rule reconcile and the verdict policy (slice 3) -- [ ] 5.1 Add `rules/report.ts`: discover every rule directory of every engine +- [x] 5.1 Add `rules/report.ts`: discover every rule directory of every engine in the snapshot, refuse duplicate ids across engines (naming both directories), and build `{ ruleId, files: [{ path, signature }] }` with POSIX paths, excluding `.tests/**`. Tests: all engines reported, fixtures excluded, a duplicate id refused, `--rule` not narrowing. -- [ ] 5.2 Add `rules/verdicts.ts`: turn a v2 reconcile response into a per-rule +- [x] 5.2 Add `rules/verdicts.ts`: turn a v2 reconcile response into a per-rule disposition (run / exclude / fail reason / notice) by engine per the table in design Decision 5, and compute accounting (a reported rule in zero or several of `rules`, `unknown`, `withheld` is unaccounted). Pure function, table-driven tests including an `unsafe` sg rule, a static and a runtime `unknown`, `missing`, withheld, unaccounted, and double-listed. -- [ ] 5.3 Replace `planRuntime` with a `planCheck` that snapshots, reports, +- [x] 5.3 Replace `planRuntime` with a `planCheck` that snapshots, reports, reconciles, applies dispositions, and removes excluded rules from the snapshot before assembly. Degrade paths (no token, `--anonymous`, no remote, 401, 404 `organization_not_found`, unreachable) run every static rule unverified and skip runtime rules, as today. Delete `repairWithheldRules`, `repair.ts`, and `run-set.ts`'s v1 helpers. -- [ ] 5.4 Rewire `commands/check.ts` onto `planCheck`: exit 1 on withheld, an +- [x] 5.4 Rewire `commands/check.ts` onto `planCheck`: exit 1 on withheld, an `unsafe` static rule, an unaccounted rule, or a duplicate id; one notice per `unsafe` naming each differing path and `taskless rule restore`; one notice per `missing`. `check.test.ts`, `runtime-check.test.ts`, and `mixed-engine-check.test.ts` cover each exit condition. -- [ ] 5.5 Assert `check` never writes `.taskless/rules/`: a test hashes the tree +- [x] 5.5 Assert `check` never writes `.taskless/rules/`: a test hashes the tree before and after a run with `unsafe` and `missing` verdicts, and asserts no restore or fetch route was called. ## 6. check --json and recipes (slice 3) -- [ ] 6.1 Add the optional `integrity` array to `schemas/check.ts` and emit it; +- [x] 6.1 Add the optional `integrity` array to `schemas/check.ts` and emit it; keep `skipped`, `failures`, `notices`, and `entitlement` (withheld names resolved by rule id). Tests cover an `unsafe` entry with files, an unaccounted entry, and its omission on a clean run. -- [ ] 6.2 Update the `check` and `ci` recipes: an edited static rule and an +- [x] 6.2 Update the `check` and `ci` recipes: an edited static rule and an unaccounted rule fail the run; the fix is `rule restore`, not editing the rule back by hand; `missing` only warns. Update `create-runtime-rule` where it describes reconcile. @@ -153,7 +152,9 @@ upgradeUrl }`, strip C0/C1 control characters except newline from output. - [ ] 7.5 Add a `recover-rule` agent recipe (restore versus rollback, what a refusal means, recovering from git per the refusal's `message`) and link - it from the `check` recipe. `recipe-cross-references.test.ts` passes. + it from the `check` recipe's "An edited rule" section (slice 3 left the + pointer out, since the topic did not exist yet). + `recipe-cross-references.test.ts` passes. ## 8. Retire v1 (slice 5) @@ -169,9 +170,12 @@ upgradeUrl }`, strip C0/C1 control characters except newline from string literal not followed by `v2/`, or delete the idea if the grep in 8.1 plus the types already make a v1 call impossible to write. Record which, and why, in the PR. -- [ ] 8.3 Remove or rewrite tests that exercised v1 (`api-deprecated-paths`, - `repair`, `repair-integration`, `reconciliation-start`, and the v1 paths - in `entitlement` and `api-rule-errors`). `pnpm test` passes. +- [ ] 8.3 Remove or rewrite what still exercises v1. Already gone in slice 3: + `repair`, `repair-integration`, `runtime-dropped-rules`, the v1 relayout + reconcile test, `api/reconcile.ts`, `api/restore.ts`, and the runtime + `plan` / `repair` / `run-set` modules. Left for here: `api-deprecated-paths`, + the v1 parser in `entitlement.test.ts`, and whatever 8.1 deletes. + `pnpm test` passes. - [ ] 8.4 Run `pnpm typecheck` and `pnpm lint` (which rebuilds and runs `pnpm cli check`) from the repository root; both pass. diff --git a/packages/cli/src/agent/check.md b/packages/cli/src/agent/check.md index cccdfcdb..22eb2aa0 100644 --- a/packages/cli/src/agent/check.md +++ b/packages/cli/src/agent/check.md @@ -1,4 +1,4 @@ -# Topic: check (CLI v%(CLI_VERSION)s / topic v3) +# Topic: check (CLI v%(CLI_VERSION)s / topic v4) ## Goal Run the applicable rules against the codebase and report matches. Two @@ -17,34 +17,69 @@ in CI (diff-only scan), or after rule create/improve to validate. ## What runs -`check` never requires auth. The two rule kinds run differently: - -- **Static rules** (`.taskless/rules/sg//.yml`) are inert ast-grep patterns - and **always run**, in every mode, with no network call. The offline - linter posture. -- **Runtime rules** (`.taskless/rules/runtime//`) execute a - `check.ts` (arbitrary code), so they run ONLY when that code is - verified: - - **Logged in** (token or API key): each rule's `check.ts` is - reconciled against the Taskless service; rules the server blessed - (`run`) execute, and the rest are withheld and reported (advisory). - **One exception fails the run:** if the organization's plan does not - include runtime rules, the service withholds them for the plan and - `check` exits 1 even with no findings. See "Withheld for the plan". - - **Logged out, `--anonymous`, no GitHub remote, or service - unavailable**, runtime rules are **skipped** (reported, never run). - Static rules still run. - - **`--dangerously-run-scripts`**: runs every runtime rule trusting - local signatures, with no network call, behind a prominent warning. - This is the only way to run runtime rules unverified. - -Notices about skipped/withheld runtime rules are human-readable stderr -only, and apart from a withhold for the plan they never change the exit -code. Under `--json` they do NOT appear -as warnings; instead an additive optional `skipped: [{ rule, reason }]` -array is included alongside the unchanged `{ success, results }`. The -authoritative allow-list is the server's; the CI backstop -(`%(TASKLESS_CLI)s agent ci`) is the enforcement point for runtime rules. +`check` never requires auth. What it verifies depends on whether you are +logged in: + +- **Logged in** (token or API key): `check` copies `.taskless/rules/` + aside, signs every file of every rule (ast-grep, Vale, and runtime), + and asks the Taskless service which of them are exactly what it + issued. Every engine then runs from that copy, so what runs is what + was checked. What happens to each rule: + + | Verdict | ast-grep / Vale | runtime | + |---|---|---| + | issued and unchanged | runs | runs | + | **edited** since it was issued | **does not run, and `check` exits 1** | does not run (reported, exit unchanged) | + | issued but missing from disk | warning only | warning only | + | written locally (never issued) | runs, silently | does not run | + | withheld for the plan | never happens | does not run, and `check` exits 1 | + + `check` also exits 1 if the service's answer leaves out a rule it was + asked about, or if two engines hold a rule with the same id. +- **Logged out, `--anonymous`, no GitHub remote, or service + unavailable**: nothing is verified. ast-grep and Vale rules run as they + are on disk, runtime rules are **skipped** (reported, never run), and + the exit code is unaffected. +- **`--dangerously-run-scripts`**: nothing is verified and no network + call is made, logged in or not. Every rule of every engine runs, + runtime included, behind a prominent warning. This is the only way to + run runtime rules unverified. + +`check` NEVER changes `.taskless/rules/`. An edited or missing rule is +reported with the command that repairs it: +`%(TASKLESS_CLI)s rule restore `. + +Notices about skipped runtime rules are human-readable stderr only. +Under `--json` they do NOT appear as warnings; instead an additive +optional `skipped: [{ rule, reason }]` array is included alongside the +unchanged `{ success, results }`, and the CI backstop +(`%(TASKLESS_CLI)s agent ci`) is the enforcement point. + +## An edited rule + +When `check` fails because a rule was edited, `failures` names the rule +and each file that differs (changed, removed, or added), and `integrity` +carries the same as data: + +```json +"integrity": [ + { "ruleId": "no-simply-1a2b3c4d", "engine": "vale", "verdict": "unsafe", + "files": [{ "path": ".vale.ini", "expected": "1;h=…", "got": "1;h=…" }] } +] +``` + +**Do not edit the rule back by hand, and do not delete it.** Run +`%(TASKLESS_CLI)s rule restore `. If the edit was intended, the +rule has to be improved through the service (`improve-rule`) or +rewritten as a local rule under a new id. An edited rule is exactly what +an agent tuning a rule until its own violation passes looks like, which +is why `check` refuses it. + +`integrity` also lists `missing` rules (with the `revisionId` restore +would bring back), runtime rules the service never issued (`unknown`), +rules the answer did not account for (`unaccounted`), and ids shared +across engines (`duplicate`). Locally written ast-grep and Vale rules +are never listed; they run. ## Withheld for the plan @@ -52,7 +87,7 @@ When the organization's Taskless plan does not include runtime rules, reconcile answers normally but declines to run them. `check` then: - does not run them, and reports each in `skipped` with the reason - `not included in your Taskless plan` (never as unsafe or drift); + `not included in your Taskless plan` (never as edited or drift); - prints ONE notice naming the rules, the reason code, and the upgrade URL; - **exits 1**, and under `--json` sets `success: false` and adds: @@ -72,7 +107,7 @@ delete the rules to make `check` pass, and do not suggest `entitlement` with an empty `withheld` does not fail the run. ## Flags -- `--json`: machine output (`{ success, results, skipped?, entitlement? }`). +- `--json`: machine output (`{ success, results, skipped?, failures?, notices?, integrity?, entitlement? }`). - `--anonymous`: run only static rules; skip runtime rules. - `--dangerously-run-scripts`: run runtime `check.ts` unverified. - `--timeout `: per-runtime-check wall-clock bound (default 10). @@ -103,7 +138,8 @@ delete the rules to make `check` pass, and do not suggest alongside `success`/`results`. Surface it so CI can tell that runtime rules did not execute. It never affects the exit code on its own; an accompanying `entitlement` with a non-empty `withheld` does (see - "Withheld for the plan"). + "Withheld for the plan"), and so does a `failures` entry for an edited + rule (see "An edited rule"). 3. **Parse the JSON output.** Shape: ```json @@ -132,8 +168,9 @@ delete the rules to make `check` pass, and do not suggest `message`, and `ruleId` for each finding; the `range.start` is the useful line/column to surface. The `success` field reflects error-severity findings: `success: false` means at least one - `severity: "error"` finding exists, or runtime rules were withheld - for the plan (check `entitlement`) (exit code 1); `success: true` + `severity: "error"` finding exists, runtime rules were withheld for + the plan (check `entitlement`), or a rule was edited or unaccounted + for (check `failures` and `integrity`) (exit code 1); `success: true` with a non-empty `results` array means there are only warning/info/hint findings (exit code 0); `success: true` with an empty `results` array means the codebase is clean. Findings are @@ -145,8 +182,9 @@ delete the rules to make `check` pass, and do not suggest - `0`: All checks passed, no rules configured, or all supplied paths missing -- `1`: Errors detected, scan failed, or runtime rules withheld because - the plan does not include them +- `1`: Errors detected, scan failed, runtime rules withheld because + the plan does not include them, an issued ast-grep or Vale rule was + edited, a rule was unaccounted for, or two engines share a rule id ## Errors diff --git a/packages/cli/src/agent/ci.md b/packages/cli/src/agent/ci.md index 29819799..147f3f12 100644 --- a/packages/cli/src/agent/ci.md +++ b/packages/cli/src/agent/ci.md @@ -1,4 +1,4 @@ -# Topic: ci (CLI v%(CLI_VERSION)s / topic v2) +# Topic: ci (CLI v%(CLI_VERSION)s / topic v3) ## Goal Wire `%(TASKLESS_CLI)s check` into the user's existing CI so rules run @@ -75,6 +75,10 @@ Run `%(TASKLESS_CLI)s check`: not a findings failure and editing the rules will not fix it. Do not add `--anonymous` or `--dangerously-run-scripts` to the CI command to get green; both hide that the rules are not running. +- A rule reported as edited (`failures` names it; `integrity` lists it + as `unsafe`) → CI will fail until it is put back. Run + `%(TASKLESS_CLI)s rule restore `; do not edit it back by hand + and do not add a flag to the CI command to skip verification. ### 4. Generate the config @@ -172,17 +176,19 @@ different structure for CircleCI. The six steps stay the same. `%(TASKLESS_CLI)s check` does NOT require authentication. The generated CI config works out of the box with no secrets and scans all local rules. -Static ast-grep rules always run in CI with no secrets. **Runtime -rules** (`.taskless/rules/runtime/`, which execute a `check.ts`) only -run when their code is server-verified, so an unauthenticated CI job -runs the static rules and skips the runtime ones. - -Exposing a `TASKLESS_TOKEN` secret turns CI into the **backstop** for -runtime rules: an authenticated `check` reconciles each runtime rule's -`check.ts` against the Taskless service and runs exactly the -server-blessed set, withholding any that drift or were never issued. -This is the enforcement point for runtime rules, local developer runs -skip them unless `--dangerously-run-scripts` is passed. To wire it, set +Static ast-grep and Vale rules always run in CI with no secrets. +**Runtime rules** (`.taskless/rules/runtime/`, which execute a +`check.ts`) only run when their code is server-verified, so an +unauthenticated CI job runs the static rules and skips the runtime ones. + +Exposing a `TASKLESS_TOKEN` secret turns CI into the **backstop**: an +authenticated `check` verifies every file of every rule against what +Taskless issued. Runtime rules run only when verified. An issued +ast-grep or Vale rule that was edited does not run and fails the job, +so a rule loosened to let its own violation through cannot pass CI. +Without the token neither check happens, which is why this is the +enforcement point; local developer runs skip runtime rules unless +`--dangerously-run-scripts` is passed. To wire it, set the token as an env var on the check step (GitHub Actions): ```yaml diff --git a/packages/cli/src/agent/create-runtime-rule.md b/packages/cli/src/agent/create-runtime-rule.md index 3a485a82..1fc8d749 100644 --- a/packages/cli/src/agent/create-runtime-rule.md +++ b/packages/cli/src/agent/create-runtime-rule.md @@ -1,4 +1,4 @@ -# Topic: create-runtime-rule (CLI v%(CLI_VERSION)s / topic v2) +# Topic: create-runtime-rule (CLI v%(CLI_VERSION)s / topic v3) ## You are here This is `create-runtime-rule`. It helps you write a runtime rule: a @@ -70,11 +70,13 @@ A runtime rule's `check.ts` is a program that runs on the developer's machine with the developer's permissions. A rule file that arrives in a pull request is code that arrives in a pull request. So `check`: -1. **Signs** each `check.ts`, a signature over the exact bytes on disk. +1. **Copies** the rules aside and **signs** every file of each rule + (`check.ts` and every capture), a signature over the exact bytes it + will run. 2. **Reconciles** those signatures with the Taskless service, which - answers with the set it will vouch for. -3. **Runs only what came back blessed.** Anything else is reported as - skipped, with the reason. + says whether each rule is exactly what it issued. +3. **Runs only what came back verified, from that copy.** Anything else + is reported as skipped, with the reason. Reconciliation is what makes the signature mean something: it is the service saying "this rule, these exact bytes, for this repository." diff --git a/packages/cli/src/api/reconcile.ts b/packages/cli/src/api/reconcile.ts deleted file mode 100644 index 385a92c5..00000000 --- a/packages/cli/src/api/reconcile.ts +++ /dev/null @@ -1,145 +0,0 @@ -import { getApiBaseUrl } from "./config"; -import { parseEntitlement, type Entitlement } from "./entitlement"; -import { CLI_VERSION, CLI_VERSION_HEADER } from "../version"; - -/** - * Server-owned rule reconciliation (TSKL-270). The CLI reports the rule files - * it holds and the server returns the exact subset that may run. The endpoint - * is now in the generated schema, but this stays on hand-typed plain `fetch` - * for its degradation contract (`ReconcileOutcome` never throws for expected - * network/auth/deployment conditions, so `check` falls back to a local scan); - * migrating it onto the typed client would mean re-expressing that handling. - */ - -/** A rule file reported for reconciliation. */ -export interface ReportedFile { - file: string; - signature: string; -} - -export interface ReconcileRequest { - /** - * Org subject: Taskless UUID (preferred) or numeric GitHub org id. Optional — - * the server falls back to the deprecated token claim when it is absent. - */ - orgId?: string | number; - repositoryUrl: string; - files: ReportedFile[]; -} - -/** A file the server blessed — execute exactly these. */ -export interface RunEntry { - ruleId: string; - file: string; - signature: string; -} - -/** A held rule whose content differs from what the server blessed. */ -export interface UnsafeEntry { - ruleId: string; - file: string; - expected: string; - got: string; -} - -/** A reported file the server never issued. */ -export interface UnknownEntry { - file: string; -} - -/** A rule the server expected that the client did not report. */ -export interface MissingEntry { - ruleId: string; - file: string; -} - -export interface ReconcileResponse { - run: RunEntry[]; - unsafe: UnsafeEntry[]; - unknown: UnknownEntry[]; - missing: MissingEntry[]; - /** - * Present only when the service withheld runtime blessing for the plan. - * Absent means the four buckets are the whole answer, as they always were. - */ - entitlement?: Entitlement; -} - -/** - * The result of an attempted reconciliation. `check` branches on this: - * - `ok`: use `result.run` as the complete allow-list; warn on the rest. - * - `unauthorized`: the token was missing/invalid (server returned 401). - * - `unavailable`: the endpoint could not be reached or is not deployed — - * degrade to a local scan. Never a thrown error for these expected cases. - */ -export type ReconcileOutcome = - | { status: "ok"; result: ReconcileResponse } - | { status: "unauthorized" } - | { status: "unavailable"; reason: string }; - -/** Coerce an untyped bucket into a typed array, tolerating a missing field. */ -function asArray(value: unknown): T[] { - return Array.isArray(value) ? (value as T[]) : []; -} - -/** - * Reconcile the reported files against the server. Returns a `ReconcileOutcome` - * and never throws for expected network/auth/deployment conditions. - */ -export async function reconcile( - token: string, - request: ReconcileRequest -): Promise { - // Schema paths include the /cli/ prefix, so the base URL is the origin. - const baseUrl = getApiBaseUrl().replace(/\/cli\/?$/, ""); - const url = `${baseUrl}/cli/api/reconcile`; - - let response: Response; - try { - response = await fetch(url, { - method: "POST", - headers: { - Authorization: `Bearer ${token}`, - "Content-Type": "application/json", - [CLI_VERSION_HEADER]: CLI_VERSION, - }, - body: JSON.stringify(request), - }); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return { status: "unavailable", reason: `network error: ${message}` }; - } - - if (response.status === 401) { - return { status: "unauthorized" }; - } - - // 404 (endpoint not deployed), 5xx, and any other non-2xx are treated as - // "unavailable" so `check` degrades to a local scan rather than failing. - if (!response.ok) { - return { - status: "unavailable", - reason: `HTTP ${String(response.status)}`, - }; - } - - let body: unknown; - try { - body = await response.json(); - } catch { - return { status: "unavailable", reason: "invalid response body" }; - } - - const data = body as Partial>; - const entitlement = parseEntitlement(data.entitlement); - return { - status: "ok", - result: { - run: asArray(data.run), - unsafe: asArray(data.unsafe), - unknown: asArray(data.unknown), - missing: asArray(data.missing), - ...(entitlement === undefined ? {} : { entitlement }), - }, - }; -} diff --git a/packages/cli/src/api/restore.ts b/packages/cli/src/api/restore.ts deleted file mode 100644 index fe2b793f..00000000 --- a/packages/cli/src/api/restore.ts +++ /dev/null @@ -1,112 +0,0 @@ -import type { paths } from "../generated/api"; -import { getApiBaseUrl } from "./config"; -import { - parseEntitlement, - type Entitlement, - type MayCarryEntitlement, -} from "./entitlement"; -import { CLI_VERSION, CLI_VERSION_HEADER } from "../version"; - -/** - * Fetch the blessed bytes for a rule the client holds wrongly, or not at all. - * - * This is the repair path for reconcile's verdicts, and it is deliberately not - * an upgrade path. `unsafe` means the server holds bytes we have drifted from; - * `missing` means it expected a rule we never reported. Both are answered by - * "send me what you blessed". `unknown` is not: a file the service never issued - * has nothing to fetch, which is why its entry carries no rule id. - * - * NOTHING RESTORED RUNS IN THE PASS THAT RESTORED IT. Restore rewrites the - * working tree and promotes nothing into the current run: an `unsafe` rule - * stays unexecuted, a `missing` rule was never a local candidate, and an - * `unknown` rule never runs. The next `check` reports the repaired signature - * and is blessed through the ordinary path. Fetching bytes and executing them - * in the same breath as discovering drift would move the gate, and the gate is - * the only reason any of this exists. - */ - -type RestoreResponse = - paths["/cli/api/request/{requestId}/restore"]["post"]["responses"]["200"]["content"]["application/json"]; - -/** A rule as the service restored it, discriminated on `engine`. */ -export type RestoredRule = NonNullable[number]; - -/** - * The result of an attempted restore. - * - * Mirrors `ReconcileOutcome`: expected conditions are values rather than - * thrown errors, because a repair that cannot happen must degrade `check` to - * "this rule was not repaired and did not run" rather than failing the run. A - * rule the service will not return is a rule that stays withheld, which is - * already a safe state. - */ -export type RestoreOutcome = - | { status: "ok"; rules: RestoredRule[]; entitlement?: Entitlement } - | { status: "unauthorized" } - | { status: "unavailable"; reason: string }; - -/** - * Ask the service for a rule's complete file set. - * - * A `POST` carrying `repositoryUrl`, which is what scopes the response to the - * organization and installation that owns the rule rather than to whoever holds - * a rule id. The verb follows that requirement rather than the other way round. - * - * **`request.ruleId` is a rule id and stays one, even though the schema now - * spells the path segment `{requestId}`.** The service renamed the resource - * because the id in `POST /cli/api/request` and its status poll was never a - * rule id, it was the generation ticket. This caller is the exception: the only - * value it ever passes comes from a reconcile `unsafe`/`missing` entry, whose - * `ruleId` the reconcile schema documents as "the model-assigned rule - * identifier, for calling restore without parsing it out of the path". Renaming - * this field to match the URL would make the name lie about the value. - */ -export async function restoreRule( - token: string, - request: { ruleId: string; repositoryUrl: string } -): Promise { - // Schema paths include the /cli/ prefix, so the base URL is the origin. - const baseUrl = getApiBaseUrl().replace(/\/cli\/?$/, ""); - const url = `${baseUrl}/cli/api/request/${encodeURIComponent(request.ruleId)}/restore`; - - let response: Response; - try { - response = await fetch(url, { - method: "POST", - headers: { - Authorization: `Bearer ${token}`, - "Content-Type": "application/json", - [CLI_VERSION_HEADER]: CLI_VERSION, - }, - body: JSON.stringify({ repositoryUrl: request.repositoryUrl }), - }); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return { status: "unavailable", reason: `network error: ${message}` }; - } - - if (response.status === 401) return { status: "unauthorized" }; - if (!response.ok) { - return { status: "unavailable", reason: `HTTP ${String(response.status)}` }; - } - - let body: unknown; - try { - body = await response.json(); - } catch { - return { status: "unavailable", reason: "invalid response body" }; - } - - const data = body as Partial>; - const rules = data.rules; - if (!Array.isArray(rules)) { - return { status: "unavailable", reason: "response carried no `rules`" }; - } - // Absent means entitled, and every service before #207 omits it. - const entitlement = parseEntitlement(data.entitlement); - return { - status: "ok", - rules, - ...(entitlement === undefined ? {} : { entitlement }), - }; -} diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index 844a0c1f..cb54455e 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -15,10 +15,8 @@ import { CLIError } from "../util/cli-error"; import { requireCurrentSchema } from "../filesystem/migrate"; import { discoverRuntimeRules } from "../rules/runtime/discover"; import { resolveRuleSelection, type RuleSelection } from "../rules/rule-filter"; -// The gate lives beside the runtime engine rather than inside this command, -// because `test` runs a rule's fixtures under exactly this policy. Sharing the -// implementation is what makes that a fact rather than an intention. -import { planRuntime } from "../rules/runtime/plan"; +import { planCheck } from "../rules/plan-check"; +import { fromProjectRoot } from "../rules/snapshot"; import { markNotice } from "../util/notices"; async function pathExists(absolutePath: string): Promise { @@ -335,29 +333,49 @@ export const checkCommand = defineCommand({ } try { - // Runtime rules are planned before dispatch, not during it: planning - // consults auth and reconcile state, which is a decision about *what* - // may run rather than part of running it. - const plan = await planRuntime(cwd, runtimeRules, { + // Planned before dispatch, not during it: planning snapshots the rules + // tree and consults auth and reconcile state, which decides WHAT runs. + // Everything below reads the snapshot, never `.taskless/rules/`, so + // the bytes that run are the bytes that were judged. + const plan = await planCheck(cwd, { anonymous: args.anonymous, dangerouslyRunScripts: Boolean(args["dangerously-run-scripts"]), }); for (const notice of plan.notices) warnNotice(notice); + for (const failure of plan.failures) warn(`Error: ${failure}`); for (const skipped of plan.skipped) { warnNotice( `runtime rule ${skipped.rule} was not run — ${skipped.reason}.` ); } - // Every engine runs concurrently and merges into one result set. An - // engine that cannot run reports a notice and the others still return. - // Assemble both engine configs from the per-rule tree. Each returns - // `undefined` when its engine has no rules, which dispatch reads as - // "nothing to run" rather than running an empty config. + // Assembled against the snapshot base, whose layout mirrors the + // project's, so both configs resolve their rules inside the snapshot. + // The engines still run from the project root; only the config path + // is relative to it. const assembled = await assembleEngineConfigs( - cwd, + plan.snapshot.base, selection === undefined ? {} : { ruleIds: selection.vale } ); + const valeAssembly = + assembled.vale?.status === "ok" + ? { + ...assembled.vale, + path: fromProjectRoot(plan.snapshot, assembled.vale.path), + } + : assembled.vale; + const sgConfig = + assembled.sg === undefined + ? undefined + : fromProjectRoot(plan.snapshot, assembled.sg); + // `--rule` narrows WHAT runs; it does not widen what may run. A runtime + // rule named here is still subject to the plan. + const runtimeExecute = + selection === undefined + ? plan.execute + : plan.execute.filter((rule) => + selection.runtime.includes(rule.name) + ); const dispatched = await runEngines({ cwd, paths: existingPaths, @@ -368,10 +386,10 @@ export const checkCommand = defineCommand({ astGrepConfigPath: selection !== undefined && selection.sg.length === 0 ? undefined - : assembled.sg, + : sgConfig, ...(selection === undefined ? {} : { astGrepRuleIds: selection.sg }), - vale: assembled.vale, - runtimeRules: plan.execute, + vale: valeAssembly, + runtimeRules: runtimeExecute, runtimeTimeoutMs: parseTimeoutMs(args.timeout), }); const results = dispatched.results; @@ -419,20 +437,26 @@ export const checkCommand = defineCommand({ // leaves the exit code alone because the CLI could not ask; this one // is the answer to asking, and a green run would say the rule is still // protecting the repository when it has stopped running. + // + // The plan's failures are the same kind of answer: an edited ast-grep + // or Vale rule, a rule the service's answer did not account for, or + // ids that collide across engines. Each is a verified outcome, never a + // degrade path, and a green run would say the rule is protecting the + // repository when it did not run. const withheldForPlan = (plan.entitlement?.withheld.length ?? 0) > 0; const exitCode = - dispatched.exitCode === 0 && withheldForPlan + dispatched.exitCode === 0 && + (withheldForPlan || plan.failures.length > 0) ? 1 : dispatched.exitCode; + const allFailures = [...plan.failures, ...dispatched.failures]; if (args.json) { const output = checkOutputSchema.parse({ success: exitCode === 0, results, ...(plan.skipped.length > 0 ? { skipped: plan.skipped } : {}), - ...(dispatched.failures.length > 0 - ? { failures: dispatched.failures } - : {}), + ...(allFailures.length > 0 ? { failures: allFailures } : {}), // BOTH sources. `plan.notices` carries the repair diagnostics — // what was restored, what could not be, and why — and they used to // reach only `warn()`, which is a no-op under `--json`. So the one @@ -442,6 +466,7 @@ export const checkCommand = defineCommand({ ...(plan.entitlement === undefined ? {} : { entitlement: plan.entitlement }), + ...(plan.integrity.length > 0 ? { integrity: plan.integrity } : {}), }); console.log(JSON.stringify(output)); } else { diff --git a/packages/cli/src/rules/plan-check.ts b/packages/cli/src/rules/plan-check.ts new file mode 100644 index 00000000..a5bdc990 --- /dev/null +++ b/packages/cli/src/rules/plan-check.ts @@ -0,0 +1,279 @@ +import { getToken } from "../auth/token"; +import { resolveOrgSubject } from "../auth/org"; +import { reconcileRules } from "../api/v2"; +import { resolveRepositoryUrl } from "../util/git-remote"; +import { getCliPrefix } from "../util/package-manager"; +import { reportRules } from "./report"; +import { RUN_SCRIPTS_WARNING } from "./runtime/harness"; +import { discoverRuntimeRulesIn, type RuntimeRule } from "./runtime/discover"; +import { + excludeFromSnapshot, + snapshotEngineDirectory, + takeSnapshot, + type Snapshot, +} from "./snapshot"; +import { + applyVerdicts, + NOT_IN_PLAN_REASON, + type IntegrityEntry, +} from "./verdicts"; + +/** + * Deciding what a `check` runs, separately from running it. + * + * Every path starts from the same snapshot, so there is one execution path and + * not a verified one and an unverified one that drift apart. What differs by + * path is only what is judged: + * + * - `--dangerously-run-scripts`: nothing. No reconcile, no signatures, every + * rule of every engine runs, runtime included. The flag does only what its + * name says. + * - logged out, `--anonymous`, no remote, or a reconcile that cannot complete: + * static rules run unverified, runtime rules are skipped, and the exit code + * is untouched. Tamper detection needs the service; its absence is not a + * failure. + * - a completed reconcile: the verdict policy in `verdicts.ts`, and whatever it + * excludes is removed from the snapshot before any engine is configured. + * + * `check` never writes `.taskless/rules/`. An edited or missing rule gets a + * notice naming `rule restore`; nothing here fetches bytes. + */ + +/** A runtime rule that will not run, with why. */ +export interface SkippedRuntimeRule { + rule: string; + reason: string; +} + +/** The plan outcome for an organization whose plan withholds runtime rules. */ +export interface PlanEntitlement { + runtimeSignatures: false; + reason?: string; + upgradeUrl?: string; + /** Local rule names the service withheld. Non-empty means `check` fails. */ + withheld: string[]; +} + +export interface CheckPlan { + snapshot: Snapshot; + /** Runtime rules to execute, from the snapshot. */ + execute: RuntimeRule[]; + skipped: SkippedRuntimeRule[]; + notices: string[]; + /** Reasons the run fails whatever the findings. */ + failures: string[]; + integrity: IntegrityEntry[]; + entitlement?: PlanEntitlement; +} + +export interface PlanOptions { + anonymous: boolean; + dangerouslyRunScripts: boolean; +} + +/** + * Snapshot the rules tree and decide what runs. + */ +export async function planCheck( + cwd: string, + options: PlanOptions +): Promise { + const snapshot = await takeSnapshot(cwd); + const runtimeRoot = snapshotEngineDirectory(snapshot, "runtime"); + const discovered = await discoverRuntimeRulesIn(runtimeRoot); + const empty = { + snapshot, + execute: [], + skipped: [], + notices: [], + failures: [], + integrity: [], + }; + + if (options.dangerouslyRunScripts) { + return { ...empty, execute: discovered, notices: [RUN_SCRIPTS_WARNING] }; + } + + const unverified = (reason: string, notice?: string): CheckPlan => ({ + ...empty, + skipped: discovered.map((rule) => ({ rule: rule.name, reason })), + notices: notice === undefined ? [] : [notice], + }); + + if (options.anonymous) { + return unverified( + "anonymous mode — runtime rules were not verified and did not run" + ); + } + const token = await getToken(cwd, { silent: true }); + if (!token) { + return unverified( + "not authenticated — runtime rules were not verified and did not run" + ); + } + let repositoryUrl: string; + try { + repositoryUrl = await resolveRepositoryUrl(cwd); + } catch { + return unverified( + "no GitHub remote — runtime rules could not be verified and did not run" + ); + } + + const report = await reportRules(snapshot); + const failures: string[] = []; + const integrity: IntegrityEntry[] = []; + + // A rule that cannot be judged must not run as though it had been. Both are + // removed from the snapshot and fail the run. + for (const duplicate of report.duplicates) { + for (const engine of duplicate.engines) { + await excludeFromSnapshot(snapshot, engine, duplicate.ruleId); + } + integrity.push({ ruleId: duplicate.ruleId, verdict: "duplicate" }); + failures.push( + `rule id ${duplicate.ruleId} is used by more than one engine (` + + duplicate.engines + .map((engine) => `.taskless/rules/${engine}/${duplicate.ruleId}/`) + .join(", ") + + "), so neither can be verified and neither ran. Rename one." + ); + } + for (const rule of report.unreadable) { + await excludeFromSnapshot(snapshot, rule.engine, rule.ruleId); + integrity.push({ + ruleId: rule.ruleId, + engine: rule.engine, + verdict: "unaccounted", + }); + failures.push( + `${rule.engine} rule ${rule.ruleId} could not be read (${rule.reason}), so it was not verified and did not run.` + ); + } + if (report.duplicates.length > 0) { + // Refused before reconcile: the report would name one id for two rules. + const remaining = await discoverRuntimeRulesIn(runtimeRoot); + return { + ...empty, + skipped: remaining.map((rule) => ({ + rule: rule.name, + reason: "rule ids collide across engines, so nothing was verified", + })), + failures, + integrity, + }; + } + + const orgSubject = await resolveOrgSubject(cwd, token); + const outcome = await reconcileRules(token, { + orgId: orgSubject, + repositoryUrl, + rules: report.rules.map(({ ruleId, files }) => ({ ruleId, files })), + }); + + if (outcome.status !== "ok") { + const cause = + outcome.status === "unauthorized" + ? `authentication was rejected — run \`${getCliPrefix()} auth login\` to re-authenticate` + : outcome.status === "error" && + outcome.code === "organization_not_found" + ? "the Taskless GitHub App installation does not cover this repository, or your login lost access to the organization" + : `the rule service was unavailable (${ + outcome.status === "unavailable" + ? outcome.reason + : outcome.status === "error" + ? outcome.code + : "unexpected refusal" + })`; + const remaining = await discoverRuntimeRulesIn(runtimeRoot); + return { + ...empty, + skipped: remaining.map((rule) => ({ rule: rule.name, reason: cause })), + notices: [ + `Rule verification could not be performed: ${cause}. Static rules ran unverified and runtime rules did not run.`, + ], + failures, + integrity, + }; + } + + const verdicts = applyVerdicts( + report.rules, + outcome.data, + (ruleId) => `${getCliPrefix()} rule restore ${ruleId}` + ); + for (const disposition of verdicts.dispositions) { + if (!disposition.run) { + await excludeFromSnapshot( + snapshot, + disposition.engine, + disposition.ruleId + ); + } + } + + const execute = await discoverRuntimeRulesIn(runtimeRoot); + const executing = new Set(execute.map((rule) => rule.name)); + const skipped: SkippedRuntimeRule[] = []; + for (const disposition of verdicts.dispositions) { + if (disposition.engine !== "runtime") continue; + if (!disposition.run) { + skipped.push({ + rule: disposition.ruleId, + reason: disposition.reason ?? "not verified", + }); + } else if (!executing.has(disposition.ruleId)) { + // Verified, then not runnable: no capture rules, or a stray module + // beside check.ts. Named, because a rule in neither list reads as a rule + // that ran and found nothing. + skipped.push({ + rule: disposition.ruleId, + reason: + "verified by the server, but it is not a runnable runtime rule (run `verify` for why)", + }); + } + } + + const entitlement: PlanEntitlement | undefined = + verdicts.entitlement === undefined + ? undefined + : { + runtimeSignatures: false, + ...(verdicts.entitlement.reason === undefined + ? {} + : { reason: verdicts.entitlement.reason }), + ...(verdicts.entitlement.upgradeUrl === undefined + ? {} + : { upgradeUrl: verdicts.entitlement.upgradeUrl }), + withheld: verdicts.withheld, + }; + + return { + snapshot, + execute, + skipped, + notices: [ + ...(entitlement === undefined || entitlement.withheld.length === 0 + ? [] + : [withheldNotice(entitlement)]), + ...verdicts.notices, + ], + failures: [...failures, ...verdicts.failures], + integrity: [...integrity, ...verdicts.integrity], + ...(entitlement === undefined ? {} : { entitlement }), + }; +} + +/** The one notice a withheld run prints, so the upgrade URL appears once. */ +function withheldNotice(entitlement: PlanEntitlement): string { + const count = entitlement.withheld.length; + return ( + `${String(count)} runtime ${count === 1 ? "rule was" : "rules were"} ` + + `withheld because runtime rules are ${NOT_IN_PLAN_REASON}` + + (entitlement.reason === undefined ? "" : ` (${entitlement.reason})`) + + `: ${entitlement.withheld.join(", ")}. \`check\` fails until they can run` + + (entitlement.upgradeUrl === undefined + ? "." + : `. Upgrade at ${entitlement.upgradeUrl}`) + ); +} diff --git a/packages/cli/src/rules/report.ts b/packages/cli/src/rules/report.ts new file mode 100644 index 00000000..67a30f9d --- /dev/null +++ b/packages/cli/src/rules/report.ts @@ -0,0 +1,124 @@ +import { readdir } from "node:fs/promises"; +import { join } from "node:path"; + +import { listRuleIds, ruleDirectory } from "./engines"; +import { ENGINES, RULE_TESTS_DIRECTORY, type EngineName } from "./layout"; +import { signRuleFile } from "./rule-hash"; +import type { Snapshot } from "./snapshot"; + +/** + * What `check` tells reconcile it holds: one entry per rule directory, of every + * engine, each file signed from the snapshot. + * + * `ruleId` is the directory name, which is what v2 addresses a rule by. The + * engine is NOT sent (the service keys on the id alone, and issued ids are + * unique across engines), but it is kept here, because it is what the verdict + * policy branches on and the reporting directory is the one fact the CLI has + * about a rule the service has never heard of. + */ + +/** One signed file of a reported rule. */ +export interface ReportedFile { + /** Relative to the rule directory, `/`-separated on every platform. */ + path: string; + signature: string; +} + +/** One reported rule. */ +export interface ReportedRule { + ruleId: string; + engine: EngineName; + files: ReportedFile[]; +} + +/** Two or more rule directories sharing one id under different engines. */ +export interface DuplicateRuleId { + ruleId: string; + engines: EngineName[]; +} + +/** A rule whose directory could not be read or signed. */ +export interface UnreadableRule { + ruleId: string; + engine: EngineName; + reason: string; +} + +export interface RuleReport { + rules: ReportedRule[]; + duplicates: DuplicateRuleId[]; + unreadable: UnreadableRule[]; +} + +/** + * Discover and sign every rule in the snapshot. + * + * A duplicate id is reported instead of either rule: reconcile carries no + * engine, so the two cannot be judged apart, and the caller refuses the run. + * Skipping the pair instead would let anyone neutralize an issued rule by + * creating a same-named directory under another engine. + */ +export async function reportRules(snapshot: Snapshot): Promise { + const engineById = new Map(); + for (const engine of ENGINES) { + for (const ruleId of await listRuleIds(snapshot.base, engine)) { + engineById.set(ruleId, [...(engineById.get(ruleId) ?? []), engine]); + } + } + + const report: RuleReport = { rules: [], duplicates: [], unreadable: [] }; + for (const [ruleId, engines] of [...engineById].toSorted(([a], [b]) => + a.localeCompare(b) + )) { + if (engines.length > 1) { + report.duplicates.push({ ruleId, engines }); + continue; + } + const engine = engines[0] as EngineName; + const directory = ruleDirectory(snapshot.base, engine, ruleId); + try { + const paths = await listReportedPaths(directory); + const files = await Promise.all( + paths.map(async (path) => ({ + path, + signature: await signRuleFile(join(directory, ...path.split("/"))), + })) + ); + report.rules.push({ ruleId, engine, files }); + } catch (error) { + report.unreadable.push({ + ruleId, + engine, + reason: error instanceof Error ? error.message : String(error), + }); + } + } + return report; +} + +/** + * Every regular file under a rule directory, recursively, except the rule's + * own `.tests/`, as sorted `/`-separated paths. + * + * The snapshot has already dereferenced links and dropped operating-system + * metadata, so every entry here is a real file an engine could read. Only the + * TOP-LEVEL `.tests/` is a fixture directory; a nested `captures/.tests/` is + * reported like any other file, because an engine reads it. + */ +async function listReportedPaths(directory: string): Promise { + const paths: string[] = []; + async function walk(absolute: string, prefix: string): Promise { + const entries = await readdir(absolute, { withFileTypes: true }); + for (const entry of entries) { + const path = prefix === "" ? entry.name : `${prefix}/${entry.name}`; + if (prefix === "" && entry.name === RULE_TESTS_DIRECTORY) continue; + if (entry.isDirectory()) { + await walk(join(absolute, entry.name), path); + } else if (entry.isFile()) { + paths.push(path); + } + } + } + await walk(directory, ""); + return paths.toSorted((a, b) => a.localeCompare(b)); +} diff --git a/packages/cli/src/rules/runtime/plan.ts b/packages/cli/src/rules/runtime/plan.ts deleted file mode 100644 index 0113a88b..00000000 --- a/packages/cli/src/rules/runtime/plan.ts +++ /dev/null @@ -1,478 +0,0 @@ -import { getToken } from "../../auth/token"; -import { resolveOrgSubject } from "../../auth/org"; -import { resolveRepositoryUrl } from "../../util/git-remote"; -import { getCliPrefix } from "../../util/package-manager"; -import { reconcile } from "../../api/reconcile"; -import type { ReconcileResponse } from "../../api/reconcile"; -import { notRunOnPlanSentence } from "../../api/entitlement"; -import { restoreRule } from "../../api/restore"; -import { writeRuleFile } from "../files"; -import { PurgeIncompleteError } from "../deliver"; -import { repairTargets, verifyRestoredCheck } from "./repair"; -import { RUN_SCRIPTS_WARNING } from "./harness"; -import { type RuntimeRule } from "./discover"; -import { - materializeRuntimeRules, - reportedCheckPath, - reportRuntimeChecks, - selectBlessedRuntimeRules, - signRuntimeChecks, -} from "./run-set"; - -/** - * Deciding WHICH runtime rules may execute during a `check`, separately from - * executing them. - * - * `check` is the only caller and the only command that needs this. It executes - * rules as a SIDE EFFECT of scanning a repository — nobody asked for code to - * run — so a reconcile stands between the scan and the execution, and every - * unverified path skips rather than fails. - * - * `test` deliberately does not come through here. It runs a rule's fixtures - * because the user typed `test`, so the verb is the consent and - * `--dangerously-run-scripts` is its whole gate; see the runtime branch of - * `rules/inspect.ts`. That is stricter than sharing this module, not looser: - * nothing runs under `test` that would not have run before, and a blessed rule - * that used to run there without the flag now needs it. - * - * It lived inside `commands/check.ts` and moved here for a reason that has - * since evaporated (sharing the gate with `test`). It stays because a second - * reason holds on its own: this is policy, `commands/check.ts` is argument - * parsing and rendering, and `repairWithheldRules` below is a long piece of - * recovery logic that a command file has no business carrying. - */ - -/** A runtime rule that will not run, with why (advisory). */ -export interface SkippedRuntimeRule { - rule: string; - reason: string; -} - -/** - * The service declined to run runtime rules for this organization's plan. - * - * Unlike every other skip, this one fails `check`. The degrade paths skip - * because the CLI could not ask; this skip is the answer to a question it did - * ask, and it will be the same answer on every run until someone acts on it. - */ -export interface PlanEntitlement { - runtimeSignatures: false; - reason?: string; - upgradeUrl?: string; - /** - * Local rule names the service withheld, plus the reported path of any - * withheld entry that matched no local rule. Non-empty means `check` fails. - */ - withheld: string[]; -} - -/** The runtime-execution plan resolved from auth state and flags. */ -export interface RuntimePlan { - /** Rules to execute — materialized when gated, live under `--dangerously-run-scripts`. */ - execute: RuntimeRule[]; - /** Rules that will not run, with a reason. */ - skipped: SkippedRuntimeRule[]; - /** Human-only notices about the runtime disposition. */ - notices: string[]; - /** Present only when reconcile answered for a plan without runtime signatures. */ - entitlement?: PlanEntitlement; -} - -/** The skip reason for a rule withheld because the plan lacks runtime rules. */ -export const NOT_IN_PLAN_REASON = "not included in your Taskless plan"; - -/** Skip every runtime rule with a shared reason (an unverified path). */ -function skipAllRuntime(rules: RuntimeRule[], reason: string): RuntimePlan { - return { - execute: [], - skipped: rules.map((rule) => ({ rule: rule.name, reason })), - notices: [], - }; -} - -/** - * The rules that were blessed but are not going to run, so a drop between the - * two is reported rather than silent. - * - * `execute` is whatever re-discovery under `.taskless/.run/` returns, which is - * a DIFFERENT question from what was blessed. A rule can be blessed and then - * vanish: a `captures/` symlink that resolves in the working tree can dangle - * once copied, a file can fail to materialize, and re-discovery then classifies - * the rule as "not a runtime rule" and drops it. - * - * Without this accounting such a rule is in neither list. Not in `execute` - * because it was dropped, and not in `withheld` because the server did bless - * it, so `check` exits 0 having said nothing and the user believes their - * runtime rule ran. - * - * Deliberately keyed on the difference rather than on any particular cause. - * Reading an unreadable `captures/` as absence was one route in and is fixed at - * its source, but a dangling symlink reports `ENOENT`, which is genuinely - * "absent" and correctly stays absent there. Only comparing the two sets - * catches that, and whatever the next route turns out to be. - */ -export function accountForDroppedRules( - blessed: readonly RuntimeRule[], - execute: readonly RuntimeRule[] -): SkippedRuntimeRule[] { - const executed = new Set(execute.map((rule) => rule.name)); - return blessed - .filter((rule) => !executed.has(rule.name)) - .map((rule) => ({ - rule: rule.name, - reason: "blessed by the server but missing after materialization", - })); -} - -/** - * Decide which runtime rules run. A runtime rule's `check.ts` is arbitrary code - * execution, so it runs only when its signature is server-validated (an - * authenticated reconcile that returns it in `run`) or `--dangerously-run-scripts` - * is set. Every unverified path — anonymous, logged out, no remote, or a - * reconcile that cannot complete — skips runtime rules without failing. - */ -export async function planRuntime( - cwd: string, - discovered: RuntimeRule[], - options: { anonymous: boolean; dangerouslyRunScripts: boolean } -): Promise { - if (discovered.length === 0) return { execute: [], skipped: [], notices: [] }; - - if (options.dangerouslyRunScripts) { - return { - execute: discovered, - skipped: [], - notices: [RUN_SCRIPTS_WARNING], - }; - } - - if (options.anonymous) { - return skipAllRuntime( - discovered, - "anonymous mode — runtime rules were not verified and did not run" - ); - } - - const token = await getToken(cwd, { silent: true }); - if (!token) { - return skipAllRuntime( - discovered, - "not authenticated — runtime rules were not verified and did not run" - ); - } - - let repositoryUrl: string; - try { - repositoryUrl = await resolveRepositoryUrl(cwd); - } catch { - return skipAllRuntime( - discovered, - "no GitHub remote — runtime rules could not be verified and did not run" - ); - } - - // A rule whose check.ts is missing/unreadable is reported, not fatal: signing - // never throws, and such rules are surfaced as skipped so static checks and - // the other runtime rules are unaffected. - const { signed, unreadable } = await signRuntimeChecks(discovered); - const unreadableSkips: SkippedRuntimeRule[] = unreadable.map((rule) => ({ - rule: rule.name, - reason: "its check.ts is missing or unreadable", - })); - - const orgSubject = await resolveOrgSubject(cwd, token); - const outcome = await reconcile(token, { - orgId: orgSubject, - repositoryUrl, - files: reportRuntimeChecks(cwd, signed), - }); - - if (outcome.status === "unauthorized") { - return skipAllRuntime( - discovered, - `authentication was rejected — run \`${getCliPrefix()} auth login\` to re-authenticate` - ); - } - if (outcome.status === "unavailable") { - return skipAllRuntime( - discovered, - `the rule service was unavailable (${outcome.reason})` - ); - } - - const { blessed, withheld } = selectBlessedRuntimeRules( - signed, - outcome.result.run - ); - // Joined by reported path, since a withheld entry carries no signature. A - // withheld rule is split out of the generic "not blessed" skips so it is - // never described as drift: nothing about its bytes is wrong. - const entitlement = outcome.result.entitlement; - const withheldFiles = new Set( - (entitlement?.withheld ?? []).map((entry) => entry.file) - ); - const planWithheld = withheld.filter((rule) => - withheldFiles.has(reportedCheckPath(cwd, rule)) - ); - const notBlessed = withheld.filter((rule) => !planWithheld.includes(rule)); - let execute: RuntimeRule[] = []; - try { - execute = - blessed.length > 0 ? await materializeRuntimeRules(cwd, blessed) : []; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return skipAllRuntime( - discovered, - `runtime rules could not be materialized (${message})` - ); - } - // Repair the working tree from the server's verdicts. This changes what the - // NEXT run sees and nothing about this one: an `unsafe` rule stays withheld - // below whether or not its bytes were just restored. Fetching code and - // executing it in the same pass that discovered the drift would move the - // gate, and the gate is the point. - // - // A file withheld for the plan is never sent to restore. The service keeps - // it out of `unsafe` and `missing` already; this holds if it ever does not, - // because restoring bytes the plan will not run fixes nothing and says the - // opposite. - const repair = await repairWithheldRules(cwd, token, { - repositoryUrl, - result: { - ...outcome.result, - unsafe: outcome.result.unsafe.filter( - (entry) => !withheldFiles.has(entry.file) - ), - missing: outcome.result.missing.filter( - (entry) => !withheldFiles.has(entry.file) - ), - }, - }); - - // A rule can be blessed and then vanish before it is executed. `execute` is - // whatever re-discovery under `.taskless/.run/` returns, and that is a - // different question from what was blessed: a `captures/` symlink that - // resolves in the working tree can dangle once copied, a file can fail to - // materialize, and re-discovery then classifies the rule as "not a runtime - // rule" and drops it. - // - // Without this, such a rule is in neither list. It is not in `execute` - // because it was dropped, and not in `withheld` because the server did bless - // it, so `check` exits 0 having said nothing and the user believes it ran. - // Accounting for the difference is what makes the drop reportable at all, - // independently of which specific route caused it. - const droppedSkips = accountForDroppedRules(blessed, execute); - - const planEntitlement = - entitlement === undefined - ? undefined - : summarizeEntitlement(cwd, entitlement, planWithheld); - - return { - execute, - skipped: [ - ...unreadableSkips, - ...notBlessed.map((rule) => ({ - rule: rule.name, - reason: "not blessed by the server (unsafe / unknown / drift)", - })), - ...planWithheld.map((rule) => ({ - rule: rule.name, - reason: NOT_IN_PLAN_REASON, - })), - ...droppedSkips, - ], - notices: [ - ...(planEntitlement === undefined || planEntitlement.withheld.length === 0 - ? [] - : [withheldNotice(planEntitlement)]), - ...repair.notices, - ], - ...(planEntitlement === undefined ? {} : { entitlement: planEntitlement }), - }; -} - -/** - * Name what the service withheld, locally where possible. - * - * A withheld entry whose file matches no local rule is kept by its reported - * path rather than dropped. The service said something will not run, and the - * CLI failing to attribute it is not a reason for the run to go green. - */ -function summarizeEntitlement( - cwd: string, - entitlement: NonNullable, - planWithheld: RuntimeRule[] -): PlanEntitlement { - const matched = new Set( - planWithheld.map((rule) => reportedCheckPath(cwd, rule)) - ); - const unmatched = entitlement.withheld - .map((entry) => entry.file) - .filter((file) => !matched.has(file)); - return { - runtimeSignatures: false, - ...(entitlement.reason === undefined ? {} : { reason: entitlement.reason }), - ...(entitlement.upgradeUrl === undefined - ? {} - : { upgradeUrl: entitlement.upgradeUrl }), - withheld: [...planWithheld.map((rule) => rule.name), ...unmatched], - }; -} - -/** The one notice a withheld run prints, so the upgrade URL appears once. */ -function withheldNotice(entitlement: PlanEntitlement): string { - const count = entitlement.withheld.length; - return ( - `${String(count)} runtime ${count === 1 ? "rule was" : "rules were"} ` + - `withheld because runtime rules are not included in your Taskless plan` + - (entitlement.reason === undefined ? "" : ` (${entitlement.reason})`) + - `: ${entitlement.withheld.join(", ")}. \`check\` fails until they can run` + - (entitlement.upgradeUrl === undefined - ? "." - : `. Upgrade at ${entitlement.upgradeUrl}`) - ); -} - -/** - * Act on the verdicts `check` used to parse and discard. - * - * `unsafe` and `missing` are repairable and are fetched; `unknown` is not, and - * gets an explanation instead. Every outcome here is a NOTICE rather than a - * failure: a rule that could not be repaired is a rule that stays withheld, - * which is already the safe state. A repair failing must never be the reason a - * `check` fails. - */ -async function repairWithheldRules( - cwd: string, - token: string, - input: { repositoryUrl: string; result: ReconcileResponse } -): Promise<{ notices: string[] }> { - const notices: string[] = []; - - // A file this disk holds that the service never issued. There is nothing to - // fetch, and saying so is the whole job: it reads as an unexplained skip - // otherwise, and the causes are ordinary (hand-written, or belonging to - // another organization or installation). - for (const entry of input.result.unknown) { - notices.push( - `${entry.file} was not issued by the rule service, so it cannot be ` + - `restored and will not run. It was written by hand, or belongs to a ` + - `different organization or installation.` - ); - } - - const { targets, unidentified } = repairTargets(input.result); - - // A repairable entry that named no rule. The service's own schema requires - // one, so reaching here means it broke that contract — and the entry is - // skipped rather than guessed at, because a rule left unrepaired stays - // withheld, which is safe, while a request built from a missing id asks for - // a rule nobody named and fails in a way nobody reads. - for (const entry of unidentified) { - notices.push( - `${entry.file} needs to be restored, but the rule service did not say ` + - `which rule it belongs to, so it could not be requested and will not ` + - `run.` - ); - } - - // Fetched concurrently: each target is a different rule id under the same - // token and repository, so they do not order against each other, and a repo - // with several drifted rules would otherwise pay one round trip per rule on - // every `check` until they reconverge. The WRITES stay sequential below, - // because two rules can share a directory prefix and a half-applied set is - // the state this whole path exists to avoid. - const fetched = await Promise.all( - targets.map(async (target) => ({ - target, - outcome: await restoreRule(token, { - ruleId: target.ruleId, - repositoryUrl: input.repositoryUrl, - }), - })) - ); - - for (const { target, outcome } of fetched) { - if (outcome.status !== "ok") { - notices.push( - `${target.file} could not be restored (${ - outcome.status === "unauthorized" - ? "authentication was rejected" - : outcome.reason - }).` - ); - continue; - } - - const rule = outcome.rules.find( - (candidate) => candidate.id === target.ruleId - ); - if (rule === undefined) { - notices.push( - `${target.file} could not be restored: the service returned no rule ` + - `called ${target.ruleId}.` - ); - continue; - } - - const verdict = await verifyRestoredCheck(target, rule); - if (!verdict.ok) { - notices.push(`${target.file} was not restored: ${verdict.reason}.`); - continue; - } - - try { - // A restored rule missing its fixtures is still the blessed bytes and is - // still worth writing; the notice rides along with the repair's own. - await writeRuleFile(cwd, rule, (message) => notices.push(message)); - } catch (error) { - // A failed WRITE and a failed CLEANUP ask the reader for opposite - // things, and saying "could not be written" for both is worse than - // saying nothing: the blessed bytes are on disk in the second case, so - // a reader acting on it re-runs a repair that already succeeded, or - // decides the rule is unrepaired and edits it by hand. - if (error instanceof PurgeIncompleteError) { - notices.push( - `${target.file} was rewritten with the bytes the service blessed, ` + - `but ${String(error.failures.length)} stale ` + - `${error.failures.length === 1 ? "entry" : "entries"} could not ` + - `be removed and an engine still reads ` + - `${error.failures.length === 1 ? "it" : "them"}: ` + - `${error.failures.join(", ")}.` - ); - continue; - } - const message = error instanceof Error ? error.message : String(error); - notices.push(`${target.file} could not be written (${message}).`); - continue; - } - // The usual notice below promises the next `check` blesses the rule, which - // a plan without runtime signatures will not do. The bytes are still the - // right bytes and are still written; only the promise is withdrawn. - if (outcome.entitlement !== undefined) { - notices.push( - `${target.file} was restored with the bytes the service blessed. ` + - notRunOnPlanSentence(outcome.entitlement) - ); - continue; - } - // Now says what the DIRECTORY contains, not just what was written. The - // delivered set is authoritative (see `writeDeliveredFileSet`), so a file - // the set does not name — a stray capture beside the rule, which reconcile - // never reported because only `check.ts` is signed — is gone rather than - // left in place still changing what the rule matches. The one exception is - // `.tests/`, which is named here rather than glossed: fixtures are data no - // engine reads, they are kept, and a reader should not have to infer that - // from silence. - notices.push( - `${target.file} was restored: its rule directory now holds exactly the ` + - `files the service delivered, apart from test fixtures under ` + - `\`.tests/\`, which are left alone. It does not run in this pass; the ` + - `next \`check\` reports the repaired signature and is blessed through ` + - `the ordinary path.` - ); - } - - return { notices }; -} diff --git a/packages/cli/src/rules/runtime/repair.ts b/packages/cli/src/rules/runtime/repair.ts deleted file mode 100644 index 6627c651..00000000 --- a/packages/cli/src/rules/runtime/repair.ts +++ /dev/null @@ -1,193 +0,0 @@ -import { canonicalHash } from "../rule-hash"; -import type { RestoredRule } from "../../api/restore"; -import type { MissingEntry, UnsafeEntry } from "../../api/reconcile"; - -/** - * Deciding what a reconcile verdict can repair, and proving a repair is one. - * - * Reconcile returns three verdicts and only two of them are repairable, which - * the entry shapes already say if you read them as a set: - * - * - `unsafe` `{ ruleId, file, expected, got }` — we hold bytes that drifted - * from what the server blessed. Repairable: the server has the real ones, - * and the entry names which rule to ask for. - * - `missing` `{ ruleId, file }` — the server expected a rule we never - * reported. Repairable, and it names its rule the same way. - * - `unknown` `{ file }` — we reported a file the service never issued. NOT - * repairable, and correctly so: there is nothing on the server to fetch, - * which is exactly why the entry carries no rule id. What it needs is an - * explanation, not a request. - * - * NOTHING REPAIRED RUNS IN THE PASS THAT REPAIRED IT. These functions rewrite - * the working tree and promote nothing into the current run. - */ - -/** The signature envelope this repair must reproduce, and the rule to ask for. */ -export interface RepairTarget { - ruleId: string; - file: string; - /** - * The signature the SERVER already told us it blessed. - * - * `undefined` for a `missing` rule, which we do not hold and therefore have - * no prior expectation for. Present for `unsafe`, and that is the case worth - * being strict about — see {@link verifyRestoredCheck}. - */ - expected: string | undefined; -} - -/** A repairable entry that did not name the rule to ask for. */ -export interface UnidentifiedEntry { - file: string; -} - -/** What the repairable buckets resolved to: what to fetch, and what cannot be. */ -export interface RepairPlan { - targets: RepairTarget[]; - /** - * Entries the published schema says must carry a `ruleId` and did not. There - * is no request to make for these, only something to say. - */ - unidentified: UnidentifiedEntry[]; -} - -/** - * The rule id an entry names, or `undefined` if it did not name one. - * - * The deployed schema requires `ruleId` on both repairable verdicts and the - * type here says `string`, but `reconcile` decodes the response with a cast - * rather than a schema, so that type is the service's PROMISE and not a fact - * about the bytes that arrived. This defends against it breaking that promise, - * which is the only reason a client validates a payload at all. - * - * Without it a rollback, a canary, or a schema regression puts `undefined` into - * a `string` and `restoreRule` asks for `/cli/api/request/undefined/restore` — - * a request that cannot succeed, made on behalf of a rule nobody can name. - * Whitespace is rejected for the same reason: it URL-encodes into an equally - * meaningless request rather than failing where it can be read. - */ -function namedRuleId(entry: { ruleId: string }): string | undefined { - const id: unknown = entry.ruleId; - return typeof id === "string" && id.trim() !== "" ? id : undefined; -} - -/** - * Repair targets for the buckets that have something to fetch. - * - * Both repairable verdicts carry `ruleId`, so the id restore is keyed on is - * read straight off the entry. Nothing is derived from the reported path: the - * rules layout has moved twice, and a parse of it would break silently on the - * next move, requesting a rule id the service never issued and leaving a - * drifted rule unrepaired and unexecuted with nothing to read. - * - * An entry that names no rule is separated out rather than guessed at. The - * answer to a missing id is to report it — a rule that is not repaired stays - * withheld, which is already the safe state, and a notice is the difference - * between that and a clean-looking pass. - */ -export function repairTargets(buckets: { - unsafe: UnsafeEntry[]; - missing: MissingEntry[]; -}): RepairPlan { - const targets: RepairTarget[] = []; - const unidentified: UnidentifiedEntry[] = []; - - for (const entry of buckets.unsafe) { - const ruleId = namedRuleId(entry); - if (ruleId === undefined) { - unidentified.push({ file: entry.file }); - continue; - } - targets.push({ ruleId, file: entry.file, expected: entry.expected }); - } - - for (const entry of buckets.missing) { - const ruleId = namedRuleId(entry); - if (ruleId === undefined) { - unidentified.push({ file: entry.file }); - continue; - } - // A missing rule is one we do not hold, so there is no prior signature to - // hold the restored bytes to. - targets.push({ ruleId, file: entry.file, expected: undefined }); - } - - return { targets, unidentified }; -} - -/** Why a restored rule was refused, in words a `check` reader can act on. */ -export type RepairVerdict = - | { ok: true; check: string } - | { ok: false; reason: string }; - -/** The `check.ts` entry of a restored file set, if it carries exactly one. */ -function restoredCheck(rule: RestoredRule): string | undefined { - // `rule.files` directly: every variant of the union declares it, so a cast - // here would re-declare a shape the generated types already know and absorb - // a future schema change instead of failing the build. - const matches = rule.files.filter((file) => file.path === "check.ts"); - return matches.length === 1 ? matches[0]?.content : undefined; -} - -/** - * Whether a restored rule is the one we asked for, and the one we were owed. - * - * THE SIGNATURE CHECKED IS THE ONE RECONCILE ALREADY SENT, not the one the - * restore response carries. That distinction is the whole guarantee. Verifying - * the response against its own `signature` field proves only that the service - * is internally consistent, which it would also be if it handed back a NEWER - * generation of the rule. That would be an upgrade wearing a repair's clothes, - * arriving mid-`check`, having never been reviewed by anyone here. - * - * With `expected` in hand there is exactly one acceptable answer, and "the - * service sent something newer" is refused by the same comparison that catches - * a corrupted transfer. - * - * A `missing` rule has no prior expectation, so it falls back to the - * response's own signature. That is a genuinely weaker check and it is the - * best available: we are not repairing a rule we hold, we are fetching one we - * do not have, and there is nothing local to disagree with. - */ -export async function verifyRestoredCheck( - target: RepairTarget, - rule: RestoredRule -): Promise { - const check = restoredCheck(rule); - if (check === undefined) { - return { - ok: false, - reason: "the restored rule carried no single `check.ts`", - }; - } - - // Read through the union rather than cast: `signature` is optional on the - // `sg`/`vale` variants and required on `runtime`, which is the distinction - // this function exists to enforce, so it must come from the types. - const claimed = rule.signature; - if (typeof claimed !== "string" || claimed === "") { - // The published schema requires this on a runtime rule, so reaching here - // means the service broke its own contract. Refuse rather than write bytes - // nothing vouches for. - return { - ok: false, - reason: "the restored runtime rule carried no signature", - }; - } - - const actual = await canonicalHash(check); - if (actual !== claimed) { - return { - ok: false, - reason: `the restored bytes do not match the signature the service sent with them (claimed ${claimed}, got ${actual})`, - }; - } - - if (target.expected !== undefined && actual !== target.expected) { - return { - ok: false, - reason: `the restored bytes are not the ones reconcile blessed — expected ${target.expected}, got ${actual}. Restore repairs a rule, it does not upgrade one`, - }; - } - - return { ok: true, check }; -} diff --git a/packages/cli/src/rules/runtime/run-set.ts b/packages/cli/src/rules/runtime/run-set.ts deleted file mode 100644 index b711aaeb..00000000 --- a/packages/cli/src/rules/runtime/run-set.ts +++ /dev/null @@ -1,118 +0,0 @@ -import { cp, mkdir, rm } from "node:fs/promises"; -import { join, relative, sep } from "node:path"; - -import { addToGitignore } from "../../filesystem/gitignore"; -import { signRuleFile } from "../rule-hash"; -import type { ReportedFile, RunEntry } from "../../api/reconcile"; -import { discoverRuntimeRulesIn, type RuntimeRule } from "./discover"; - -/** Directory (relative to `.taskless/`) holding materialized runtime rules. */ -export const RUNTIME_RUN_DIR = ".run/runtime-rules"; - -/** A runtime rule paired with its `check.ts` signature — the reconcile gate. */ -export interface SignedRuntimeRule { - rule: RuntimeRule; - /** Canonical signature envelope for the rule's `check.ts` bytes. */ - signature: string; -} - -/** Result of signing a set of runtime rules' `check.ts` files. */ -export interface RuntimeSigningResult { - /** Rules whose `check.ts` was read and signed. */ - signed: SignedRuntimeRule[]; - /** Rules whose `check.ts` was missing or unreadable (cannot be reconciled). */ - unreadable: RuntimeRule[]; -} - -/** - * Sign each runtime rule's `check.ts` (only) — the sole artifact carrying - * arbitrary code execution. Capture `*.yml` are inert and are neither signed nor - * reported. A rule whose `check.ts` cannot be read is returned in `unreadable` - * (never thrown) so one malformed rule never aborts the whole `check`. - */ -export async function signRuntimeChecks( - rules: RuntimeRule[] -): Promise { - const signed: SignedRuntimeRule[] = []; - const unreadable: RuntimeRule[] = []; - await Promise.all( - rules.map(async (rule) => { - try { - signed.push({ rule, signature: await signRuleFile(rule.checkFile) }); - } catch { - unreadable.push(rule); - } - }) - ); - return { signed, unreadable }; -} - -/** - * The path a rule's `check.ts` is reported under. One function, because - * `entitlement.withheld` carries no signature and is joined back to local rules - * by this path: a second spelling of it would silently match nothing. - */ -export function reportedCheckPath(cwd: string, rule: RuntimeRule): string { - // Reconcile paths are repo-relative POSIX; normalize Windows separators. - return relative(cwd, rule.checkFile).split(sep).join("/"); -} - -/** Map signed runtime rules to the reconcile report (`check.ts` path + signature). */ -export function reportRuntimeChecks( - cwd: string, - signed: SignedRuntimeRule[] -): ReportedFile[] { - return signed.map(({ rule, signature }) => ({ - file: reportedCheckPath(cwd, rule), - signature, - })); -} - -/** The blessed/withheld split from a successful reconciliation. */ -export interface RuntimeSelection { - /** Rules whose `check.ts` signature is in the server `run` set. */ - blessed: RuntimeRule[]; - /** Rules whose `check.ts` was not blessed (advisory). */ - withheld: RuntimeRule[]; -} - -/** - * Split signed runtime rules into those whose `check.ts` is present in the - * server's `run` set (blessed, execute) and the rest (withheld, advisory). The - * join is by signature — content-based, so a moved-but-unchanged rule resolves. - */ -export function selectBlessedRuntimeRules( - signed: SignedRuntimeRule[], - run: RunEntry[] -): RuntimeSelection { - const runSignatures = new Set(run.map((entry) => entry.signature)); - const blessed: RuntimeRule[] = []; - const withheld: RuntimeRule[] = []; - for (const { rule, signature } of signed) { - if (runSignatures.has(signature)) blessed.push(rule); - else withheld.push(rule); - } - return { blessed, withheld }; -} - -/** - * Materialize blessed runtime rules into the gitignored - * `.taskless/.run/runtime-rules/` and return them re-discovered from there, so - * the narrow and `check.ts` execute the blessed bytes rather than whatever is - * live in `.taskless/rules/runtime/`. - */ -export async function materializeRuntimeRules( - cwd: string, - blessed: RuntimeRule[] -): Promise { - const runtimeRunRoot = join(cwd, ".taskless", RUNTIME_RUN_DIR); - await rm(runtimeRunRoot, { recursive: true, force: true }); - await mkdir(runtimeRunRoot, { recursive: true }); - await Promise.all( - blessed.map((rule) => - cp(rule.dir, join(runtimeRunRoot, rule.name), { recursive: true }) - ) - ); - await addToGitignore(cwd, [".run/"]); - return discoverRuntimeRulesIn(runtimeRunRoot); -} diff --git a/packages/cli/src/rules/snapshot.ts b/packages/cli/src/rules/snapshot.ts new file mode 100644 index 00000000..130f0f8c --- /dev/null +++ b/packages/cli/src/rules/snapshot.ts @@ -0,0 +1,171 @@ +import { + copyFile, + mkdir, + readdir, + realpath, + rm, + stat, + writeFile, +} from "node:fs/promises"; +import { join, relative } from "node:path"; + +import { isMissingDirectory } from "./errno"; +import { engineRulesDirectory, ruleDirectory, rulesRoot } from "./engines"; +import type { EngineName } from "./layout"; + +/** + * The copy of `.taskless/rules/` that one `check` signs, reports, and runs. + * + * Copy, then sign, then run the copy. Signing the live tree and running it + * afterwards leaves a window in which an edit runs unjudged; copying after the + * verdict (what runtime rules used to do) leaves the same window on the other + * side. Taking the copy first and never reading the live tree again closes both: + * whatever the verdict describes is exactly what runs. + * + * **The snapshot mirrors the project's layout** under a base directory: + * `.taskless/.run/snapshot/.taskless/rules/`. Every path helper in this package + * takes a project root and appends `.taskless/rules/...`, and both config + * assemblers write root-relative paths (`StylesPath`, `ruleDirs`), so handing + * them the base instead of the project root points all of it at the snapshot + * with no new parameter to thread through, and none to forget. Measured before + * relying on it: identical findings from both config locations, including a + * Vale rule scoped to a subdirectory glob. + */ + +/** The snapshot base, relative to `.taskless/`. Gitignored with the rest of `.run/`. */ +const SNAPSHOT_DIRECTORY = join(".run", "snapshot"); + +/** + * Operating-system metadata that is neither copied nor reported. + * + * The service marks a rule `unsafe` when its directory holds a file it never + * issued, and a Finder window must not fail CI. Leaving these out is safe only + * because they are also absent from what runs: no engine reads any of them. The + * list is closed on purpose. Growing it to "files that look harmless" would be a + * way to hide a file from the verdict while an engine still reads it. + */ +export const IGNORED_METADATA_FILES: ReadonlySet = new Set([ + ".DS_Store", + "Thumbs.db", + "desktop.ini", +]); + +/** A taken snapshot. */ +export interface Snapshot { + /** The project root the engines run from. */ + cwd: string; + /** + * The base that mirrors the project root: pass it wherever a function takes + * a project root to reach the snapshot's rules and assembled configs. + */ + base: string; +} + +/** + * Replace the snapshot with a fresh copy of `.taskless/rules/`. + * + * Symbolic links are DEREFERENCED: what is signed has to be what runs, and a + * link resolved at run time is bytes nobody signed. A link that does not + * resolve is left out, which surfaces as a missing file in the verdict rather + * than as a surprise when an engine follows it. A directory reached twice + * through links is copied once, so a cycle cannot recurse forever. + */ +export async function takeSnapshot(cwd: string): Promise { + const base = join(cwd, ".taskless", SNAPSHOT_DIRECTORY); + await rm(base, { recursive: true, force: true }); + await mkdir(join(base, ".taskless"), { recursive: true }); + // The run directory ignores ITSELF. Adding `.run/` to `.taskless/.gitignore` + // instead would make every `check` rewrite a tracked file, and `check` + // writes nothing under `.taskless/` outside `.taskless/.run/`. git, and the + // ignore walkers ast-grep and Vale use, all honor a nested `.gitignore`. + await writeFile(join(cwd, ".taskless", ".run", ".gitignore"), "*\n"); + + const source = rulesRoot(cwd); + const target = rulesRoot(base); + const visited = new Set(); + try { + await copyTree(source, target, visited); + } catch (error) { + if (!isMissingDirectory(error)) throw error; + // No rules tree: an empty snapshot, which the callers read as no rules. + await mkdir(target, { recursive: true }); + } + return { cwd, base }; +} + +async function copyTree( + source: string, + target: string, + visited: Set +): Promise { + const real = await realpath(source); + if (visited.has(real)) return; + visited.add(real); + + await mkdir(target, { recursive: true }); + const entries = await readdir(source, { withFileTypes: true }); + for (const entry of entries) { + if (IGNORED_METADATA_FILES.has(entry.name)) continue; + const from = join(source, entry.name); + const to = join(target, entry.name); + + let kind: "file" | "directory" | "other"; + if (entry.isSymbolicLink()) { + let resolved; + try { + resolved = await stat(from); + } catch (error) { + // Dangling: nothing to sign, and nothing an engine could read. + if (isMissingDirectory(error)) continue; + throw error; + } + kind = resolved.isDirectory() + ? "directory" + : resolved.isFile() + ? "file" + : "other"; + } else { + kind = entry.isDirectory() + ? "directory" + : entry.isFile() + ? "file" + : "other"; + } + + if (kind === "directory") await copyTree(from, to, visited); + else if (kind === "file") await copyFile(from, to); + // Sockets, FIFOs, devices: not rule files, and not copyable as bytes. + } +} + +/** Remove one rule from the snapshot, so no engine configuration can reach it. */ +export async function excludeFromSnapshot( + snapshot: Snapshot, + engine: EngineName, + ruleId: string +): Promise { + await rm(ruleDirectory(snapshot.base, engine, ruleId), { + recursive: true, + force: true, + }); +} + +/** The snapshot's rules directory for one engine. */ +export function snapshotEngineDirectory( + snapshot: Snapshot, + engine: EngineName +): string { + return engineRulesDirectory(snapshot.base, engine); +} + +/** + * A path inside the snapshot, relative to the project root, for handing to an + * engine that runs from the project root (an assembled config path is relative + * to the base it was assembled against). + */ +export function fromProjectRoot( + snapshot: Snapshot, + pathInBase: string +): string { + return relative(snapshot.cwd, join(snapshot.base, pathInBase)); +} diff --git a/packages/cli/src/rules/verdicts.ts b/packages/cli/src/rules/verdicts.ts new file mode 100644 index 00000000..79328b06 --- /dev/null +++ b/packages/cli/src/rules/verdicts.ts @@ -0,0 +1,284 @@ +import { parseEntitlementV2, type EntitlementV2 } from "../api/entitlement"; +import type { EngineName } from "./layout"; +import { isKnownEngine } from "./layout"; +import type { ReportedRule } from "./report"; + +/** + * Turning a v2 reconcile answer into what `check` does with each rule. + * + * Pure, and deliberately so: this is the policy, and every row of it is a + * decision about whether edited code runs or a run goes green. It is tested as + * a table rather than through a mocked network, because the network is not + * what can be wrong here. + * + * | Verdict | runtime | sg / vale | + * |--------------|---------------------------------|----------------------------------| + * | `run` | execute | run | + * | withheld | not executed; fails | (never sent) | + * | `unsafe` | not executed; restore offered | not run; FAILS; restore offered | + * | `missing` | warn; restore offered | warn; restore offered | + * | `unknown` | not executed | run, silently | + * | unaccounted | not executed; fails | not run; fails | + * + * **Accounting is computed, not trusted.** Every reported rule must land in + * exactly one of `rules`, `unknown`, and `entitlement.withheld`. A rule the + * answer drops, or answers twice, is not run and fails the run. That is what + * turns a parser that silently drops withheld entries (#403), or a service + * that forgets a rule, into a red run instead of a green one. + */ + +/** A differing file, as the service reported it. */ +export interface DifferingFile { + path: string; + /** What was issued. Absent for a file the service never issued. */ + expected?: string; + /** What was reported. Absent for an issued file that was not reported. */ + got?: string; +} + +export type IntegrityVerdict = + | "unsafe" + | "missing" + | "unknown" + | "unaccounted" + | "duplicate"; + +/** A non-`run` outcome worth reporting, for `check --json`'s `integrity`. */ +export interface IntegrityEntry { + ruleId: string; + engine?: EngineName; + verdict: IntegrityVerdict; + files?: DifferingFile[]; + revisionId?: string; +} + +/** What happens to one reported rule. */ +export interface RuleDisposition { + ruleId: string; + engine: EngineName; + /** Whether it stays in the snapshot the engines read. */ + run: boolean; + /** Why it did not run, for `skipped` (runtime) and notices. */ + reason?: string; +} + +export interface VerdictPlan { + dispositions: RuleDisposition[]; + integrity: IntegrityEntry[]; + /** Human notices, one per rule that needs attention. */ + notices: string[]; + /** Each reason that fails the run, besides a plan withhold. */ + failures: string[]; + /** Reported rule ids the service withheld for the plan. */ + withheld: string[]; + /** Present only for an unentitled organization. */ + entitlement?: EntitlementV2; +} + +/** The reason a runtime rule the plan withholds did not run. */ +export const NOT_IN_PLAN_REASON = "not included in your Taskless plan"; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function records(value: unknown): Record[] { + return Array.isArray(value) ? value.filter((entry) => isRecord(entry)) : []; +} + +function readFiles(value: unknown): DifferingFile[] { + return records(value) + .filter((entry) => typeof entry.path === "string") + .map((entry) => ({ + path: entry.path as string, + ...(typeof entry.expected === "string" + ? { expected: entry.expected } + : {}), + ...(typeof entry.got === "string" ? { got: entry.got } : {}), + })); +} + +/** "changed .vale.ini; removed captures/a.yml; added extra.yml" */ +export function describeDifferences(files: readonly DifferingFile[]): string { + if (files.length === 0) return "its files differ from what was issued"; + return files + .map((file) => + file.expected !== undefined && file.got !== undefined + ? `changed ${file.path}` + : file.expected === undefined + ? `added ${file.path}` + : `removed ${file.path}` + ) + .join("; "); +} + +/** Record a reported rule the answer did not account for: it does not run, and the run fails. */ +function unaccounted(plan: VerdictPlan, rule: ReportedRule, why: string): void { + const { ruleId, engine } = rule; + plan.dispositions.push({ ruleId, engine, run: false, reason: why }); + plan.integrity.push({ ruleId, engine, verdict: "unaccounted" }); + plan.failures.push(`${engine} rule ${ruleId} did not run: ${why}.`); +} + +/** + * Apply a reconcile response to the rules that were reported. + * + * `restoreCommand` renders the command a notice points at, so this stays free + * of how the CLI was invoked. + */ +export function applyVerdicts( + reported: readonly ReportedRule[], + response: unknown, + restoreCommand: (ruleId: string) => string +): VerdictPlan { + const body = isRecord(response) ? response : {}; + const entitlement = parseEntitlementV2(body.entitlement); + const withheldIds = (entitlement?.withheld ?? []).map( + (entry) => entry.ruleId + ); + const unknownIds = records(body.unknown) + .map((entry) => entry.ruleId) + .filter((id): id is string => typeof id === "string"); + const verdicts = records(body.rules).filter( + (entry) => typeof entry.ruleId === "string" + ); + + const plan: VerdictPlan = { + dispositions: [], + integrity: [], + notices: [], + failures: [], + withheld: [], + ...(entitlement === undefined ? {} : { entitlement }), + }; + + const reportedIds = new Set(reported.map((rule) => rule.ruleId)); + + for (const rule of reported) { + const { ruleId, engine } = rule; + const answers = verdicts.filter((entry) => entry.ruleId === ruleId); + const inUnknown = unknownIds.filter((id) => id === ruleId).length; + const inWithheld = withheldIds.filter((id) => id === ruleId).length; + const count = answers.length + inUnknown + inWithheld; + + if (count !== 1) { + unaccounted( + plan, + rule, + count === 0 + ? "the rule service's answer did not account for it" + : "the rule service answered for it more than once" + ); + continue; + } + + if (inWithheld === 1) { + plan.withheld.push(ruleId); + plan.dispositions.push({ + ruleId, + engine, + run: false, + reason: NOT_IN_PLAN_REASON, + }); + continue; + } + + if (inUnknown === 1) { + if (engine === "runtime") { + const reason = + "not issued by the rule service for this repository, so it runs only with --dangerously-run-scripts"; + plan.dispositions.push({ ruleId, engine, run: false, reason }); + plan.integrity.push({ ruleId, engine, verdict: "unknown" }); + } else { + // Locally written static rules are first-class, and every one of them + // is `unknown`. A notice per rule per run would be noise that trains + // people to skip notices. + plan.dispositions.push({ ruleId, engine, run: true }); + } + continue; + } + + const answer = answers[0] as Record; + // The service says which engine it judged. A different answer means it + // judged something other than what this directory is, and the verdict + // cannot be applied to it. + if ( + typeof answer.engine !== "string" || + !isKnownEngine(answer.engine) || + answer.engine !== engine + ) { + unaccounted( + plan, + rule, + `the rule service judged it as a ${String(answer.engine)} rule, but it is a ${engine} rule here` + ); + continue; + } + + switch (answer.verdict) { + case "run": { + plan.dispositions.push({ ruleId, engine, run: true }); + break; + } + case "unsafe": { + const files = readFiles(answer.files); + const changes = describeDifferences(files); + plan.integrity.push({ ruleId, engine, verdict: "unsafe", files }); + if (engine === "runtime") { + plan.dispositions.push({ + ruleId, + engine, + run: false, + reason: `edited since Taskless issued it (${changes})`, + }); + plan.notices.push( + `runtime rule ${ruleId} was edited since Taskless issued it (${changes}), so it did not run. Run \`${restoreCommand(ruleId)}\` to put back the issued version.` + ); + } else { + plan.dispositions.push({ + ruleId, + engine, + run: false, + reason: `edited since Taskless issued it (${changes})`, + }); + plan.failures.push( + `${engine} rule ${ruleId} was edited since Taskless issued it (${changes}), so it did not run and \`check\` fails. Run \`${restoreCommand(ruleId)}\` to put back the issued version.` + ); + } + break; + } + default: { + // `missing` names a rule that was NOT reported, so it cannot be the + // answer for one that was; anything else is not a verdict at all. + unaccounted( + plan, + rule, + `the rule service answered \`${String(answer.verdict)}\`, which is not a verdict for a rule that was reported` + ); + } + } + } + + for (const entry of verdicts) { + if (entry.verdict !== "missing") continue; + const ruleId = entry.ruleId as string; + if (reportedIds.has(ruleId)) continue; // already failed as unaccounted + const engine = + typeof entry.engine === "string" && isKnownEngine(entry.engine) + ? entry.engine + : undefined; + const revisionId = + typeof entry.revisionId === "string" ? entry.revisionId : undefined; + plan.integrity.push({ + ruleId, + ...(engine === undefined ? {} : { engine }), + verdict: "missing", + ...(revisionId === undefined ? {} : { revisionId }), + }); + plan.notices.push( + `${engine ?? "A"} rule ${ruleId} was issued for this repository but is not in .taskless/rules/. Run \`${restoreCommand(ruleId)}\` to bring it back, or ignore this if it was removed on purpose.` + ); + } + + return plan; +} diff --git a/packages/cli/src/schemas/check.ts b/packages/cli/src/schemas/check.ts index fde389bf..681a0800 100644 --- a/packages/cli/src/schemas/check.ts +++ b/packages/cli/src/schemas/check.ts @@ -44,7 +44,9 @@ export const outputSchema = z.object({ failures: z .array(z.string()) .optional() - .describe("Engines that were present and failed"), + .describe( + "Why the run failed besides findings: engines that were present and failed, and rules that were edited, unaccounted for, or collide" + ), notices: z .array(z.string()) .optional() @@ -71,6 +73,46 @@ export const outputSchema = z.object({ }) .optional() .describe("Runtime rules withheld because the plan does not include them"), + // One entry per rule whose verified outcome needs attention: edited, missing, + // a runtime rule the service never issued, unaccounted for, or an id shared + // across engines. Locally written ast-grep and Vale rules are `unknown` too + // and are deliberately NOT listed: they run, and every run would repeat them. + integrity: z + .array( + z.object({ + ruleId: z.string(), + engine: z.enum(["sg", "vale", "runtime"]).optional(), + verdict: z + .enum(["unsafe", "missing", "unknown", "unaccounted", "duplicate"]) + .describe( + "unsafe: edited since issued; missing: issued but not on disk; unknown: a runtime rule the service never issued; unaccounted: the service's answer did not account for it; duplicate: its id is used by more than one engine" + ), + files: z + .array( + z.object({ + path: z.string(), + expected: z + .string() + .optional() + .describe("Issued signature; absent for an added file"), + got: z + .string() + .optional() + .describe("Reported signature; absent for a removed file"), + }) + ) + .optional() + .describe("For unsafe: each file that differs from what was issued"), + revisionId: z + .string() + .optional() + .describe("For missing: the revision `rule restore` brings back"), + }) + ) + .optional() + .describe( + "Rules whose verified state needs attention. `taskless rule restore ` repairs unsafe and missing ones" + ), }); /** Error schema for `taskless check --json` on failure */ diff --git a/packages/cli/test/check-snapshot.test.ts b/packages/cli/test/check-snapshot.test.ts new file mode 100644 index 00000000..2aaeb1b2 --- /dev/null +++ b/packages/cli/test/check-snapshot.test.ts @@ -0,0 +1,262 @@ +import { execFileSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { + cp, + mkdir, + mkdtemp, + readFile, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { assembleEngineConfigs } from "../src/rules/assemble"; +import { runEngines } from "../src/rules/dispatch"; +import { reportRules } from "../src/rules/report"; +import type { CheckResult } from "../src/types/check"; +import { canonicalHash } from "../src/rules/rule-hash"; +import { + excludeFromSnapshot, + fromProjectRoot, + takeSnapshot, +} from "../src/rules/snapshot"; + +describe("the check snapshot", () => { + let cwd: string; + const rules = () => join(cwd, ".taskless", "rules"); + + beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "tskl-snapshot-")); + await mkdir(join(rules(), "sg", "no-eval-3fa9c21b", ".tests"), { + recursive: true, + }); + await writeFile( + join(rules(), "sg", "no-eval-3fa9c21b", "no-eval-3fa9c21b.yml"), + "id: no-eval-3fa9c21b\n" + ); + await writeFile( + join(rules(), "sg", "no-eval-3fa9c21b", ".tests", "case.ts"), + "eval(x);\n" + ); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); + }); + + it("mirrors the project layout under the base, and ignores itself", async () => { + const snapshot = await takeSnapshot(cwd); + expect(snapshot.base).toBe(join(cwd, ".taskless", ".run", "snapshot")); + expect( + existsSync( + join( + snapshot.base, + ".taskless", + "rules", + "sg", + "no-eval-3fa9c21b", + "no-eval-3fa9c21b.yml" + ) + ) + ).toBe(true); + // Ignored from inside, so `check` never rewrites a tracked file. + expect( + await readFile(join(cwd, ".taskless", ".run", ".gitignore"), "utf8") + ).toBe("*\n"); + expect(existsSync(join(cwd, ".taskless", ".gitignore"))).toBe(false); + expect(fromProjectRoot(snapshot, ".taskless/.vale.ini")).toBe( + join(".taskless", ".run", "snapshot", ".taskless", ".vale.ini") + ); + }); + + it("is what gets signed: an edit after the snapshot does not reach the report", async () => { + const snapshot = await takeSnapshot(cwd); + await writeFile( + join(rules(), "sg", "no-eval-3fa9c21b", "no-eval-3fa9c21b.yml"), + "id: edited\n" + ); + const report = await reportRules(snapshot); + expect(report.rules[0]?.files).toEqual([ + { + path: "no-eval-3fa9c21b.yml", + signature: await canonicalHash("id: no-eval-3fa9c21b\n"), + }, + ]); + }); + + it("dereferences a symlinked file, so the bytes signed are the bytes run", async () => { + const outside = join(cwd, "outside.yml"); + await writeFile(outside, "id: linked\n"); + await rm(join(rules(), "sg", "no-eval-3fa9c21b", "no-eval-3fa9c21b.yml")); + await symlink( + outside, + join(rules(), "sg", "no-eval-3fa9c21b", "no-eval-3fa9c21b.yml") + ); + const snapshot = await takeSnapshot(cwd); + await writeFile(outside, "id: changed after the snapshot\n"); + const report = await reportRules(snapshot); + expect(report.rules[0]?.files[0]?.signature).toBe( + await canonicalHash("id: linked\n") + ); + }); + + it("drops a dangling link, which then shows up as a missing file", async () => { + await symlink( + join(cwd, "nowhere.yml"), + join(rules(), "sg", "no-eval-3fa9c21b", "dangling.yml") + ); + const report = await reportRules(await takeSnapshot(cwd)); + expect(report.rules[0]?.files.map((file) => file.path)).toEqual([ + "no-eval-3fa9c21b.yml", + ]); + }); + + it("neither copies nor reports operating-system metadata", async () => { + await writeFile(join(rules(), "sg", "no-eval-3fa9c21b", ".DS_Store"), "x"); + const snapshot = await takeSnapshot(cwd); + const report = await reportRules(snapshot); + expect(report.rules[0]?.files.map((file) => file.path)).toEqual([ + "no-eval-3fa9c21b.yml", + ]); + expect( + existsSync( + join( + snapshot.base, + ".taskless", + "rules", + "sg", + "no-eval-3fa9c21b", + ".DS_Store" + ) + ) + ).toBe(false); + }); + + it("reports a nested .tests/ as an ordinary file; only the top-level one is fixtures", async () => { + const nested = join(rules(), "sg", "no-eval-3fa9c21b", "extra", ".tests"); + await mkdir(nested, { recursive: true }); + await writeFile(join(nested, "a.yml"), "id: a\n"); + const report = await reportRules(await takeSnapshot(cwd)); + expect(report.rules[0]?.files.map((file) => file.path)).toEqual([ + "extra/.tests/a.yml", + "no-eval-3fa9c21b.yml", + ]); + }); + + it("reports an id used by two engines as a duplicate, never as either rule", async () => { + await mkdir(join(rules(), "vale", "no-eval-3fa9c21b"), { recursive: true }); + await writeFile( + join(rules(), "vale", "no-eval-3fa9c21b", "no-eval-3fa9c21b.yml"), + "extends: existence\n" + ); + const report = await reportRules(await takeSnapshot(cwd)); + expect(report.rules).toEqual([]); + expect(report.duplicates).toEqual([ + { ruleId: "no-eval-3fa9c21b", engines: ["sg", "vale"] }, + ]); + }); + + it("excluding a rule removes it from the snapshot only", async () => { + const snapshot = await takeSnapshot(cwd); + await excludeFromSnapshot(snapshot, "sg", "no-eval-3fa9c21b"); + const { rules: reported } = await reportRules(snapshot); + expect(reported).toEqual([]); + expect(existsSync(join(rules(), "sg", "no-eval-3fa9c21b"))).toBe(true); + }); + + it("an empty project snapshots to no rules", async () => { + await rm(rules(), { recursive: true }); + const report = await reportRules(await takeSnapshot(cwd)); + expect(report).toEqual({ rules: [], duplicates: [], unreadable: [] }); + }); +}); + +function keys(results: readonly CheckResult[]): string[] { + return results + .map((result) => + JSON.stringify([result.file, result.ruleId, result.range, result.message]) + ) + .toSorted(); +} + +describe("engines read the snapshot exactly as they read the live tree", () => { + const fixture = join( + import.meta.dirname, + "fixtures", + "mixed-engines-project" + ); + let cwd: string; + + beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "tskl-snapshot-engines-")); + await cp(fixture, cwd, { recursive: true }); + // Scoped to a SUBDIRECTORY glob: the case that would move if Vale resolved + // section globs against the config file's location rather than the files + // it is handed. + const rule = join(cwd, ".taskless", "rules", "vale", "no-basically"); + await mkdir(rule, { recursive: true }); + await writeFile( + join(rule, "no-basically.yml"), + "extends: existence\nmessage: \"Avoid '%s'.\"\nlevel: error\nignorecase: true\ntokens:\n - basically\n" + ); + await writeFile( + join(rule, ".vale.ini"), + "[docs/**/*.md]\ntskl) rule = no-basically\nno-basically.no-basically = YES\n" + ); + await mkdir(join(cwd, "docs", "deep"), { recursive: true }); + await writeFile( + join(cwd, "docs", "deep", "a.md"), + "This is basically simple. Obviously, simply do it.\n" + ); + await writeFile(join(cwd, "top.md"), "This is basically fine.\n"); + // No `.run/` entry anywhere: the snapshot's own `.gitignore` has to be what + // keeps the engines from linting the copied rules as project files. + execFileSync("git", ["init", "-q"], { cwd }); + }); + + afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); + }); + + it("finds the same results, with the same subdirectory scoping", async () => { + const live = await assembleEngineConfigs(cwd); + const fromLive = await runEngines({ + cwd, + paths: [], + astGrepConfigPath: live.sg, + vale: live.vale, + runtimeRules: [], + }); + + const snapshot = await takeSnapshot(cwd); + const assembled = await assembleEngineConfigs(snapshot.base); + const fromSnapshot = await runEngines({ + cwd, + paths: [], + astGrepConfigPath: + assembled.sg === undefined + ? undefined + : fromProjectRoot(snapshot, assembled.sg), + vale: + assembled.vale?.status === "ok" + ? { + ...assembled.vale, + path: fromProjectRoot(snapshot, assembled.vale.path), + } + : assembled.vale, + runtimeRules: [], + }); + + expect(fromSnapshot.failures).toEqual([]); + expect(keys(fromSnapshot.results)).toEqual(keys(fromLive.results)); + const basically = fromSnapshot.results.filter( + (result) => result.ruleId === "no-basically" + ); + expect(basically.map((result) => result.file)).toEqual([ + join("docs", "deep", "a.md"), + ]); + }); +}); diff --git a/packages/cli/test/engine-dispatch.test.ts b/packages/cli/test/engine-dispatch.test.ts index 7e31d4c2..195f9dad 100644 --- a/packages/cli/test/engine-dispatch.test.ts +++ b/packages/cli/test/engine-dispatch.test.ts @@ -28,14 +28,7 @@ import { writeRuleTestFile, } from "../src/rules/files"; import { verifyOneRule } from "../src/rules/inspect"; -import { - discoverRuntimeRules, - discoverRuntimeRulesIn, -} from "../src/rules/runtime/discover"; -import { - reportRuntimeChecks, - signRuntimeChecks, -} from "../src/rules/runtime/run-set"; +import { discoverRuntimeRules } from "../src/rules/runtime/discover"; import type { GeneratedRule } from "../src/api/rules"; import { CLIError } from "../src/util/cli-error"; import { migrateFixture } from "./support/current-project"; @@ -743,55 +736,3 @@ describe("service-delivered rule ingest", () => { expect(() => resolveIngestEngine({ engine: "nope" })).toThrow(/nope/); }); }); - -describe("reconcile compatibility across the relayout", () => { - let temporaryDirectory: string; - - beforeEach(async () => { - temporaryDirectory = await mkdtemp(join(tmpdir(), "tskl-reconcile-")); - }); - - afterEach(async () => { - await rm(temporaryDirectory, { recursive: true, force: true }); - }); - - it("keeps signatures identical and reports the moved path", async () => { - const tasklessDirectory = join(temporaryDirectory, ".taskless"); - const legacyRule = join(tasklessDirectory, "runtime-rules", "demo"); - await mkdir(legacyRule, { recursive: true }); - await writeFile( - join(tasklessDirectory, "taskless.json"), - JSON.stringify({ version: 3 }), - "utf8" - ); - await writeFile(join(legacyRule, "logs.yml"), RUNTIME_CAPTURE, "utf8"); - await writeFile(join(legacyRule, "check.ts"), RUNTIME_CHECK, "utf8"); - - // Discovery reads the current layout only, so the pre-migration tree - // cannot be discovered — it is described directly. What the test is about - // is the signature, which is computed over `check.ts` bytes and must - // survive the move. - const before = await signRuntimeChecks([ - { - name: "demo", - dir: legacyRule, - captureRules: [], - checkFile: join(legacyRule, "check.ts"), - }, - ]); - const beforeReport = reportRuntimeChecks(temporaryDirectory, before.signed); - - await ensureTasklessDirectory(temporaryDirectory); - - const after = await signRuntimeChecks( - await discoverRuntimeRulesIn(join(tasklessDirectory, "rules", "runtime")) - ); - const afterReport = reportRuntimeChecks(temporaryDirectory, after.signed); - - // The path follows the moved tree... - expect(beforeReport[0]?.file).toBe(".taskless/runtime-rules/demo/check.ts"); - expect(afterReport[0]?.file).toBe(".taskless/rules/runtime/demo/check.ts"); - // ...while the signature — what the server joins on — does not change. - expect(afterReport[0]?.signature).toBe(beforeReport[0]?.signature); - }); -}); diff --git a/packages/cli/test/repair-integration.test.ts b/packages/cli/test/repair-integration.test.ts deleted file mode 100644 index fad492c2..00000000 --- a/packages/cli/test/repair-integration.test.ts +++ /dev/null @@ -1,835 +0,0 @@ -import { execFile } from "node:child_process"; -import { existsSync } from "node:fs"; -import { - chmod, - mkdir, - mkdtemp, - readFile, - rm, - writeFile, -} from "node:fs/promises"; -import { createServer, type Server } from "node:http"; -import { tmpdir } from "node:os"; -import { join, resolve } from "node:path"; -import { promisify } from "node:util"; - -import { afterEach, beforeEach, describe, expect, it } from "vitest"; - -import { restoreRule } from "../src/api/restore"; -import { canonicalHash } from "../src/rules/rule-hash"; -import { migrateFixture } from "./support/current-project"; - -const execFileAsync = promisify(execFile); -const binPath = resolve(import.meta.dirname, "../dist/index.js"); - -/** - * The repair path end to end: reconcile reports drift, restore answers, the - * file is rewritten, and the run says so. - * - * `repair.test.ts` covers the decisions as pure functions. What it cannot see - * is the wiring, and the wiring is where this feature failed review: every - * repair notice reached `warn()` only, which is a no-op under `--json`, so the - * one channel a CI run reads dropped the entire output of the thing whose - * purpose is explaining a rule that did not run. A test at this level is what - * would have caught it, so it is the first assertion below. - */ - -/** A mock serving both endpoints the repair path uses. */ -interface Mock { - apiUrl: string; - restoreCalls: string[]; - close: () => Promise; -} - -/** - * A reconcile handler normally returns the response body directly (always - * HTTP 200). A handler that instead needs to exercise a non-2xx reconcile - * response (e.g. the unauthorized branch of `planRuntime`) returns this - * wrapper, distinguished by its `statusCode` field — no ordinary reconcile - * response body (`{ run, unsafe, unknown, missing }`) has one. - */ -interface HttpOverride { - statusCode: number; - body: unknown; -} - -function isHttpOverride(value: unknown): value is HttpOverride { - return ( - typeof value === "object" && - value !== null && - "statusCode" in value && - typeof (value as { statusCode: unknown }).statusCode === "number" && - "body" in value - ); -} - -function startMock(handlers: { - reconcile: (body: { - files: { file: string; signature: string }[]; - }) => unknown; - restore: (ruleId: string) => { statusCode: number; body: unknown }; -}): Promise { - const restoreCalls: string[] = []; - const server: Server = createServer((request, response) => { - let raw = ""; - request.on("data", (chunk: Buffer) => (raw += chunk.toString())); - request.on("end", () => { - const url = request.url ?? ""; - if (url === "/cli/api/reconcile") { - const result = handlers.reconcile( - JSON.parse(raw) as { files: { file: string; signature: string }[] } - ); - const { statusCode, body } = isHttpOverride(result) - ? result - : { statusCode: 200, body: result }; - response.writeHead(statusCode, { "content-type": "application/json" }); - response.end(JSON.stringify(body)); - return; - } - const restore = /^\/cli\/api\/request\/([^/]+)\/restore$/.exec(url); - if (restore) { - const ruleId = decodeURIComponent(restore[1] ?? ""); - restoreCalls.push(ruleId); - const { statusCode, body } = handlers.restore(ruleId); - response.writeHead(statusCode, { "content-type": "application/json" }); - response.end(JSON.stringify(body)); - return; - } - response.writeHead(404).end("{}"); - }); - }); - return new Promise((resolvePromise) => { - server.listen(0, "127.0.0.1", () => { - const address = server.address(); - const port = typeof address === "object" && address ? address.port : 0; - resolvePromise({ - apiUrl: `http://127.0.0.1:${String(port)}/cli`, - restoreCalls, - close: () => new Promise((done) => server.close(() => done())), - }); - }); - }); -} - -async function runCli( - args: string[], - env: Record -): Promise<{ stdout: string; exitCode: number }> { - // The fixture writes a current-layout tree but no manifest, and `check` - // refuses a project it cannot confirm is current — a tree without a - // manifest reads as version 0, and it cannot tell this one from a - // pre-`0004` project by looking. So the scaffold is completed here, which - // is what a real project has. - await migrateFixture(args); - try { - const { stdout } = await execFileAsync("node", [binPath, ...args], { - env: { ...process.env, ...env }, - }); - return { stdout, exitCode: 0 }; - } catch (error) { - const failure = error as { stdout: string; code: number }; - return { stdout: failure.stdout ?? "", exitCode: failure.code }; - } -} - -/** The `--json` envelope, ignoring any migration chatter before it. */ -function envelope(stdout: string): { notices?: string[] } { - const line = stdout - .trim() - .split("\n") - .findLast((l) => l.trim().startsWith("{")); - return JSON.parse(line ?? "{}") as { notices?: string[] }; -} - -const CAPTURE = [ - "id: logs-abc12345", - "language: typescript", - "rule:", - " pattern: console.log($A)", - "metadata:", - " taskless:", - " version: 1", - " kind: runtime", - " name: logs", - " check: check.ts", - " match: anchor", - "", -].join("\n"); - -const DRIFTED = "export default async () => [];\n"; -const BLESSED = "export default async function () {\n return [];\n}\n"; -const REPORTED = ".taskless/rules/runtime/demo/check.ts"; - -/** - * Reconcile reporting the reported check as drifted from `expected`. - * - * `ruleId` is what restore is keyed on. It defaults to the rule directory's - * name only so the ordinary fixtures read naturally; nothing derives it from - * `REPORTED`, and the disagreeing-id test below passes one that does not match - * the path at all. - */ -const driftedReconcile = - (expected: string, ruleId = "demo") => - () => ({ - run: [], - unsafe: [{ ruleId, file: REPORTED, expected, got: "1;h=sha-256;d=stale" }], - unknown: [], - missing: [], - }); - -describe("repairing a drifted runtime rule, end to end", () => { - let directory: string; - let checkFile: string; - - beforeEach(async () => { - directory = await mkdtemp(join(tmpdir(), "tskl-repair-")); - const rule = join(directory, ".taskless", "rules", "runtime", "demo"); - await mkdir(join(rule, "captures"), { recursive: true }); - await writeFile(join(rule, "captures", "logs.yml"), CAPTURE, "utf8"); - checkFile = join(rule, "check.ts"); - await writeFile(checkFile, DRIFTED, "utf8"); - await writeFile(join(directory, "src.ts"), 'console.log("hi");\n', "utf8"); - await execFileAsync("git", ["init"], { cwd: directory }); - await execFileAsync( - "git", - ["remote", "add", "origin", "https://github.com/acme/widgets.git"], - { cwd: directory } - ); - }); - - afterEach(async () => { - await rm(directory, { recursive: true, force: true }); - }); - - it("reports the repair on the --json envelope, not only on stderr", async () => { - // The regression this test exists for. `--json` is what CI reads, and the - // repair notices used to reach `warn()` alone. - const blessed = await canonicalHash(BLESSED); - const mock = await startMock({ - reconcile: driftedReconcile(blessed), - restore: () => ({ - statusCode: 200, - body: { - ruleId: "demo", - rules: [ - { - id: "demo", - engine: "runtime", - // The COMPLETE set. The schema calls `files` "every file the - // rule directory must contain", and `writeRuleFile` enforces - // that: a runtime rule restored without its captures would be - // written, verify as incomplete, and never fire. - files: [ - { path: "check.ts", content: BLESSED }, - { path: "captures/logs.yml", content: CAPTURE }, - ], - signature: blessed, - }, - ], - }, - }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - - const notices = envelope(stdout).notices ?? []; - expect(notices.join("\n")).toContain(REPORTED); - expect(notices.join("\n")).toMatch(/blessed/); - expect(mock.restoreCalls).toEqual(["demo"]); - - // And the bytes actually landed. - await expect(readFile(checkFile, "utf8")).resolves.toBe(BLESSED); - } finally { - await mock.close(); - } - }); - - it("does not promise blessing when the plan will not run the restored rule", async () => { - // The ordinary notice says the next `check` blesses the repaired bytes. - // Under a plan without runtime signatures it will not, so the restore - // still writes the blessed bytes and withdraws only the promise. - const upgradeUrl = "https://app.taskless.io/o/acme/upgrade?from=restore"; - const blessed = await canonicalHash(BLESSED); - const restoreBody = (entitlement: unknown) => ({ - statusCode: 200, - body: { - ruleId: "demo", - entitlement, - rules: [ - { - id: "demo", - engine: "runtime", - files: [ - { path: "check.ts", content: BLESSED }, - { path: "captures/logs.yml", content: CAPTURE }, - ], - signature: blessed, - }, - ], - }, - }); - - const mock = await startMock({ - reconcile: driftedReconcile(blessed), - restore: () => restoreBody({ runtimeSignatures: false, upgradeUrl }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - const notices = (envelope(stdout).notices ?? []).join("\n"); - expect(notices).toContain(`${REPORTED} was restored`); - expect(notices).toMatch(/will not run/); - expect(notices).toContain(upgradeUrl); - expect(notices).not.toMatch(/next `check`/); - await expect(readFile(checkFile, "utf8")).resolves.toBe(BLESSED); - } finally { - await mock.close(); - } - - // An entitled response keeps the ordinary notice and warns about nothing. - await writeFile(checkFile, DRIFTED, "utf8"); - const entitled = await startMock({ - reconcile: driftedReconcile(blessed), - restore: () => restoreBody({ runtimeSignatures: true }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: entitled.apiUrl, - }); - const notices = (envelope(stdout).notices ?? []).join("\n"); - expect(notices).toMatch(/next `check`/); - expect(notices).not.toMatch(/will not run/); - } finally { - await entitled.close(); - } - }); - - it("leaves the rule directory holding exactly the blessed set", async () => { - // #233. Repair runs BECAUSE the directory's trustworthiness is in - // question, and only `check.ts` is signed — so a stray capture beside the - // rule is never reported by reconcile, was never replaced by the repair, - // and went on changing what the rule matched while the rule read as - // repaired. The delivered set is now the directory. - const rule = join(directory, ".taskless", "rules", "runtime", "demo"); - await writeFile( - join(rule, "captures", "stray.yml"), - CAPTURE.replace("logs-abc12345", "stray-abc12345"), - "utf8" - ); - // A fixture no delivered set will ever name: this CLI writes timestamped - // ones itself. Kept, and the notice says so rather than leaving a reader - // to infer it. - await mkdir(join(rule, ".tests"), { recursive: true }); - await writeFile(join(rule, ".tests", "demo-1970-test.yml"), "id: demo\n"); - - const blessed = await canonicalHash(BLESSED); - const mock = await startMock({ - reconcile: driftedReconcile(blessed), - restore: () => ({ - statusCode: 200, - body: { - ruleId: "demo", - rules: [ - { - id: "demo", - engine: "runtime", - files: [ - { path: "check.ts", content: BLESSED }, - { path: "captures/logs.yml", content: CAPTURE }, - ], - signature: blessed, - }, - ], - }, - }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - - expect(existsSync(join(rule, "captures", "stray.yml"))).toBe(false); - // A served set is the whole directory, fixtures included, so a fixture - // it does not name is stale and goes with the rest. - expect(existsSync(join(rule, ".tests", "demo-1970-test.yml"))).toBe( - false - ); - // The rule is whole afterwards. A repair that leaves it inert would be - // the bug, not a trade-off. - await expect(readFile(checkFile, "utf8")).resolves.toBe(BLESSED); - expect(existsSync(join(rule, "captures", "logs.yml"))).toBe(true); - - const notices = (envelope(stdout).notices ?? []).join("\n"); - expect(notices).toMatch(/was restored/); - expect(notices).toContain(".tests/"); - } finally { - await mock.close(); - } - }); - - it("refuses bytes that are not the ones reconcile blessed, and says so", async () => { - // The service answers consistently — it signs exactly what it sends — and - // sends a NEWER rule than the one we were owed. Nothing is written. - const owed = await canonicalHash(BLESSED); - const newer = "export default async () => [{ file: 'x' }];\n"; - const newerSignature = await canonicalHash(newer); - const mock = await startMock({ - reconcile: driftedReconcile(owed), - restore: () => ({ - statusCode: 200, - body: { - ruleId: "demo", - rules: [ - { - id: "demo", - engine: "runtime", - files: [ - { path: "check.ts", content: newer }, - { path: "captures/logs.yml", content: CAPTURE }, - ], - signature: newerSignature, - }, - ], - }, - }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - const notices = (envelope(stdout).notices ?? []).join("\n"); - expect(notices).toMatch(/was not restored/); - expect(notices).toMatch(/does not upgrade/); - await expect(readFile(checkFile, "utf8")).resolves.toBe(DRIFTED); - } finally { - await mock.close(); - } - }); - - it("explains a rule the service will not return", async () => { - const mock = await startMock({ - reconcile: driftedReconcile(await canonicalHash(BLESSED)), - restore: () => ({ statusCode: 404, body: {} }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - const notices = (envelope(stdout).notices ?? []).join("\n"); - expect(notices).toMatch(/could not be restored/); - // A repair that cannot happen is a notice, never a failed run: the rule - // stays withheld, which is already the safe state. - await expect(readFile(checkFile, "utf8")).resolves.toBe(DRIFTED); - } finally { - await mock.close(); - } - }); - - it("asks for the id the unsafe entry carries, not the one in its path", async () => { - // #240. The entry names `logs-abc12345` while its path's rule directory is - // `demo`, and the entry wins. This is the assertion a path-parsing - // implementation fails: it would ask for `demo`, and after a layout move it - // would ask for something the service never issued, take a 404, and leave - // the rule unrepaired and unexecuted with nothing said on any channel. - const blessed = await canonicalHash(BLESSED); - const mock = await startMock({ - reconcile: driftedReconcile(blessed, "logs-abc12345"), - restore: (ruleId) => ({ - statusCode: 200, - body: { - ruleId, - rules: [ - { - id: ruleId, - engine: "runtime", - files: [ - { path: "check.ts", content: BLESSED }, - { path: "captures/logs.yml", content: CAPTURE }, - ], - signature: blessed, - }, - ], - }, - }), - }); - try { - await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - expect(mock.restoreCalls).toEqual(["logs-abc12345"]); - // And the repair completed, so the entry's id was what reached restore - // rather than the request merely landing somewhere harmless. The bytes - // land under the id's own directory, since the delivered set decides - // where a rule lives and the reported path never did. - await expect( - readFile( - join(directory, ".taskless/rules/runtime/logs-abc12345/check.ts"), - "utf8" - ) - ).resolves.toBe(BLESSED); - } finally { - await mock.close(); - } - }); - - it("says why an unknown file cannot be restored, and asks for nothing", async () => { - const mock = await startMock({ - reconcile: () => ({ - run: [], - unsafe: [], - unknown: [{ file: REPORTED }], - missing: [], - }), - restore: () => ({ statusCode: 500, body: {} }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - const notices = (envelope(stdout).notices ?? []).join("\n"); - expect(notices).toMatch(/was not issued by the rule service/); - // Nothing on the server to ask for, so nothing is asked. - expect(mock.restoreCalls).toEqual([]); - } finally { - await mock.close(); - } - }); - - it("asks for nothing when an unsafe entry carries no rule id, and says why", async () => { - // The deployed schema requires `ruleId` on `unsafe`, and `reconcile` - // decodes with a cast rather than a schema — so a rollback, a canary, or a - // regression can put an entry like this on the wire despite the `string` - // type. Without a guard `restoreRule` interpolates `undefined` and the CLI - // requests `/cli/api/request/undefined/restore`: a call that cannot - // succeed, made for a rule nobody named. - const mock = await startMock({ - reconcile: () => ({ - run: [], - unsafe: [ - { - file: REPORTED, - expected: "1;h=sha-256;d=a", - got: "1;h=sha-256;d=b", - }, - ], - unknown: [], - missing: [], - }), - restore: () => ({ statusCode: 500, body: {} }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - const notices = (envelope(stdout).notices ?? []).join("\n"); - - // The whole point: no request at all, and in particular not the literal - // `undefined` the unguarded path would have sent. - expect(mock.restoreCalls).toEqual([]); - // Not a silent skip. `--json` is what CI reads, so the notice has to be - // here and it has to name the file. - expect(notices).toContain(REPORTED); - expect(notices).toMatch(/did not say which rule it belongs to/); - // Withheld, not repaired: the drifted bytes are untouched, which is - // already the safe state for a rule that could not be verified. - await expect(readFile(checkFile, "utf8")).resolves.toBe(DRIFTED); - } finally { - await mock.close(); - } - }); - - it("says '1 stale entry ... it' when the repair leaves exactly one behind", async () => { - // plan.ts's `repairWithheldRules` wraps `PurgeIncompleteError` in its own - // wording rather than reusing the error's message, and pluralizes it by - // hand — a seam `deliver.test.ts` cannot see, since it only ever throws - // the error, never renders this notice. - const rule = join(directory, ".taskless", "rules", "runtime", "demo"); - await mkdir(join(rule, "blocked"), { recursive: true }); - await writeFile(join(rule, "blocked", "a.txt"), "stuck\n", "utf8"); - await chmod(join(rule, "blocked"), 0o500); - - const blessed = await canonicalHash(BLESSED); - const mock = await startMock({ - reconcile: driftedReconcile(blessed), - restore: () => ({ - statusCode: 200, - body: { - ruleId: "demo", - rules: [ - { - id: "demo", - engine: "runtime", - files: [ - { path: "check.ts", content: BLESSED }, - { path: "captures/logs.yml", content: CAPTURE }, - ], - signature: blessed, - }, - ], - }, - }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - const notices = (envelope(stdout).notices ?? []).join("\n"); - expect(notices).toMatch( - /was rewritten with the bytes the service blessed, but 1 stale entry could not be removed and an engine still reads it/ - ); - expect(notices).toContain("blocked/a.txt"); - // The bytes still landed — a failed cleanup is not a failed write. - await expect(readFile(checkFile, "utf8")).resolves.toBe(BLESSED); - } finally { - await chmod(join(rule, "blocked"), 0o700); - await mock.close(); - } - }); - - it("says ' stale entries ... them' when the repair leaves several behind", async () => { - const rule = join(directory, ".taskless", "rules", "runtime", "demo"); - await mkdir(join(rule, "blocked-one"), { recursive: true }); - await writeFile(join(rule, "blocked-one", "a.txt"), "stuck\n", "utf8"); - await chmod(join(rule, "blocked-one"), 0o500); - await mkdir(join(rule, "blocked-two"), { recursive: true }); - await writeFile(join(rule, "blocked-two", "b.txt"), "stuck\n", "utf8"); - await chmod(join(rule, "blocked-two"), 0o500); - - const blessed = await canonicalHash(BLESSED); - const mock = await startMock({ - reconcile: driftedReconcile(blessed), - restore: () => ({ - statusCode: 200, - body: { - ruleId: "demo", - rules: [ - { - id: "demo", - engine: "runtime", - files: [ - { path: "check.ts", content: BLESSED }, - { path: "captures/logs.yml", content: CAPTURE }, - ], - signature: blessed, - }, - ], - }, - }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - const notices = (envelope(stdout).notices ?? []).join("\n"); - expect(notices).toMatch( - /was rewritten with the bytes the service blessed, but 2 stale entries could not be removed and an engine still reads them/ - ); - expect(notices).toContain("blocked-one/a.txt"); - expect(notices).toContain("blocked-two/b.txt"); - await expect(readFile(checkFile, "utf8")).resolves.toBe(BLESSED); - } finally { - await chmod(join(rule, "blocked-one"), 0o700); - await chmod(join(rule, "blocked-two"), 0o700); - await mock.close(); - } - }); - - it("skips every runtime rule when reconcile's own authentication is rejected", async () => { - // `planRuntime`'s `unauthorized` branch (plan.ts) — distinct from - // restore's `unauthorized`, and reachable only when the reconcile call - // itself, not the later restore call, gets a 401. - const mock = await startMock({ - reconcile: () => ({ statusCode: 401, body: {} }), - restore: () => ({ statusCode: 500, body: {} }), - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }); - const output = envelope(stdout) as { - skipped?: { rule: string; reason: string }[]; - }; - expect(output.skipped).toHaveLength(1); - expect(output.skipped?.[0]?.rule).toBe("demo"); - expect(output.skipped?.[0]?.reason).toContain( - "authentication was rejected" - ); - // Rejected before any restore could even be considered. - expect(mock.restoreCalls).toEqual([]); - await expect(readFile(checkFile, "utf8")).resolves.toBe(DRIFTED); - } finally { - await mock.close(); - } - }); - - it("skips every runtime rule when materialization fails, without failing the run", async () => { - // plan.ts's materialize-failure catch. Blessing whatever `check.ts` was - // actually reported sidesteps needing to know `signRuleFile`'s exact - // envelope format — the point of this test is the catch, not the sign. - // - // `.taskless/.gitignore` is replaced with a directory so `addToGitignore`'s - // `writeFile` (called from `materializeRuntimeRules`) fails with EISDIR. - // The scaffold migration (`migrateFixture`, inside `runCli`) writes that - // file itself as part of bringing the fixture current, so the block has to - // go on AFTER migration — doing it first makes the migration itself throw, - // before `check` ever runs. - await migrateFixture(["check", "-d", directory]); - await rm(join(directory, ".taskless", ".gitignore"), { force: true }); - await mkdir(join(directory, ".taskless", ".gitignore")); - - const mock = await startMock({ - reconcile: (body) => ({ - run: body.files.map((file) => ({ - ruleId: "demo", - file: file.file, - signature: file.signature, - })), - unsafe: [], - unknown: [], - missing: [], - }), - restore: () => ({ statusCode: 500, body: {} }), - }); - try { - let stdout: string; - let exitCode: number; - try { - ({ stdout } = await execFileAsync( - "node", - [binPath, "check", "-d", directory, "--json"], - { - env: { - ...process.env, - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: mock.apiUrl, - }, - } - )); - exitCode = 0; - } catch (error) { - const failure = error as { stdout: string; code: number }; - stdout = failure.stdout ?? ""; - exitCode = failure.code; - } - // A materialize failure is a notice, never a failed run. - expect(exitCode).toBe(0); - const output = envelope(stdout) as { - skipped?: { rule: string; reason: string }[]; - }; - expect(output.skipped).toHaveLength(1); - expect(output.skipped?.[0]?.rule).toBe("demo"); - expect(output.skipped?.[0]?.reason).toContain( - "runtime rules could not be materialized" - ); - expect(mock.restoreCalls).toEqual([]); - } finally { - await mock.close(); - } - }); -}); - -describe("restoreRule's HTTP contract", () => { - let server: Server; - let originalApiUrl: string | undefined; - let respond: (response: import("node:http").ServerResponse) => void; - - beforeEach(async () => { - originalApiUrl = process.env.TASKLESS_API_URL; - server = createServer((request, response) => { - request.resume(); - request.on("end", () => respond(response)); - }); - await new Promise((done) => { - server.listen(0, "127.0.0.1", () => { - const address = server.address(); - const port = typeof address === "object" && address ? address.port : 0; - process.env.TASKLESS_API_URL = `http://127.0.0.1:${String(port)}/cli`; - done(); - }); - }); - }); - - afterEach(async () => { - await new Promise((done) => server.close(() => done())); - if (originalApiUrl === undefined) { - delete process.env.TASKLESS_API_URL; - } else { - process.env.TASKLESS_API_URL = originalApiUrl; - } - }); - - const request = { - ruleId: "demo", - repositoryUrl: "https://github.com/acme/widgets.git", - }; - - it("maps HTTP 401 to `unauthorized`", async () => { - respond = (response) => { - response.writeHead(401, { "content-type": "application/json" }); - response.end("{}"); - }; - await expect(restoreRule("token", request)).resolves.toEqual({ - status: "unauthorized", - }); - }); - - it("maps a non-ok status to `unavailable`", async () => { - respond = (response) => { - response.writeHead(500, { "content-type": "application/json" }); - response.end("{}"); - }; - await expect(restoreRule("token", request)).resolves.toEqual({ - status: "unavailable", - reason: "HTTP 500", - }); - }); - - it("maps a body `response.json()` cannot parse to `unavailable`", async () => { - respond = (response) => { - response.writeHead(200, { "content-type": "application/json" }); - response.end("not json"); - }; - await expect(restoreRule("token", request)).resolves.toEqual({ - status: "unavailable", - reason: "invalid response body", - }); - }); - - it("maps a body without an array `rules` to `unavailable`", async () => { - respond = (response) => { - response.writeHead(200, { "content-type": "application/json" }); - response.end(JSON.stringify({ ruleId: "demo" })); - }; - await expect(restoreRule("token", request)).resolves.toEqual({ - status: "unavailable", - reason: "response carried no `rules`", - }); - }); - - it("maps a fetch that throws to `unavailable`", async () => { - // No handler is registered on the running mock server for this one — the - // request is pointed at a closed local port instead, so `fetch` itself - // rejects (ECONNREFUSED) rather than the server answering. - process.env.TASKLESS_API_URL = "http://127.0.0.1:1/cli"; - const outcome = await restoreRule("token", request); - expect(outcome.status).toBe("unavailable"); - expect( - (outcome as { status: "unavailable"; reason: string }).reason - ).toMatch(/network error/); - }); -}); diff --git a/packages/cli/test/repair.test.ts b/packages/cli/test/repair.test.ts deleted file mode 100644 index a97621d4..00000000 --- a/packages/cli/test/repair.test.ts +++ /dev/null @@ -1,265 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { canonicalHash } from "../src/rules/rule-hash"; -import { - repairTargets, - verifyRestoredCheck, - type RepairTarget, -} from "../src/rules/runtime/repair"; -import type { RestoredRule } from "../src/api/restore"; - -const CHECK = "export default async () => [];\n"; -const TAMPERED = "export default async () => [{ file: 'x' }];\n"; - -/** A restored runtime rule carrying `check.ts` and a signature over it. */ -async function restored( - content: string, - signature?: string -): Promise { - return { - id: "logs-abc12345", - engine: "runtime", - files: [{ path: "check.ts", content }], - signature: signature ?? (await canonicalHash(content)), - } as unknown as RestoredRule; -} - -describe("which reconcile verdicts can be repaired", () => { - it("routes unsafe and missing, and never unknown", () => { - // `unknown` is not in the input type at all, and that is the point: there - // is nothing on the server to fetch for a file it never issued, which is - // why its entry carries no rule id to fetch it by. - const { targets } = repairTargets({ - unsafe: [ - { - ruleId: "logs-abc12345", - file: ".taskless/rules/runtime/logs-abc12345/check.ts", - expected: "1;h=sha-256;d=aaa", - got: "1;h=sha-256;d=bbb", - }, - ], - missing: [ - { - ruleId: "evals-def67890", - file: ".taskless/rules/runtime/x/check.ts", - }, - ], - }); - - expect(targets).toHaveLength(2); - expect(targets[0]?.ruleId).toBe("logs-abc12345"); - expect(targets[0]?.expected).toBe("1;h=sha-256;d=aaa"); - // A missing rule is one we do not hold, so there is no prior expectation. - expect(targets[1]?.ruleId).toBe("evals-def67890"); - expect(targets[1]?.expected).toBeUndefined(); - }); - - it.each([["unsafe" as const], ["missing" as const]])( - "takes a %s entry's id rather than parsing its path", - (bucket) => { - // The path parses to `x`; the entry says `evals-def67890`. The entry wins, - // and nothing reads the path at all. A path-parsing implementation asks - // restore for `x`, gets a 404, and leaves the rule unrepaired in silence, - // which is the failure this assertion exists to catch. - const entry = { - ruleId: "evals-def67890", - file: ".taskless/rules/runtime/x/check.ts", - }; - const { targets } = repairTargets({ - unsafe: - bucket === "unsafe" - ? [{ ...entry, expected: "1;h=sha-256;d=aaa", got: "b" }] - : [], - missing: bucket === "missing" ? [entry] : [], - }); - expect(targets).toHaveLength(1); - expect(targets[0]?.ruleId).toBe("evals-def67890"); - } - ); - - it("identifies an unsafe entry whose file sits outside the rules tree", () => { - // Nothing about the reported path decides anything now, so a rule held - // somewhere the layout does not describe is still repairable. Under path - // parsing this entry was unidentifiable and dropped. - const { targets } = repairTargets({ - unsafe: [ - { - ruleId: "logs-abc12345", - file: "src/check.ts", - expected: "1;h=sha-256;d=a", - got: "b", - }, - ], - missing: [], - }); - expect(targets).toHaveLength(1); - expect(targets[0]?.ruleId).toBe("logs-abc12345"); - }); - - // The published schema requires `ruleId` on both repairable verdicts, and - // `reconcile` decodes with a cast rather than a schema, so the `string` type - // is what the service PROMISES rather than what arrived. A rollback or a - // canary that breaks that promise must not become a request: without the - // guard `restoreRule` is handed `undefined` and asks the service for - // `/cli/api/request/undefined/restore`, which cannot succeed and says - // nothing when it fails. - describe.each([["unsafe" as const], ["missing" as const]])( - "a %s entry that names no rule", - (bucket) => { - const plan = (ruleId?: unknown) => { - const entry = { ruleId, file: `.taskless/rules/runtime/x/check.ts` }; - return repairTargets({ - unsafe: - bucket === "unsafe" - ? [{ ...entry, expected: "1;h=sha-256;d=a", got: "b" }] - : [], - missing: bucket === "missing" ? [entry] : [], - } as unknown as Parameters[0]); - }; - - it.each([ - ["absent", undefined], - ["empty", ""], - ["whitespace", " "], - ])("is not requested when its id is %s", (_label, ruleId) => { - const { targets, unidentified } = plan(ruleId); - // Nothing to ask for: no target means no request built from a rule id - // nobody supplied. - expect(targets).toEqual([]); - // And it is said out loud, so the skip is not a silent pass. - expect(unidentified).toEqual([ - { file: ".taskless/rules/runtime/x/check.ts" }, - ]); - }); - - it("is not rescued by parsing its path", () => { - // The path contains `x`, which the deleted fallback would have taken. - // The id is reported as absent, not invented. - const { targets } = plan(); - expect(targets).toEqual([]); - }); - } - ); - - it("keeps identified entries when another names no rule", () => { - // One bad entry withholds itself, not the rules beside it. - const { targets, unidentified } = repairTargets({ - unsafe: [ - { - ruleId: "logs-abc12345", - file: ".taskless/rules/runtime/logs-abc12345/check.ts", - expected: "1;h=sha-256;d=a", - got: "b", - }, - { file: "src/orphan.ts", expected: "1;h=sha-256;d=c", got: "d" }, - ], - missing: [{ ruleId: "evals-def67890", file: "src/present.ts" }], - } as unknown as Parameters[0]); - - expect(targets.map((target) => target.ruleId)).toEqual([ - "logs-abc12345", - "evals-def67890", - ]); - expect(unidentified).toEqual([{ file: "src/orphan.ts" }]); - }); -}); - -describe("verifying restored bytes", () => { - const unsafeTarget = async (): Promise => ({ - ruleId: "logs-abc12345", - file: ".taskless/rules/runtime/logs-abc12345/check.ts", - expected: await canonicalHash(CHECK), - }); - - it("accepts the bytes reconcile blessed", async () => { - const verdict = await verifyRestoredCheck( - await unsafeTarget(), - await restored(CHECK) - ); - expect(verdict.ok).toBe(true); - }); - - /** - * The property task 6.4 asks for, and the reason `expected` is checked at - * all: restore repairs, it does not upgrade. - */ - it("refuses bytes that are newer than the ones reconcile blessed", async () => { - // Internally consistent — the service signed exactly what it sent — and - // still refused, because it is not what we were owed. Verifying only the - // response against itself would install this silently, mid-`check`. - const newer = await restored(TAMPERED); - const verdict = await verifyRestoredCheck(await unsafeTarget(), newer); - - expect(verdict.ok).toBe(false); - expect(verdict.ok === false && verdict.reason).toMatch( - /not the ones reconcile blessed/ - ); - expect(verdict.ok === false && verdict.reason).toMatch(/does not upgrade/); - }); - - it("refuses bytes that do not match the signature sent with them", async () => { - // A corrupted or substituted transfer: the claim and the content disagree. - const inconsistent = await restored(TAMPERED, await canonicalHash(CHECK)); - const verdict = await verifyRestoredCheck( - await unsafeTarget(), - inconsistent - ); - - expect(verdict.ok).toBe(false); - expect(verdict.ok === false && verdict.reason).toMatch( - /do not match the signature/ - ); - }); - - it("refuses a runtime rule the service returned without a signature", async () => { - // The published schema requires it, so this means the service broke its - // own contract. Writing unvouched-for bytes is not the way to find out. - const unsigned = { - id: "logs-abc12345", - engine: "runtime", - files: [{ path: "check.ts", content: CHECK }], - } as unknown as RestoredRule; - - const verdict = await verifyRestoredCheck(await unsafeTarget(), unsigned); - expect(verdict.ok).toBe(false); - expect(verdict.ok === false && verdict.reason).toMatch(/no signature/); - }); - - it.each([ - ["no check.ts", []], - [ - "two check.ts entries", - [ - { path: "check.ts", content: CHECK }, - { path: "check.ts", content: TAMPERED }, - ], - ], - ])("refuses a restored set with %s", async (_label, files) => { - const rule = { - id: "logs-abc12345", - engine: "runtime", - files, - signature: await canonicalHash(CHECK), - } as unknown as RestoredRule; - - const verdict = await verifyRestoredCheck(await unsafeTarget(), rule); - expect(verdict.ok).toBe(false); - expect(verdict.ok === false && verdict.reason).toMatch( - /single `check\.ts`/ - ); - }); - - it("falls back to the response signature for a missing rule", async () => { - // Nothing local to disagree with, so the weaker check is the only one - // available. Stated in the module rather than left as an inconsistency. - const verdict = await verifyRestoredCheck( - { - ruleId: "evals-def67890", - file: ".taskless/rules/runtime/evals-def67890/check.ts", - expected: undefined, - }, - await restored(TAMPERED) - ); - expect(verdict.ok).toBe(true); - }); -}); diff --git a/packages/cli/test/runtime-check.test.ts b/packages/cli/test/runtime-check.test.ts index 167e9cbe..18f9a0a0 100644 --- a/packages/cli/test/runtime-check.test.ts +++ b/packages/cli/test/runtime-check.test.ts @@ -1,6 +1,14 @@ import { execFile } from "node:child_process"; +import { createHash } from "node:crypto"; import { createServer, type Server } from "node:http"; -import { mkdtemp, rm, mkdir, writeFile } from "node:fs/promises"; +import { + mkdtemp, + readdir, + readFile, + rm, + mkdir, + writeFile, +} from "node:fs/promises"; import { resolve, join } from "node:path"; import { tmpdir } from "node:os"; import { promisify } from "node:util"; @@ -10,13 +18,13 @@ import { migrateFixture } from "./support/current-project"; const execFileAsync = promisify(execFile); const binPath = resolve(import.meta.dirname, "../dist/index.js"); -interface ReportedFile { - file: string; - signature: string; +interface ReportedRule { + ruleId: string; + files: { path: string; signature: string }[]; } interface ReconcileRequestBody { repositoryUrl: string; - files: ReportedFile[]; + rules: ReportedRule[]; } type Responder = (request: ReconcileRequestBody) => { statusCode: number; @@ -25,16 +33,20 @@ type Responder = (request: ReconcileRequestBody) => { interface MockServer { apiUrl: string; requests: ReconcileRequestBody[]; + /** Every path requested, so a test can assert nothing but reconcile was called. */ + paths: string[]; headers: Record[]; close: () => Promise; } -/** Start a mock reconcile endpoint on a random port. */ +/** Start a mock v2 reconcile endpoint on a random port. */ function startMockServer(responder: Responder): Promise { const requests: ReconcileRequestBody[] = []; + const paths: string[] = []; const headers: Record[] = []; const server: Server = createServer((request, response) => { - if (request.method !== "POST" || request.url !== "/cli/api/reconcile") { + paths.push(`${request.method ?? ""} ${request.url ?? ""}`); + if (request.method !== "POST" || request.url !== "/cli/api/v2/reconcile") { response.writeHead(404).end("{}"); return; } @@ -56,6 +68,7 @@ function startMockServer(responder: Responder): Promise { resolvePromise({ apiUrl: `http://127.0.0.1:${String(port)}/cli`, requests, + paths, headers, close: () => new Promise((done) => server.close(() => done())), }); @@ -63,9 +76,105 @@ function startMockServer(responder: Responder): Promise { }); } -/** Echo a reported file's signature back so the mock can bless it. */ -function sig(request: ReconcileRequestBody, endsWith: string): string { - return request.files.find((f) => f.file.endsWith(endsWith))?.signature ?? ""; +/** + * Every call except the identity lookup, which resolves the org subject and is + * not a data route: the assertion is that nothing but reconcile touched rules. + */ +function dataCalls(server: MockServer): string[] { + return server.paths.filter((path) => !path.endsWith("/whoami")); +} + +const ENGINES: Record = { + "no-console": "sg", + demo: "runtime", + other: "runtime", + broken: "runtime", +}; + +type Answer = + | "run" + | "unknown" + | "withheld" + | "omit" + | { unsafe: { path: string; expected?: string; got?: string }[] }; + +const UPGRADE_URL = "https://app.taskless.io/o/acme/upgrade?from=reconcile"; + +/** + * A v2 reconcile answer: each reported rule gets the verdict \`answers\` names, + * \`unknown\` by default. \`withheld\` rules make the organization unentitled. + */ +function answer( + request: ReconcileRequestBody, + answers: Record = {}, + extra: { + missing?: { ruleId: string; engine: string; revisionId: string }[]; + } = {} +) { + const rules: unknown[] = []; + const unknown: { ruleId: string }[] = []; + const withheld: { ruleId: string; revisionId: string }[] = []; + for (const { ruleId } of request.rules) { + const verdict = answers[ruleId] ?? "unknown"; + const engine = ENGINES[ruleId] ?? "sg"; + switch (verdict) { + case "omit": { + break; + } + case "unknown": { + unknown.push({ ruleId }); + break; + } + case "withheld": { + withheld.push({ ruleId, revisionId: "rev-1" }); + break; + } + case "run": { + rules.push({ ruleId, engine, verdict: "run", revisionId: "rev-1" }); + break; + } + default: { + rules.push({ + ruleId, + engine, + verdict: "unsafe", + files: verdict.unsafe, + }); + } + } + } + for (const missing of extra.missing ?? []) { + rules.push({ ...missing, verdict: "missing" }); + } + return { + rules, + unknown, + entitlement: + withheld.length === 0 + ? { runtimeSignatures: true } + : { + runtimeSignatures: false, + reason: "RUNTIME_SIGNATURES_NOT_IN_PLAN", + upgradeUrl: UPGRADE_URL, + withheld, + }, + }; +} + +/** A digest of every file under \`.taskless/rules/\`, to prove \`check\` wrote nothing there. */ +async function treeDigest(directory: string): Promise { + const root = join(directory, ".taskless", "rules"); + const hash = createHash("sha256"); + const entries = await readdir(root, { recursive: true, withFileTypes: true }); + const files = entries + .filter((entry) => entry.isFile()) + .map((entry) => join(entry.parentPath, entry.name)) + .toSorted(); + for (const file of files) { + hash.update(file); + hash.update(await readFile(file)); + } + return hash.digest("hex"); } async function runCli( @@ -90,20 +199,33 @@ async function runCli( } /** The check `--json` line, ignoring any preceding migration output. */ -function parseJson(stdout: string): { +interface CheckJson { success: boolean; results: { source: string; ruleId: string }[]; skipped?: { rule: string; reason: string }[]; -} { + failures?: string[]; + notices?: string[]; + integrity?: { + ruleId: string; + engine?: string; + verdict: string; + files?: unknown[]; + revisionId?: string; + }[]; + entitlement?: { + runtimeSignatures: false; + reason?: string; + upgradeUrl?: string; + withheld: string[]; + }; +} + +function parseJson(stdout: string): CheckJson { const line = stdout .trim() .split("\n") .findLast((l) => l.trim().startsWith("{")); - return JSON.parse(line ?? "{}") as { - success: boolean; - results: { source: string; ruleId: string }[]; - skipped?: { rule: string; reason: string }[]; - }; + return JSON.parse(line ?? "{}") as CheckJson; } const STATIC_RULE = [ @@ -136,40 +258,6 @@ const RUNTIME_CHECK = `export default async function (root, matches) { } `; -const CHECK_REPORT_PATH = ".taskless/runtime/rules/demo/check.ts"; - -const UPGRADE_URL = "https://app.taskless.io/o/acme/upgrade?from=reconcile"; - -/** The `--json` line with the entitlement field this suite asserts on. */ -function parseEntitlementJson(stdout: string): ReturnType & { - entitlement?: { - runtimeSignatures: false; - reason?: string; - upgradeUrl?: string; - withheld: string[]; - }; -} { - return parseJson(stdout) as ReturnType; -} - -/** A reconcile body withholding the reported files ending in `endsWith`. */ -function withholding(request: ReconcileRequestBody, ...endsWith: string[]) { - return { - run: [], - unsafe: [], - unknown: [], - missing: [], - entitlement: { - runtimeSignatures: false, - reason: "RUNTIME_SIGNATURES_NOT_IN_PLAN", - upgradeUrl: UPGRADE_URL, - withheld: request.files - .filter((f) => endsWith.some((suffix) => f.file.endsWith(suffix))) - .map((f) => ({ ruleId: "r", file: f.file })), - }, - }; -} - describe("check: static vs runtime dispatch", () => { let directory: string; @@ -218,384 +306,369 @@ describe("check: static vs runtime dispatch", () => { expect(stdout).toContain("no-console"); }); - it("authed + blessed check.ts: runtime runs; only check.ts is reported", async () => { - const server = await startMockServer((request) => ({ - statusCode: 200, - body: { - run: [ - { - ruleId: "demo", - file: CHECK_REPORT_PATH, - signature: sig(request, "check.ts"), - }, - ], - unsafe: [], - unknown: [], - missing: [], - }, - })); + /** Run \`check\` authenticated against a mock that answers with \`responder\`. */ + async function authedCheck( + responder: Responder, + extraArguments: string[] = ["--json"] + ) { + const server = await startMockServer(responder); try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: server.apiUrl, - }); - // Only the runtime check.ts is reported — never the static rule. - expect(server.requests).toHaveLength(1); - expect(server.requests[0]!.files).toHaveLength(1); - expect(server.requests[0]!.files[0]!.file.endsWith("check.ts")).toBe( - true - ); - const output = parseJson(stdout); - expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( - true + const result = await runCli( + ["check", "-d", directory, ...extraArguments], + { + TASKLESS_TOKEN: "fake.token", + TASKLESS_API_URL: server.apiUrl, + } ); - expect(output.results.some((r) => r.ruleId === "no-console")).toBe(true); + return { ...result, server }; } finally { await server.close(); } - }); + } - it("authed + empty run set: runtime withheld, static still runs", async () => { - const server = await startMockServer(() => ({ + it("reports every rule directory of every engine, files relative, fixtures excluded", async () => { + await migrateFixture(["-d", directory]); + const fixtures = join( + directory, + ".taskless", + "rules", + "runtime", + "demo", + ".tests" + ); + await mkdir(fixtures, { recursive: true }); + await writeFile(join(fixtures, "case.ts"), "console.log(1);\n"); + const { server } = await authedCheck((request) => ({ statusCode: 200, - body: { run: [], unsafe: [], unknown: [], missing: [] }, + body: answer(request, { "no-console": "run", demo: "run" }), })); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: server.apiUrl, - }); - const output = parseJson(stdout); - expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( - false - ); - expect(output.results.some((r) => r.ruleId === "no-console")).toBe(true); - expect(output.skipped?.some((s) => s.rule === "demo")).toBe(true); - } finally { - await server.close(); - } - }); - - it("reconcile unavailable: runtime skipped, static runs, exit 0", async () => { - const server = await startMockServer(() => ({ statusCode: 503 })); - try { - const { stdout, exitCode } = await runCli( - ["check", "-d", directory, "--json"], - { TASKLESS_TOKEN: "fake.token", TASKLESS_API_URL: server.apiUrl } - ); - const output = parseJson(stdout); - expect(exitCode).toBe(0); - expect(output.results.some((r) => r.ruleId === "no-console")).toBe(true); - expect(output.skipped?.some((s) => s.rule === "demo")).toBe(true); - } finally { - await server.close(); + expect(dataCalls(server)).toEqual(["POST /cli/api/v2/reconcile"]); + const rules = server.requests[0]!.rules; + const byId = Object.fromEntries( + rules.map((rule) => [rule.ruleId, rule.files.map((file) => file.path)]) + ); + expect(byId).toEqual({ + "no-console": ["no-console.yml"], + demo: ["captures/logs.yml", "check.ts"], + }); + for (const rule of rules) { + for (const file of rule.files) { + expect(file.signature).toMatch(/^1;h=sha-256;d=[0-9a-f]{64}$/); + } } + const version = server.headers[0]?.["x-taskless-cli-version"]; + expect(typeof version).toBe("string"); + expect(version).not.toBe(""); }); - it("--anonymous with a token: skips runtime and never calls reconcile", async () => { - const server = await startMockServer(() => ({ + it("run for every rule: runtime executes and static runs, from the snapshot", async () => { + const { stdout, exitCode } = await authedCheck((request) => ({ statusCode: 200, - body: { run: [], unsafe: [], unknown: [], missing: [] }, + body: answer(request, { "no-console": "run", demo: "run" }), })); - try { - const { stdout } = await runCli( - ["check", "-d", directory, "--json", "--anonymous"], - { TASKLESS_TOKEN: "fake.token", TASKLESS_API_URL: server.apiUrl } - ); - expect(server.requests).toHaveLength(0); - const output = parseJson(stdout); - expect(output.skipped?.some((s) => s.rule === "demo")).toBe(true); - } finally { - await server.close(); - } + const output = parseJson(stdout); + expect(exitCode).toBe(0); + expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( + true + ); + expect(output.results.some((r) => r.ruleId === "no-console")).toBe(true); + expect(output).not.toHaveProperty("integrity"); }); - it("--dangerously-run-scripts: runs runtime offline behind a warning", async () => { - const { stdout, stderr } = await runCli([ - "check", - "-d", - directory, - "--dangerously-run-scripts", - ]); - // The warning is a runtime PLAN notice, and plan notices used to be - // printed by their own loop with no marker at all while the dispatched - // ones were marked — so the same message looked like two different kinds - // of thing depending on which list it arrived on, and `--json` mixed both - // into one `notices` array. Every notice `check` prints is marked now. - const warningLines = stderr - .split("\n") - .filter((line) => line.includes("dangerously-run-scripts")); - expect(warningLines.length).toBeGreaterThan(0); - for (const line of warningLines) { - expect(line.startsWith("Notice: ")).toBe(true); - } - expect(stdout).toContain("demo"); // runtime finding surfaced + it("unknown: a local static rule runs silently, a local runtime rule does not execute", async () => { + const { stdout, stderr, exitCode } = await authedCheck( + (request) => ({ statusCode: 200, body: answer(request) }), + [] + ); + expect(exitCode).toBe(0); + expect(stdout).toContain("no-console"); + expect(stderr).not.toMatch(/no-console/); + expect(stderr).toMatch( + /runtime rule demo was not run — not issued by the rule service/ + ); }); - it("a runtime rule missing check.ts is skipped, not fatal; static still runs", async () => { - // A malformed rule (capture yml, no check.ts) must not abort the whole check. - const broken = join(directory, ".taskless", "runtime", "rules", "broken"); - await mkdir(broken, { recursive: true }); - await writeFile(join(broken, "logs.yml"), RUNTIME_CAPTURE, "utf8"); - - const server = await startMockServer((request) => ({ + it("an edited static rule does not run, fails the run, and names restore", async () => { + await migrateFixture(["-d", directory]); + const before = await treeDigest(directory); + const { stdout, exitCode } = await authedCheck((request) => ({ statusCode: 200, - body: { - run: [ + body: answer(request, { + demo: "run", + "no-console": { + unsafe: [ + { + path: "no-console.yml", + expected: "1;h=sha-256;d=00", + got: "1;h=sha-256;d=11", + }, + ], + }, + }), + })); + const output = parseJson(stdout); + expect(exitCode).toBe(1); + expect(output.success).toBe(false); + expect(output.results.some((r) => r.ruleId === "no-console")).toBe(false); + expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( + true + ); + expect(output.failures?.join("\n")).toMatch( + /sg rule no-console was edited .*changed no-console\.yml.*rule restore no-console/ + ); + expect(output.integrity).toEqual([ + { + ruleId: "no-console", + engine: "sg", + verdict: "unsafe", + files: [ { - ruleId: "demo", - file: CHECK_REPORT_PATH, - signature: sig(request, "check.ts"), + path: "no-console.yml", + expected: "1;h=sha-256;d=00", + got: "1;h=sha-256;d=11", }, ], - unsafe: [], - unknown: [], - missing: [], }, - })); - try { - const { stdout, exitCode } = await runCli( - ["check", "-d", directory, "--json"], - { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: server.apiUrl, - } - ); - const output = parseJson(stdout); - expect(exitCode).toBe(0); // not SCAN_FAILED - // The good runtime rule still ran and static still ran. - expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( - true - ); - expect(output.results.some((r) => r.ruleId === "no-console")).toBe(true); - // The broken rule is reported as skipped, not crashed. - expect(output.skipped?.some((s) => s.rule === "broken")).toBe(true); - // Only the readable check.ts was reported to the server. - expect(server.requests[0]!.files).toHaveLength(1); - } finally { - await server.close(); - } + ]); + // `check` never writes the rules tree, even to repair. + expect(await treeDigest(directory)).toBe(before); }); - it("withheld for the plan: fails the run and names the cause, not drift", async () => { - const server = await startMockServer((request) => ({ + it("an edited runtime rule does not execute and does not fail the run", async () => { + const { stdout, exitCode } = await authedCheck((request) => ({ statusCode: 200, - body: withholding(request, "demo/check.ts"), + body: answer(request, { + "no-console": "run", + demo: { + unsafe: [{ path: "captures/extra.yml", got: "1;h=sha-256;d=22" }], + }, + }), })); - try { - const { stdout, exitCode } = await runCli( - ["check", "-d", directory, "--json"], - { TASKLESS_TOKEN: "fake.token", TASKLESS_API_URL: server.apiUrl } - ); - const output = parseEntitlementJson(stdout); - // The only finding is a warning, so this is the withhold alone failing. - expect(exitCode).toBe(1); - expect(output.success).toBe(false); - expect(output.results.some((r) => r.ruleId === "no-console")).toBe(true); - expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( - false - ); - const skip = output.skipped?.find((s) => s.rule === "demo"); - expect(skip?.reason).toBe("not included in your Taskless plan"); - expect(skip?.reason).not.toMatch(/unsafe|unknown|drift/); - expect(output.entitlement).toEqual({ - runtimeSignatures: false, - reason: "RUNTIME_SIGNATURES_NOT_IN_PLAN", - upgradeUrl: UPGRADE_URL, - withheld: ["demo"], - }); - } finally { - await server.close(); - } + const output = parseJson(stdout); + expect(exitCode).toBe(0); + expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( + false + ); + expect(output.skipped?.find((s) => s.rule === "demo")?.reason).toMatch( + /edited .*added captures\/extra\.yml/ + ); + expect(output.notices?.join("\n")).toMatch(/rule restore demo/); }); - it("withheld for the plan, human output: one notice carries the upgrade URL", async () => { - const server = await startMockServer((request) => ({ + it("missing warns, names restore, and fetches nothing", async () => { + const { stdout, exitCode, server } = await authedCheck((request) => ({ statusCode: 200, - body: withholding(request, "demo/check.ts"), + body: answer( + request, + { "no-console": "run", demo: "run" }, + { + missing: [ + { ruleId: "gone-3fa9c21b", engine: "vale", revisionId: "rev-9" }, + ], + } + ), })); - try { - const { stderr, exitCode } = await runCli(["check", "-d", directory], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: server.apiUrl, - }); - expect(exitCode).toBe(1); - expect(stderr.split(UPGRADE_URL)).toHaveLength(2); - expect(stderr).toContain("RUNTIME_SIGNATURES_NOT_IN_PLAN"); - } finally { - await server.close(); - } + const output = parseJson(stdout); + expect(exitCode).toBe(0); + expect(dataCalls(server)).toEqual(["POST /cli/api/v2/reconcile"]); + expect(output.integrity).toEqual([ + { + ruleId: "gone-3fa9c21b", + engine: "vale", + verdict: "missing", + revisionId: "rev-9", + }, + ]); + expect(output.notices?.join("\n")).toMatch(/rule restore gone-3fa9c21b/); }); - it("blessed and withheld together: the blessed rule runs and the run still fails", async () => { - const other = join(directory, ".taskless", "runtime", "rules", "other"); - await mkdir(other, { recursive: true }); - await writeFile(join(other, "logs.yml"), RUNTIME_CAPTURE, "utf8"); - await writeFile(join(other, "check.ts"), RUNTIME_CHECK + "// other\n"); - - const server = await startMockServer((request) => ({ + it("a reported rule the answer does not account for does not run and fails the run", async () => { + const { stdout, exitCode } = await authedCheck((request) => ({ statusCode: 200, - body: { - ...withholding(request, "other/check.ts"), - run: [ - { - ruleId: "demo", - file: CHECK_REPORT_PATH, - signature: sig(request, "demo/check.ts"), - }, - ], - }, + body: answer(request, { "no-console": "run", demo: "omit" }), })); - try { - const { stdout, exitCode } = await runCli( - ["check", "-d", directory, "--json"], - { TASKLESS_TOKEN: "fake.token", TASKLESS_API_URL: server.apiUrl } - ); - const output = parseEntitlementJson(stdout); - expect(exitCode).toBe(1); - expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( - true - ); - expect(output.entitlement?.withheld).toEqual(["other"]); - } finally { - await server.close(); - } + const output = parseJson(stdout); + expect(exitCode).toBe(1); + expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( + false + ); + expect(output.integrity).toEqual([ + { ruleId: "demo", engine: "runtime", verdict: "unaccounted" }, + ]); }); - it("a withheld file matching no local rule still fails, named by its path", async () => { - const server = await startMockServer(() => ({ + it("an id shared across engines stops the run before reconcile", async () => { + const decoy = join(directory, ".taskless", "rules", "sg", "demo"); + await migrateFixture(["-d", directory]); + await mkdir(decoy, { recursive: true }); + await writeFile( + join(decoy, "demo.yml"), + STATIC_RULE.replace("no-console", "demo") + ); + const { stdout, exitCode, server } = await authedCheck((request) => ({ statusCode: 200, - body: { - run: [], - unsafe: [], - unknown: [], - missing: [], - entitlement: { - runtimeSignatures: false, - withheld: [{ ruleId: "r", file: "elsewhere/check.ts" }], - }, - }, + body: answer(request, { "no-console": "run", demo: "run" }), })); - try { - const { stdout, exitCode } = await runCli( - ["check", "-d", directory, "--json"], - { TASKLESS_TOKEN: "fake.token", TASKLESS_API_URL: server.apiUrl } - ); - const output = parseEntitlementJson(stdout); - expect(exitCode).toBe(1); - expect(output.entitlement?.withheld).toEqual(["elsewhere/check.ts"]); - // `demo` was reported and not withheld, so it keeps the ordinary reason. - expect(output.skipped?.find((s) => s.rule === "demo")?.reason).toMatch( - /not blessed/ - ); - } finally { - await server.close(); - } + const output = parseJson(stdout); + expect(exitCode).toBe(1); + expect(server.requests).toHaveLength(0); + expect(output.results.some((r) => r.ruleId === "demo")).toBe(false); + expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( + false + ); + expect(output.failures?.join("\n")).toMatch( + /\.taskless\/rules\/sg\/demo\/.*\.taskless\/rules\/runtime\/demo\// + ); + }); + + it("reconcile unavailable: runtime skipped, static runs, exit 0, and it says so", async () => { + const { stdout, exitCode } = await authedCheck(() => ({ statusCode: 503 })); + const output = parseJson(stdout); + expect(exitCode).toBe(0); + expect(output.results.some((r) => r.ruleId === "no-console")).toBe(true); + expect(output.skipped?.some((s) => s.rule === "demo")).toBe(true); + expect(output.notices?.join("\n")).toMatch( + /verification could not be performed/ + ); + }); + + it("--anonymous with a token: skips runtime and never calls reconcile", async () => { + const { stdout, server } = await authedCheck( + (request) => ({ statusCode: 200, body: answer(request) }), + ["--json", "--anonymous"] + ); + expect(dataCalls(server)).toHaveLength(0); + expect(parseJson(stdout).skipped?.some((s) => s.rule === "demo")).toBe( + true + ); }); - it("a file withheld for the plan is never sent to restore", async () => { - // The service keeps withheld files out of `unsafe`; this is the guard for - // the day it does not. Restoring bytes the plan will not run fixes nothing. - const server = await startMockServer((request) => { - const body = withholding(request, "demo/check.ts"); - const file = body.entitlement.withheld[0]!.file; - return { + it("--dangerously-run-scripts while authenticated: no reconcile, no checksums, everything runs", async () => { + const { stdout, server } = await authedCheck( + (request) => ({ statusCode: 200, - body: { - ...body, - unsafe: [ - { ruleId: "r", file, expected: "1;h=sha-256;d=00", got: "x" }, - ], - }, - }; - }); - try { - const { stdout } = await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: server.apiUrl, - }); - const output = JSON.parse( - stdout - .trim() - .split("\n") - .findLast((l) => l.startsWith("{")) ?? "{}" - ) as { notices?: string[] }; - expect( - (output.notices ?? []).some((notice) => /restor/.test(notice)) - ).toBe(false); - } finally { - await server.close(); + body: answer(request, { "no-console": { unsafe: [] } }), + }), + ["--json", "--dangerously-run-scripts"] + ); + expect(dataCalls(server)).toHaveLength(0); + const output = parseJson(stdout); + expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( + true + ); + expect(output.results.some((r) => r.ruleId === "no-console")).toBe(true); + }); + + it("--dangerously-run-scripts: runs runtime offline behind a warning", async () => { + const { stdout, stderr } = await runCli([ + "check", + "-d", + directory, + "--dangerously-run-scripts", + ]); + const warningLines = stderr + .split("\n") + .filter((line) => line.includes("dangerously-run-scripts")); + expect(warningLines.length).toBeGreaterThan(0); + for (const line of warningLines) { + expect(line.startsWith("Notice: ")).toBe(true); } + expect(stdout).toContain("demo"); }); - it("unentitled with nothing withheld: exit 0, entitlement still reported", async () => { - const server = await startMockServer(() => ({ + it("a malformed runtime rule is reported and skipped, never fatal", async () => { + await migrateFixture(["-d", directory]); + const broken = join(directory, ".taskless", "rules", "runtime", "broken"); + await mkdir(join(broken, "captures"), { recursive: true }); + await writeFile( + join(broken, "captures", "logs.yml"), + RUNTIME_CAPTURE, + "utf8" + ); + const { stdout, exitCode, server } = await authedCheck((request) => ({ statusCode: 200, - body: { - run: [], - unsafe: [], - unknown: [], - missing: [], - entitlement: { runtimeSignatures: false, withheld: [] }, - }, + body: answer(request, { "no-console": "run", demo: "run" }), })); - try { - const { stdout, exitCode } = await runCli( - ["check", "-d", directory, "--json"], - { TASKLESS_TOKEN: "fake.token", TASKLESS_API_URL: server.apiUrl } - ); - const output = parseEntitlementJson(stdout); - expect(exitCode).toBe(0); - expect(output.success).toBe(true); - expect(output.entitlement).toEqual({ - runtimeSignatures: false, - withheld: [], - }); - } finally { - await server.close(); - } + const output = parseJson(stdout); + expect(exitCode).toBe(0); + expect(server.requests[0]!.rules.map((rule) => rule.ruleId)).toContain( + "broken" + ); + expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( + true + ); + expect(output.skipped?.some((s) => s.rule === "broken")).toBe(true); + }); + + it("withheld for the plan: fails the run and names the cause, not drift", async () => { + const { stdout, exitCode } = await authedCheck((request) => ({ + statusCode: 200, + body: answer(request, { "no-console": "run", demo: "withheld" }), + })); + const output = parseJson(stdout); + expect(exitCode).toBe(1); + expect(output.success).toBe(false); + expect(output.results.some((r) => r.ruleId === "no-console")).toBe(true); + expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( + false + ); + const skip = output.skipped?.find((s) => s.rule === "demo"); + expect(skip?.reason).toBe("not included in your Taskless plan"); + expect(output.entitlement).toEqual({ + runtimeSignatures: false, + reason: "RUNTIME_SIGNATURES_NOT_IN_PLAN", + upgradeUrl: UPGRADE_URL, + withheld: ["demo"], + }); + expect(output).not.toHaveProperty("integrity"); }); - it("entitled or legacy responses are unchanged: exit 0, no entitlement field", async () => { - for (const entitlement of [undefined, { runtimeSignatures: true }]) { - const server = await startMockServer(() => ({ + it("withheld for the plan, human output: one notice carries the upgrade URL", async () => { + const { stderr, exitCode } = await authedCheck( + (request) => ({ statusCode: 200, - body: { run: [], unsafe: [], unknown: [], missing: [], entitlement }, - })); - try { - const { stdout, exitCode } = await runCli( - ["check", "-d", directory, "--json"], - { TASKLESS_TOKEN: "fake.token", TASKLESS_API_URL: server.apiUrl } - ); - const output = parseEntitlementJson(stdout); - expect(exitCode).toBe(0); - expect(output).not.toHaveProperty("entitlement"); - expect(output.skipped?.find((s) => s.rule === "demo")?.reason).toMatch( - /not blessed/ - ); - } finally { - await server.close(); - } - } + body: answer(request, { "no-console": "run", demo: "withheld" }), + }), + [] + ); + expect(exitCode).toBe(1); + expect(stderr.split(UPGRADE_URL)).toHaveLength(2); + expect(stderr).toContain("RUNTIME_SIGNATURES_NOT_IN_PLAN"); + expect(stderr).not.toMatch(/restore demo/); }); - it("declares the CLI version via the x-taskless-cli-version header", async () => { - const server = await startMockServer(() => ({ + it("run and withheld together: the run rule executes and the run still fails", async () => { + await migrateFixture(["-d", directory]); + const other = join(directory, ".taskless", "rules", "runtime", "other"); + await mkdir(join(other, "captures"), { recursive: true }); + await writeFile( + join(other, "captures", "logs.yml"), + RUNTIME_CAPTURE, + "utf8" + ); + await writeFile(join(other, "check.ts"), RUNTIME_CHECK + "// other\n"); + const { stdout, exitCode } = await authedCheck((request) => ({ statusCode: 200, - body: { run: [], unsafe: [], unknown: [], missing: [] }, + body: answer(request, { + "no-console": "run", + demo: "run", + other: "withheld", + }), })); - try { - await runCli(["check", "-d", directory, "--json"], { - TASKLESS_TOKEN: "fake.token", - TASKLESS_API_URL: server.apiUrl, - }); - expect(server.requests).toHaveLength(1); - const version = server.headers[0]?.["x-taskless-cli-version"]; - expect(typeof version).toBe("string"); - expect(version).not.toBe(""); - expect(version).not.toBe("unknown"); - } finally { - await server.close(); - } + const output = parseJson(stdout); + expect(exitCode).toBe(1); + expect(output.results.some((r) => r.source === "taskless-runtime")).toBe( + true + ); + expect(output.entitlement?.withheld).toEqual(["other"]); + }); + + it("an entitled organization: exit 0 and no entitlement field", async () => { + const { stdout, exitCode } = await authedCheck((request) => ({ + statusCode: 200, + body: answer(request, { "no-console": "run", demo: "run" }), + })); + expect(exitCode).toBe(0); + expect(parseJson(stdout)).not.toHaveProperty("entitlement"); }); }); diff --git a/packages/cli/test/runtime-dropped-rules.test.ts b/packages/cli/test/runtime-dropped-rules.test.ts deleted file mode 100644 index 81fa2700..00000000 --- a/packages/cli/test/runtime-dropped-rules.test.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { describe, expect, it } from "vitest"; - -import { accountForDroppedRules } from "../src/rules/runtime/plan"; -import type { RuntimeRule } from "../src/rules/runtime/discover"; - -/** - * A rule is identified by `name` here, which is all the accounting reads. The - * rest of a `RuntimeRule` is irrelevant to the question this helper answers, - * so the casts keep the fixtures to the field under test. - */ -function rule(name: string): RuntimeRule { - return { name } as unknown as RuntimeRule; -} - -describe("accounting for blessed rules that never ran", () => { - it("reports a rule that was blessed but is absent from the executed set", () => { - // The silent case. The server blessed it, so it is not `withheld`; - // re-discovery dropped it, so it is not in `execute`. Before this, it - // appeared in neither list and `check` exited 0 having said nothing. - const skipped = accountForDroppedRules( - [rule("env-read"), rule("logs-write")], - [rule("env-read")] - ); - - expect(skipped).toEqual([ - { - rule: "logs-write", - reason: "blessed by the server but missing after materialization", - }, - ]); - }); - - it("reports nothing when every blessed rule survived", () => { - expect( - accountForDroppedRules( - [rule("env-read"), rule("logs-write")], - [rule("env-read"), rule("logs-write")] - ) - ).toEqual([]); - }); - - it("reports nothing when nothing was blessed", () => { - expect(accountForDroppedRules([], [])).toEqual([]); - }); - - it("does not report a rule that appeared without being blessed", () => { - // The asymmetry is deliberate. This helper answers "what did we promise to - // run and then not run"; an unexpected EXTRA rule in the executed set is a - // different question, and reporting it here as skipped would be false. - expect( - accountForDroppedRules( - [rule("env-read")], - [rule("env-read"), rule("surprise")] - ) - ).toEqual([]); - }); -}); diff --git a/packages/cli/test/verdicts.test.ts b/packages/cli/test/verdicts.test.ts new file mode 100644 index 00000000..87caccd0 --- /dev/null +++ b/packages/cli/test/verdicts.test.ts @@ -0,0 +1,251 @@ +import { describe, expect, it } from "vitest"; + +import type { ReportedRule } from "../src/rules/report"; +import { applyVerdicts, NOT_IN_PLAN_REASON } from "../src/rules/verdicts"; + +const restore = (id: string) => `taskless rule restore ${id}`; + +const SG: ReportedRule = { + ruleId: "no-eval-3fa9c21b", + engine: "sg", + files: [], +}; +const VALE: ReportedRule = { + ruleId: "no-simply-1a2b3c4d", + engine: "vale", + files: [], +}; +const RT: ReportedRule = { + ruleId: "no-env-leak-00000000", + engine: "runtime", + files: [], +}; + +const entitled = { runtimeSignatures: true }; + +function run(rule: ReportedRule) { + return { + ruleId: rule.ruleId, + engine: rule.engine, + verdict: "run", + revisionId: "r1", + }; +} + +describe("applyVerdicts", () => { + it("runs every rule answered run", () => { + const plan = applyVerdicts( + [SG, VALE, RT], + { + rules: [run(SG), run(VALE), run(RT)], + unknown: [], + entitlement: entitled, + }, + restore + ); + expect(plan.dispositions.every((d) => d.run)).toBe(true); + expect(plan.failures).toEqual([]); + expect(plan.integrity).toEqual([]); + expect(plan.notices).toEqual([]); + }); + + it("an unsafe static rule does not run, fails, and names restore", () => { + const plan = applyVerdicts( + [VALE], + { + rules: [ + { + ruleId: VALE.ruleId, + engine: "vale", + verdict: "unsafe", + files: [ + { path: ".vale.ini", expected: "e", got: "g" }, + { path: "extra.yml", got: "g" }, + { path: "no-simply-1a2b3c4d.yml", expected: "e" }, + ], + }, + ], + unknown: [], + entitlement: entitled, + }, + restore + ); + expect(plan.dispositions).toMatchObject([{ run: false }]); + expect(plan.failures).toHaveLength(1); + expect(plan.failures[0]).toContain( + "changed .vale.ini; added extra.yml; removed no-simply-1a2b3c4d.yml" + ); + expect(plan.failures[0]).toContain(restore(VALE.ruleId)); + expect(plan.integrity[0]).toMatchObject({ + verdict: "unsafe", + engine: "vale", + }); + }); + + it("an unsafe runtime rule does not run and does NOT fail", () => { + const plan = applyVerdicts( + [RT], + { + rules: [ + { + ruleId: RT.ruleId, + engine: "runtime", + verdict: "unsafe", + files: [], + }, + ], + unknown: [], + entitlement: entitled, + }, + restore + ); + expect(plan.dispositions).toMatchObject([{ run: false }]); + expect(plan.failures).toEqual([]); + expect(plan.notices[0]).toContain(restore(RT.ruleId)); + }); + + it("unknown: static runs silently, runtime does not run", () => { + const plan = applyVerdicts( + [SG, RT], + { + rules: [], + unknown: [{ ruleId: SG.ruleId }, { ruleId: RT.ruleId }], + entitlement: entitled, + }, + restore + ); + expect(plan.dispositions).toEqual([ + { ruleId: SG.ruleId, engine: "sg", run: true }, + expect.objectContaining({ ruleId: RT.ruleId, run: false }), + ]); + expect(plan.notices).toEqual([]); + expect(plan.integrity).toEqual([ + { ruleId: RT.ruleId, engine: "runtime", verdict: "unknown" }, + ]); + }); + + it("withheld is matched by rule id, never runs, and is not offered restore", () => { + const plan = applyVerdicts( + [RT], + { + rules: [], + unknown: [], + entitlement: { + runtimeSignatures: false, + withheld: [{ ruleId: RT.ruleId, revisionId: "r1" }], + }, + }, + restore + ); + expect(plan.withheld).toEqual([RT.ruleId]); + expect(plan.dispositions).toEqual([ + { + ruleId: RT.ruleId, + engine: "runtime", + run: false, + reason: NOT_IN_PLAN_REASON, + }, + ]); + expect(plan.notices.join("")).not.toContain("restore"); + }); + + it("a reported rule in none of the lists is unaccounted: no run, fails", () => { + const plan = applyVerdicts( + [SG], + { rules: [], unknown: [], entitlement: entitled }, + restore + ); + expect(plan.dispositions).toMatchObject([{ run: false }]); + expect(plan.failures[0]).toContain("did not account for it"); + expect(plan.integrity).toEqual([ + { ruleId: SG.ruleId, engine: "sg", verdict: "unaccounted" }, + ]); + }); + + it("a rule answered twice is unaccounted", () => { + const plan = applyVerdicts( + [RT], + { + rules: [run(RT)], + unknown: [], + entitlement: { + runtimeSignatures: false, + withheld: [{ ruleId: RT.ruleId, revisionId: "r1" }], + }, + }, + restore + ); + expect(plan.failures[0]).toContain("more than once"); + expect(plan.withheld).toEqual([]); + }); + + it("an answer judging a different engine is unaccounted", () => { + const plan = applyVerdicts( + [SG], + { + rules: [{ ...run(SG), engine: "vale" }], + unknown: [], + entitlement: entitled, + }, + restore + ); + expect(plan.dispositions).toMatchObject([{ run: false }]); + expect(plan.failures[0]).toContain("judged it as a vale rule"); + }); + + it("missing for an unreported rule warns with its revision and never fails", () => { + const plan = applyVerdicts( + [], + { + rules: [ + { + ruleId: "gone-3fa9c21b", + engine: "sg", + verdict: "missing", + revisionId: "r9", + }, + ], + unknown: [], + entitlement: entitled, + }, + restore + ); + expect(plan.failures).toEqual([]); + expect(plan.integrity).toEqual([ + { + ruleId: "gone-3fa9c21b", + engine: "sg", + verdict: "missing", + revisionId: "r9", + }, + ]); + expect(plan.notices[0]).toContain(restore("gone-3fa9c21b")); + }); + + it("missing for a REPORTED rule is not a verdict for it, so it is unaccounted", () => { + const plan = applyVerdicts( + [SG], + { + rules: [ + { + ruleId: SG.ruleId, + engine: "sg", + verdict: "missing", + revisionId: "r1", + }, + ], + unknown: [], + entitlement: entitled, + }, + restore + ); + expect(plan.dispositions).toMatchObject([{ run: false }]); + expect(plan.failures).toHaveLength(1); + }); + + it("a malformed body accounts for nothing, so every reported rule fails", () => { + const plan = applyVerdicts([SG, RT], "not an object", restore); + expect(plan.dispositions.every((d) => !d.run)).toBe(true); + expect(plan.failures).toHaveLength(2); + }); +}); From 39133c291927516f66925135a6471db34b765ee6 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Tue, 29 Sep 2026 15:34:13 -0700 Subject: [PATCH 2/3] fix(check): copy every symlink that converges on one directory into the snapshot copyTree kept one visited set for the whole walk, so a second link to a directory already copied was skipped: its files were never copied, signed, or reported. Only a directory on the current path is now a cycle. --- packages/cli/src/rules/snapshot.ts | 23 ++++++++++++++-------- packages/cli/test/check-snapshot.test.ts | 25 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/rules/snapshot.ts b/packages/cli/src/rules/snapshot.ts index 130f0f8c..d4d64a7d 100644 --- a/packages/cli/src/rules/snapshot.ts +++ b/packages/cli/src/rules/snapshot.ts @@ -67,8 +67,11 @@ export interface Snapshot { * Symbolic links are DEREFERENCED: what is signed has to be what runs, and a * link resolved at run time is bytes nobody signed. A link that does not * resolve is left out, which surfaces as a missing file in the verdict rather - * than as a surprise when an engine follows it. A directory reached twice - * through links is copied once, so a cycle cannot recurse forever. + * than as a surprise when an engine follows it. A link back to a directory + * the walk is already inside is a cycle and is not followed, so the copy + * cannot recurse forever. Two links that merely converge on one directory are + * NOT a cycle, and each is copied in full: skipping the second would drop a + * rule's files from the snapshot without any verdict saying so. */ export async function takeSnapshot(cwd: string): Promise { const base = join(cwd, ".taskless", SNAPSHOT_DIRECTORY); @@ -82,9 +85,8 @@ export async function takeSnapshot(cwd: string): Promise { const source = rulesRoot(cwd); const target = rulesRoot(base); - const visited = new Set(); try { - await copyTree(source, target, visited); + await copyTree(source, target, new Set()); } catch (error) { if (!isMissingDirectory(error)) throw error; // No rules tree: an empty snapshot, which the callers read as no rules. @@ -93,14 +95,19 @@ export async function takeSnapshot(cwd: string): Promise { return { cwd, base }; } +/** + * `ancestors` holds the real paths of the directories on the CURRENT path from + * the root, not every directory seen so far: only re-entering one of those is a + * cycle. Each call extends its own copy, so siblings never see each other's. + */ async function copyTree( source: string, target: string, - visited: Set + ancestors: ReadonlySet ): Promise { const real = await realpath(source); - if (visited.has(real)) return; - visited.add(real); + if (ancestors.has(real)) return; + const chain = new Set(ancestors).add(real); await mkdir(target, { recursive: true }); const entries = await readdir(source, { withFileTypes: true }); @@ -132,7 +139,7 @@ async function copyTree( : "other"; } - if (kind === "directory") await copyTree(from, to, visited); + if (kind === "directory") await copyTree(from, to, chain); else if (kind === "file") await copyFile(from, to); // Sockets, FIFOs, devices: not rule files, and not copyable as bytes. } diff --git a/packages/cli/test/check-snapshot.test.ts b/packages/cli/test/check-snapshot.test.ts index 2aaeb1b2..34ed3cf4 100644 --- a/packages/cli/test/check-snapshot.test.ts +++ b/packages/cli/test/check-snapshot.test.ts @@ -114,6 +114,31 @@ describe("the check snapshot", () => { ]); }); + it("copies two links that converge on one directory, each in full", async () => { + const shared = join(cwd, "shared"); + await mkdir(shared); + await writeFile(join(shared, "helper.yml"), "x: 1\n"); + await symlink(shared, join(rules(), "sg", "no-eval-3fa9c21b", "a")); + await symlink(shared, join(rules(), "sg", "no-eval-3fa9c21b", "b")); + const report = await reportRules(await takeSnapshot(cwd)); + expect(report.rules[0]?.files.map((file) => file.path)).toEqual([ + "a/helper.yml", + "b/helper.yml", + "no-eval-3fa9c21b.yml", + ]); + }); + + it("does not follow a link back into a directory it is already inside", async () => { + await symlink( + join(rules(), "sg", "no-eval-3fa9c21b"), + join(rules(), "sg", "no-eval-3fa9c21b", "loop") + ); + const report = await reportRules(await takeSnapshot(cwd)); + expect(report.rules[0]?.files.map((file) => file.path)).toEqual([ + "no-eval-3fa9c21b.yml", + ]); + }); + it("neither copies nor reports operating-system metadata", async () => { await writeFile(join(rules(), "sg", "no-eval-3fa9c21b", ".DS_Store"), "x"); const snapshot = await takeSnapshot(cwd); From b5922c1a6b4d3ce27ea67fe3263849ae06d16bf0 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Tue, 29 Sep 2026 15:34:16 -0700 Subject: [PATCH 3/3] refactor(check): use the shared isRecord guard in verdicts --- packages/cli/src/rules/verdicts.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/cli/src/rules/verdicts.ts b/packages/cli/src/rules/verdicts.ts index 79328b06..0d34b00a 100644 --- a/packages/cli/src/rules/verdicts.ts +++ b/packages/cli/src/rules/verdicts.ts @@ -1,4 +1,5 @@ import { parseEntitlementV2, type EntitlementV2 } from "../api/entitlement"; +import { isRecord } from "../util/is-record"; import type { EngineName } from "./layout"; import { isKnownEngine } from "./layout"; import type { ReportedRule } from "./report"; @@ -78,10 +79,6 @@ export interface VerdictPlan { /** The reason a runtime rule the plan withholds did not run. */ export const NOT_IN_PLAN_REASON = "not included in your Taskless plan"; -function isRecord(value: unknown): value is Record { - return typeof value === "object" && value !== null && !Array.isArray(value); -} - function records(value: unknown): Record[] { return Array.isArray(value) ? value.filter((entry) => isRecord(entry)) : []; }