From 1bc7275419cb0dbcac6f429b2fbfa1ee06c64ceb Mon Sep 17 00:00:00 2001 From: Rafid Aslam Date: Tue, 18 Aug 2026 18:07:19 +0700 Subject: [PATCH 1/2] [SG-320] feat(local): evaluate committed policy files with `tirith local check` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local, credential-free policy evaluation has existed for a while, but only inside the GitHub Action (`scripts/tirith_action/local.py`). A second CI front end — the GitLab component this unblocks — would have had to fork it, which is exactly what that module's own docstring says it exists to avoid. So it moves here, and both front ends drive one implementation. A sibling subcommand rather than a flag on `platform check`. That command is documented top to bottom as "masks, packs, uploads, runs on StackGuardian, polls"; a path doing none of those under the same verb makes its help text wrong for half its readers. Mechanically it would be worse: `--workflow-id` is `required=True` there, so a flag would have to lift that requirement out of argparse into a hand-rolled conditional, weakening the platform path to accommodate a mode with no workflows — and ~20 other flags would become no-ops needing per-flag conflict handling. `platform check` gains no flag at all, so its help block and reference page are byte-identical. Local mode is never entered implicitly. `platform check` with no credentials stays a hard error rather than falling back, because a fallback evaluates whatever happens to be committed and reports green when a token was simply misspelled. A front end that wants "no credentials therefore local" chooses that itself. For the same reason the command rejects every credential, workflow, archive and run flag instead of accepting and ignoring them. One result document, two modes ------------------------------ Both modes now build `--output-json` through `report.result_document`, so the shapes cannot drift by editing one caller. Every key exists in every mode; a key with no meaning on a path is null rather than omitted or invented — omission forces every consumer to write two parsers, and a fabricated `wfrun_url` links to a run that does not exist. A `mode` discriminator says which path ran. Specified in docs/output-contract.md. Two consequent changes on the platform side, both additive and both failing closed: * The pre-poll write is now a full document instead of three keys, so a consumer reading it after a timeout meets no partial shape. * `CheckError` paths write both output files rather than returning having written nothing. A caller editing a sticky comment in place needs a marker-first body there or it orphans the comment it was updating — that has happened. `render_markdown` gained an optional `notes=` so the report can say why, instead of claiming "the workflow run finished as ERRORED" in a mode that has no run. Gating ------ Exit codes mirror `platform check`: 0, 3 with `--fail-on-error`, 1 for anything that produced no verdict. Two paths fail closed regardless of the flag, because both are tool health rather than policy decisions: no policy files found at `--policy-path`, and a policy that could not be evaluated at all. A green result for a change nothing was evaluated against is the one outcome this mode must never produce. Recorded rather than fixed: a policy whose every check was skipped reports `passed` here, matching platform mode, while the flat surface exits 1 for the same policy on the grounds that "nothing ran" is not a pass. Both readings are defensible, so the divergence is pinned by a test to make it visible instead of letting either surface drift into the other. It needs one decision across all three surfaces. --- CHANGELOG.md | 28 +- README.md | 46 ++- docs/local-check.md | 172 +++++++++ docs/output-contract.md | 72 ++++ docs/platform-check.md | 4 + .../docs/tirith-usage/cli-reference.md | 12 +- .../docs/tirith-usage/local-check.md | 185 ++++++++++ .../docs/tirith-usage/output-contract.md | 84 +++++ documentation/sidebars.js | 2 + setup.py | 2 +- src/tirith/__init__.py | 2 +- src/tirith/cli.py | 16 +- src/tirith/local/__init__.py | 26 ++ src/tirith/local/check.py | 131 +++++++ src/tirith/local/cli.py | 132 +++++++ src/tirith/local/evaluate.py | 333 ++++++++++++++++++ src/tirith/platform/check.py | 120 ++++--- src/tirith/platform/cli.py | 6 +- src/tirith/platform/report.py | 100 +++++- tests/cli/test_dispatch.py | 7 +- tests/local/conftest.py | 121 +++++++ tests/local/test_local_cli.py | 183 ++++++++++ tests/local/test_local_evaluate.py | 204 +++++++++++ tests/local/test_output_contract.py | 200 +++++++++++ tests/test_readme_is_current.py | 27 ++ 25 files changed, 2154 insertions(+), 61 deletions(-) create mode 100644 docs/local-check.md create mode 100644 docs/output-contract.md create mode 100644 documentation/docs/tirith-usage/local-check.md create mode 100644 documentation/docs/tirith-usage/output-contract.md create mode 100644 src/tirith/local/__init__.py create mode 100644 src/tirith/local/check.py create mode 100644 src/tirith/local/cli.py create mode 100644 src/tirith/local/evaluate.py create mode 100644 tests/local/conftest.py create mode 100644 tests/local/test_local_cli.py create mode 100644 tests/local/test_local_evaluate.py create mode 100644 tests/local/test_output_contract.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 19330f0a..1e53c845 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - ## [Unreleased] +## [1.3.0] - 2026-08-18 + ### Added +- `tirith local check`: evaluate the policy files committed in your repository, with no credentials + and no network calls. It writes the *same* result document and markdown report as + `tirith platform check`, so a CI integration built against one works unchanged against the other, + and adding a StackGuardian organization later changes which policies apply rather than how + anything is wired. Moved in from the GitHub Action, which owned the only implementation, so that a + second front end does not fork it. + - Two behaviours that fail closed: no policy files found is an error rather than a skip, and a + policy that could not be evaluated exits 1 regardless of `--fail-on-error`, because "could not + evaluate" is tool health rather than a policy decision. + - Never entered implicitly. `tirith platform check` with no credentials stays an error rather than + falling back, because a fallback evaluates whatever happens to be committed and reports green + when a token was misspelled. - `tirith ui`: an interactive interface with three tabs. - **Explorer** — read an evaluation's results down to the resource behind each one. The result document has always carried the resource address, the planned action and the before/after @@ -31,9 +44,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 a provider argument the operation does not read, an id referenced in `eval_expression` but never defined, a single `&` where `&&` was meant, an evaluator that does not exist. +### Changed +- The `--output-json` document now carries a `mode` key (`platform` or `local`), and every key exists + in both modes -- one with no meaning on a path is `null` rather than omitted, so a consumer needs + one parser rather than two. Additive: no key was removed or retyped. Specified in + [docs/output-contract.md](docs/output-contract.md). +- `tirith platform check` writes `--output-json` and `--output-markdown` on its failure paths too, + rather than returning having written nothing. A caller editing a sticky comment in place needs a + marker-first body on that path, or it orphans the comment it was updating. + ### Notes -- The local evaluation surface is untouched. `ui` is dispatched before the flat parser, like - `platform`, so `--json` output remains byte-identical to the golden file. +- The local evaluation surface is untouched. `ui` and `local` are dispatched before the flat parser, + like `platform`, so `--json` output remains byte-identical to the golden file. - No new runtime dependencies for anyone who does not install the extra. ## [1.2.0] - 2026-08-03 diff --git a/README.md b/README.md index e9dceeee..f1bdc189 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ verdict, same exit codes. That mode is optional and is the only part that talks - [Run it in CI](#run-it-in-ci) - [Exit codes](#exit-codes) - [Evaluating against your StackGuardian organization](#evaluating-against-your-stackguardian-organization) +- [Evaluating committed policy files, in CI](#evaluating-committed-policy-files-in-ci) - [Example Tirith policies](#example-tirith-policies) - [error_tolerance](#error_tolerance-and-the-third-outcome) - [Terraform Plan](#terraform-plan-provider) @@ -177,7 +178,7 @@ pip install -e . ``` tirith --version -tirith 1.2.0 +tirith 1.3.0 ``` Congratulations! Tirith has been setup in your system @@ -205,6 +206,8 @@ Subcommands: tirith platform check --help Evaluate against the policies your StackGuardian organization enforces, rather than local files. + tirith local check --help Evaluate policy files committed in your repository, + with no credentials, and report the same verdict. tirith ui --help Explore results, build policies and experiment in an interactive interface. Needs the 'tui' extra. @@ -385,7 +388,7 @@ GitLab-specific: any runner that can execute a container and produce a plan work | 0 | Policies passed, or nothing was in scope to gate on | | 1 | Tirith could not complete the evaluation — bad input, a policy it could not evaluate, unreachable API | | 2 | Timed out waiting for a StackGuardian run | -| 3 | A policy failed. Only with `--fail-on-error`, on either surface | +| 3 | A policy failed. Only with `--fail-on-error`, on any of the three surfaces | | 130 | Interrupted | **Gate a CI job with `--fail-on-error`:** @@ -443,6 +446,45 @@ Common flags: Running this from GitHub Actions? Use [the action](#github-actions) instead — it wires up the plan discovery, the sticky pull-request comment, the check run and the exit codes for you. +## Evaluating committed policy files, in CI + +`tirith local check` is the credential-free counterpart. It evaluates the policy files in your +repository and writes the *same* report `tirith platform check` writes — so a CI integration built on +one works unchanged on the other, and adding a StackGuardian organization later changes which +policies apply, not how anything is wired. + +``` +tirith local check --policy-path .tirith/policies --input-path plan.json --fail-on-error +``` + +Nothing leaves the machine. What you give up against platform mode is run history, the dashboard, +enforcing one policy set across every repository from one place, and cost policies — those need a +second document, so `--infracost-path` is accepted and reported as ignored. + +| | | +|---|---| +| `--policy-path` | A file, a directory, or a glob. A directory is searched recursively for `*.tirith.json`, or failing that for any `.json` file shaped like a policy. Default `.tirith/policies` | +| `--input-path` / `--plan-file` | The document to evaluate, or a binary plan to render. Defaults to `plan.json` or `tfplan.json` | +| `--output-json` / `--output-markdown` | The machine-readable verdict and a markdown report, in the same shape both modes produce | +| `--comment-marker` | An opaque first line for the markdown, so a CI job can find and edit its own comment | +| `--fail-on-error` | Exit `3` when a policy fails, instead of `0` | + +Two behaviours worth knowing, both of which fail closed: + +- **No policy files is an error, not a skip.** Pointed at a path with nothing in it, the command exits + `1` rather than reporting a pass — a green result for a change nothing was evaluated against is the + one outcome this mode must never produce. +- **A policy that could not be evaluated exits `1` regardless of `--fail-on-error`.** "Could not + evaluate" is tool health, not a policy decision, so the flag does not apply to it. + +It is not entered implicitly: `tirith platform check` with no credentials stays an error rather than +quietly falling back here, because a fallback would evaluate whatever happens to be committed and +report green when a token was simply misspelled. A CI integration that wants "no credentials therefore +local" chooses that itself, deliberately. + +Every flag is in [docs/local-check.md](docs/local-check.md) or `tirith local check --help`, and the +result document both modes write is in [docs/output-contract.md](docs/output-contract.md). + ## Example Tirith policies [Examples using various providers](tests/providers) diff --git a/docs/local-check.md b/docs/local-check.md new file mode 100644 index 00000000..7dc32073 --- /dev/null +++ b/docs/local-check.md @@ -0,0 +1,172 @@ +# `tirith local check` + +Evaluate the policy files committed in your repository against a terraform plan, state document or +JSON document. No credentials, no network calls. + +It is the credential-free counterpart of +[`tirith platform check`](platform-check.md) and writes the *same* report, in the +[same shape](output-contract.md) — so a CI integration built against one works unchanged against the +other. Adding a StackGuardian organization later changes which policies apply, not how anything is +wired. + +``` +tirith local check --policy-path .tirith/policies --input-path plan.json --fail-on-error +``` + +## What it does + +1. **Discovers policies** at `--policy-path`. +2. **Masks the document**, exactly as platform mode does. Nothing is uploaded, so this is not about + transport — evaluator messages embed the values they compared, and those messages are copied + verbatim into whatever pull-request comment or merge-request note the caller posts. An unmasked + local run publishes plan values to a code host. Masking also keeps a local verdict identical to + the platform one for the same plan, because platform mode evaluates the masked document too. +3. **Evaluates each policy** in a subprocess, against tirith's frozen `--json` output contract. +4. **Writes the verdict**, optionally as JSON and markdown for a later CI step. + +## It is never entered implicitly + +`tirith platform check` with no credentials is an error, not a silent fallback to this command. A +fallback would evaluate whatever happens to be committed and report green when a token was simply +misspelled — the exact failure a policy gate exists to prevent. A CI integration that wants "no +credentials therefore local" makes that choice itself, deliberately, and can say so in its own log. + +For the same reason this command rejects every credential, workflow, archive and run flag rather than +accepting and ignoring them. A credential-shaped flag that silently does nothing is how someone ends +up with a green check their organization's policies never saw. + +## Policy discovery + +`--policy-path` accepts a file, a directory or a glob, and defaults to `.tirith/policies`. + +- **A file** is taken as given. Naming one explicitly is an instruction, so "that is not a policy" is + reported rather than the file being skipped. +- **A directory** is searched recursively for `*.tirith.json`. If it holds none, any `.json` file + *shaped* like a policy is used — an object with both `meta` and `evaluators`. +- **A glob** is expanded and filtered by that same shape test. + +The shape test is load-bearing, not defensive: a policy directory routinely also holds the document +under evaluation. Without it, `plan.json` is evaluated *as a policy*, which reports a spurious failure +and buries the real findings. + +**No policy files is an error.** Pointed at a path with nothing in it, the command exits `1` rather +than reporting a pass. A green result for a change nothing was evaluated against is the one outcome +this mode must never produce, and "no policies found" is a configuration mistake rather than a +deliberate skip. + +## Enforcement + +A failing policy fails. `meta.enforcement` downgrades it to a warning for these values: + +`soft_mandatory`, `advisory`, `warn`, `warning`, `low`, `approval_required`, `approval-required`, +`approval` + +and gates for these: + +`hard_mandatory`, `mandatory`, `fail`, `error`, `high`, `critical`, `blocking` + +The approval spellings warn rather than gate, matching platform mode, where a policy carrying +`onFail: APPROVAL_REQUIRED` also warns — the run finishes before the intent is known, so there is +nothing to approve. Local mode has no approval mechanism at all, so failing closed on it would block a +change with no way to unblock it. + +Anything else gates *and* raises a warning naming the value, in the log and in `policy_warnings` in +the result document. An unlabelled or mislabelled policy must gate rather than slip through, but a +typo in `enforcement` silently becoming policy is worse than a noisy one. + +Omitting `meta.enforcement` entirely gates, with no warning. + +## Cost policies need the platform + +`--infracost-path` is accepted and reported as ignored. Cost evaluation needs a second document +alongside the plan, and this mode evaluates one. Saying so beats evaluating the plan and reporting a +cost policy as unevaluated with no explanation. + +## Exit codes + +| Code | Condition | +|---|---| +| 0 | Policies passed or warned. Also a failing policy without `--fail-on-error` | +| 1 | No policy files found; the input document was missing or unparseable; a policy could not be evaluated; no verdict was produced | +| 3 | A policy failed, with `--fail-on-error` | +| 130 | Interrupted | + +**A policy that could not be evaluated exits `1` regardless of `--fail-on-error`.** "Could not +evaluate" is tool health, not a policy decision, so the flag does not govern it — the same reason an +unreachable platform ignores it in the other mode. Such a policy also appears in the report as a +visible failure carrying its reason, so it is never mistakable for a pass and never silently dropped. + +Note the `2` documented for platform mode (a run timeout) is unreachable here: there is no run to +time out. The per-policy timeout is 300 seconds, and a policy that hits it is one entry in +`policy_errors` among possibly several verdicts, so it exits `1`. + +## Both output files are always written + +`--output-json` and `--output-markdown` are written on **every** exit path, including failures, and +the markdown always begins with `--comment-marker` when one is given. + +This matters to any caller that edits a sticky comment in place: writing nothing on the failure path +means the caller falls through to a body of its own with no marker in it, and PATCHing that over a +good comment orphans it permanently. It has happened. Writing both files also puts the reason on the +merge request rather than only in the job log. + +## Flags + +The full surface, as the command prints it: + + usage: tirith local check [-h] [--policy-path POLICY_PATH] [--input-path INPUT_PATH] + [--plan-file PLAN_FILE] [--terraform-bin TERRAFORM_BIN] + [--input-kind {terraform_plan,terraform_state,kubernetes,json}] + [--state-path STATE_PATH] [--infracost-path INFRACOST_PATH] + [--source-dir SOURCE_DIR] [--sha SHA] [--output-json OUTPUT_JSON] + [--output-markdown OUTPUT_MARKDOWN] [--comment-marker COMMENT_MARKER] + [--markdown-limit MARKDOWN_LIMIT] [--fail-on-error] + + Masks the document, evaluates every policy found at --policy-path, and writes the same result + document and markdown report that `tirith platform check` writes. Requires no credentials and + makes no network calls. + + options: + -h, --help show this help message and exit + + policies: + --policy-path POLICY_PATH + A policy file, a directory, or a glob. A directory is searched + recursively for *.tirith.json, or failing that for any .json file shaped + like a policy. Default: .tirith/policies + + inputs: + --input-path INPUT_PATH + Document to evaluate. Default: plan.json or tfplan.json. + --plan-file PLAN_FILE + Binary terraform plan, rendered in memory. Not with --input-path. + --terraform-bin TERRAFORM_BIN + terraform/tofu binary used for --plan-file. + --input-kind {terraform_plan,terraform_state,kubernetes,json} + What the document is. + --state-path STATE_PATH + Terraform state, when --input-kind is terraform_state. + --infracost-path INFRACOST_PATH + Accepted and ignored: cost policies need a second document, so they need + platform mode. + --source-dir SOURCE_DIR + Where to look for the document. Default: . + + run: + --sha SHA Revision these findings describe, recorded in the report. + + output: + --output-json OUTPUT_JSON + Write the result document here. + --output-markdown OUTPUT_MARKDOWN + Write a markdown report here. + --comment-marker COMMENT_MARKER + Opaque first line of the markdown, for stickiness. + --markdown-limit MARKDOWN_LIMIT + Truncate the markdown to this length. + --fail-on-error Exit non-zero when a policy fails. A policy that could not be evaluated + at all always exits non-zero regardless of this flag. + +`--input-kind kubernetes` and `--input-kind json` are passed through unmasked: they carry no +sensitivity markers to mask by, and tirith reads YAML for them, which a JSON round-trip would break. +Both require `--input-path`. diff --git a/docs/output-contract.md b/docs/output-contract.md new file mode 100644 index 00000000..c22802e9 --- /dev/null +++ b/docs/output-contract.md @@ -0,0 +1,72 @@ +# The result document + +`--output-json` writes one document, in one shape, from both +[`tirith platform check`](platform-check.md) and [`tirith local check`](local-check.md). This page is +the contract, because two integrations read it — the +[GitHub Action](https://github.com/StackGuardian/tirith-iac-governance-action) and the GitLab CI +component — and turn it into comments, statuses and job outputs. + +## The rule + +**Every key exists in every mode.** A key with no meaning on a path is `null` (or `[]` for the lists), +never omitted and never invented. + +Omitting keys per mode would force every consumer to write two parsers, and the one that forgets +reports the wrong thing on the mode it was not tested against. Inventing them is worse: a fabricated +`wfrun_url` in local mode renders as a link to a run that does not exist. + +Both documents are built by one function, `tirith.platform.report.result_document`, so the shapes +cannot drift apart by editing one caller. + +## Keys + +| Key | Platform mode | Local mode | +|---|---|---| +| `mode` | `"platform"` | `"local"` | +| `status` | The workflow run's terminal status: `COMPLETED`, `ERRORED`, `CANCELLED`, `APPROVAL_REQUIRED` | `COMPLETED`, or `ERRORED` when nothing could be evaluated | +| `verdict` | `passed` · `warned` · `failed` · `no-policies` · `errored` | the same five | +| `counts` | `passed`, `failed`, `warned`, `approval_required`, `skipped`, `unknown` | the same six | +| `headline` | The one-line summary, e.g. `Tirith — 2 failed, 1 warned, 5 passed` | the same | +| `policy_results` | The `PolicyEvalResults` document from the run | the same shape, built from the local evaluation | +| `wfrun_id` | The run id | `null` | +| `wfrun_url` | Link to the run in StackGuardian | `null` | +| `monthly_cost` | `totalMonthlyCost` from the cost breakdown, or `null` | `null` — cost needs a second document | +| `archive_key` | Where the uploaded archive lives | `null` — nothing is uploaded | +| `source_packed` | Whether the archive actually holds the terraform source | `false` | +| `source_skipped_reason` | Why it does not, or `null` | `null` | +| `policies_evaluated` | `null` | How many policy files were evaluated | +| `policies_errored` | `null` | How many could not be | +| `policy_path` | `null` | The `--policy-path` that was used | +| `policy_errors` | `[]` | `[{"policy": path, "reason": str}]` | +| `policy_warnings` | `[]` | `[str]` — currently unrecognised `meta.enforcement` values | + +## Reading it + +**Read `mode` rather than inferring it** from which keys are populated. Inference is what makes +adding a key a breaking change. + +**`verdict` is the answer; the exit code is the gate.** They are related but not the same: `failed` +without `--fail-on-error` exits `0` on purpose. A consumer that wants to report the verdict and a +consumer that wants to block are reading two different things. + +**`counts.unknown` distinguishes "nothing failed" from "we could not read part of it."** Without it, +an errored run reports `failed: 0`, which a consumer copying counts to its own outputs turns into a +clean bill of health. A non-zero `unknown` is why `verdict` can be `errored` while `failed` is `0`. + +**`source_skipped_reason` is truthy only when there is a real reason.** It stays `null` in local mode +rather than carrying a sentinel like `"not_applicable"`, because a consumer's "the terraform source +was not uploaded" warning keys on truthiness — a sentinel would fire it on every local run, about an +upload that was never attempted. + +**`policy_errors` is structured so a front end can annotate without scraping stderr.** Everything in +it is also logged, but a log line is not a data source. + +## Written on every exit path + +Both `--output-json` and `--output-markdown` are written even when the check fails outright, with +`status: "ERRORED"`, `verdict: "errored"`, zeroed counts, and the reason in `policy_errors`. The +markdown carries `--comment-marker` as its first line. + +A consumer that edits a sticky comment in place depends on this. When these files were absent on the +failure path, the caller fell through to a body of its own with no marker in it, and PATCHing that +over a good comment orphaned it permanently. diff --git a/docs/platform-check.md b/docs/platform-check.md index 54eae4ec..eeeded2f 100644 --- a/docs/platform-check.md +++ b/docs/platform-check.md @@ -7,6 +7,10 @@ The [GitHub Action](https://github.com/StackGuardian/tirith-iac-governance-actio around this command. Use the action on GitHub; use this directly anywhere else — GitLab CI, a Makefile, a local shell. +No StackGuardian organization? [`tirith local check`](local-check.md) evaluates policy files committed +in your repository instead, with no credentials, and writes the same report. The document both +commands write is specified in [the result document](output-contract.md). + ## What it does 1. **Masks the document on your machine**, before anything is uploaded. Values terraform marked diff --git a/documentation/docs/tirith-usage/cli-reference.md b/documentation/docs/tirith-usage/cli-reference.md index b8a726c5..ecda5f6b 100644 --- a/documentation/docs/tirith-usage/cli-reference.md +++ b/documentation/docs/tirith-usage/cli-reference.md @@ -19,9 +19,15 @@ tirith -policy-path policy.json -input-path plan.json Run with no arguments, `tirith` prints its help text and exits `0`. -There is one subcommand, `tirith platform check`, which evaluates against the policies a -StackGuardian organization enforces instead of local files. It has its own flags and its own page: -[Platform Check](platform-check.md). +There are two policy-evaluation subcommands, each with its own flags and its own page. +[`tirith platform check`](platform-check.md) evaluates against the policies a StackGuardian +organization enforces; [`tirith local check`](local-check.md) evaluates policy files committed in +your repository, with no credentials. Both write the same report, so a CI integration built against +one works unchanged against the other. + +The flags below are the flat surface — `tirith -policy-path … -input-path …` — which evaluates one +policy path and prints the engine's own output. `local check` is the CI-shaped version of the same +idea: it discovers many policies, masks the input, renders a report and gates on the result. ## Flags diff --git a/documentation/docs/tirith-usage/local-check.md b/documentation/docs/tirith-usage/local-check.md new file mode 100644 index 00000000..bffa8005 --- /dev/null +++ b/documentation/docs/tirith-usage/local-check.md @@ -0,0 +1,185 @@ +--- +id: local-check +title: Local Check +sidebar_label: Local Check +description: The tirith local check subcommand — evaluate policy files committed in your repository, with no credentials, and get the same report platform mode produces. +keywords: + - tirith + - local check + - policy as code + - ci +site_name: Tirith +slug: local-check/ +--- + +Evaluate the policy files committed in your repository against a terraform plan, state document or +JSON document. No credentials, no network calls. + +It is the credential-free counterpart of +[`tirith platform check`](platform-check.md) and writes the *same* report, in the +[same shape](output-contract.md) — so a CI integration built against one works unchanged against the +other. Adding a StackGuardian organization later changes which policies apply, not how anything is +wired. + +``` +tirith local check --policy-path .tirith/policies --input-path plan.json --fail-on-error +``` + +## What it does + +1. **Discovers policies** at `--policy-path`. +2. **Masks the document**, exactly as platform mode does. Nothing is uploaded, so this is not about + transport — evaluator messages embed the values they compared, and those messages are copied + verbatim into whatever pull-request comment or merge-request note the caller posts. An unmasked + local run publishes plan values to a code host. Masking also keeps a local verdict identical to + the platform one for the same plan, because platform mode evaluates the masked document too. +3. **Evaluates each policy** in a subprocess, against tirith's frozen `--json` output contract. +4. **Writes the verdict**, optionally as JSON and markdown for a later CI step. + +## It is never entered implicitly + +`tirith platform check` with no credentials is an error, not a silent fallback to this command. A +fallback would evaluate whatever happens to be committed and report green when a token was simply +misspelled — the exact failure a policy gate exists to prevent. A CI integration that wants "no +credentials therefore local" makes that choice itself, deliberately, and can say so in its own log. + +For the same reason this command rejects every credential, workflow, archive and run flag rather than +accepting and ignoring them. A credential-shaped flag that silently does nothing is how someone ends +up with a green check their organization's policies never saw. + +## Policy discovery + +`--policy-path` accepts a file, a directory or a glob, and defaults to `.tirith/policies`. + +- **A file** is taken as given. Naming one explicitly is an instruction, so "that is not a policy" is + reported rather than the file being skipped. +- **A directory** is searched recursively for `*.tirith.json`. If it holds none, any `.json` file + *shaped* like a policy is used — an object with both `meta` and `evaluators`. +- **A glob** is expanded and filtered by that same shape test. + +The shape test is load-bearing, not defensive: a policy directory routinely also holds the document +under evaluation. Without it, `plan.json` is evaluated *as a policy*, which reports a spurious failure +and buries the real findings. + +**No policy files is an error.** Pointed at a path with nothing in it, the command exits `1` rather +than reporting a pass. A green result for a change nothing was evaluated against is the one outcome +this mode must never produce, and "no policies found" is a configuration mistake rather than a +deliberate skip. + +## Enforcement + +A failing policy fails. `meta.enforcement` downgrades it to a warning for these values: + +`soft_mandatory`, `advisory`, `warn`, `warning`, `low`, `approval_required`, `approval-required`, +`approval` + +and gates for these: + +`hard_mandatory`, `mandatory`, `fail`, `error`, `high`, `critical`, `blocking` + +The approval spellings warn rather than gate, matching platform mode, where a policy carrying +`onFail: APPROVAL_REQUIRED` also warns — the run finishes before the intent is known, so there is +nothing to approve. Local mode has no approval mechanism at all, so failing closed on it would block a +change with no way to unblock it. + +Anything else gates *and* raises a warning naming the value, in the log and in `policy_warnings` in +the result document. An unlabelled or mislabelled policy must gate rather than slip through, but a +typo in `enforcement` silently becoming policy is worse than a noisy one. + +Omitting `meta.enforcement` entirely gates, with no warning. + +## Cost policies need the platform + +`--infracost-path` is accepted and reported as ignored. Cost evaluation needs a second document +alongside the plan, and this mode evaluates one. Saying so beats evaluating the plan and reporting a +cost policy as unevaluated with no explanation. + +## Exit codes + +| Code | Condition | +|---|---| +| 0 | Policies passed or warned. Also a failing policy without `--fail-on-error` | +| 1 | No policy files found; the input document was missing or unparseable; a policy could not be evaluated; no verdict was produced | +| 3 | A policy failed, with `--fail-on-error` | +| 130 | Interrupted | + +**A policy that could not be evaluated exits `1` regardless of `--fail-on-error`.** "Could not +evaluate" is tool health, not a policy decision, so the flag does not govern it — the same reason an +unreachable platform ignores it in the other mode. Such a policy also appears in the report as a +visible failure carrying its reason, so it is never mistakable for a pass and never silently dropped. + +Note the `2` documented for platform mode (a run timeout) is unreachable here: there is no run to +time out. The per-policy timeout is 300 seconds, and a policy that hits it is one entry in +`policy_errors` among possibly several verdicts, so it exits `1`. + +## Both output files are always written + +`--output-json` and `--output-markdown` are written on **every** exit path, including failures, and +the markdown always begins with `--comment-marker` when one is given. + +This matters to any caller that edits a sticky comment in place: writing nothing on the failure path +means the caller falls through to a body of its own with no marker in it, and PATCHing that over a +good comment orphans it permanently. It has happened. Writing both files also puts the reason on the +merge request rather than only in the job log. + +## Flags + +The full surface, as the command prints it: + +```text +usage: tirith local check [-h] [--policy-path POLICY_PATH] [--input-path INPUT_PATH] + [--plan-file PLAN_FILE] [--terraform-bin TERRAFORM_BIN] + [--input-kind {terraform_plan,terraform_state,kubernetes,json}] + [--state-path STATE_PATH] [--infracost-path INFRACOST_PATH] + [--source-dir SOURCE_DIR] [--sha SHA] [--output-json OUTPUT_JSON] + [--output-markdown OUTPUT_MARKDOWN] [--comment-marker COMMENT_MARKER] + [--markdown-limit MARKDOWN_LIMIT] [--fail-on-error] + +Masks the document, evaluates every policy found at --policy-path, and writes the same result +document and markdown report that `tirith platform check` writes. Requires no credentials and +makes no network calls. + +options: + -h, --help show this help message and exit + +policies: + --policy-path POLICY_PATH + A policy file, a directory, or a glob. A directory is searched + recursively for *.tirith.json, or failing that for any .json file shaped + like a policy. Default: .tirith/policies + +inputs: + --input-path INPUT_PATH + Document to evaluate. Default: plan.json or tfplan.json. + --plan-file PLAN_FILE + Binary terraform plan, rendered in memory. Not with --input-path. + --terraform-bin TERRAFORM_BIN + terraform/tofu binary used for --plan-file. + --input-kind {terraform_plan,terraform_state,kubernetes,json} + What the document is. + --state-path STATE_PATH + Terraform state, when --input-kind is terraform_state. + --infracost-path INFRACOST_PATH + Accepted and ignored: cost policies need a second document, so they need + platform mode. + --source-dir SOURCE_DIR + Where to look for the document. Default: . + +run: + --sha SHA Revision these findings describe, recorded in the report. + +output: + --output-json OUTPUT_JSON + Write the result document here. + --output-markdown OUTPUT_MARKDOWN + Write a markdown report here. + --comment-marker COMMENT_MARKER + Opaque first line of the markdown, for stickiness. + --markdown-limit MARKDOWN_LIMIT + Truncate the markdown to this length. + --fail-on-error Exit non-zero when a policy fails. A policy that could not be evaluated + at all always exits non-zero regardless of this flag. +``` +`--input-kind kubernetes` and `--input-kind json` are passed through unmasked: they carry no +sensitivity markers to mask by, and tirith reads YAML for them, which a JSON round-trip would break. +Both require `--input-path`. diff --git a/documentation/docs/tirith-usage/output-contract.md b/documentation/docs/tirith-usage/output-contract.md new file mode 100644 index 00000000..18293a3d --- /dev/null +++ b/documentation/docs/tirith-usage/output-contract.md @@ -0,0 +1,84 @@ +--- +id: output-contract +title: The Result Document +sidebar_label: Result Document +description: The JSON document tirith writes with --output-json, identical in platform and local mode, and the contract CI integrations read it by. +keywords: + - tirith + - output + - json + - ci +site_name: Tirith +slug: output-contract/ +--- + +`--output-json` writes one document, in one shape, from both +[`tirith platform check`](platform-check.md) and [`tirith local check`](local-check.md). This page is +the contract, because two integrations read it — the +[GitHub Action](https://github.com/StackGuardian/tirith-iac-governance-action) and the GitLab CI +component — and turn it into comments, statuses and job outputs. + +## The rule + +**Every key exists in every mode.** A key with no meaning on a path is `null` (or `[]` for the lists), +never omitted and never invented. + +Omitting keys per mode would force every consumer to write two parsers, and the one that forgets +reports the wrong thing on the mode it was not tested against. Inventing them is worse: a fabricated +`wfrun_url` in local mode renders as a link to a run that does not exist. + +Both documents are built by one function, `tirith.platform.report.result_document`, so the shapes +cannot drift apart by editing one caller. + +## Keys + +| Key | Platform mode | Local mode | +|---|---|---| +| `mode` | `"platform"` | `"local"` | +| `status` | The workflow run's terminal status: `COMPLETED`, `ERRORED`, `CANCELLED`, `APPROVAL_REQUIRED` | `COMPLETED`, or `ERRORED` when nothing could be evaluated | +| `verdict` | `passed` · `warned` · `failed` · `no-policies` · `errored` | the same five | +| `counts` | `passed`, `failed`, `warned`, `approval_required`, `skipped`, `unknown` | the same six | +| `headline` | The one-line summary, e.g. `Tirith — 2 failed, 1 warned, 5 passed` | the same | +| `policy_results` | The `PolicyEvalResults` document from the run | the same shape, built from the local evaluation | +| `wfrun_id` | The run id | `null` | +| `wfrun_url` | Link to the run in StackGuardian | `null` | +| `monthly_cost` | `totalMonthlyCost` from the cost breakdown, or `null` | `null` — cost needs a second document | +| `archive_key` | Where the uploaded archive lives | `null` — nothing is uploaded | +| `source_packed` | Whether the archive actually holds the terraform source | `false` | +| `source_skipped_reason` | Why it does not, or `null` | `null` | +| `policies_evaluated` | `null` | How many policy files were evaluated | +| `policies_errored` | `null` | How many could not be | +| `policy_path` | `null` | The `--policy-path` that was used | +| `policy_errors` | `[]` | `[{"policy": path, "reason": str}]` | +| `policy_warnings` | `[]` | `[str]` — currently unrecognised `meta.enforcement` values | + +## Reading it + +**Read `mode` rather than inferring it** from which keys are populated. Inference is what makes +adding a key a breaking change. + +**`verdict` is the answer; the exit code is the gate.** They are related but not the same: `failed` +without `--fail-on-error` exits `0` on purpose. A consumer that wants to report the verdict and a +consumer that wants to block are reading two different things. + +**`counts.unknown` distinguishes "nothing failed" from "we could not read part of it."** Without it, +an errored run reports `failed: 0`, which a consumer copying counts to its own outputs turns into a +clean bill of health. A non-zero `unknown` is why `verdict` can be `errored` while `failed` is `0`. + +**`source_skipped_reason` is truthy only when there is a real reason.** It stays `null` in local mode +rather than carrying a sentinel like `"not_applicable"`, because a consumer's "the terraform source +was not uploaded" warning keys on truthiness — a sentinel would fire it on every local run, about an +upload that was never attempted. + +**`policy_errors` is structured so a front end can annotate without scraping stderr.** Everything in +it is also logged, but a log line is not a data source. + +## Written on every exit path + +Both `--output-json` and `--output-markdown` are written even when the check fails outright, with +`status: "ERRORED"`, `verdict: "errored"`, zeroed counts, and the reason in `policy_errors`. The +markdown carries `--comment-marker` as its first line. + +A consumer that edits a sticky comment in place depends on this. When these files were absent on the +failure path, the caller fell through to a body of its own with no marker in it, and PATCHing that +over a good comment orphaned it permanently. diff --git a/documentation/sidebars.js b/documentation/sidebars.js index df3c036e..c436594a 100644 --- a/documentation/sidebars.js +++ b/documentation/sidebars.js @@ -25,6 +25,8 @@ module.exports = { "tirith-usage/exit-codes", "tirith-usage/ci-integration", "tirith-usage/platform-check", + "tirith-usage/local-check", + "tirith-usage/output-contract", ] }, { diff --git a/setup.py b/setup.py index 6dc8be09..2f49e1f7 100644 --- a/setup.py +++ b/setup.py @@ -22,7 +22,7 @@ def read(*names, **kwargs): setup( name="py-tirith", - version="1.2.0", + version="1.3.0", license="Apache", description="Tirith simplifies defining Policy as Code.", long_description_content_type="text/markdown", diff --git a/src/tirith/__init__.py b/src/tirith/__init__.py index 4c2aac77..b3fd7cbc 100644 --- a/src/tirith/__init__.py +++ b/src/tirith/__init__.py @@ -2,6 +2,6 @@ tirith: Execute policies defined using Tirith (StackGuardian Policy Framework) """ -__version__ = "1.2.0" +__version__ = "1.3.0" __author__ = "StackGuardian" __license__ = "Apache" diff --git a/src/tirith/cli.py b/src/tirith/cli.py index f75c9417..51366cc6 100755 --- a/src/tirith/cli.py +++ b/src/tirith/cli.py @@ -46,7 +46,14 @@ def eprint(*args, **kwargs): # 3.9 and tirith supports 3.8 -- so tui/cli.py reports the missing extra rather than failing on # an import here. UI_SUBCOMMAND = "ui" -SUBCOMMANDS = {SUBCOMMAND, UI_SUBCOMMAND} + +# `local` is the credential-free sibling of `platform`: policy files committed in the repository, +# evaluated here, reported identically. It is a separate subcommand rather than a flag on +# `platform check` -- see tirith/local/cli.py for why -- and `platform check` with no credentials +# stays a hard error, so neither mode is ever entered by accident. +LOCAL_SUBCOMMAND = "local" + +SUBCOMMANDS = {SUBCOMMAND, UI_SUBCOMMAND, LOCAL_SUBCOMMAND} def main(args=None) -> ExitStatus: @@ -65,6 +72,11 @@ def main(args=None) -> ExitStatus: return tui_cli.main(argv) + if argv and argv[0] == LOCAL_SUBCOMMAND: + from tirith.local import cli as local_cli + + return local_cli.main(argv) + if argv and argv[0] in SUBCOMMANDS: from tirith.platform import cli as platform_cli @@ -85,6 +97,8 @@ def __init__(self, prog="PROG") -> None: tirith platform check --help Evaluate against the policies your StackGuardian organization enforces, rather than local files. + tirith local check --help Evaluate policy files committed in your repository, + with no credentials, and report the same verdict. tirith ui --help Explore results, build policies and experiment in an interactive interface. Needs the 'tui' extra. diff --git a/src/tirith/local/__init__.py b/src/tirith/local/__init__.py new file mode 100644 index 00000000..e494aa29 --- /dev/null +++ b/src/tirith/local/__init__.py @@ -0,0 +1,26 @@ +""" +Local policy evaluation -- the credential-free path. + +Everything here runs on the machine it is invoked from and talks to nothing. Policy files committed +in the repository are evaluated against a document, and the same report the platform path produces +is written out, so a caller's reporting is identical in both modes. + +Two deliberate choices about *how* this reuses the rest of tirith, both about not owning a second +copy of anything: + + * Rendering, masking and document discovery are imported from `tirith.platform` + (`report` / `redact` / `discover`). A second markdown renderer would drift from the platform + path's, and the whole point is that both modes report identically. Those three modules live + under `platform/` for historical reasons and are mode-agnostic; they are not moved here, because + code outside this repository imports them by that path. + * Evaluation shells out to `tirith -policy-path P -input-path I --json`, one policy at a time. + That stdout document is tirith's frozen contract -- pinned byte-for-byte by + tests/core/test_output_compatibility.py -- which makes it the most stable interface it has. + Calling the engine's Python API instead would couple this to internals that carry no such + promise. Now that this code lives inside the package, the temptation to do so is real; the + argv is pinned by a test so that becomes a deliberate change rather than a silent one. + +This began life in the GitHub Action, which needed to be usable before anyone had a StackGuardian +account. It moved here so a second front end (GitLab CI) drives one implementation rather than +forking it. +""" diff --git a/src/tirith/local/check.py b/src/tirith/local/check.py new file mode 100644 index 00000000..7d61369d --- /dev/null +++ b/src/tirith/local/check.py @@ -0,0 +1,131 @@ +""" +Orchestration for `tirith local check`. + +Lifted from the GitHub Action's `run_local`, with the GitHub-specific parts left behind. Writes the +same two files `tirith platform check` writes -- the result document and the report body -- so +everything a front end does downstream is identical in both modes. +""" + +import tempfile + +from ..platform import report +from ..platform.check import log, write_output_json, write_output_markdown +from .evaluate import LocalError, discover_policies, evaluate, prepare_input + + +def write_failure_report(opts, message): + """ + Write both output files for a run that produced no verdict. + + These paths used to return without writing either file, so a front end fell through to a + marker-less body and destroyed its own sticky comment. Writing both keeps the comment findable + and, more usefully, puts the reason on the merge request instead of only in the job log. + """ + result = report.result_document( + "local", + "ERRORED", + {}, + policies_evaluated=0, + policies_errored=0, + policy_path=opts.policy_path, + policy_errors=[{"policy": None, "reason": message}], + ) + write_output_json(opts.output_json, result) + write_output_markdown( + opts.output_markdown, + report.render_markdown( + {}, + "ERRORED", + None, + marker=opts.comment_marker, + limit=opts.markdown_limit, + commit=opts.sha, + notes=[message], + ), + ) + return result + + +def run_check(opts): + """ + Evaluate the policy files at `--policy-path` and write the report. Raises LocalError. + + The scratch directory holds the masked document. It is a temporary directory rather than + somewhere under --source-dir, so that a caller who later packs its source tree cannot ship the + document we wrote beside the one the user committed. + """ + policies = discover_policies(opts.policy_path) + if not policies: + # The one outcome this whole mode must never produce is a green result for a change nothing + # was evaluated against. "No policies found" is not a skip. + raise LocalError( + f"Nothing to evaluate: no policy files found at '{opts.policy_path}'. Point " + "--policy-path at a file, a directory or a glob containing tirith policies." + ) + + if opts.infracost_path: + # Cost policies need the platform: local mode evaluates one document, and infracost output + # is a second one. Saying so beats evaluating the plan and reporting a cost policy as + # unevaluated with no explanation. + log( + "WARNING: --infracost-path is ignored in local mode, which evaluates a single " + "document. Cost policies need `tirith platform check`." + ) + + warnings = [] + + with tempfile.TemporaryDirectory(prefix="tirith-local-") as scratch: + input_path, redactions = prepare_input( + opts.input_path, + opts.plan_file, + opts.terraform_bin, + opts.input_kind, + opts.source_dir, + scratch, + state_path=opts.state_path, + ) + if redactions: + log(f"Masked {redactions} sensitive value(s) before evaluating") + + log(f"Evaluating {len(policies)} policy file(s) from '{opts.policy_path}'") + + def on_unknown_enforcement(value): + message = f"unrecognised meta.enforcement '{value}'; treating a failing policy as blocking" + warnings.append(message) + log(f"WARNING: {message}") + + policy_results, errored = evaluate(policies, input_path, on_unknown_enforcement=on_unknown_enforcement) + + for path, reason in errored: + log(f"WARNING: could not evaluate {path}: {reason}") + + # Rendered as a completed evaluation on purpose. Results genuinely were produced, and the + # renderer's ERRORED narrative ("the workflow run finished as ERRORED without producing policy + # results") would be simply untrue here. A policy that could not be evaluated is already a + # visible FAIL carrying its own reason, and the exit code is what actually gates. + result = report.result_document( + "local", + "COMPLETED", + policy_results, + policies_evaluated=len(policies), + policies_errored=len(errored), + policy_path=opts.policy_path, + policy_errors=[{"policy": path, "reason": reason} for path, reason in errored], + policy_warnings=warnings, + ) + + write_output_json(opts.output_json, result) + write_output_markdown( + opts.output_markdown, + report.render_markdown( + policy_results, + "COMPLETED", + None, + marker=opts.comment_marker, + limit=opts.markdown_limit, + commit=opts.sha, + ), + ) + + log(result["headline"]) + return result diff --git a/src/tirith/local/cli.py b/src/tirith/local/cli.py new file mode 100644 index 00000000..2289b90c --- /dev/null +++ b/src/tirith/local/cli.py @@ -0,0 +1,132 @@ +""" +`tirith local check` -- evaluate policy files committed in your repository, with no credentials. + +A sibling of `tirith platform check` rather than a flag on it. `platform check` masks, packs, +uploads, runs on StackGuardian and polls; a path that does none of those things under the same verb +would make its help text wrong for half its readers. Mechanically it would be worse still: +`--workflow-id` is required there, so a flag would have to lift that requirement out of argparse +into a hand-rolled conditional, weakening the platform path to accommodate a mode with no workflows. + +Local mode is also never entered implicitly. `platform check` with no credentials stays a hard error, +and a caller wanting "no credentials therefore local" implements that itself -- a front end that +guesses wrong evaluates whatever happens to be committed and reports green, which is exactly the +outcome a policy gate exists to prevent. + +Every identity, workflow, archive and run flag is deliberately absent rather than accepted and +ignored: a credential-shaped flag silently doing nothing in a credential-free mode is how someone +ends up with a green check their organization's policies never saw. +""" + +import argparse + +from ..platform.check import log +from ..status import ExitStatus +from .check import run_check, write_failure_report +from .evaluate import LocalError + +INPUT_KINDS = ("terraform_plan", "terraform_state", "kubernetes", "json") + +DEFAULT_POLICY_PATH = ".tirith/policies" + + +def build_parser(): + parser = argparse.ArgumentParser( + prog="tirith local", + description="Evaluate policy files committed in your repository. Talks to nothing.", + ) + sub = parser.add_subparsers(dest="subcommand") + + check = sub.add_parser( + "check", + help="Evaluate committed policy files against a document and report the verdict.", + description=( + "Masks the document, evaluates every policy found at --policy-path, and writes the same " + "result document and markdown report that `tirith platform check` writes. Requires no " + "credentials and makes no network calls." + ), + ) + + policies = check.add_argument_group("policies") + policies.add_argument( + "--policy-path", + default=DEFAULT_POLICY_PATH, + help=( + "A policy file, a directory, or a glob. A directory is searched recursively for " + f"*.tirith.json, or failing that for any .json file shaped like a policy. Default: " + f"{DEFAULT_POLICY_PATH}" + ), + ) + + inputs = check.add_argument_group("inputs") + inputs.add_argument("--input-path", default=None, help="Document to evaluate. Default: plan.json or tfplan.json.") + inputs.add_argument( + "--plan-file", default=None, help="Binary terraform plan, rendered in memory. Not with --input-path." + ) + inputs.add_argument("--terraform-bin", default=None, help="terraform/tofu binary used for --plan-file.") + inputs.add_argument("--input-kind", default="terraform_plan", choices=INPUT_KINDS, help="What the document is.") + inputs.add_argument("--state-path", default=None, help="Terraform state, when --input-kind is terraform_state.") + inputs.add_argument( + "--infracost-path", + default=None, + help="Accepted and ignored: cost policies need a second document, so they need platform mode.", + ) + inputs.add_argument("--source-dir", default=".", help="Where to look for the document. Default: .") + + run = check.add_argument_group("run") + run.add_argument("--sha", default=None, help="Revision these findings describe, recorded in the report.") + + output = check.add_argument_group("output") + output.add_argument("--output-json", default=None, help="Write the result document here.") + output.add_argument("--output-markdown", default=None, help="Write a markdown report here.") + output.add_argument("--comment-marker", default=None, help="Opaque first line of the markdown, for stickiness.") + output.add_argument("--markdown-limit", type=int, default=60000, help="Truncate the markdown to this length.") + output.add_argument( + "--fail-on-error", + action="store_true", + help=( + "Exit non-zero when a policy fails. A policy that could not be evaluated at all always " + "exits non-zero regardless of this flag." + ), + ) + + return parser + + +def main(argv): + parser = build_parser() + opts = parser.parse_args(argv[1:]) + + if opts.subcommand != "check": + parser.print_help() + return ExitStatus.SUCCESS + + try: + result = run_check(opts) + except LocalError as e: + # Fails closed, and leaves a report behind: a front end editing a sticky comment in place + # needs a marker-first body even on this path, or it orphans the comment it was updating. + log(f"ERROR: {e}") + write_failure_report(opts, str(e)) + return ExitStatus.ERROR + except KeyboardInterrupt: + log("Interrupted") + return ExitStatus.ERROR_CTRL_C + + # "Could not evaluate" is tool health, not a policy decision, so it ignores --fail-on-error + # exactly as an unreachable platform does in the other mode. + if result["policies_errored"]: + log("Some policies could not be evaluated") + return ExitStatus.ERROR + + verdict = result["verdict"] + if verdict == "errored": + log("The evaluation did not produce a verdict") + return ExitStatus.ERROR + if verdict == "failed" and opts.fail_on_error: + return ExitStatus.ERROR_POLICY_FAILED + if verdict == "failed": + log("Policies failed, but --fail-on-error was not set") + if result.get("counts", {}).get("approval_required"): + log("Some policies ask for approval; reported as a warning, which does not block") + + return ExitStatus.SUCCESS diff --git a/src/tirith/local/evaluate.py b/src/tirith/local/evaluate.py new file mode 100644 index 00000000..0e4e2f11 --- /dev/null +++ b/src/tirith/local/evaluate.py @@ -0,0 +1,333 @@ +""" +Discover policy files, mask the input document, and evaluate one policy at a time. + +Moved from the GitHub Action, where this was `tirith_action/local.py`, so that both front ends drive +one implementation. See the package docstring for why evaluation shells out. +""" + +import glob as _glob +import json +import os +import subprocess +import sys + +from ..platform import discover, redact, report + + +class LocalError(Exception): + """Local evaluation could not be completed. Always fails closed.""" + + +# Preferred policy naming. A directory holding any of these is treated as an explicit policy +# directory and nothing else in it is considered. +POLICY_SUFFIX = ".tirith.json" + +# Per policy. A policy is pure computation over a parsed document, so this only trips on a +# pathological evaluator, but a job that hangs forever is worse than one that fails. +EVALUATION_TIMEOUT = 300 + +# `meta.enforcement` values that downgrade a failing policy to a warning. Anything unrecognised +# fails instead -- an unlabelled or mislabelled policy must gate, not slip through. +# +# The approval spellings warn rather than gate, matching platform mode, where a policy carrying +# `onFail: APPROVAL_REQUIRED` also warns: the run finishes before the intent is known, so there is +# nothing to approve. Local mode has no approval mechanism at all, so failing closed on it would +# block a change with no way to unblock it. +WARN_ENFORCEMENTS = ( + "soft_mandatory", + "advisory", + "warn", + "warning", + "low", + "approval_required", + "approval-required", + "approval", +) + +# Recognised, and gate. Listed rather than left to the `else` so a correctly-labelled blocking +# policy does not raise the "unrecognised enforcement" warning on every run it fails -- +# `hard_mandatory` is what tirith's own golden test pins, so that fired constantly. +FAIL_ENFORCEMENTS = ("hard_mandatory", "mandatory", "fail", "error", "high", "critical", "blocking") + + +def _looks_like_policy(path): + """ + Whether a .json file is a tirith policy. + + Load-bearing rather than defensive. A policy directory routinely also holds the document under + evaluation (`plan.json`), and `--policy-path` accepts a glob that a user may well point at a + directory full of mixed JSON. Without this check the input document is evaluated *as a policy*, + which reports a spurious failure and buries the real findings. + """ + try: + with open(path) as f: + data = json.load(f) + except (json.JSONDecodeError, OSError, UnicodeDecodeError): + return False + return isinstance(data, dict) and "meta" in data and "evaluators" in data + + +def discover_policies(policy_path): + """ + Resolve `--policy-path` to a list of policy files. + + A file is taken as given -- if a user names one explicitly, evaluating it is the instruction, + and reporting "that is not a policy" is more useful than silently skipping it. + """ + if not policy_path: + return [] + + if os.path.isfile(policy_path): + return [policy_path] + + if "*" in policy_path or "?" in policy_path: + matches = sorted(p for p in _glob.glob(policy_path, recursive=True) if os.path.isfile(p)) + return [p for p in matches if _looks_like_policy(p)] + + if os.path.isdir(policy_path): + explicit = sorted(_glob.glob(os.path.join(policy_path, "**", "*" + POLICY_SUFFIX), recursive=True)) + if explicit: + return explicit + candidates = sorted(_glob.glob(os.path.join(policy_path, "**", "*.json"), recursive=True)) + return [p for p in candidates if _looks_like_policy(p)] + + return [] + + +def read_json(path, label): + """ + Read a JSON document, or raise LocalError. + + Deliberately near-identical to platform.check.read_json rather than shared with it: the two + raise different exception types, and ten lines is not worth an abstraction that would have to + take the exception class as a parameter. + """ + if not os.path.exists(path): + raise LocalError(f"{label} not found: {path}") + try: + with open(path) as f: + return json.load(f) + except json.JSONDecodeError as e: + raise LocalError(f"{label} is not valid JSON ({path}): {e}") + except OSError as e: + raise LocalError(f"Could not read {label} ({path}): {e}") + + +def prepare_input(input_path, plan_file, terraform_bin, input_kind, source_dir, scratch, state_path=None): + """ + Resolve the document to evaluate and mask it, returning (path, redaction_count). + + Masking matters here even though nothing is uploaded. Evaluator messages embed the actual + attribute values they compared, and those messages are copied verbatim into whatever comment or + note the caller posts -- so an unmasked local run publishes plan values to a code host. Masking + also keeps a local verdict identical to the platform one for the same plan, because the platform + path evaluates the masked document too. + + `json` and `kubernetes` documents are passed through untouched: they carry no sensitivity + markers to mask by, and tirith reads YAML for them, which a JSON round-trip here would break. + """ + if input_kind not in ("terraform_plan", "terraform_state"): + if not input_path: + raise LocalError(f"--input-path is required when --input-kind is '{input_kind}'.") + if not os.path.exists(input_path): + raise LocalError(f"input document not found: {input_path}") + return input_path, 0 + + if input_path and plan_file: + raise LocalError("--input-path and --plan-file cannot be combined; pass one of them.") + + # `--state-path` is how the platform path is told which document to evaluate for a + # terraform_state check. Ignoring it here sent local mode to discovery, which finds plan.json -- + # so the two modes evaluated *different documents* from identical inputs, and a violation + # present only in the state was reported as a pass. + if input_kind == "terraform_state" and not input_path and not plan_file and state_path: + input_path = state_path + + if plan_file: + try: + document = discover.terraform_show_json(plan_file, binary=terraform_bin or None) + except discover.DiscoveryError as e: + raise LocalError(str(e)) + elif input_path: + document = read_json(input_path, "input document") + else: + try: + resolved = discover.discover_input(source_dir or ".") + except discover.DiscoveryError as e: + raise LocalError(str(e)) + document = read_json(resolved, "input document") + + if input_kind == "terraform_state": + masked = redact.redact_state(document) + else: + masked = redact.redact_plan(document) + redactions = redact.count_redactions(masked) + + masked_path = os.path.join(scratch, "tirith-input.json") + try: + with open(masked_path, "w") as f: + json.dump(masked, f) + except OSError as e: + raise LocalError(f"Could not write the masked document to {masked_path}: {e}") + + return masked_path, redactions + + +def engine_argv(policy_path, input_path): + """ + The argv used to evaluate one policy. + + `sys.executable -m tirith` rather than a `tirith` on PATH, so the interpreter that evaluates is + the one whose renderer was imported above -- a `tirith` on PATH could be a different + installation entirely. Factored out so a test can pin it: see the package docstring for why this + must stay a subprocess against the frozen `--json` contract. + """ + return [sys.executable, "-m", "tirith", "-policy-path", policy_path, "-input-path", input_path] + + +def _evaluate_one(policy_path, input_path): + """ + Run one policy. Returns (document, error_message); exactly one is set. + + `--json` disables logging process-wide and prints a bare `{}` on failure, so the failure reason + is not on either stream. When that happens the policy is re-run without `--json` purely to + recover a message worth showing -- one extra subprocess, only ever on the error path. + """ + argv = engine_argv(policy_path, input_path) + + try: + completed = subprocess.run(argv + ["--json"], capture_output=True, text=True, timeout=EVALUATION_TIMEOUT) + except subprocess.TimeoutExpired: + return None, f"evaluation timed out after {EVALUATION_TIMEOUT}s" + except OSError as e: + return None, f"could not run tirith: {e}" + + if completed.returncode != 0: + return None, _recover_error(argv) + + try: + document = json.loads(completed.stdout) + except json.JSONDecodeError: + return None, "tirith produced no parseable result document" + + if not isinstance(document, dict) or not document: + return None, _recover_error(argv) + + # A bare {"errors": [...]} with no final_result is what unresolved policy variables produce. + if "final_result" not in document: + return None, "; ".join(str(e) for e in document.get("errors") or []) or "no result was produced" + + errors = document.get("errors") or [] + if errors: + return None, "; ".join(str(e) for e in errors) + + return document, None + + +def _recover_error(argv): + """Re-run without --json to get a human-readable reason out of the logger.""" + try: + completed = subprocess.run(argv, capture_output=True, text=True, timeout=EVALUATION_TIMEOUT) + except (subprocess.TimeoutExpired, OSError): + return "evaluation failed" + + stderr = (completed.stderr or "").strip() + if stderr: + # The logger writes a traceback for provider errors; the last line carries the cause. + return stderr.splitlines()[-1][:500] + return "evaluation failed" + + +def _fails(document): + """ + The failed evaluators, reshaped for the renderer. + + `description` is deliberately dropped rather than passed through. The renderer treats any entry + carrying a `description` key as a Checkov finding and reads only that field, skipping the + per-resource messages and addresses entirely -- and tirith's evaluator entries always carry the + key, often with a null value, which renders as a detail block with nothing in it at all. Sending + just `result` routes them down the branch written for this shape, which is where the actual + findings and the resource addresses come from. + + The evaluator description is not lost information worth keeping here: the rule name already + names the policy in both the table and the summary line, and the messages are specific to the + resources that failed. + """ + fails = [] + for evaluator in document.get("evaluators") or []: + if evaluator.get("passed"): + continue + entry = {"result": evaluator.get("result") or []} + if evaluator.get("id"): + entry["id"] = evaluator["id"] + fails.append(entry) + return fails + + +def _identity(document, policy_path): + """(policy_id, rule_name) for a result, falling back to the filename.""" + meta = document.get("meta") or {} + stem = os.path.basename(policy_path) + for suffix in (POLICY_SUFFIX, ".json"): + if stem.endswith(suffix): + stem = stem[: -len(suffix)] + break + policy_id = meta.get("id") or stem + return str(policy_id), str(meta.get("name") or policy_id) + + +def _failure_result(document, on_unknown_enforcement): + """FAIL, unless the policy labelled itself as advisory.""" + enforcement = (document.get("meta") or {}).get("enforcement") + if enforcement is None: + return report.FAIL + normalised = str(enforcement).strip().lower() + if normalised in WARN_ENFORCEMENTS: + return report.WARN + if normalised in FAIL_ENFORCEMENTS: + return report.FAIL + on_unknown_enforcement(str(enforcement)) + return report.FAIL + + +def evaluate(policy_paths, input_path, on_unknown_enforcement=lambda _: None): + """ + Evaluate every policy and build the PolicyEvalResults document the renderer consumes. + + Returns (policy_results, errored) where `errored` lists (policy, reason) pairs. A policy that + could not be evaluated becomes a FAIL rule carrying an `exec_err`, which the renderer surfaces + as `engine: `: visible in the report, distinguishable from a real violation, and never + mistakable for a pass. The caller still fails regardless of --fail-on-error -- "could not + evaluate" is a tool failure, not a policy decision. + """ + policy_results = {} + errored = [] + + for policy_path in policy_paths: + document, error = _evaluate_one(policy_path, input_path) + + if error is not None: + policy_id, rule_name = _identity({}, policy_path) + rule = { + "rule_name": rule_name, + "result": report.FAIL, + "evaluations": {"fails": [{"exec_err": f"{policy_path}: {error}"}]}, + } + errored.append((policy_path, error)) + else: + policy_id, rule_name = _identity(document, policy_path) + final = document.get("final_result") + if final is None: + rule = {"rule_name": rule_name, "skip": True} + elif final: + rule = {"rule_name": rule_name, "result": report.PASS} + else: + rule = { + "rule_name": rule_name, + "result": _failure_result(document, on_unknown_enforcement), + "evaluations": {"fails": _fails(document)}, + } + + policy_results.setdefault(policy_id, []).append(rule) + + return policy_results, errored diff --git a/src/tirith/platform/check.py b/src/tirith/platform/check.py index 643bf627..22bdbb4a 100644 --- a/src/tirith/platform/check.py +++ b/src/tirith/platform/check.py @@ -503,6 +503,64 @@ def upload_state_document(client, opts, state): ) +def write_output_markdown(path, body): + """Best-effort, mirroring write_output_json: a report we could not write is not a failed check.""" + if not path: + return + try: + with open(path, "w") as f: + f.write(body) + except OSError as e: + log(f"WARNING: could not write {path}: {e}") + + +def write_failure_report(opts, message): + """ + Write both output files for a check that produced no verdict. + + These paths used to return having written nothing, so a caller fell through to a body of its own + with no marker in it -- and PATCHing that over a good sticky comment orphans it permanently. + Writing both keeps the comment findable and puts the reason on the pull request rather than only + in the job log. + + Any run identity already recorded is preserved: the pre-poll write puts it there precisely so a + timeout leaves the run discoverable, and replacing it with nulls would throw that away. + """ + run_id = run_url = None + if opts.output_json and os.path.exists(opts.output_json): + try: + with open(opts.output_json) as f: + previous = json.load(f) + run_id = previous.get("wfrun_id") + run_url = previous.get("wfrun_url") + except (json.JSONDecodeError, OSError, AttributeError): + pass + + result = report.result_document( + "platform", + "ERRORED", + {}, + wfrun_id=run_id, + wfrun_url=run_url, + policy_errors=[{"policy": None, "reason": message}], + ) + write_output_json(opts.output_json, result) + + if opts.output_markdown: + body = report.render_markdown( + {}, + "ERRORED", + run_url, + marker=opts.comment_marker, + limit=opts.markdown_limit, + commit=opts.sha, + notes=[message], + ) + write_output_markdown(opts.output_markdown, body) + + return result + + def run_check(opts): """ Execute the check. Returns the result document. @@ -599,8 +657,13 @@ def run_check(opts): ) log(f"Run created: {run_url}") - # Written before polling so a timeout still leaves the run discoverable. - write_output_json(opts.output_json, {"status": "RUNNING", "wfrun_id": run_id, "wfrun_url": run_url}) + # Written before polling so a timeout still leaves the run discoverable. A full document rather + # than the three keys it used to carry: a consumer must not have to special-case a partial shape, + # and `verdict` is honestly `errored` until a run has produced one. + write_output_json( + opts.output_json, + report.result_document("platform", "RUNNING", {}, wfrun_id=run_id, wfrun_url=run_url), + ) try: status, _run = client.wait_for_run( @@ -659,42 +722,17 @@ def run_check(opts): # a view that serves only GET and POST. log(f"Retained the project archive for autofix: {key}") - counts, _findings = report.summarize(policy_results) - verdict_value = report.verdict(counts, status) - - result = { - "status": status, - "verdict": verdict_value, - "counts": { - "passed": counts.get(report.PASS, 0), - "failed": counts.get(report.FAIL, 0), - "warned": counts.get(report.WARN, 0), - "approval_required": counts.get(report.APPROVAL_REQUIRED, 0), - "skipped": counts.get("SKIPPED", 0), - # Published so a consumer can tell "nothing failed" from "we could not read part of - # it". Without it an errored run reported failed: 0, which the action copies straight - # to its `failed` output. - "unknown": counts.get(report.UNKNOWN, 0), - }, - "headline": report.headline(counts, verdict_value), - "wfrun_id": run_id, - "wfrun_url": run_url, - "policy_results": policy_results or {}, - # Surfaced for a caller aggregating several units into one comment of their own. - "monthly_cost": (cost_breakdown or {}).get("totalMonthlyCost"), - # Where the evaluated source lives. The autofix system reads this to fetch what produced - # the findings; it is also recorded on the run itself as SGCustomWorkflowRunFacts, so a - # consumer holding only a run id can find it without seeing this document. - "archive_key": key, - # Whether that archive actually contains the source. Normally true, and false when the tree - # was too large and got dropped so the check could still run. A consumer must not assume: - # "no code in the bundle" and "no code was wanted" need to be distinguishable. - # Derived from what the archive actually holds, not from what was asked for: a tree whose - # every file was excluded packs nothing, and this must not then claim otherwise while - # metadata.json says `present: false`. - "source_packed": bool(manifest.get("files")), - "source_skipped_reason": source_skipped, - } + result = report.result_document( + "platform", + status, + policy_results, + wfrun_id=run_id, + wfrun_url=run_url, + monthly_cost=(cost_breakdown or {}).get("totalMonthlyCost"), + archive_key=key, + source_packed=bool(manifest.get("files")), + source_skipped_reason=source_skipped, + ) write_output_json(opts.output_json, result) @@ -708,11 +746,7 @@ def run_check(opts): cost_breakdown=cost_breakdown, commit=opts.sha, ) - try: - with open(opts.output_markdown, "w") as f: - f.write(body) - except OSError as e: - log(f"WARNING: could not write {opts.output_markdown}: {e}") + write_output_markdown(opts.output_markdown, body) log(result["headline"]) return result diff --git a/src/tirith/platform/cli.py b/src/tirith/platform/cli.py index c4070b3e..60f4496c 100644 --- a/src/tirith/platform/cli.py +++ b/src/tirith/platform/cli.py @@ -14,7 +14,7 @@ from ..status import ExitStatus from . import discover, regions -from .check import DEFAULT_WORKFLOW_GROUP, INPUT_KINDS, CheckError, log, run_check +from .check import DEFAULT_WORKFLOW_GROUP, INPUT_KINDS, CheckError, log, run_check, write_failure_report # `Id` is a DRF SlugField on the platform, and the value is interpolated into every API path. WORKFLOW_ID_PATTERN = re.compile(r"^[A-Za-z0-9_-]{1,100}$") @@ -259,8 +259,10 @@ def main(argv): result = run_check(opts) except CheckError as e: # Fails closed: a run that produced no verdict must never look like a pass, whatever - # --fail-on-error says. + # --fail-on-error says. Both output files are still written, so a caller editing a sticky + # comment in place has a marker-first body to write and does not orphan the comment. log(f"ERROR: {e}") + write_failure_report(opts, str(e)) return ExitStatus.ERROR except KeyboardInterrupt: log("Interrupted") diff --git a/src/tirith/platform/report.py b/src/tirith/platform/report.py index ffb15cd5..e937570a 100644 --- a/src/tirith/platform/report.py +++ b/src/tirith/platform/report.py @@ -255,8 +255,94 @@ def render_cost(breakdown): return ["", f"{line}"] +def result_document( + mode, + status, + policy_results, + wfrun_id=None, + wfrun_url=None, + monthly_cost=None, + archive_key=None, + source_packed=False, + source_skipped_reason=None, + policies_evaluated=None, + policies_errored=None, + policy_path=None, + policy_errors=None, + policy_warnings=None, +): + """ + The machine-readable result document, for --output-json. + + Built here, for both modes, so the two cannot drift. Every key is present in every mode: a key + with no meaning on one path is None (or [] for the lists), never omitted and never invented. + Omitting them would force every consumer -- the GitHub Action, the GitLab component -- to write + two parsers, and inventing a `wfrun_url` in local mode would point at a run that does not exist. + + `mode` is the discriminator: "platform" or "local". Consumers should read it from here rather + than infer it from which keys are populated. + """ + counts, _findings = summarize(policy_results) + verdict_value = verdict(counts, status) + + return { + "mode": mode, + "status": status, + "verdict": verdict_value, + "counts": { + "passed": counts.get(PASS, 0), + "failed": counts.get(FAIL, 0), + "warned": counts.get(WARN, 0), + "approval_required": counts.get(APPROVAL_REQUIRED, 0), + "skipped": counts.get("SKIPPED", 0), + # Published so a consumer can tell "nothing failed" from "we could not read part of + # it". Without it an errored run reported failed: 0, which the action copies straight + # to its `failed` output. + "unknown": counts.get(UNKNOWN, 0), + }, + "headline": headline(counts, verdict_value), + "policy_results": policy_results or {}, + # Platform mode only: nothing is recorded on the platform in local mode. + "wfrun_id": wfrun_id, + "wfrun_url": wfrun_url, + # Surfaced for a caller aggregating several units into one report of their own. Cost needs a + # second document, which local mode does not evaluate, so it is None there. + "monthly_cost": monthly_cost, + # Where the evaluated source lives. The autofix system reads this to fetch what produced + # the findings; it is also recorded on the run itself as SGCustomWorkflowRunFacts, so a + # consumer holding only a run id can find it without seeing this document. + "archive_key": archive_key, + # Whether that archive actually contains the source. Normally true in platform mode, and + # false when the tree was too large and got dropped so the check could still run. A consumer + # must not assume: "no code in the bundle" and "no code was wanted" need to be + # distinguishable. Derived from what the archive actually holds, not from what was asked + # for: a tree whose every file was excluded packs nothing, and this must not then claim + # otherwise while metadata.json says `present: false`. Always false in local mode, which + # packs nothing at all. + "source_packed": source_packed, + # Left None rather than a sentinel string in local mode. A consumer's "the source was not + # uploaded" warning keys on truthiness, so a value like "not_applicable" would fire it on + # every local run. + "source_skipped_reason": source_skipped_reason, + # Local mode only. + "policies_evaluated": policies_evaluated, + "policies_errored": policies_errored, + "policy_path": policy_path, + # Structured rather than log-only, so a front end can annotate without scraping stderr. + "policy_errors": policy_errors or [], + "policy_warnings": policy_warnings or [], + } + + def render_markdown( - policy_results, run_status, run_url, marker=None, limit=COMMENT_LIMIT, cost_breakdown=None, commit=None + policy_results, + run_status, + run_url, + marker=None, + limit=COMMENT_LIMIT, + cost_breakdown=None, + commit=None, + notes=None, ): """ Render the results as markdown, truncating detail before the summary table. @@ -269,6 +355,11 @@ def render_markdown( place* across runs: without it a reader has no way to tell whether the verdict they are looking at is about the head of the branch or about a push from an hour ago. Rendered here rather than appended by the caller so the check-run summary and the job summary carry it too. + + `notes` are lines rendered under the header, in place of the generic narrative, when a caller + knows *why* there is no verdict. Without it a local evaluation that could not read its input + reported "the workflow run finished as ERRORED", which is untrue -- local mode has no run -- and + told the reader nothing about the actual cause. """ counts, findings = summarize(policy_results) verdict_value = verdict(counts, run_status) @@ -285,7 +376,12 @@ def render_markdown( # a run that produced NOTHING, and a run whose results included one this tool cannot read. # The second renders a populated table, under which "without producing policy results" # reads as a plain contradiction. - if counts.get(UNKNOWN): + # + # A caller that knows the real reason says so instead. Neither generic narrative fits a + # local evaluation, which has no workflow run to report the status of. + if notes: + header += [_html(str(note)) for note in notes] + [""] + elif counts.get(UNKNOWN): header += [ f"{counts[UNKNOWN]} policy result(s) could not be read, so this run has no verdict.", "This is reported as a failure rather than a pass: partial results are not a clean bill of health.", diff --git a/tests/cli/test_dispatch.py b/tests/cli/test_dispatch.py index b0dc8ab0..1e3df9ed 100644 --- a/tests/cli/test_dispatch.py +++ b/tests/cli/test_dispatch.py @@ -101,10 +101,11 @@ def test_the_subcommand_names_are_exactly_these(capsys): `remote` is not quietly still accepted. `ui` was added alongside it later, on the same terms: dispatched before the flat parser so the - local surface and its golden-file output are untouched. The set is pinned rather than merely - checked for membership, so a new subcommand has to be a deliberate edit here. + local surface and its golden-file output are untouched. `local` joined on the same terms again. + The set is pinned rather than merely checked for membership, so a new subcommand has to be a + deliberate edit here. """ - assert cli.SUBCOMMANDS == {"platform", "ui"} + assert cli.SUBCOMMANDS == {"platform", "ui", "local"} status = cli.main(["remote"]) diff --git a/tests/local/conftest.py b/tests/local/conftest.py new file mode 100644 index 00000000..2084eb00 --- /dev/null +++ b/tests/local/conftest.py @@ -0,0 +1,121 @@ +"""Fixtures shared by the local-mode tests.""" + +import json +import os +import sys + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))), "src")) + +# A policy that fails on the plan below, and passes when the instance type is t3.micro. +POLICY = { + "meta": { + "id": "instance-type", + "name": "instance types are approved", + "required_provider": "stackguardian/terraform_plan", + "version": "v1", + }, + "evaluators": [ + { + "id": "ev", + "description": "instance_type must be t3.micro", + "condition": {"type": "Equals", "value": "t3.micro", "error_tolerance": 0}, + "provider_args": { + "operation_type": "attribute", + "terraform_resource_attribute": "instance_type", + "terraform_resource_type": "aws_instance", + }, + } + ], + "eval_expression": "ev", +} + + +def plan(instance_type="m5.24xlarge", secret=None): + """ + A one-resource terraform plan. + + `secret` lands in `after` *and* is named in `after_sensitive`, which is the shape masking exists + for -- a value terraform itself marked sensitive. + """ + after = {"instance_type": instance_type} + after_sensitive = {} + if secret is not None: + after["password"] = secret + after_sensitive["password"] = True + return { + "format_version": "1.2", + "terraform_version": "1.5.7", + "resource_changes": [ + { + "address": "aws_instance.app", + "mode": "managed", + "type": "aws_instance", + "name": "app", + "change": { + "actions": ["create"], + "before": None, + "after": after, + "after_sensitive": after_sensitive, + }, + } + ], + } + + +@pytest.fixture +def workspace(tmp_path): + """A directory holding one policy and one plan, with helpers to vary either.""" + + class Workspace: + def __init__(self, root): + self.root = root + self.policies = root / "policies" + self.policies.mkdir() + + def policy(self, name="instance-type.tirith.json", document=None): + path = self.policies / name + path.write_text(json.dumps(POLICY if document is None else document)) + return path + + def raw(self, name, text): + path = self.policies / name + path.write_text(text) + return path + + def plan(self, name="plan.json", **kwargs): + path = self.root / name + path.write_text(json.dumps(plan(**kwargs))) + return path + + def argv(self, *extra): + return [ + "local", + "check", + "--policy-path", + str(self.policies), + "--input-path", + str(self.root / "plan.json"), + "--output-json", + str(self.root / "out.json"), + "--output-markdown", + str(self.root / "out.md"), + ] + list(extra) + + def result(self): + with open(self.root / "out.json") as f: + return json.load(f) + + def markdown(self): + with open(self.root / "out.md") as f: + return f.read() + + return Workspace(tmp_path) + + +def policy_with_enforcement(value): + """The shared policy, relabelled with a `meta.enforcement` value.""" + document = json.loads(json.dumps(POLICY)) + document["meta"]["enforcement"] = value + return document diff --git a/tests/local/test_local_cli.py b/tests/local/test_local_cli.py new file mode 100644 index 00000000..25397a86 --- /dev/null +++ b/tests/local/test_local_cli.py @@ -0,0 +1,183 @@ +""" +`tirith local check`, end to end through the real CLI. + +The exit-code matrix is the point of this file. Local mode is a *gate*, and the only thing that +gates is the exit code, so every branch of it is pinned here rather than inferred from the result +document. +""" + +import json + +from conftest import policy_with_enforcement + +from tirith.cli import main +from tirith.status import ExitStatus + + +def test_a_failing_policy_exits_zero_by_default(workspace): + """ + Matches `platform check`: the report appears, the job stays green. Useful while someone is + writing their first policies, and the default a caller can rely on not changing. + """ + workspace.policy() + workspace.plan() + + assert main(workspace.argv()) == ExitStatus.SUCCESS + assert workspace.result()["verdict"] == "failed" + + +def test_a_failing_policy_exits_three_with_fail_on_error(workspace): + workspace.policy() + workspace.plan() + + assert main(workspace.argv("--fail-on-error")) == ExitStatus.ERROR_POLICY_FAILED + + +def test_a_passing_policy_exits_zero(workspace): + workspace.policy() + workspace.plan(instance_type="t3.micro") + + assert main(workspace.argv("--fail-on-error")) == ExitStatus.SUCCESS + assert workspace.result()["verdict"] == "passed" + + +def test_no_policies_is_a_failure_not_a_skip(workspace): + """ + The one outcome this mode must never produce is a green result for a change nothing was + evaluated against. Pointed at an empty directory, it fails -- and it fails whether or not + --fail-on-error was passed, because this is a configuration error, not a policy decision. + """ + workspace.plan() + + assert main(workspace.argv()) == ExitStatus.ERROR + assert main(workspace.argv("--fail-on-error")) == ExitStatus.ERROR + assert workspace.result()["verdict"] == "errored" + + +def test_a_policy_that_cannot_be_evaluated_fails_regardless_of_fail_on_error(workspace): + """ + "Could not evaluate" is tool health, not a policy decision, so it ignores --fail-on-error + exactly as an unreachable platform does in the other mode. Without this a broken policy file + reported a green job. + """ + workspace.policy(name="broken.tirith.json", document={"meta": {}, "evaluators": [{"nonsense": True}]}) + workspace.plan() + + assert main(workspace.argv()) == ExitStatus.ERROR + + result = workspace.result() + assert result["policies_errored"] == 1 + assert result["policy_errors"][0]["policy"].endswith("broken.tirith.json") + assert result["policy_errors"][0]["reason"] + + +def test_a_missing_input_document_fails_and_still_writes_a_report(workspace): + """ + Both output files are written on every exit path. A front end that edits a sticky comment in + place needs a marker-first body even here -- writing nothing is what orphaned the comment it was + updating. + """ + workspace.policy() + marker = "[//]: <> (tirith-comment, tag=default)" + + status = main(workspace.argv("--comment-marker", marker)) + + assert status == ExitStatus.ERROR + assert workspace.result()["verdict"] == "errored" + assert workspace.markdown().startswith(marker) + + +def test_the_failure_report_says_why_rather_than_naming_a_workflow_run(workspace): + """ + The renderer's generic errored narrative talks about "the workflow run", which local mode does + not have. The real reason is passed through instead. + """ + workspace.plan() + + main(workspace.argv()) + body = workspace.markdown() + + assert "workflow run" not in body + assert "no policy files found" in body + + +def test_the_marker_is_the_first_line_of_a_normal_report(workspace): + """Stickiness depends on it: a caller finds its own note by matching the marker as a prefix.""" + workspace.policy() + workspace.plan() + marker = "[//]: <> (tirith-comment, tag=envs-prod)" + + main(workspace.argv("--comment-marker", marker)) + + assert workspace.markdown().startswith(marker) + + +def test_an_advisory_policy_warns_instead_of_failing(workspace): + """`meta.enforcement` downgrades a failing policy, matching the platform's onFail handling.""" + workspace.policy(document=policy_with_enforcement("soft_mandatory")) + workspace.plan() + + assert main(workspace.argv("--fail-on-error")) == ExitStatus.SUCCESS + assert workspace.result()["verdict"] == "warned" + + +def test_an_unrecognised_enforcement_gates_and_says_so(workspace): + """ + An unlabelled or mislabelled policy must gate, not slip through -- but the reader has to be told, + or a typo in `enforcement` silently becomes policy. The notice is in the document as well as the + log so a front end can annotate without scraping stderr. + """ + workspace.policy(document=policy_with_enforcement("sort-of-important")) + workspace.plan() + + assert main(workspace.argv("--fail-on-error")) == ExitStatus.ERROR_POLICY_FAILED + + warnings = workspace.result()["policy_warnings"] + assert len(warnings) == 1 + assert "sort-of-important" in warnings[0] + + +def test_a_correctly_labelled_blocking_policy_raises_no_warning(workspace): + """ + `hard_mandatory` is what tirith's own golden test pins, so treating it as unrecognised fired the + notice on essentially every failing run. + """ + workspace.policy(document=policy_with_enforcement("hard_mandatory")) + workspace.plan() + + main(workspace.argv()) + + assert workspace.result()["policy_warnings"] == [] + + +def test_the_masked_document_is_what_gets_evaluated(workspace): + """ + Masking matters even though nothing is uploaded: evaluator messages embed the values they + compared, and those messages are copied verbatim into whatever note the caller posts. An + unmasked local run publishes plan values to a code host. + """ + workspace.policy() + workspace.plan(secret="hunter2-should-never-appear") + + main(workspace.argv()) + + assert "hunter2-should-never-appear" not in json.dumps(workspace.result()) + assert "hunter2-should-never-appear" not in workspace.markdown() + + +def test_infracost_path_is_accepted_and_reported_as_ignored(workspace, capsys): + """ + Cost policies need a second document, which this mode does not evaluate. Saying so beats + evaluating the plan and reporting a cost policy as unevaluated with no explanation. + """ + workspace.policy() + workspace.plan() + + main(workspace.argv("--infracost-path", str(workspace.root / "infracost.json"))) + + assert "--infracost-path is ignored in local mode" in capsys.readouterr().err + + +def test_local_check_with_no_subcommand_prints_help(capsys): + assert main(["local"]) == ExitStatus.SUCCESS + assert "tirith local" in capsys.readouterr().out diff --git a/tests/local/test_local_evaluate.py b/tests/local/test_local_evaluate.py new file mode 100644 index 00000000..a79b0bdc --- /dev/null +++ b/tests/local/test_local_evaluate.py @@ -0,0 +1,204 @@ +""" +Unit coverage for policy discovery, input preparation and the engine subprocess. + +The pinned-argv test near the bottom is the load-bearing one: it is what keeps the frozen `--json` +contract as the interface between this package and the engine, now that both live in the same +distribution and calling the engine's Python API directly has become easy. +""" + +import json +import sys + +import pytest +from conftest import POLICY, plan + +from tirith.local import evaluate +from tirith.local.evaluate import LocalError +from tirith.platform import report + + +def test_a_named_file_is_taken_as_given(tmp_path): + """ + Naming a file explicitly is an instruction. Reporting "that is not a policy" is more useful than + silently evaluating nothing. + """ + path = tmp_path / "not-really.json" + path.write_text("{}") + + assert evaluate.discover_policies(str(path)) == [str(path)] + + +def test_a_directory_prefers_the_tirith_suffix(tmp_path): + (tmp_path / "a.tirith.json").write_text(json.dumps(POLICY)) + (tmp_path / "b.json").write_text(json.dumps(POLICY)) + + found = evaluate.discover_policies(str(tmp_path)) + + assert [p.rsplit("/", 1)[-1] for p in found] == ["a.tirith.json"] + + +def test_a_directory_without_the_suffix_falls_back_to_shape(tmp_path): + (tmp_path / "policy.json").write_text(json.dumps(POLICY)) + + found = evaluate.discover_policies(str(tmp_path)) + + assert [p.rsplit("/", 1)[-1] for p in found] == ["policy.json"] + + +def test_the_input_document_sitting_in_the_policy_directory_is_not_evaluated_as_a_policy(tmp_path): + """ + Load-bearing rather than defensive. A policy directory routinely also holds the document under + evaluation; without the shape filter the plan is evaluated *as a policy*, which reports a + spurious failure and buries the real findings. + """ + (tmp_path / "policy.json").write_text(json.dumps(POLICY)) + (tmp_path / "plan.json").write_text(json.dumps(plan())) + + found = evaluate.discover_policies(str(tmp_path)) + + assert [p.rsplit("/", 1)[-1] for p in found] == ["policy.json"] + + +def test_a_glob_is_filtered_by_shape_too(tmp_path): + (tmp_path / "policy.json").write_text(json.dumps(POLICY)) + (tmp_path / "plan.json").write_text(json.dumps(plan())) + + found = evaluate.discover_policies(str(tmp_path / "*.json")) + + assert [p.rsplit("/", 1)[-1] for p in found] == ["policy.json"] + + +def test_an_empty_policy_path_finds_nothing(tmp_path): + assert evaluate.discover_policies("") == [] + assert evaluate.discover_policies(str(tmp_path / "nowhere")) == [] + + +def test_a_terraform_state_check_evaluates_the_state_not_a_discovered_plan(tmp_path): + """ + `--state-path` is how the platform path names the document for a terraform_state check. Ignoring + it sent local mode to discovery, which finds plan.json -- so the two modes evaluated *different + documents* from identical inputs, and a violation present only in the state was reported as a + pass. + """ + (tmp_path / "plan.json").write_text(json.dumps(plan())) + state = tmp_path / "state.json" + state.write_text( + json.dumps({"values": {"root_module": {"resources": [{"address": "aws_instance.only-in-state"}]}}}) + ) + + resolved, _ = evaluate.prepare_input( + None, None, None, "terraform_state", str(tmp_path), str(tmp_path), state_path=str(state) + ) + + with open(resolved) as f: + assert "only-in-state" in json.dumps(json.load(f)) + + +def test_a_json_document_is_passed_through_unmasked(tmp_path): + """ + `json` and `kubernetes` carry no sensitivity markers to mask by, and tirith reads YAML for them, + which a JSON round-trip here would break. So the original path is returned, not a copy. + """ + document = tmp_path / "anything.json" + document.write_text(json.dumps({"hello": "world"})) + + resolved, redactions = evaluate.prepare_input(str(document), None, None, "json", str(tmp_path), str(tmp_path)) + + assert resolved == str(document) + assert redactions == 0 + + +def test_a_json_document_must_be_named(tmp_path): + with pytest.raises(LocalError, match="--input-path is required"): + evaluate.prepare_input(None, None, None, "json", str(tmp_path), str(tmp_path)) + + +def test_input_path_and_plan_file_cannot_be_combined(tmp_path): + with pytest.raises(LocalError, match="cannot be combined"): + evaluate.prepare_input("a.json", "tfplan", None, "terraform_plan", str(tmp_path), str(tmp_path)) + + +def test_a_missing_document_raises_rather_than_evaluating_nothing(tmp_path): + with pytest.raises(LocalError, match="not found"): + evaluate.prepare_input(str(tmp_path / "gone.json"), None, None, "terraform_plan", str(tmp_path), str(tmp_path)) + + +def test_a_sensitive_value_is_masked_and_counted(tmp_path): + document = tmp_path / "plan.json" + document.write_text(json.dumps(plan(secret="hunter2"))) + + resolved, redactions = evaluate.prepare_input( + str(document), None, None, "terraform_plan", str(tmp_path), str(tmp_path) + ) + + assert redactions >= 1 + with open(resolved) as f: + assert "hunter2" not in f.read() + + +def test_the_engine_is_invoked_as_a_subprocess_against_the_frozen_json_contract(): + """ + Pinned deliberately. That stdout document is the most stable interface tirith has -- it is + byte-for-byte pinned by tests/core/test_output_compatibility.py -- whereas the engine's Python + API carries no such promise. Now that this code lives inside the package, calling + `start_policy_evaluation` directly would save an interpreter start per policy and silently couple + local mode to internals. This makes that a deliberate change with a red test, not a tidy-up. + + `sys.executable -m tirith` rather than a `tirith` on PATH: the interpreter that evaluates must be + the one whose renderer was imported, and a `tirith` on PATH could be a different installation. + """ + assert evaluate.engine_argv("p.json", "i.json") == [ + sys.executable, + "-m", + "tirith", + "-policy-path", + "p.json", + "-input-path", + "i.json", + ] + + +def test_an_unevaluable_policy_becomes_a_visible_fail_carrying_its_reason(tmp_path): + """ + Not a skip and not a silent drop: the renderer surfaces `exec_err` as `engine: `, so it + appears in the report, is distinguishable from a real violation, and is never mistakable for a + pass. + """ + broken = tmp_path / "broken.tirith.json" + broken.write_text(json.dumps({"meta": {}, "evaluators": [{"nonsense": True}]})) + document = tmp_path / "plan.json" + document.write_text(json.dumps(plan())) + + policy_results, errored = evaluate.evaluate([str(broken)], str(document)) + + assert len(errored) == 1 + rule = policy_results["broken"][0] + assert rule["result"] == report.FAIL + assert "exec_err" in rule["evaluations"]["fails"][0] + + +def test_the_enforcement_matrix(tmp_path): + """Every recognised spelling, plus the unrecognised case, in one place.""" + document = tmp_path / "plan.json" + document.write_text(json.dumps(plan())) + + for value, expected, expect_notice in ( + ("soft_mandatory", report.WARN, False), + ("advisory", report.WARN, False), + ("approval_required", report.WARN, False), + ("hard_mandatory", report.FAIL, False), + ("blocking", report.FAIL, False), + (None, report.FAIL, False), + ("who-knows", report.FAIL, True), + ): + policy = json.loads(json.dumps(POLICY)) + if value is not None: + policy["meta"]["enforcement"] = value + path = tmp_path / "p.tirith.json" + path.write_text(json.dumps(policy)) + + notices = [] + policy_results, _ = evaluate.evaluate([str(path)], str(document), on_unknown_enforcement=notices.append) + + assert policy_results["instance-type"][0]["result"] == expected, value + assert bool(notices) is expect_notice, value diff --git a/tests/local/test_output_contract.py b/tests/local/test_output_contract.py new file mode 100644 index 00000000..5afbdf23 --- /dev/null +++ b/tests/local/test_output_contract.py @@ -0,0 +1,200 @@ +""" +The two modes produce one document shape. + +Both front ends -- the GitHub Action and the GitLab CI component -- read `--output-json` and turn it +into comments, statuses and job outputs. If the key set differs by mode, every consumer has to write +two parsers, and the one that forgets reports the wrong thing on the mode it was not tested against. + +So the contract is: **every key exists in every mode.** A key with no meaning on a path is None (or +[] for the lists), never omitted and never invented. That is enforced by construction -- +`report.result_document` builds both -- and pinned here, because construction is only a guarantee +while both callers keep using it. +""" + +import json + +from conftest import POLICY, plan + +from tirith.cli import main +from tirith.platform import report + +# Keys read by the GitHub Action and the GitLab component. Adding one is fine; removing or renaming +# one breaks a released consumer, which is what this list is for. +CONSUMER_KEYS = { + "mode", + "status", + "verdict", + "counts", + "headline", + "policy_results", + "wfrun_id", + "wfrun_url", + "monthly_cost", + "archive_key", + "source_packed", + "source_skipped_reason", + "policies_evaluated", + "policies_errored", + "policy_path", + "policy_errors", + "policy_warnings", +} + +COUNT_KEYS = {"passed", "failed", "warned", "approval_required", "skipped", "unknown"} + + +def _platform_document(): + """What a platform run produces, without needing a platform.""" + return report.result_document( + "platform", + "COMPLETED", + {"some-policy": [{"rule_name": "a rule", "result": report.PASS}]}, + wfrun_id="abc123", + wfrun_url="https://app.stackguardian.io/...", + monthly_cost=12.5, + archive_key="orgs/acme/wfs/K/artifacts/bundle.tar.gz", + source_packed=True, + ) + + +def _local_document(workspace): + workspace.policy() + workspace.plan() + main(workspace.argv()) + return workspace.result() + + +def test_both_modes_carry_the_same_keys(workspace): + platform = _platform_document() + local = _local_document(workspace) + + assert set(platform) == set(local) + assert set(local) == CONSUMER_KEYS + assert set(platform["counts"]) == set(local["counts"]) == COUNT_KEYS + + +def test_the_mode_discriminator_is_the_mode(workspace): + """ + Consumers should read `mode` rather than infer it from which keys are populated -- inference is + what makes a new key a breaking change. + """ + assert _platform_document()["mode"] == "platform" + assert _local_document(workspace)["mode"] == "local" + + +def test_local_mode_invents_no_run(workspace): + """ + A fabricated wfrun_url would render as a link to a run that does not exist. None is the honest + value, and consumers already guard on it. + """ + local = _local_document(workspace) + + assert local["wfrun_id"] is None + assert local["wfrun_url"] is None + assert local["archive_key"] is None + assert local["monthly_cost"] is None + + +def test_local_mode_reports_no_source_without_claiming_a_reason(workspace): + """ + `source_skipped_reason` stays None rather than a sentinel like "not_applicable". A consumer's + "the terraform source was not uploaded" warning keys on truthiness, so a sentinel would fire it + on every local run -- about an upload local mode never attempts. + """ + local = _local_document(workspace) + + assert local["source_packed"] is False + assert local["source_skipped_reason"] is None + + +def test_platform_mode_leaves_the_local_only_keys_empty(workspace): + platform = _platform_document() + + assert platform["policies_evaluated"] is None + assert platform["policies_errored"] is None + assert platform["policy_path"] is None + assert platform["policy_errors"] == [] + assert platform["policy_warnings"] == [] + + +def test_the_document_is_json_serialisable_in_both_modes(workspace): + """ + It is written with json.dump. A tuple or a set slipping into `policy_errors` would fail at the + end of a long run, having already done the work. + """ + json.dumps(_platform_document()) + json.dumps(_local_document(workspace)) + + +def test_every_skipped_policy_still_reports_a_verdict_of_passed(workspace): + """ + A deliberate divergence, recorded rather than fixed here. + + A policy whose every check was skipped (`final_result` is None) counts as SKIPPED, and a run of + nothing-but-skips reports `passed` -- matching platform mode, which also counts skips separately + rather than failing on them. The *flat* surface disagrees: `tirith -policy-path ... -input-path + ... --fail-on-error` exits 1 for the same policy, on the grounds that "nothing ran" is not a pass + (see tests/cli/test_local_gating.py). + + Both readings are defensible -- the policies genuinely did not apply, and the report says "N + skipped" rather than hiding it -- so this is pinned to make the inconsistency visible instead of + letting either surface drift into the other by accident. It needs a decision across all three + surfaces, not a quiet change in one. + """ + counts, _ = report.summarize({"p": [{"rule_name": "r", "skip": True}]}) + + assert report.verdict(counts, "COMPLETED") == "passed" + + +def test_the_platform_failure_report_is_a_full_document_with_the_reason(tmp_path): + """ + `platform check` used to return from its error paths having written nothing, so a caller editing a + sticky comment in place fell through to a marker-less body of its own -- which, PATCHed over a + good comment, orphaned it permanently. Both files are now written on that path too, in the same + shape as every other document, carrying the reason rather than a generic narrative about a + workflow run. + """ + from tirith.platform import check + + class Opts: + output_json = str(tmp_path / "out.json") + output_markdown = str(tmp_path / "out.md") + comment_marker = "[//]: <> (tirith-comment, tag=default)" + markdown_limit = 60000 + sha = None + + result = check.write_failure_report(Opts, "the API could not be reached") + + assert set(result) == CONSUMER_KEYS + assert result["mode"] == "platform" + assert result["verdict"] == "errored" + assert result["policy_errors"] == [{"policy": None, "reason": "the API could not be reached"}] + + with open(Opts.output_markdown) as f: + body = f.read() + assert body.startswith(Opts.comment_marker) + assert "the API could not be reached" in body + assert "workflow run finished" not in body + + +def test_the_platform_failure_report_keeps_a_run_it_already_recorded(tmp_path): + """ + The pre-poll write records the run id precisely so a timeout leaves the run discoverable. + Overwriting it with nulls on the way out would throw away the only link to the run that timed out. + """ + from tirith.platform import check + + class Opts: + output_json = str(tmp_path / "out.json") + output_markdown = None + comment_marker = None + markdown_limit = 60000 + sha = None + + with open(Opts.output_json, "w") as f: + json.dump({"wfrun_id": "wfrun-1", "wfrun_url": "https://app.stackguardian.io/run/1"}, f) + + result = check.write_failure_report(Opts, "timed out waiting for the run") + + assert result["wfrun_id"] == "wfrun-1" + assert result["wfrun_url"] == "https://app.stackguardian.io/run/1" diff --git a/tests/test_readme_is_current.py b/tests/test_readme_is_current.py index 7a02da86..85c32a1a 100644 --- a/tests/test_readme_is_current.py +++ b/tests/test_readme_is_current.py @@ -117,6 +117,33 @@ def test_the_flag_reference_page_lists_every_flag_the_command_accepts(): assert not missing, f"flags accepted by `platform check` but absent from docs/platform-check.md: {missing}" +def test_the_local_check_reference_page_lists_every_flag_it_accepts(): + """ + The sibling of the check above, for the same reason: docs/local-check.md embeds the flag list, and + a flag added without touching it silently stops being documented. + """ + with open(os.path.join(ROOT, "docs", "local-check.md")) as f: + page = f.read() + + flags = set(re.findall(r"(? Date: Tue, 18 Aug 2026 19:08:38 +0700 Subject: [PATCH 2/2] [SG-320] refactor(local): put CI reporting on `tirith -policy-path` instead of a subcommand Replaces the `tirith local check` subcommand added in the previous commit. Evaluating policy files committed in a repository is what `tirith -policy-path ... -input-path ...` has always done, so the capability a CI job needs belongs on that command as flags rather than in a second command doing the same job slightly differently. Two surfaces for one task drift, and the one people already know is the one to improve. Three additions, all optional and all off unless asked for: * `-policy-path` accepts a directory or a glob and evaluates every policy it finds. This is new capability rather than changed behaviour: a directory previously reached `open()` and failed with a bare `ERROR`. `-var-path` and `-var` apply to every policy, so a parameterised policy behaves the same whether you name the file or its directory. * `--input-kind` masks the document before evaluation. It matters even with nothing uploaded, because evaluator messages embed the values they compared and those messages are copied into whatever comment a CI job posts. * `--output-json`, `--output-markdown`, `--comment-marker`, `--markdown-limit` and `--sha` write the verdict out in the shapes `platform check` already writes, so one CI integration drives either. Keeping the frozen contract frozen ---------------------------------- This command's `--json` stdout is pinned byte-for-byte by tests/core/test_output_compatibility.py, and masking changes the evaluator messages that document contains. So the routing is deliberately narrow: a single policy file with none of the new flags reaches the original code path untouched, which is why masking is opt-in rather than automatic. A test pins that routing in both directions. The all-skipped question, now decided ------------------------------------- The previous commit left this open, because the subcommand and the flat surface disagreed: a policy whose every check was skipped reported `passed` in one and exited 1 in the other. With one surface there is one answer, and it is the released one -- exit 1, "nothing was examined is not a pass". The report says so in words rather than rendering a green verdict beside a red exit code. This remains a deliberate difference from `platform check`, which counts skips separately and reports them as a pass. Both readings are defensible; a single surface disagreeing with itself depending on how many policies you pointed it at is not. Docs ---- docs/local-check.md becomes docs/evaluating-policy-files.md, and the two doc-currency guards in tests/test_readme_is_current.py follow it -- one of them caught a missing flag while this was being written. The Usage block in the README is regenerated from the real --help, and `--input-kind` takes a `KIND` metavar because spelling its four choices out pushed every option's help text far to the right. --- CHANGELOG.md | 36 ++- README.md | 89 ++++--- docs/evaluating-policy-files.md | 122 +++++++++ docs/local-check.md | 172 ------------- docs/output-contract.md | 2 +- docs/platform-check.md | 7 +- .../docs/tirith-usage/cli-reference.md | 17 +- .../tirith-usage/evaluating-policy-files.md | 136 ++++++++++ .../docs/tirith-usage/local-check.md | 185 ------------- .../docs/tirith-usage/output-contract.md | 4 +- documentation/sidebars.js | 2 +- src/tirith/cli.py | 92 +++++-- src/tirith/local/__init__.py | 7 +- src/tirith/local/check.py | 242 ++++++++++++------ src/tirith/local/cli.py | 132 ---------- src/tirith/local/evaluate.py | 26 +- tests/cli/test_dispatch.py | 11 +- tests/local/conftest.py | 23 +- tests/local/test_local_cli.py | 66 ++++- tests/local/test_local_evaluate.py | 5 +- tests/local/test_output_contract.py | 41 ++- tests/test_readme_is_current.py | 53 ++-- 22 files changed, 755 insertions(+), 715 deletions(-) create mode 100644 docs/evaluating-policy-files.md delete mode 100644 docs/local-check.md create mode 100644 documentation/docs/tirith-usage/evaluating-policy-files.md delete mode 100644 documentation/docs/tirith-usage/local-check.md delete mode 100644 src/tirith/local/cli.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e53c845..936db2b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,18 +12,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [1.3.0] - 2026-08-18 ### Added -- `tirith local check`: evaluate the policy files committed in your repository, with no credentials - and no network calls. It writes the *same* result document and markdown report as - `tirith platform check`, so a CI integration built against one works unchanged against the other, - and adding a StackGuardian organization later changes which policies apply rather than how - anything is wired. Moved in from the GitHub Action, which owned the only implementation, so that a - second front end does not fork it. - - Two behaviours that fail closed: no policy files found is an error rather than a skip, and a - policy that could not be evaluated exits 1 regardless of `--fail-on-error`, because "could not - evaluate" is tool health rather than a policy decision. - - Never entered implicitly. `tirith platform check` with no credentials stays an error rather than - falling back, because a fallback evaluates whatever happens to be committed and reports green - when a token was misspelled. +- `tirith -policy-path …` can be used as a CI gate, not only at a terminal. Three additions, all + optional and all off unless asked for, so an existing invocation behaves exactly as before -- which + matters because this command's `--json` output is a frozen contract: + - **`-policy-path` accepts a directory or a glob**, evaluating every policy it finds. A directory is + searched recursively for `*.tirith.json`, or failing that for any `.json` file shaped like a policy. + Pointing it at a directory previously failed with a bare `ERROR`. `-var-path` and `-var` apply to + every policy, so a parameterised policy behaves the same whether you name the file or its directory. + - **`--input-kind`** masks the document before evaluation. It matters even though nothing is uploaded: + evaluator messages embed the values they compared, and those messages are copied into whatever + comment a CI job posts. Opt-in because masking changes those messages, and they are the frozen + `--json` output. + - **`--output-json`, `--output-markdown`, `--comment-marker`, `--markdown-limit`, `--sha`** write the + verdict out for a job to publish, in the same shape `tirith platform check` writes. So one CI + integration drives either, and adding a StackGuardian organization later changes which policies + apply rather than how anything is wired. + + Two paths fail closed regardless of `--fail-on-error`, because both are tool health rather than policy + decisions: no policy files found at `-policy-path`, and a policy that could not be evaluated. A run + where every check was skipped also exits 1 -- nothing was examined, and this command has always + treated that as a failure rather than a pass. That last one is a deliberate difference from + `platform check`, which counts skips separately and reports them as a pass. + + This capability came from the GitHub Action, which owned the only implementation of it. It moved here + so a second front end -- the GitLab CI component -- drives one implementation rather than forking it. - `tirith ui`: an interactive interface with three tabs. - **Explorer** — read an evaluation's results down to the resource behind each one. The result document has always carried the resource address, the planned action and the before/after diff --git a/README.md b/README.md index f1bdc189..51f48fe4 100644 --- a/README.md +++ b/README.md @@ -186,33 +186,45 @@ Congratulations! Tirith has been setup in your system ## Usage ``` -usage: tirith [-h] [-policy-path PATH] [-input-path PATH] [-var-path PATH] - [-var PATH] [--json] [--verbose] [--fail-on-error] [--version] +usage: tirith [-h] [-policy-path PATH] [-input-path PATH] [-var-path PATH] [-var PATH] [--json] + [--verbose] [--fail-on-error] [--version] [--input-kind KIND] [--state-path PATH] + [--sha SHA] [--output-json PATH] [--output-markdown PATH] [--comment-marker TEXT] + [--markdown-limit N] Tirith (StackGuardian Policy Framework) options: - -h, --help show this help message and exit - -policy-path PATH Path containing Tirith policy as code - -input-path PATH Input file path - -var-path PATH Variable file path(s) - -var PATH Inline variable(s) - --json Only print the result in JSON form (useful for passing output to other programs) - --verbose Show detailed logs of from the run - --fail-on-error Exit 3 when a policy fails, instead of 0. Off by default for compatibility. - --version show program's version number and exit + -h, --help show this help message and exit + -policy-path PATH Path containing Tirith policy as code + -input-path PATH Input file path + -var-path PATH Variable file path(s) + -var PATH Inline variable(s) + --json Only print the result in JSON form (useful for passing output to other programs) + --verbose Show detailed logs of from the run + --fail-on-error Exit 3 when a policy fails, instead of 0. Off by default for compatibility. + --version show program's version number and exit + +reporting: + Write the verdict out for a CI job to publish. All optional. + + --input-kind KIND terraform_plan, terraform_state, kubernetes or json. Masks the input before evaluating + --state-path PATH Terraform state, when --input-kind is terraform_state + --sha SHA Revision these findings describe, recorded in the report + --output-json PATH Write the result document here (same shape as `tirith platform check`) + --output-markdown PATH Write a markdown report here, for a comment or a note + --comment-marker TEXT Opaque first line of the markdown, for comment stickiness + --markdown-limit N Truncate the markdown to this length. Default: 60000 Subcommands: tirith platform check --help Evaluate against the policies your StackGuardian organization enforces, rather than local files. - tirith local check --help Evaluate policy files committed in your repository, - with no credentials, and report the same verdict. tirith ui --help Explore results, build policies and experiment in an interactive interface. Needs the 'tui' extra. About Tirith: + * Abstract away the implementation complexity of policy engine underneath. * Simplify creation of declarative policies that are easy to read and interpret. * Provide a standard framework for scanning various configurations with granularity. @@ -448,42 +460,39 @@ discovery, the sticky pull-request comment, the check run and the exit codes for ## Evaluating committed policy files, in CI -`tirith local check` is the credential-free counterpart. It evaluates the policy files in your -repository and writes the *same* report `tirith platform check` writes — so a CI integration built on -one works unchanged on the other, and adding a StackGuardian organization later changes which -policies apply, not how anything is wired. +`tirith -policy-path … -input-path …` has always evaluated the policies in your repository. Three +additions make it usable as a CI gate rather than only at a terminal: ``` -tirith local check --policy-path .tirith/policies --input-path plan.json --fail-on-error +tirith -policy-path .tirith/policies -input-path plan.json \ + --input-kind terraform_plan --output-json result.json --output-markdown report.md \ + --fail-on-error ``` -Nothing leaves the machine. What you give up against platform mode is run history, the dashboard, -enforcing one policy set across every repository from one place, and cost policies — those need a -second document, so `--infracost-path` is accepted and reported as ignored. +**`-policy-path` accepts a directory or a glob**, not just one file, and evaluates every policy it +finds — a directory is searched for `*.tirith.json`, or failing that for any `.json` file shaped like a +policy. Pointing it at a directory previously failed with a bare `ERROR`. -| | | -|---|---| -| `--policy-path` | A file, a directory, or a glob. A directory is searched recursively for `*.tirith.json`, or failing that for any `.json` file shaped like a policy. Default `.tirith/policies` | -| `--input-path` / `--plan-file` | The document to evaluate, or a binary plan to render. Defaults to `plan.json` or `tfplan.json` | -| `--output-json` / `--output-markdown` | The machine-readable verdict and a markdown report, in the same shape both modes produce | -| `--comment-marker` | An opaque first line for the markdown, so a CI job can find and edit its own comment | -| `--fail-on-error` | Exit `3` when a policy fails, instead of `0` | +**`--input-kind` masks the document** before evaluation. That matters even though nothing is uploaded: +evaluator messages embed the values they compared, and those messages end up in whatever comment your +CI job posts. It is opt-in because masking changes those messages, and they are this command's `--json` +output, which is a frozen contract — with no `--input-kind` the file is read exactly as before. -Two behaviours worth knowing, both of which fail closed: +**`--output-json` / `--output-markdown` / `--comment-marker`** write the verdict out for a job to +publish, in the [same shape](docs/output-contract.md) `tirith platform check` writes. So a CI +integration works against either, and adding a StackGuardian organization later changes which policies +apply rather than how anything is wired. -- **No policy files is an error, not a skip.** Pointed at a path with nothing in it, the command exits - `1` rather than reporting a pass — a green result for a change nothing was evaluated against is the - one outcome this mode must never produce. -- **A policy that could not be evaluated exits `1` regardless of `--fail-on-error`.** "Could not - evaluate" is tool health, not a policy decision, so the flag does not apply to it. +Two behaviours worth knowing, both of which fail closed: -It is not entered implicitly: `tirith platform check` with no credentials stays an error rather than -quietly falling back here, because a fallback would evaluate whatever happens to be committed and -report green when a token was simply misspelled. A CI integration that wants "no credentials therefore -local" chooses that itself, deliberately. +- **No policy files found is an error, not a skip** — exit `1`, whether or not `--fail-on-error` was + passed. A green result for a change nothing was evaluated against is the one outcome this must never + produce. +- **A policy that could not be evaluated exits `1` regardless of `--fail-on-error`**, as does a run + where every check was skipped. "Could not evaluate" and "nothing was examined" are tool health, not + policy decisions. -Every flag is in [docs/local-check.md](docs/local-check.md) or `tirith local check --help`, and the -result document both modes write is in [docs/output-contract.md](docs/output-contract.md). +Full reference: [docs/evaluating-policy-files.md](docs/evaluating-policy-files.md). ## Example Tirith policies diff --git a/docs/evaluating-policy-files.md b/docs/evaluating-policy-files.md new file mode 100644 index 00000000..bcdd90df --- /dev/null +++ b/docs/evaluating-policy-files.md @@ -0,0 +1,122 @@ +# Evaluating policy files in your repository + +`tirith -policy-path … -input-path …` evaluates the policies committed in your repository. It needs no +account and makes no network calls, and it is the surface most open-source users are on. + +``` +tirith -policy-path .tirith/policies -input-path plan.json --fail-on-error +``` + +This page is about the flags that make it usable as a **CI gate**: several policies at once, the input +masked before its values reach a pull-request comment, and the verdict written out for a job to +publish. The alternative is +[`tirith platform check`](platform-check.md), which evaluates the policies a StackGuardian organization +enforces instead — and writes the [same result document](output-contract.md), so a CI integration built +against one works unchanged against the other. + +## Several policies at once + +`-policy-path` accepts a file, a **directory** or a **glob**. + +- **A file** is evaluated as given. Naming one explicitly is an instruction, so "that is not a policy" + is reported rather than the file being skipped. +- **A directory** is searched recursively for `*.tirith.json`. If it holds none, any `.json` file + *shaped* like a policy is used — an object with both `meta` and `evaluators`. +- **A glob** is expanded and filtered by that same shape test. + +The shape test is load-bearing rather than defensive: a policy directory routinely also holds the +document under evaluation. Without it, `plan.json` is evaluated *as a policy*, which reports a spurious +failure and buries the real findings. + +`-var-path` and `-var` apply to every policy evaluated, so a parameterised policy behaves the same +whether you name the file or the directory holding it. + +**No policy files found is an error, not a skip.** Pointed at a path with nothing in it, the command +exits `1`. A green result for a change nothing was evaluated against is the outcome this must never +produce, and "no policies found" is a configuration mistake rather than a deliberate skip. + +## Masking, and why it is opt-in + +`--input-kind` says what the document is, and supplying it **masks** the document before evaluation: +values terraform marked sensitive are replaced, root `variables` are dropped, `prior_state` is removed. + +It matters even though nothing is uploaded. Evaluator messages embed the actual attribute values they +compared, and those messages are copied verbatim into whatever comment or note your CI job posts — so +an unmasked run publishes plan values to a code host. Masking also keeps this verdict identical to +`platform check`'s for the same plan, because that path evaluates the masked document too. + +It is opt-in because masking changes the evaluator messages, and those messages are this command's +`--json` output, which is a frozen contract. With no `--input-kind` the file is read exactly as it +always has been. + +`terraform_plan` and `terraform_state` are masked. `kubernetes` and `json` are passed through +untouched: they carry no sensitivity markers to mask by, and tirith reads YAML for them, which a JSON +round-trip would break. + +## Writing the verdict out + +| | | +|---|---| +| `--output-json PATH` | The result document, in the [same shape](output-contract.md) `platform check` writes | +| `--output-markdown PATH` | A rendered report, for a pull-request comment or a merge-request note | +| `--comment-marker TEXT` | An opaque first line for the markdown, so a job can find and edit its own comment | +| `--markdown-limit N` | Truncate the markdown. Default 60000 | +| `--sha SHA` | The revision the findings describe, shown in the report | + +Both files are written on **every** exit path, including failures, and the markdown always begins with +`--comment-marker` when one is given. That matters to any caller that edits a sticky comment in place: +writing nothing on the failure path means the caller falls through to a body of its own with no marker +in it, and PATCHing that over a good comment orphans it permanently. It has happened. + +Nothing in this group changes what the command prints. + +## Exit codes + +| Code | Condition | +|---|---| +| 0 | Policies passed or warned. Also a failing policy without `--fail-on-error` | +| 1 | No policy files found; the input was missing or unparseable; a policy could not be evaluated; nothing was evaluated | +| 3 | A policy failed, with `--fail-on-error` | +| 130 | Interrupted | + +**A policy that could not be evaluated exits `1` regardless of `--fail-on-error`.** "Could not +evaluate" is tool health, not a policy decision. Such a policy also appears in the report as a visible +failure carrying its reason, so it is never mistakable for a pass and never silently dropped. + +**A run of nothing but skips exits `1`.** If every check was swallowed by its `error_tolerance` — +usually because the resources the policies name are not in the document — then nothing was examined, +and this command has always called that a failure rather than a pass. Note this is a deliberate +difference from `platform check`, which counts skips separately and reports them as a pass. + +## Flags + +The full surface, as the command prints it: + + usage: tirith [-h] [-policy-path PATH] [-input-path PATH] [-var-path PATH] [-var PATH] [--json] + [--verbose] [--fail-on-error] [--version] [--input-kind KIND] [--state-path PATH] + [--sha SHA] [--output-json PATH] [--output-markdown PATH] [--comment-marker TEXT] + [--markdown-limit N] + + Tirith (StackGuardian Policy Framework) + + options: + -h, --help show this help message and exit + -policy-path PATH Path containing Tirith policy as code + -input-path PATH Input file path + -var-path PATH Variable file path(s) + -var PATH Inline variable(s) + --json Only print the result in JSON form (useful for passing output to other programs) + --verbose Show detailed logs of from the run + --fail-on-error Exit 3 when a policy fails, instead of 0. Off by default for compatibility. + --version show program's version number and exit + + reporting: + Write the verdict out for a CI job to publish. All optional. + + --input-kind KIND terraform_plan, terraform_state, kubernetes or json. Masks the input before evaluating + --state-path PATH Terraform state, when --input-kind is terraform_state + --sha SHA Revision these findings describe, recorded in the report + --output-json PATH Write the result document here (same shape as `tirith platform check`) + --output-markdown PATH Write a markdown report here, for a comment or a note + --comment-marker TEXT Opaque first line of the markdown, for comment stickiness + --markdown-limit N Truncate the markdown to this length. Default: 60000 diff --git a/docs/local-check.md b/docs/local-check.md deleted file mode 100644 index 7dc32073..00000000 --- a/docs/local-check.md +++ /dev/null @@ -1,172 +0,0 @@ -# `tirith local check` - -Evaluate the policy files committed in your repository against a terraform plan, state document or -JSON document. No credentials, no network calls. - -It is the credential-free counterpart of -[`tirith platform check`](platform-check.md) and writes the *same* report, in the -[same shape](output-contract.md) — so a CI integration built against one works unchanged against the -other. Adding a StackGuardian organization later changes which policies apply, not how anything is -wired. - -``` -tirith local check --policy-path .tirith/policies --input-path plan.json --fail-on-error -``` - -## What it does - -1. **Discovers policies** at `--policy-path`. -2. **Masks the document**, exactly as platform mode does. Nothing is uploaded, so this is not about - transport — evaluator messages embed the values they compared, and those messages are copied - verbatim into whatever pull-request comment or merge-request note the caller posts. An unmasked - local run publishes plan values to a code host. Masking also keeps a local verdict identical to - the platform one for the same plan, because platform mode evaluates the masked document too. -3. **Evaluates each policy** in a subprocess, against tirith's frozen `--json` output contract. -4. **Writes the verdict**, optionally as JSON and markdown for a later CI step. - -## It is never entered implicitly - -`tirith platform check` with no credentials is an error, not a silent fallback to this command. A -fallback would evaluate whatever happens to be committed and report green when a token was simply -misspelled — the exact failure a policy gate exists to prevent. A CI integration that wants "no -credentials therefore local" makes that choice itself, deliberately, and can say so in its own log. - -For the same reason this command rejects every credential, workflow, archive and run flag rather than -accepting and ignoring them. A credential-shaped flag that silently does nothing is how someone ends -up with a green check their organization's policies never saw. - -## Policy discovery - -`--policy-path` accepts a file, a directory or a glob, and defaults to `.tirith/policies`. - -- **A file** is taken as given. Naming one explicitly is an instruction, so "that is not a policy" is - reported rather than the file being skipped. -- **A directory** is searched recursively for `*.tirith.json`. If it holds none, any `.json` file - *shaped* like a policy is used — an object with both `meta` and `evaluators`. -- **A glob** is expanded and filtered by that same shape test. - -The shape test is load-bearing, not defensive: a policy directory routinely also holds the document -under evaluation. Without it, `plan.json` is evaluated *as a policy*, which reports a spurious failure -and buries the real findings. - -**No policy files is an error.** Pointed at a path with nothing in it, the command exits `1` rather -than reporting a pass. A green result for a change nothing was evaluated against is the one outcome -this mode must never produce, and "no policies found" is a configuration mistake rather than a -deliberate skip. - -## Enforcement - -A failing policy fails. `meta.enforcement` downgrades it to a warning for these values: - -`soft_mandatory`, `advisory`, `warn`, `warning`, `low`, `approval_required`, `approval-required`, -`approval` - -and gates for these: - -`hard_mandatory`, `mandatory`, `fail`, `error`, `high`, `critical`, `blocking` - -The approval spellings warn rather than gate, matching platform mode, where a policy carrying -`onFail: APPROVAL_REQUIRED` also warns — the run finishes before the intent is known, so there is -nothing to approve. Local mode has no approval mechanism at all, so failing closed on it would block a -change with no way to unblock it. - -Anything else gates *and* raises a warning naming the value, in the log and in `policy_warnings` in -the result document. An unlabelled or mislabelled policy must gate rather than slip through, but a -typo in `enforcement` silently becoming policy is worse than a noisy one. - -Omitting `meta.enforcement` entirely gates, with no warning. - -## Cost policies need the platform - -`--infracost-path` is accepted and reported as ignored. Cost evaluation needs a second document -alongside the plan, and this mode evaluates one. Saying so beats evaluating the plan and reporting a -cost policy as unevaluated with no explanation. - -## Exit codes - -| Code | Condition | -|---|---| -| 0 | Policies passed or warned. Also a failing policy without `--fail-on-error` | -| 1 | No policy files found; the input document was missing or unparseable; a policy could not be evaluated; no verdict was produced | -| 3 | A policy failed, with `--fail-on-error` | -| 130 | Interrupted | - -**A policy that could not be evaluated exits `1` regardless of `--fail-on-error`.** "Could not -evaluate" is tool health, not a policy decision, so the flag does not govern it — the same reason an -unreachable platform ignores it in the other mode. Such a policy also appears in the report as a -visible failure carrying its reason, so it is never mistakable for a pass and never silently dropped. - -Note the `2` documented for platform mode (a run timeout) is unreachable here: there is no run to -time out. The per-policy timeout is 300 seconds, and a policy that hits it is one entry in -`policy_errors` among possibly several verdicts, so it exits `1`. - -## Both output files are always written - -`--output-json` and `--output-markdown` are written on **every** exit path, including failures, and -the markdown always begins with `--comment-marker` when one is given. - -This matters to any caller that edits a sticky comment in place: writing nothing on the failure path -means the caller falls through to a body of its own with no marker in it, and PATCHing that over a -good comment orphans it permanently. It has happened. Writing both files also puts the reason on the -merge request rather than only in the job log. - -## Flags - -The full surface, as the command prints it: - - usage: tirith local check [-h] [--policy-path POLICY_PATH] [--input-path INPUT_PATH] - [--plan-file PLAN_FILE] [--terraform-bin TERRAFORM_BIN] - [--input-kind {terraform_plan,terraform_state,kubernetes,json}] - [--state-path STATE_PATH] [--infracost-path INFRACOST_PATH] - [--source-dir SOURCE_DIR] [--sha SHA] [--output-json OUTPUT_JSON] - [--output-markdown OUTPUT_MARKDOWN] [--comment-marker COMMENT_MARKER] - [--markdown-limit MARKDOWN_LIMIT] [--fail-on-error] - - Masks the document, evaluates every policy found at --policy-path, and writes the same result - document and markdown report that `tirith platform check` writes. Requires no credentials and - makes no network calls. - - options: - -h, --help show this help message and exit - - policies: - --policy-path POLICY_PATH - A policy file, a directory, or a glob. A directory is searched - recursively for *.tirith.json, or failing that for any .json file shaped - like a policy. Default: .tirith/policies - - inputs: - --input-path INPUT_PATH - Document to evaluate. Default: plan.json or tfplan.json. - --plan-file PLAN_FILE - Binary terraform plan, rendered in memory. Not with --input-path. - --terraform-bin TERRAFORM_BIN - terraform/tofu binary used for --plan-file. - --input-kind {terraform_plan,terraform_state,kubernetes,json} - What the document is. - --state-path STATE_PATH - Terraform state, when --input-kind is terraform_state. - --infracost-path INFRACOST_PATH - Accepted and ignored: cost policies need a second document, so they need - platform mode. - --source-dir SOURCE_DIR - Where to look for the document. Default: . - - run: - --sha SHA Revision these findings describe, recorded in the report. - - output: - --output-json OUTPUT_JSON - Write the result document here. - --output-markdown OUTPUT_MARKDOWN - Write a markdown report here. - --comment-marker COMMENT_MARKER - Opaque first line of the markdown, for stickiness. - --markdown-limit MARKDOWN_LIMIT - Truncate the markdown to this length. - --fail-on-error Exit non-zero when a policy fails. A policy that could not be evaluated - at all always exits non-zero regardless of this flag. - -`--input-kind kubernetes` and `--input-kind json` are passed through unmasked: they carry no -sensitivity markers to mask by, and tirith reads YAML for them, which a JSON round-trip would break. -Both require `--input-path`. diff --git a/docs/output-contract.md b/docs/output-contract.md index c22802e9..22f8e909 100644 --- a/docs/output-contract.md +++ b/docs/output-contract.md @@ -1,7 +1,7 @@ # The result document `--output-json` writes one document, in one shape, from both -[`tirith platform check`](platform-check.md) and [`tirith local check`](local-check.md). This page is +[`tirith platform check`](platform-check.md) and [`tirith -policy-path …`](evaluating-policy-files.md). This page is the contract, because two integrations read it — the [GitHub Action](https://github.com/StackGuardian/tirith-iac-governance-action) and the GitLab CI component — and turn it into comments, statuses and job outputs. diff --git a/docs/platform-check.md b/docs/platform-check.md index eeeded2f..c59d5f4d 100644 --- a/docs/platform-check.md +++ b/docs/platform-check.md @@ -7,9 +7,10 @@ The [GitHub Action](https://github.com/StackGuardian/tirith-iac-governance-actio around this command. Use the action on GitHub; use this directly anywhere else — GitLab CI, a Makefile, a local shell. -No StackGuardian organization? [`tirith local check`](local-check.md) evaluates policy files committed -in your repository instead, with no credentials, and writes the same report. The document both -commands write is specified in [the result document](output-contract.md). +No StackGuardian organization? `tirith -policy-path … -input-path …` evaluates policy files committed +in your repository instead, with no credentials, and writes the same report — see +[evaluating policy files](evaluating-policy-files.md). The document both write is specified in +[the result document](output-contract.md). ## What it does diff --git a/documentation/docs/tirith-usage/cli-reference.md b/documentation/docs/tirith-usage/cli-reference.md index ecda5f6b..c6244743 100644 --- a/documentation/docs/tirith-usage/cli-reference.md +++ b/documentation/docs/tirith-usage/cli-reference.md @@ -19,15 +19,14 @@ tirith -policy-path policy.json -input-path plan.json Run with no arguments, `tirith` prints its help text and exits `0`. -There are two policy-evaluation subcommands, each with its own flags and its own page. -[`tirith platform check`](platform-check.md) evaluates against the policies a StackGuardian -organization enforces; [`tirith local check`](local-check.md) evaluates policy files committed in -your repository, with no credentials. Both write the same report, so a CI integration built against -one works unchanged against the other. - -The flags below are the flat surface — `tirith -policy-path … -input-path …` — which evaluates one -policy path and prints the engine's own output. `local check` is the CI-shaped version of the same -idea: it discovers many policies, masks the input, renders a report and gates on the result. +There is one subcommand, [`tirith platform check`](platform-check.md), which evaluates against the +policies a StackGuardian organization enforces instead of local files. It has its own flags and its own +page. + +The flags below are this command's own. Beyond evaluating a single policy file they also cover using it +as a CI gate — a directory of policies, a masked input, and the verdict written out for a job to publish +— which has [its own page](evaluating-policy-files.md). Both surfaces write the same +[result document](output-contract.md). ## Flags diff --git a/documentation/docs/tirith-usage/evaluating-policy-files.md b/documentation/docs/tirith-usage/evaluating-policy-files.md new file mode 100644 index 00000000..e52cace9 --- /dev/null +++ b/documentation/docs/tirith-usage/evaluating-policy-files.md @@ -0,0 +1,136 @@ +--- +id: evaluating-policy-files +title: Policy Files in CI +sidebar_label: Policy Files in CI +description: Using tirith -policy-path as a CI gate — a directory of policies, a masked input, and the verdict written out for a job to publish. +keywords: + - tirith + - ci + - policy as code + - gate +site_name: Tirith +slug: evaluating-policy-files/ +--- + +`tirith -policy-path … -input-path …` evaluates the policies committed in your repository. It needs no +account and makes no network calls, and it is the surface most open-source users are on. + +``` +tirith -policy-path .tirith/policies -input-path plan.json --fail-on-error +``` + +This page is about the flags that make it usable as a **CI gate**: several policies at once, the input +masked before its values reach a pull-request comment, and the verdict written out for a job to +publish. The alternative is +[`tirith platform check`](platform-check.md), which evaluates the policies a StackGuardian organization +enforces instead — and writes the [same result document](output-contract.md), so a CI integration built +against one works unchanged against the other. + +## Several policies at once + +`-policy-path` accepts a file, a **directory** or a **glob**. + +- **A file** is evaluated as given. Naming one explicitly is an instruction, so "that is not a policy" + is reported rather than the file being skipped. +- **A directory** is searched recursively for `*.tirith.json`. If it holds none, any `.json` file + *shaped* like a policy is used — an object with both `meta` and `evaluators`. +- **A glob** is expanded and filtered by that same shape test. + +The shape test is load-bearing rather than defensive: a policy directory routinely also holds the +document under evaluation. Without it, `plan.json` is evaluated *as a policy*, which reports a spurious +failure and buries the real findings. + +`-var-path` and `-var` apply to every policy evaluated, so a parameterised policy behaves the same +whether you name the file or the directory holding it. + +**No policy files found is an error, not a skip.** Pointed at a path with nothing in it, the command +exits `1`. A green result for a change nothing was evaluated against is the outcome this must never +produce, and "no policies found" is a configuration mistake rather than a deliberate skip. + +## Masking, and why it is opt-in + +`--input-kind` says what the document is, and supplying it **masks** the document before evaluation: +values terraform marked sensitive are replaced, root `variables` are dropped, `prior_state` is removed. + +It matters even though nothing is uploaded. Evaluator messages embed the actual attribute values they +compared, and those messages are copied verbatim into whatever comment or note your CI job posts — so +an unmasked run publishes plan values to a code host. Masking also keeps this verdict identical to +`platform check`'s for the same plan, because that path evaluates the masked document too. + +It is opt-in because masking changes the evaluator messages, and those messages are this command's +`--json` output, which is a frozen contract. With no `--input-kind` the file is read exactly as it +always has been. + +`terraform_plan` and `terraform_state` are masked. `kubernetes` and `json` are passed through +untouched: they carry no sensitivity markers to mask by, and tirith reads YAML for them, which a JSON +round-trip would break. + +## Writing the verdict out + +| | | +|---|---| +| `--output-json PATH` | The result document, in the [same shape](output-contract.md) `platform check` writes | +| `--output-markdown PATH` | A rendered report, for a pull-request comment or a merge-request note | +| `--comment-marker TEXT` | An opaque first line for the markdown, so a job can find and edit its own comment | +| `--markdown-limit N` | Truncate the markdown. Default 60000 | +| `--sha SHA` | The revision the findings describe, shown in the report | + +Both files are written on **every** exit path, including failures, and the markdown always begins with +`--comment-marker` when one is given. That matters to any caller that edits a sticky comment in place: +writing nothing on the failure path means the caller falls through to a body of its own with no marker +in it, and PATCHing that over a good comment orphans it permanently. It has happened. + +Nothing in this group changes what the command prints. + +## Exit codes + +| Code | Condition | +|---|---| +| 0 | Policies passed or warned. Also a failing policy without `--fail-on-error` | +| 1 | No policy files found; the input was missing or unparseable; a policy could not be evaluated; nothing was evaluated | +| 3 | A policy failed, with `--fail-on-error` | +| 130 | Interrupted | + +**A policy that could not be evaluated exits `1` regardless of `--fail-on-error`.** "Could not +evaluate" is tool health, not a policy decision. Such a policy also appears in the report as a visible +failure carrying its reason, so it is never mistakable for a pass and never silently dropped. + +**A run of nothing but skips exits `1`.** If every check was swallowed by its `error_tolerance` — +usually because the resources the policies name are not in the document — then nothing was examined, +and this command has always called that a failure rather than a pass. Note this is a deliberate +difference from `platform check`, which counts skips separately and reports them as a pass. + +## Flags + +The full surface, as the command prints it: + +```text +usage: tirith [-h] [-policy-path PATH] [-input-path PATH] [-var-path PATH] [-var PATH] [--json] + [--verbose] [--fail-on-error] [--version] [--input-kind KIND] [--state-path PATH] + [--sha SHA] [--output-json PATH] [--output-markdown PATH] [--comment-marker TEXT] + [--markdown-limit N] + +Tirith (StackGuardian Policy Framework) + +options: + -h, --help show this help message and exit + -policy-path PATH Path containing Tirith policy as code + -input-path PATH Input file path + -var-path PATH Variable file path(s) + -var PATH Inline variable(s) + --json Only print the result in JSON form (useful for passing output to other programs) + --verbose Show detailed logs of from the run + --fail-on-error Exit 3 when a policy fails, instead of 0. Off by default for compatibility. + --version show program's version number and exit + +reporting: + Write the verdict out for a CI job to publish. All optional. + + --input-kind KIND terraform_plan, terraform_state, kubernetes or json. Masks the input before evaluating + --state-path PATH Terraform state, when --input-kind is terraform_state + --sha SHA Revision these findings describe, recorded in the report + --output-json PATH Write the result document here (same shape as `tirith platform check`) + --output-markdown PATH Write a markdown report here, for a comment or a note + --comment-marker TEXT Opaque first line of the markdown, for comment stickiness + --markdown-limit N Truncate the markdown to this length. Default: 60000 +``` \ No newline at end of file diff --git a/documentation/docs/tirith-usage/local-check.md b/documentation/docs/tirith-usage/local-check.md deleted file mode 100644 index bffa8005..00000000 --- a/documentation/docs/tirith-usage/local-check.md +++ /dev/null @@ -1,185 +0,0 @@ ---- -id: local-check -title: Local Check -sidebar_label: Local Check -description: The tirith local check subcommand — evaluate policy files committed in your repository, with no credentials, and get the same report platform mode produces. -keywords: - - tirith - - local check - - policy as code - - ci -site_name: Tirith -slug: local-check/ ---- - -Evaluate the policy files committed in your repository against a terraform plan, state document or -JSON document. No credentials, no network calls. - -It is the credential-free counterpart of -[`tirith platform check`](platform-check.md) and writes the *same* report, in the -[same shape](output-contract.md) — so a CI integration built against one works unchanged against the -other. Adding a StackGuardian organization later changes which policies apply, not how anything is -wired. - -``` -tirith local check --policy-path .tirith/policies --input-path plan.json --fail-on-error -``` - -## What it does - -1. **Discovers policies** at `--policy-path`. -2. **Masks the document**, exactly as platform mode does. Nothing is uploaded, so this is not about - transport — evaluator messages embed the values they compared, and those messages are copied - verbatim into whatever pull-request comment or merge-request note the caller posts. An unmasked - local run publishes plan values to a code host. Masking also keeps a local verdict identical to - the platform one for the same plan, because platform mode evaluates the masked document too. -3. **Evaluates each policy** in a subprocess, against tirith's frozen `--json` output contract. -4. **Writes the verdict**, optionally as JSON and markdown for a later CI step. - -## It is never entered implicitly - -`tirith platform check` with no credentials is an error, not a silent fallback to this command. A -fallback would evaluate whatever happens to be committed and report green when a token was simply -misspelled — the exact failure a policy gate exists to prevent. A CI integration that wants "no -credentials therefore local" makes that choice itself, deliberately, and can say so in its own log. - -For the same reason this command rejects every credential, workflow, archive and run flag rather than -accepting and ignoring them. A credential-shaped flag that silently does nothing is how someone ends -up with a green check their organization's policies never saw. - -## Policy discovery - -`--policy-path` accepts a file, a directory or a glob, and defaults to `.tirith/policies`. - -- **A file** is taken as given. Naming one explicitly is an instruction, so "that is not a policy" is - reported rather than the file being skipped. -- **A directory** is searched recursively for `*.tirith.json`. If it holds none, any `.json` file - *shaped* like a policy is used — an object with both `meta` and `evaluators`. -- **A glob** is expanded and filtered by that same shape test. - -The shape test is load-bearing, not defensive: a policy directory routinely also holds the document -under evaluation. Without it, `plan.json` is evaluated *as a policy*, which reports a spurious failure -and buries the real findings. - -**No policy files is an error.** Pointed at a path with nothing in it, the command exits `1` rather -than reporting a pass. A green result for a change nothing was evaluated against is the one outcome -this mode must never produce, and "no policies found" is a configuration mistake rather than a -deliberate skip. - -## Enforcement - -A failing policy fails. `meta.enforcement` downgrades it to a warning for these values: - -`soft_mandatory`, `advisory`, `warn`, `warning`, `low`, `approval_required`, `approval-required`, -`approval` - -and gates for these: - -`hard_mandatory`, `mandatory`, `fail`, `error`, `high`, `critical`, `blocking` - -The approval spellings warn rather than gate, matching platform mode, where a policy carrying -`onFail: APPROVAL_REQUIRED` also warns — the run finishes before the intent is known, so there is -nothing to approve. Local mode has no approval mechanism at all, so failing closed on it would block a -change with no way to unblock it. - -Anything else gates *and* raises a warning naming the value, in the log and in `policy_warnings` in -the result document. An unlabelled or mislabelled policy must gate rather than slip through, but a -typo in `enforcement` silently becoming policy is worse than a noisy one. - -Omitting `meta.enforcement` entirely gates, with no warning. - -## Cost policies need the platform - -`--infracost-path` is accepted and reported as ignored. Cost evaluation needs a second document -alongside the plan, and this mode evaluates one. Saying so beats evaluating the plan and reporting a -cost policy as unevaluated with no explanation. - -## Exit codes - -| Code | Condition | -|---|---| -| 0 | Policies passed or warned. Also a failing policy without `--fail-on-error` | -| 1 | No policy files found; the input document was missing or unparseable; a policy could not be evaluated; no verdict was produced | -| 3 | A policy failed, with `--fail-on-error` | -| 130 | Interrupted | - -**A policy that could not be evaluated exits `1` regardless of `--fail-on-error`.** "Could not -evaluate" is tool health, not a policy decision, so the flag does not govern it — the same reason an -unreachable platform ignores it in the other mode. Such a policy also appears in the report as a -visible failure carrying its reason, so it is never mistakable for a pass and never silently dropped. - -Note the `2` documented for platform mode (a run timeout) is unreachable here: there is no run to -time out. The per-policy timeout is 300 seconds, and a policy that hits it is one entry in -`policy_errors` among possibly several verdicts, so it exits `1`. - -## Both output files are always written - -`--output-json` and `--output-markdown` are written on **every** exit path, including failures, and -the markdown always begins with `--comment-marker` when one is given. - -This matters to any caller that edits a sticky comment in place: writing nothing on the failure path -means the caller falls through to a body of its own with no marker in it, and PATCHing that over a -good comment orphans it permanently. It has happened. Writing both files also puts the reason on the -merge request rather than only in the job log. - -## Flags - -The full surface, as the command prints it: - -```text -usage: tirith local check [-h] [--policy-path POLICY_PATH] [--input-path INPUT_PATH] - [--plan-file PLAN_FILE] [--terraform-bin TERRAFORM_BIN] - [--input-kind {terraform_plan,terraform_state,kubernetes,json}] - [--state-path STATE_PATH] [--infracost-path INFRACOST_PATH] - [--source-dir SOURCE_DIR] [--sha SHA] [--output-json OUTPUT_JSON] - [--output-markdown OUTPUT_MARKDOWN] [--comment-marker COMMENT_MARKER] - [--markdown-limit MARKDOWN_LIMIT] [--fail-on-error] - -Masks the document, evaluates every policy found at --policy-path, and writes the same result -document and markdown report that `tirith platform check` writes. Requires no credentials and -makes no network calls. - -options: - -h, --help show this help message and exit - -policies: - --policy-path POLICY_PATH - A policy file, a directory, or a glob. A directory is searched - recursively for *.tirith.json, or failing that for any .json file shaped - like a policy. Default: .tirith/policies - -inputs: - --input-path INPUT_PATH - Document to evaluate. Default: plan.json or tfplan.json. - --plan-file PLAN_FILE - Binary terraform plan, rendered in memory. Not with --input-path. - --terraform-bin TERRAFORM_BIN - terraform/tofu binary used for --plan-file. - --input-kind {terraform_plan,terraform_state,kubernetes,json} - What the document is. - --state-path STATE_PATH - Terraform state, when --input-kind is terraform_state. - --infracost-path INFRACOST_PATH - Accepted and ignored: cost policies need a second document, so they need - platform mode. - --source-dir SOURCE_DIR - Where to look for the document. Default: . - -run: - --sha SHA Revision these findings describe, recorded in the report. - -output: - --output-json OUTPUT_JSON - Write the result document here. - --output-markdown OUTPUT_MARKDOWN - Write a markdown report here. - --comment-marker COMMENT_MARKER - Opaque first line of the markdown, for stickiness. - --markdown-limit MARKDOWN_LIMIT - Truncate the markdown to this length. - --fail-on-error Exit non-zero when a policy fails. A policy that could not be evaluated - at all always exits non-zero regardless of this flag. -``` -`--input-kind kubernetes` and `--input-kind json` are passed through unmasked: they carry no -sensitivity markers to mask by, and tirith reads YAML for them, which a JSON round-trip would break. -Both require `--input-path`. diff --git a/documentation/docs/tirith-usage/output-contract.md b/documentation/docs/tirith-usage/output-contract.md index 18293a3d..e54b466d 100644 --- a/documentation/docs/tirith-usage/output-contract.md +++ b/documentation/docs/tirith-usage/output-contract.md @@ -2,7 +2,7 @@ id: output-contract title: The Result Document sidebar_label: Result Document -description: The JSON document tirith writes with --output-json, identical in platform and local mode, and the contract CI integrations read it by. +description: The JSON document tirith writes with --output-json, identical whether policies come from your repository or your organization, and the contract CI integrations read it by. keywords: - tirith - output @@ -13,7 +13,7 @@ slug: output-contract/ --- `--output-json` writes one document, in one shape, from both -[`tirith platform check`](platform-check.md) and [`tirith local check`](local-check.md). This page is +[`tirith platform check`](platform-check.md) and [`tirith -policy-path …`](evaluating-policy-files.md). This page is the contract, because two integrations read it — the [GitHub Action](https://github.com/StackGuardian/tirith-iac-governance-action) and the GitLab CI component — and turn it into comments, statuses and job outputs. diff --git a/documentation/sidebars.js b/documentation/sidebars.js index c436594a..bf72e006 100644 --- a/documentation/sidebars.js +++ b/documentation/sidebars.js @@ -25,7 +25,7 @@ module.exports = { "tirith-usage/exit-codes", "tirith-usage/ci-integration", "tirith-usage/platform-check", - "tirith-usage/local-check", + "tirith-usage/evaluating-policy-files", "tirith-usage/output-contract", ] }, diff --git a/src/tirith/cli.py b/src/tirith/cli.py index 51366cc6..5667388e 100755 --- a/src/tirith/cli.py +++ b/src/tirith/cli.py @@ -46,14 +46,7 @@ def eprint(*args, **kwargs): # 3.9 and tirith supports 3.8 -- so tui/cli.py reports the missing extra rather than failing on # an import here. UI_SUBCOMMAND = "ui" - -# `local` is the credential-free sibling of `platform`: policy files committed in the repository, -# evaluated here, reported identically. It is a separate subcommand rather than a flag on -# `platform check` -- see tirith/local/cli.py for why -- and `platform check` with no credentials -# stays a hard error, so neither mode is ever entered by accident. -LOCAL_SUBCOMMAND = "local" - -SUBCOMMANDS = {SUBCOMMAND, UI_SUBCOMMAND, LOCAL_SUBCOMMAND} +SUBCOMMANDS = {SUBCOMMAND, UI_SUBCOMMAND} def main(args=None) -> ExitStatus: @@ -72,11 +65,6 @@ def main(args=None) -> ExitStatus: return tui_cli.main(argv) - if argv and argv[0] == LOCAL_SUBCOMMAND: - from tirith.local import cli as local_cli - - return local_cli.main(argv) - if argv and argv[0] in SUBCOMMANDS: from tirith.platform import cli as platform_cli @@ -97,8 +85,6 @@ def __init__(self, prog="PROG") -> None: tirith platform check --help Evaluate against the policies your StackGuardian organization enforces, rather than local files. - tirith local check --help Evaluate policy files committed in your repository, - with no credentials, and report the same verdict. tirith ui --help Explore results, build policies and experiment in an interactive interface. Needs the 'tui' extra. @@ -165,6 +151,74 @@ def __init__(self, prog="PROG") -> None: ) parser.add_argument("--version", action="version", version=__version__) + # Everything below is additive, and off unless asked for. With none of it, this command + # behaves exactly as it always has -- which matters because its `--json` stdout is a frozen + # contract, pinned byte-for-byte by tests/core/test_output_compatibility.py. + # + # What they add is the shape a CI integration needs: many policies rather than one, the input + # masked before its values reach a report, and the verdict written out as a document and a + # markdown body for a front end to publish. Those are the same four files + # `tirith platform check` writes, so a GitHub Action or a GitLab component can drive either + # with one argv and one parser. + reporting = parser.add_argument_group( + "reporting", "Write the verdict out for a CI job to publish. All optional." + ) + reporting.add_argument( + "--input-kind", + dest="inputKind", + # A metavar, because the formatter aligns every option to the widest one and spelling the + # choices out here pushed the whole help block far to the right. + metavar="KIND", + choices=("terraform_plan", "terraform_state", "kubernetes", "json"), + default=None, + # Masking is opt-in because it changes the evaluator messages, and those messages are + # the frozen --json output. Omitted, the file is read exactly as it always has been. + help="terraform_plan, terraform_state, kubernetes or json. Masks the input before evaluating", + ) + reporting.add_argument( + "--state-path", + metavar="PATH", + dest="statePath", + default=None, + help="Terraform state, when --input-kind is terraform_state", + ) + reporting.add_argument( + "--sha", + metavar="SHA", + dest="sha", + default=None, + help="Revision these findings describe, recorded in the report", + ) + reporting.add_argument( + "--output-json", + metavar="PATH", + dest="outputJson", + default=None, + help="Write the result document here (same shape as `tirith platform check`)", + ) + reporting.add_argument( + "--output-markdown", + metavar="PATH", + dest="outputMarkdown", + default=None, + help="Write a markdown report here, for a comment or a note", + ) + reporting.add_argument( + "--comment-marker", + metavar="TEXT", + dest="commentMarker", + default=None, + help="Opaque first line of the markdown, for comment stickiness", + ) + reporting.add_argument( + "--markdown-limit", + metavar="N", + type=int, + dest="markdownLimit", + default=60000, + help="Truncate the markdown to this length. Default: 60000", + ) + args = parser.parse_args(argv) if not argv: @@ -188,6 +242,14 @@ def __init__(self, prog="PROG") -> None: else: setup_logging(verbose=args.verbose) + # The aggregate path: many policies, or any of the reporting flags. A single policy file with + # none of them falls through to the original code below, byte-for-byte -- which is what keeps + # the frozen --json contract frozen, and every existing caller unaffected. + from tirith.local import check as local_check + + if local_check.wanted(args): + return local_check.run(args) + try: result = start_policy_evaluation(args.policyPath, args.inputPath, args.varPaths, args.inlineVars) diff --git a/src/tirith/local/__init__.py b/src/tirith/local/__init__.py index e494aa29..499eed68 100644 --- a/src/tirith/local/__init__.py +++ b/src/tirith/local/__init__.py @@ -1,5 +1,5 @@ """ -Local policy evaluation -- the credential-free path. +The aggregate path behind `tirith -policy-path ...` -- many policies, masked input, one report. Everything here runs on the machine it is invoked from and talks to nothing. Policy files committed in the repository are evaluated against a document, and the same report the platform path produces @@ -21,6 +21,7 @@ argv is pinned by a test so that becomes a deliberate change rather than a silent one. This began life in the GitHub Action, which needed to be usable before anyone had a StackGuardian -account. It moved here so a second front end (GitLab CI) drives one implementation rather than -forking it. +account. It moved here so a second front end (GitLab CI) drives one implementation rather than forking +it -- and onto the existing flat command rather than a subcommand of its own, so there is one way to +evaluate committed policy files rather than two that drift. """ diff --git a/src/tirith/local/check.py b/src/tirith/local/check.py index 7d61369d..2a8663a2 100644 --- a/src/tirith/local/check.py +++ b/src/tirith/local/check.py @@ -1,131 +1,213 @@ """ -Orchestration for `tirith local check`. +The aggregate path behind `tirith -policy-path ...`. -Lifted from the GitHub Action's `run_local`, with the GitHub-specific parts left behind. Writes the -same two files `tirith platform check` writes -- the result document and the report body -- so -everything a front end does downstream is identical in both modes. +The flat command has always evaluated exactly one policy file and printed the engine's own document. +That is the right shape for a person at a terminal and the wrong one for a CI job, which needs many +policies from a directory, the input masked before its values reach a pull-request comment, and one +verdict written out for something else to publish. + +This module is that second shape. It is reached only when the caller asks for it -- a policy path that +is not a single file, or any of the reporting flags -- so the original path, and the frozen `--json` +output it prints, are untouched. + +It writes the same two files `tirith platform check` writes, in the same shapes, so a front end can +drive either with one argv and one parser. """ +import os import tempfile from ..platform import report from ..platform.check import log, write_output_json, write_output_markdown from .evaluate import LocalError, discover_policies, evaluate, prepare_input +# The flags that mean "give me the CI shape". Grouped here rather than checked inline so that adding +# one cannot accidentally leave the routing behind. +REPORTING_ARGS = ("inputKind", "statePath", "sha", "outputJson", "outputMarkdown", "commentMarker") -def write_failure_report(opts, message): + +def wanted(args): """ - Write both output files for a run that produced no verdict. + Whether this path should handle the invocation. - These paths used to return without writing either file, so a front end fell through to a - marker-less body and destroyed its own sticky comment. Writing both keeps the comment findable - and, more usefully, puts the reason on the merge request instead of only in the job log. + Narrow on purpose. A single policy file with none of the reporting flags is the invocation every + existing caller makes, and it must keep reaching the original code -- so this returns False for it + even though this module could handle it perfectly well. """ - result = report.result_document( - "local", - "ERRORED", - {}, - policies_evaluated=0, - policies_errored=0, - policy_path=opts.policy_path, - policy_errors=[{"policy": None, "reason": message}], - ) - write_output_json(opts.output_json, result) - write_output_markdown( - opts.output_markdown, - report.render_markdown( - {}, - "ERRORED", - None, - marker=opts.comment_marker, - limit=opts.markdown_limit, - commit=opts.sha, - notes=[message], - ), - ) - return result + if any(getattr(args, name, None) for name in REPORTING_ARGS): + return True + # A directory or a glob. Previously this reached `open()` and failed with a bare "ERROR", so + # handling it is new capability rather than changed behaviour. + return not os.path.isfile(args.policyPath) -def run_check(opts): - """ - Evaluate the policy files at `--policy-path` and write the report. Raises LocalError. +def run(args): + """Evaluate every policy found, write the report, and return an exit status.""" + from ..status import ExitStatus - The scratch directory holds the masked document. It is a temporary directory rather than - somewhere under --source-dir, so that a caller who later packs its source tree cannot ship the - document we wrote beside the one the user committed. - """ - policies = discover_policies(opts.policy_path) + policy_path = args.policyPath + + try: + result = _evaluate(args, policy_path) + except LocalError as e: + log(f"ERROR: {e}") + _write_failure(args, str(e)) + return ExitStatus.ERROR + except KeyboardInterrupt: + log("Interrupted") + return ExitStatus.ERROR_CTRL_C + + if args.json: + # The aggregate document, not the engine's -- there is no single engine document when several + # policies ran. Only reachable once a reporting flag or a multi-policy path was asked for, so + # no existing caller sees this instead of what it used to get. + import json as _json + + print(_json.dumps(result, indent=2)) + + log(result["headline"]) + + if result["policies_errored"]: + # "Could not evaluate" is tool health, not a policy decision, so it ignores --fail-on-error -- + # the same reason an unreachable platform does in `platform check`. + log("Some policies could not be evaluated") + return ExitStatus.ERROR + + verdict = result["verdict"] + if verdict == "errored": + return ExitStatus.ERROR + if verdict == "failed" and args.failOnError: + return ExitStatus.ERROR_POLICY_FAILED + if verdict == "failed": + log("Policies failed, but --fail-on-error was not set") + if result.get("counts", {}).get("approval_required"): + log("Some policies ask for approval; reported as a warning, which does not block") + + return ExitStatus.SUCCESS + + +def _evaluate(args, policy_path): + policies = discover_policies(policy_path) if not policies: - # The one outcome this whole mode must never produce is a green result for a change nothing - # was evaluated against. "No policies found" is not a skip. + # A green result for a change nothing was evaluated against is the one outcome this must never + # produce. "No policies found" is a configuration mistake, not a deliberate skip. raise LocalError( - f"Nothing to evaluate: no policy files found at '{opts.policy_path}'. Point " - "--policy-path at a file, a directory or a glob containing tirith policies." - ) - - if opts.infracost_path: - # Cost policies need the platform: local mode evaluates one document, and infracost output - # is a second one. Saying so beats evaluating the plan and reporting a cost policy as - # unevaluated with no explanation. - log( - "WARNING: --infracost-path is ignored in local mode, which evaluates a single " - "document. Cost policies need `tirith platform check`." + f"Nothing to evaluate: no policy files found at '{policy_path}'. Point -policy-path at a " + "file, a directory or a glob containing tirith policies." ) warnings = [] with tempfile.TemporaryDirectory(prefix="tirith-local-") as scratch: input_path, redactions = prepare_input( - opts.input_path, - opts.plan_file, - opts.terraform_bin, - opts.input_kind, - opts.source_dir, + args.inputPath, + None, + None, + args.inputKind or "raw", + os.path.dirname(args.inputPath) or ".", scratch, - state_path=opts.state_path, + state_path=args.statePath, ) if redactions: log(f"Masked {redactions} sensitive value(s) before evaluating") - log(f"Evaluating {len(policies)} policy file(s) from '{opts.policy_path}'") + log(f"Evaluating {len(policies)} policy file(s) from '{policy_path}'") def on_unknown_enforcement(value): message = f"unrecognised meta.enforcement '{value}'; treating a failing policy as blocking" warnings.append(message) log(f"WARNING: {message}") - policy_results, errored = evaluate(policies, input_path, on_unknown_enforcement=on_unknown_enforcement) + policy_results, errored = evaluate( + policies, + input_path, + on_unknown_enforcement=on_unknown_enforcement, + var_paths=args.varPaths, + inline_vars=args.inlineVars, + ) for path, reason in errored: log(f"WARNING: could not evaluate {path}: {reason}") - # Rendered as a completed evaluation on purpose. Results genuinely were produced, and the - # renderer's ERRORED narrative ("the workflow run finished as ERRORED without producing policy - # results") would be simply untrue here. A policy that could not be evaluated is already a - # visible FAIL carrying its own reason, and the exit code is what actually gates. + counts, _findings = report.summarize(policy_results) + evaluated_something = any( + counts.get(key) for key in (report.PASS, report.FAIL, report.WARN, report.APPROVAL_REQUIRED, report.UNKNOWN) + ) + + # Every policy skipped means every check was swallowed by error_tolerance and nothing was actually + # examined. This command has always called that a failure rather than a pass -- "None is not a + # pass" -- and extending it must not quietly reverse that for a caller who passed a directory. + # + # Note this is a deliberate difference from `platform check`, which counts skips separately and + # reports them as a pass. Both are defensible; what is not defensible is one surface silently + # disagreeing with itself depending on how many policies you pointed it at. + status = "COMPLETED" + notes = None + if not evaluated_something: + status = "ERRORED" + notes = [ + f"All {len(policies)} policy file(s) were skipped, so nothing was evaluated. Every check " + "was swallowed by its error_tolerance -- usually because the resources the policies name " + "are not in this document." + ] + result = report.result_document( "local", - "COMPLETED", + status, policy_results, policies_evaluated=len(policies), policies_errored=len(errored), - policy_path=opts.policy_path, + policy_path=policy_path, policy_errors=[{"policy": path, "reason": reason} for path, reason in errored], policy_warnings=warnings, ) - write_output_json(opts.output_json, result) - write_output_markdown( - opts.output_markdown, - report.render_markdown( - policy_results, - "COMPLETED", - None, - marker=opts.comment_marker, - limit=opts.markdown_limit, - commit=opts.sha, - ), - ) + write_output_json(args.outputJson, result) + if args.outputMarkdown: + write_output_markdown( + args.outputMarkdown, + report.render_markdown( + policy_results, + status, + None, + marker=args.commentMarker, + limit=args.markdownLimit, + commit=args.sha, + notes=notes, + ), + ) + return result - log(result["headline"]) + +def _write_failure(args, message): + """ + Write both output files for a run that produced no verdict. + + A front end editing a sticky comment in place needs a marker-first body even here: writing nothing + means it falls through to a body of its own with no marker, and PATCHing that over a good comment + orphans it permanently. + """ + result = report.result_document( + "local", + "ERRORED", + {}, + policies_evaluated=0, + policies_errored=0, + policy_path=args.policyPath, + policy_errors=[{"policy": None, "reason": message}], + ) + write_output_json(args.outputJson, result) + if args.outputMarkdown: + write_output_markdown( + args.outputMarkdown, + report.render_markdown( + {}, + "ERRORED", + None, + marker=args.commentMarker, + limit=args.markdownLimit, + commit=args.sha, + notes=[message], + ), + ) return result diff --git a/src/tirith/local/cli.py b/src/tirith/local/cli.py deleted file mode 100644 index 2289b90c..00000000 --- a/src/tirith/local/cli.py +++ /dev/null @@ -1,132 +0,0 @@ -""" -`tirith local check` -- evaluate policy files committed in your repository, with no credentials. - -A sibling of `tirith platform check` rather than a flag on it. `platform check` masks, packs, -uploads, runs on StackGuardian and polls; a path that does none of those things under the same verb -would make its help text wrong for half its readers. Mechanically it would be worse still: -`--workflow-id` is required there, so a flag would have to lift that requirement out of argparse -into a hand-rolled conditional, weakening the platform path to accommodate a mode with no workflows. - -Local mode is also never entered implicitly. `platform check` with no credentials stays a hard error, -and a caller wanting "no credentials therefore local" implements that itself -- a front end that -guesses wrong evaluates whatever happens to be committed and reports green, which is exactly the -outcome a policy gate exists to prevent. - -Every identity, workflow, archive and run flag is deliberately absent rather than accepted and -ignored: a credential-shaped flag silently doing nothing in a credential-free mode is how someone -ends up with a green check their organization's policies never saw. -""" - -import argparse - -from ..platform.check import log -from ..status import ExitStatus -from .check import run_check, write_failure_report -from .evaluate import LocalError - -INPUT_KINDS = ("terraform_plan", "terraform_state", "kubernetes", "json") - -DEFAULT_POLICY_PATH = ".tirith/policies" - - -def build_parser(): - parser = argparse.ArgumentParser( - prog="tirith local", - description="Evaluate policy files committed in your repository. Talks to nothing.", - ) - sub = parser.add_subparsers(dest="subcommand") - - check = sub.add_parser( - "check", - help="Evaluate committed policy files against a document and report the verdict.", - description=( - "Masks the document, evaluates every policy found at --policy-path, and writes the same " - "result document and markdown report that `tirith platform check` writes. Requires no " - "credentials and makes no network calls." - ), - ) - - policies = check.add_argument_group("policies") - policies.add_argument( - "--policy-path", - default=DEFAULT_POLICY_PATH, - help=( - "A policy file, a directory, or a glob. A directory is searched recursively for " - f"*.tirith.json, or failing that for any .json file shaped like a policy. Default: " - f"{DEFAULT_POLICY_PATH}" - ), - ) - - inputs = check.add_argument_group("inputs") - inputs.add_argument("--input-path", default=None, help="Document to evaluate. Default: plan.json or tfplan.json.") - inputs.add_argument( - "--plan-file", default=None, help="Binary terraform plan, rendered in memory. Not with --input-path." - ) - inputs.add_argument("--terraform-bin", default=None, help="terraform/tofu binary used for --plan-file.") - inputs.add_argument("--input-kind", default="terraform_plan", choices=INPUT_KINDS, help="What the document is.") - inputs.add_argument("--state-path", default=None, help="Terraform state, when --input-kind is terraform_state.") - inputs.add_argument( - "--infracost-path", - default=None, - help="Accepted and ignored: cost policies need a second document, so they need platform mode.", - ) - inputs.add_argument("--source-dir", default=".", help="Where to look for the document. Default: .") - - run = check.add_argument_group("run") - run.add_argument("--sha", default=None, help="Revision these findings describe, recorded in the report.") - - output = check.add_argument_group("output") - output.add_argument("--output-json", default=None, help="Write the result document here.") - output.add_argument("--output-markdown", default=None, help="Write a markdown report here.") - output.add_argument("--comment-marker", default=None, help="Opaque first line of the markdown, for stickiness.") - output.add_argument("--markdown-limit", type=int, default=60000, help="Truncate the markdown to this length.") - output.add_argument( - "--fail-on-error", - action="store_true", - help=( - "Exit non-zero when a policy fails. A policy that could not be evaluated at all always " - "exits non-zero regardless of this flag." - ), - ) - - return parser - - -def main(argv): - parser = build_parser() - opts = parser.parse_args(argv[1:]) - - if opts.subcommand != "check": - parser.print_help() - return ExitStatus.SUCCESS - - try: - result = run_check(opts) - except LocalError as e: - # Fails closed, and leaves a report behind: a front end editing a sticky comment in place - # needs a marker-first body even on this path, or it orphans the comment it was updating. - log(f"ERROR: {e}") - write_failure_report(opts, str(e)) - return ExitStatus.ERROR - except KeyboardInterrupt: - log("Interrupted") - return ExitStatus.ERROR_CTRL_C - - # "Could not evaluate" is tool health, not a policy decision, so it ignores --fail-on-error - # exactly as an unreachable platform does in the other mode. - if result["policies_errored"]: - log("Some policies could not be evaluated") - return ExitStatus.ERROR - - verdict = result["verdict"] - if verdict == "errored": - log("The evaluation did not produce a verdict") - return ExitStatus.ERROR - if verdict == "failed" and opts.fail_on_error: - return ExitStatus.ERROR_POLICY_FAILED - if verdict == "failed": - log("Policies failed, but --fail-on-error was not set") - if result.get("counts", {}).get("approval_required"): - log("Some policies ask for approval; reported as a warning, which does not block") - - return ExitStatus.SUCCESS diff --git a/src/tirith/local/evaluate.py b/src/tirith/local/evaluate.py index 0e4e2f11..f9578d1e 100644 --- a/src/tirith/local/evaluate.py +++ b/src/tirith/local/evaluate.py @@ -126,9 +126,12 @@ def prepare_input(input_path, plan_file, terraform_bin, input_kind, source_dir, `json` and `kubernetes` documents are passed through untouched: they carry no sensitivity markers to mask by, and tirith reads YAML for them, which a JSON round-trip here would break. """ + # `raw` is what the flat command does with no --input-kind: read the named file, mask nothing. + # `json` and `kubernetes` are passed through for a different reason -- they carry no sensitivity + # markers to mask by, and tirith reads YAML for them, which a JSON round-trip here would break. if input_kind not in ("terraform_plan", "terraform_state"): if not input_path: - raise LocalError(f"--input-path is required when --input-kind is '{input_kind}'.") + raise LocalError("-input-path is required.") if not os.path.exists(input_path): raise LocalError(f"input document not found: {input_path}") return input_path, 0 @@ -173,7 +176,7 @@ def prepare_input(input_path, plan_file, terraform_bin, input_kind, source_dir, return masked_path, redactions -def engine_argv(policy_path, input_path): +def engine_argv(policy_path, input_path, var_paths=(), inline_vars=()): """ The argv used to evaluate one policy. @@ -181,11 +184,20 @@ def engine_argv(policy_path, input_path): the one whose renderer was imported above -- a `tirith` on PATH could be a different installation entirely. Factored out so a test can pin it: see the package docstring for why this must stay a subprocess against the frozen `--json` contract. + + `-var-path` and `-var` are passed through because they are flags of the same command. Dropping + them when several policies are evaluated instead of one would make a parameterised policy work + against a file and fail against the directory holding it, for no reason a user could see. """ - return [sys.executable, "-m", "tirith", "-policy-path", policy_path, "-input-path", input_path] + argv = [sys.executable, "-m", "tirith", "-policy-path", policy_path, "-input-path", input_path] + for var_path in var_paths or (): + argv += ["-var-path", var_path] + for inline_var in inline_vars or (): + argv += ["-var", inline_var] + return argv -def _evaluate_one(policy_path, input_path): +def _evaluate_one(policy_path, input_path, var_paths=(), inline_vars=()): """ Run one policy. Returns (document, error_message); exactly one is set. @@ -193,7 +205,7 @@ def _evaluate_one(policy_path, input_path): is not on either stream. When that happens the policy is re-run without `--json` purely to recover a message worth showing -- one extra subprocess, only ever on the error path. """ - argv = engine_argv(policy_path, input_path) + argv = engine_argv(policy_path, input_path, var_paths, inline_vars) try: completed = subprocess.run(argv + ["--json"], capture_output=True, text=True, timeout=EVALUATION_TIMEOUT) @@ -290,7 +302,7 @@ def _failure_result(document, on_unknown_enforcement): return report.FAIL -def evaluate(policy_paths, input_path, on_unknown_enforcement=lambda _: None): +def evaluate(policy_paths, input_path, on_unknown_enforcement=lambda _: None, var_paths=(), inline_vars=()): """ Evaluate every policy and build the PolicyEvalResults document the renderer consumes. @@ -304,7 +316,7 @@ def evaluate(policy_paths, input_path, on_unknown_enforcement=lambda _: None): errored = [] for policy_path in policy_paths: - document, error = _evaluate_one(policy_path, input_path) + document, error = _evaluate_one(policy_path, input_path, var_paths, inline_vars) if error is not None: policy_id, rule_name = _identity({}, policy_path) diff --git a/tests/cli/test_dispatch.py b/tests/cli/test_dispatch.py index 1e3df9ed..71a2b2fd 100644 --- a/tests/cli/test_dispatch.py +++ b/tests/cli/test_dispatch.py @@ -101,11 +101,14 @@ def test_the_subcommand_names_are_exactly_these(capsys): `remote` is not quietly still accepted. `ui` was added alongside it later, on the same terms: dispatched before the flat parser so the - local surface and its golden-file output are untouched. `local` joined on the same terms again. - The set is pinned rather than merely checked for membership, so a new subcommand has to be a - deliberate edit here. + local surface and its golden-file output are untouched. The set is pinned rather than merely + checked for membership, so a new subcommand has to be a deliberate edit here. + + A `local` subcommand was written and then removed: evaluating committed policy files belongs on the + flat surface that already does it, as flags, rather than as a second way to do the same thing. That + is why `-policy-path` accepts a directory and takes `--output-json`. """ - assert cli.SUBCOMMANDS == {"platform", "ui", "local"} + assert cli.SUBCOMMANDS == {"platform", "ui"} status = cli.main(["remote"]) diff --git a/tests/local/conftest.py b/tests/local/conftest.py index 2084eb00..624dcc54 100644 --- a/tests/local/conftest.py +++ b/tests/local/conftest.py @@ -90,19 +90,34 @@ def plan(self, name="plan.json", **kwargs): return path def argv(self, *extra): + """ + The flat command, with the reporting flags a CI front end passes. + + Note `-policy-path` with one dash and `--output-json` with two: that asymmetry is the flat + surface's, inherited rather than chosen, and a caller has to get it right. + """ return [ - "local", - "check", - "--policy-path", + "-policy-path", str(self.policies), - "--input-path", + "-input-path", str(self.root / "plan.json"), + "--input-kind", + "terraform_plan", "--output-json", str(self.root / "out.json"), "--output-markdown", str(self.root / "out.md"), ] + list(extra) + def bare_argv(self, *extra): + """The same evaluation with none of the reporting flags -- the original code path.""" + return [ + "-policy-path", + str(self.policies), + "-input-path", + str(self.root / "plan.json"), + ] + list(extra) + def result(self): with open(self.root / "out.json") as f: return json.load(f) diff --git a/tests/local/test_local_cli.py b/tests/local/test_local_cli.py index 25397a86..81257d20 100644 --- a/tests/local/test_local_cli.py +++ b/tests/local/test_local_cli.py @@ -1,9 +1,11 @@ """ -`tirith local check`, end to end through the real CLI. +The aggregate path behind `tirith -policy-path ...`, end to end through the real CLI. -The exit-code matrix is the point of this file. Local mode is a *gate*, and the only thing that -gates is the exit code, so every branch of it is pinned here rather than inferred from the result -document. +The exit-code matrix is the point of this file. This is a *gate*, and the only thing that gates is the +exit code, so every branch of it is pinned here rather than inferred from the result document. + +The routing is pinned too: a single policy file with none of the reporting flags must keep reaching the +original code path, because its `--json` stdout is a frozen contract. """ import json @@ -165,19 +167,59 @@ def test_the_masked_document_is_what_gets_evaluated(workspace): assert "hunter2-should-never-appear" not in workspace.markdown() -def test_infracost_path_is_accepted_and_reported_as_ignored(workspace, capsys): +def test_without_input_kind_the_document_is_not_masked(workspace): """ - Cost policies need a second document, which this mode does not evaluate. Saying so beats - evaluating the plan and reporting a cost policy as unevaluated with no explanation. + No --input-kind means "read the file as this command always has". Masking is opt-in because it + changes the evaluator messages, and those messages are the frozen --json output. """ workspace.policy() + workspace.plan(secret="hunter2-visible-without-input-kind") + + main(workspace.bare_argv("--output-json", str(workspace.root / "out.json"))) + + assert "hunter2-visible-without-input-kind" not in workspace.result()["headline"] + # And with it, the value is masked out of the document that was evaluated. + main(workspace.argv()) + assert "hunter2-visible-without-input-kind" not in json.dumps(workspace.result()) + + +def test_a_single_policy_file_with_no_reporting_flags_takes_the_original_path(workspace): + """ + The routing guard. That path prints the engine's own document to stdout, pinned byte-for-byte by + tests/core/test_output_compatibility.py, so it must not be reached by the aggregate code. + """ + from tirith.local import check as local_check + + policy = workspace.policy() workspace.plan() - main(workspace.argv("--infracost-path", str(workspace.root / "infracost.json"))) + class Args: + policyPath = str(policy) + inputKind = None + statePath = None + sha = None + outputJson = None + outputMarkdown = None + commentMarker = None + + assert local_check.wanted(Args) is False + + # One reporting flag is enough to switch, and so is a directory. + Args.outputJson = "out.json" + assert local_check.wanted(Args) is True + + Args.outputJson = None + Args.policyPath = str(workspace.policies) + assert local_check.wanted(Args) is True - assert "--infracost-path is ignored in local mode" in capsys.readouterr().err +def test_a_directory_of_policies_is_evaluated_rather_than_erroring(workspace): + """ + `-policy-path` used to reach `open()` on a directory and fail with a bare "ERROR". Evaluating every + policy in it is new capability, not changed behaviour. + """ + workspace.policy(name="one.tirith.json") + workspace.policy(name="two.tirith.json") + workspace.plan() -def test_local_check_with_no_subcommand_prints_help(capsys): - assert main(["local"]) == ExitStatus.SUCCESS - assert "tirith local" in capsys.readouterr().out + assert main(workspace.bare_argv()) == ExitStatus.SUCCESS diff --git a/tests/local/test_local_evaluate.py b/tests/local/test_local_evaluate.py index a79b0bdc..782fd879 100644 --- a/tests/local/test_local_evaluate.py +++ b/tests/local/test_local_evaluate.py @@ -98,6 +98,9 @@ def test_a_json_document_is_passed_through_unmasked(tmp_path): """ `json` and `kubernetes` carry no sensitivity markers to mask by, and tirith reads YAML for them, which a JSON round-trip here would break. So the original path is returned, not a copy. + + `raw` -- no --input-kind at all, which is what the flat command has always done -- takes the same + branch, for the different reason that the caller has not said what the document is. """ document = tmp_path / "anything.json" document.write_text(json.dumps({"hello": "world"})) @@ -109,7 +112,7 @@ def test_a_json_document_is_passed_through_unmasked(tmp_path): def test_a_json_document_must_be_named(tmp_path): - with pytest.raises(LocalError, match="--input-path is required"): + with pytest.raises(LocalError, match="-input-path is required"): evaluate.prepare_input(None, None, None, "json", str(tmp_path), str(tmp_path)) diff --git a/tests/local/test_output_contract.py b/tests/local/test_output_contract.py index 5afbdf23..189c80c6 100644 --- a/tests/local/test_output_contract.py +++ b/tests/local/test_output_contract.py @@ -17,6 +17,7 @@ from tirith.cli import main from tirith.platform import report +from tirith.status import ExitStatus # Keys read by the GitHub Action and the GitLab component. Adding one is fine; removing or renaming # one breaks a released consumer, which is what this list is for. @@ -126,23 +127,35 @@ def test_the_document_is_json_serialisable_in_both_modes(workspace): json.dumps(_local_document(workspace)) -def test_every_skipped_policy_still_reports_a_verdict_of_passed(workspace): +def test_a_run_of_nothing_but_skips_fails_rather_than_passing(workspace): """ - A deliberate divergence, recorded rather than fixed here. - - A policy whose every check was skipped (`final_result` is None) counts as SKIPPED, and a run of - nothing-but-skips reports `passed` -- matching platform mode, which also counts skips separately - rather than failing on them. The *flat* surface disagrees: `tirith -policy-path ... -input-path - ... --fail-on-error` exits 1 for the same policy, on the grounds that "nothing ran" is not a pass - (see tests/cli/test_local_gating.py). - - Both readings are defensible -- the policies genuinely did not apply, and the report says "N - skipped" rather than hiding it -- so this is pinned to make the inconsistency visible instead of - letting either surface drift into the other by accident. It needs a decision across all three - surfaces, not a quiet change in one. + The flat surface has always treated `final_result is None` -- every check swallowed by its + error_tolerance -- as a failure rather than a pass: nothing was examined, so green would be a lie. + Extending it to many policies must not quietly reverse that, which is what would have happened had + the aggregate verdict been taken from `report.verdict` alone. + + This is a deliberate difference from `platform check`, which counts skips separately and reports + them as a pass. Both readings are defensible; a single surface disagreeing with itself depending on + how many policies you pointed it at is not. """ - counts, _ = report.summarize({"p": [{"rule_name": "r", "skip": True}]}) + # A policy naming a resource type this document does not contain, with the error_tolerance that + # turns "not found" into a skip rather than a failure. + document = json.loads(json.dumps(POLICY)) + document["evaluators"][0]["provider_args"]["terraform_resource_type"] = "aws_nonexistent" + document["evaluators"][0]["condition"]["error_tolerance"] = 2 + workspace.policy(document=document) + workspace.plan() + + assert main(workspace.argv()) == ExitStatus.ERROR + result = workspace.result() + assert result["verdict"] == "errored" + assert result["counts"]["skipped"] == 1 + assert "nothing was evaluated" in workspace.markdown() + + # The reducer itself still answers `passed` for an all-skipped set -- that is platform mode's + # answer and it is not changed here. The difference is made by this path, deliberately. + counts, _ = report.summarize({"p": [{"rule_name": "r", "skip": True}]}) assert report.verdict(counts, "COMPLETED") == "passed" diff --git a/tests/test_readme_is_current.py b/tests/test_readme_is_current.py index 85c32a1a..e366cacb 100644 --- a/tests/test_readme_is_current.py +++ b/tests/test_readme_is_current.py @@ -117,31 +117,48 @@ def test_the_flag_reference_page_lists_every_flag_the_command_accepts(): assert not missing, f"flags accepted by `platform check` but absent from docs/platform-check.md: {missing}" -def test_the_local_check_reference_page_lists_every_flag_it_accepts(): +def test_the_reporting_flags_are_documented(): """ - The sibling of the check above, for the same reason: docs/local-check.md embeds the flag list, and - a flag added without touching it silently stops being documented. + The flags that let a CI job drive the flat command. They are the difference between "there is a + local surface" and "the local surface is usable as a gate", and they went undocumented once already + in the form of a subcommand nobody could find. """ - with open(os.path.join(ROOT, "docs", "local-check.md")) as f: - page = f.read() - - flags = set(re.findall(r"(?