Skip to content

ci-runners: audit query must match all Actions SKUs, not just Linux - #126

Open
DJRHails wants to merge 1 commit into
mainfrom
ci-runners/audit-all-actions-skus
Open

ci-runners: audit query must match all Actions SKUs, not just Linux#126
DJRHails wants to merge 1 commit into
mainfrom
ci-runners/audit-all-actions-skus

Conversation

@DJRHails

Copy link
Copy Markdown
Owner

Follow-up to the post-merge review of #125 (finding F1).

The billing audit snippet filtered sku=="Actions Linux" exactly, which
silently drops every other billed Actions SKU — Actions macOS (10× the
Linux rate), Actions Windows, and larger runners (Actions Linux 4-core).
That re-introduces the failure class the section exists to warn about: a
scoping choice that under-reports spend with no visible sign. Verified
against a mock payload — a macOS repo at $0.80 and a 4-core repo at $0.40
were both invisible to the exact-match query.

Now a prefix match on Actions, with a README line explaining why. Storage
rows ride along at pennies, which errs toward over-reporting — the right
direction for an audit.

via gantry

The billing audit snippet added in #125 filtered sku=="Actions Linux"
exactly, silently dropping macOS (10x rate), Windows, and larger-runner
SKUs — the same silent-scoping failure the section warns about. Prefix
match on "Actions" instead; storage rows ride along at pennies, erring
toward over-reporting.

Resolves the P2 finding from the post-merge review of #125.
@DJRHails

Copy link
Copy Markdown
Owner Author

Review Summary

Direct single-pass review (7-line docs-only diff, one README). No P1–P3 findings — the change is correct and does exactly what it claims.

Findings

None. Two things considered and dismissed (informational):

# Severity Finding Resolution
1 P4 select(.product=="actions") would match on the product field instead of a SKU prefix Dismissed: the prefix match is deliberate, documented in the new paragraph, and behaves identically on all known Actions SKUs
2 P4 `(.net*100 round/100)` renders $0.3 rather than $0.30

Verification

  • jq query, mock payload (the real billing API needs the user scope this container's token deliberately lacks): built an 8-row mock matching the enhanced-billing usageItems shape. The old exact match reported only linux-repo $0.3, silently dropping macOS ($0.80), Windows ($0.20), and Linux 4-core ($0.40) rows — confirming the PR's premise verbatim. The new prefix match catches all four, folds Actions Storage pennies into repo totals, and correctly excludes non-Actions products (Packages, Copilot) and $0 public-repo rows.
  • Prose accuracy: the 10× macOS claim matches GitHub's published rates ($0.08/min vs $0.008/min); the "storage rides along" note matches observed behavior in the mock run.
  • Cross-module consistency: spend-watch sums all usage items with no SKU filter, so no inconsistency is introduced.
  • Secrets: gitleaks git origin/main..HEAD — 1 commit scanned, no leaks. Repo has no CI workflows (hooks-only by design), and the change touches no scripts, so the local test suites don't apply.

Commit

No fix commit needed — nothing to change.

Verdict: approve

via gantry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant