fix: reject non-plan input, report unknown costs instead of guessing, expose Action outputs

.github/workflows/ci.yml

@@ -8,11 +8,11 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: ["3.10", "3.11", "3.12"]
+        python-version: ["3.10", "3.11", "3.12", "3.13"]
     steps:
       - uses: actions/checkout@v4
       - uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
-      - run: pip install -e . pytest
+      - run: pip install -e '.[dev]'
       - run: pytest

README.md

@@ -13,8 +13,11 @@ pull request. Optionally fails the check when the delta crosses a threshold.
 ## How it works
 
 1. Parses `terraform show -json <plan>` into create / update / destroy sets.
+   Input that is not plan JSON — state output, a truncated download — is
+   **rejected**, never reported as "no changes".
 2. Prices cost-relevant resources with a small built-in AWS sheet (override with
-   your own JSON via `--price-sheet`).
+   your own JSON via `--price-sheet`). A resource whose cost cannot be
+   determined is reported as **unknown**, never counted as $0.
 3. Renders a Markdown table and upserts one sticky PR comment.
 
 ## CLI
@@ -53,12 +56,61 @@ steps:
       terraform plan -out tf.plan
       terraform show -json tf.plan > plan.json
   - uses: moveeeax/tf-cost-diff@v0
+    id: cost
     with:
-      plan: plan.json
+      plan: plan.json       # relative to the workspace
       threshold: "100"      # optional: fail if Δ > $100/mo
       # price-sheet: prices.json   # optional override
 ```
 
+### Outputs
+
+Every figure the tool computes is readable by later steps. They are written
+*before* the threshold gate is applied, so they are still available on the run
+where the gate fails — use `if: always()` to read them there.
+
+| Output | Example | Meaning |
+| --- | --- | --- |
+| `total-delta` | `97.96` | Monthly delta in dollars (negative when the plan saves money). |
+| `created` / `updated` / `destroyed` | `2` / `1` / `1` | Cost-relevant resource counts. |
+| `unpriced` | `0` | Resources excluded from the total because their cost is unknown. |
+| `complete` | `true` | `false` when `unpriced > 0`, i.e. the total is a partial figure. |
+| `threshold-exceeded` | `false` | Whether the delta crossed `threshold`. |
+| `comment-url` | `https://github.com/...` | The sticky comment, empty if none was posted. |
+
+```yaml
+  - if: always() && steps.cost.outputs.complete == 'false'
+    run: echo "::warning::${{ steps.cost.outputs.unpriced }} resources could not be priced"
+```
+
+### Exit codes
+
+| Code | Meaning |
+| --- | --- |
+| `0` | Success. |
+| `1` | The delta exceeded `--threshold`. |
+| `2` | Bad input: not a Terraform plan, unreadable file, or an invalid price sheet. |
+
+Posting the comment is best effort: a pull request from a fork gets a read-only
+`GITHUB_TOKEN`, so the comment is skipped with a warning rather than failing the
+run. The threshold gate still applies.
+
+## Unknown costs
+
+The built-in sheet covers a handful of resource types and a handful of instance
+sizes within them. Anything outside that is treated as **unknown**, not free:
+
+- A resource type with no price model (IAM roles, security groups) is excluded
+  from the estimate silently — the comment carries a standing caveat.
+- A *modelled* type the sheet cannot price for this configuration — an
+  unrecognised `instance_type`, or a size that is not known until apply — is
+  listed in a separate "Cost unknown" table, excluded from the total, and the
+  headline is marked `(partial)`.
+
+This matters for the threshold gate: guessing a rate for an unrecognised
+`m5.24xlarge` would report ~$36/mo for a resource that really costs over
+$3,000/mo, and it would sail straight through `threshold: 100`.
+
 ## Price sheet override
 
 `--price-sheet prices.json` where the file maps a Terraform resource type to a
@@ -71,12 +123,13 @@ flat monthly price:
 }
 ```
 
-Anything not in the built-in sheet or your override is treated as $0 and skipped.
+Values must be finite, non-negative numbers; anything else is rejected with exit
+code `2` rather than silently producing a nonsense total.
 
 ## Development
 
 ```bash
-pip install -e . pytest
+pip install -e '.[dev]'
 pytest
 ```
 

action.yml

@@ -5,29 +5,70 @@ branding:
   color: "green"
 inputs:
   plan:
-    description: "Path to `terraform show -json` output (a JSON file)."
+    description: "Path to `terraform show -json` output (a JSON file), relative to the workspace."
     required: true
   threshold:
     description: "Fail the check when the monthly delta exceeds this many dollars."
     required: false
   price-sheet:
-    description: "Optional path to a JSON price-sheet override."
+    description: "Optional path to a JSON price-sheet override, relative to the workspace."
     required: false
   github-token:
     description: "Token used to post the PR comment."
     required: false
     default: ${{ github.token }}
+outputs:
+  total-delta:
+    description: "Estimated monthly cost delta in dollars, e.g. `-12.40`."
+    value: ${{ steps.cost.outputs.total-delta }}
+  created:
+    description: "Number of cost-relevant resources being created."
+    value: ${{ steps.cost.outputs.created }}
+  updated:
+    description: "Number of cost-relevant resources being changed or replaced."
+    value: ${{ steps.cost.outputs.updated }}
+  destroyed:
+    description: "Number of cost-relevant resources being destroyed."
+    value: ${{ steps.cost.outputs.destroyed }}
+  unpriced:
+    description: "Number of resources excluded from the total because their cost is unknown."
+    value: ${{ steps.cost.outputs.unpriced }}
+  complete:
+    description: "`true` when every cost-relevant resource could be priced."
+    value: ${{ steps.cost.outputs.complete }}
+  threshold-exceeded:
+    description: "`true` when the delta exceeded `threshold`."
+    value: ${{ steps.cost.outputs.threshold-exceeded }}
+  comment-url:
+    description: "URL of the sticky pull request comment, empty if none was posted."
+    value: ${{ steps.cost.outputs.comment-url }}
 runs:
   using: "composite"
   steps:
-    - shell: bash
+    - id: cost
+      shell: bash
+      # Inputs are passed through the environment rather than interpolated into
+      # the script, so a value containing shell metacharacters cannot execute.
+      # PYTHONPATH (not working-directory) makes the package importable, so that
+      # `plan` stays relative to the caller's workspace as documented.
       env:
         GITHUB_TOKEN: ${{ inputs.github-token }}
         GITHUB_REPOSITORY: ${{ github.repository }}
+        PYTHONPATH: ${{ github.action_path }}
+        TFCD_PLAN: ${{ inputs.plan }}
+        TFCD_PRICE_SHEET: ${{ inputs.price-sheet }}
+        TFCD_THRESHOLD: ${{ inputs.threshold }}
+        TFCD_PR: ${{ github.event.pull_request.number }}
       run: |
-        python3 -m tf_cost_diff \
-          --plan "${{ inputs.plan }}" \
-          ${{ inputs.price-sheet && format('--price-sheet {0}', inputs.price-sheet) || '' }} \
-          ${{ inputs.threshold && format('--threshold {0}', inputs.threshold) || '' }} \
-          ${{ github.event.pull_request.number && format('--pr {0}', github.event.pull_request.number) || '' }}
-      working-directory: ${{ github.action_path }}
+        set -euo pipefail
+        args=(--plan "$TFCD_PLAN" --github-output "$GITHUB_OUTPUT")
+        if [ -n "${TFCD_PRICE_SHEET:-}" ]; then
+          args+=(--price-sheet "$TFCD_PRICE_SHEET")
+        fi
+        if [ -n "${TFCD_THRESHOLD:-}" ]; then
+          args+=(--threshold "$TFCD_THRESHOLD")
+        fi
+        if [ -n "${TFCD_PR:-}" ]; then
+          args+=(--pr "$TFCD_PR")
+        fi
+        python3 -m tf_cost_diff "${args[@]}"

pyproject.toml

@@ -4,14 +4,17 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "tf-cost-diff"
-version = "0.1.0"
+version = "0.2.0"
 description = "Estimate the monthly cost delta of a Terraform plan and post it to a pull request."
 readme = "README.md"
 requires-python = ">=3.10"
 license = { text = "MIT" }
 authors = [{ name = "Michael Tarassov", email = "michael@tarassov.me" }]
 keywords = ["terraform", "finops", "cost", "github-actions", "aws"]
 
+[project.optional-dependencies]
+dev = ["pytest>=7", "pyyaml>=6"]
+
 [project.urls]
 Homepage = "https://github.com/moveeeax/tf-cost-diff"
 

tests/test_action.py

@@ -0,0 +1,65 @@
+"""Keep action.yml honest about what the CLI actually produces.
+
+The failure mode this guards against is drift: the tool computes numbers, the
+Action declares no `outputs:` for them, and the calling workflow silently has
+nothing to read.
+"""
+import pathlib
+
+import pytest
+
+yaml = pytest.importorskip("yaml")
+
+ACTION = pathlib.Path(__file__).resolve().parent.parent / "action.yml"
+
+
+@pytest.fixture(scope="module")
+def action():
+    return yaml.safe_load(ACTION.read_text(encoding="utf-8"))
+
+
+def _cli_output_names(tmp_path):
+    from tf_cost_diff.__main__ import write_github_output
+    from tf_cost_diff.plan import PlanSummary
+
+    out = tmp_path / "gh-output"
+    write_github_output(str(out), PlanSummary(resources=[]), False, "")
+    return {line.split("=", 1)[0] for line in out.read_text().splitlines()}
+
+
+def test_action_declares_every_output_the_cli_writes(action, tmp_path):
+    declared = set(action.get("outputs") or {})
+    assert declared == _cli_output_names(tmp_path)
+
+
+def test_every_declared_output_is_wired_to_the_step(action):
+    step_ids = {s.get("id") for s in action["runs"]["steps"]}
+    for name, spec in action["outputs"].items():
+        assert spec.get("description"), f"output {name} has no description"
+        value = spec["value"]
+        step = value.split("steps.", 1)[1].split(".", 1)[0]
+        assert step in step_ids, f"output {name} references unknown step {step}"
+        assert value.endswith(f"outputs.{name} }}}}"), f"output {name} is mis-wired"
+
+
+def test_inputs_are_passed_through_env_not_interpolated_into_the_script(action):
+    """`run:` must not interpolate inputs, or a crafted value runs as shell."""
+    for step in action["runs"]["steps"]:
+        assert "${{" not in step.get("run", ""), "inputs must reach the script via env:"
+
+
+def test_plan_path_is_resolved_against_the_workspace(action):
+    # working-directory: ${{ github.action_path }} would make the documented
+    # relative `plan: plan.json` resolve inside the action checkout instead.
+    for step in action["runs"]["steps"]:
+        assert "working-directory" not in step
+    assert "PYTHONPATH" in action["runs"]["steps"][0]["env"]
+
+
+def test_declared_inputs_are_all_consumed(action):
+    script = "".join(s.get("run", "") for s in action["runs"]["steps"])
+    env = {k: str(v) for s in action["runs"]["steps"] for k, v in (s.get("env") or {}).items()}
+    for name in action["inputs"]:
+        referenced = any(f"inputs.{name}" in value for value in env.values())
+        assert referenced, f"input {name} is declared but never used"
+    assert "--plan" in script