From 26b29eacde382a6c8a5fe5b88dc1a96e6fba0d45 Mon Sep 17 00:00:00 2001 From: vincent-k2026 Date: Wed, 22 Jul 2026 11:55:28 +0000 Subject: [PATCH 1/2] docs: add resource-bounding and CI-workflow-security to review rubric --- .github/actions/claude-pr-review/rubric.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/actions/claude-pr-review/rubric.md b/.github/actions/claude-pr-review/rubric.md index b7d9e99..51619a5 100644 --- a/.github/actions/claude-pr-review/rubric.md +++ b/.github/actions/claude-pr-review/rubric.md @@ -42,6 +42,10 @@ Are the core types right? Bad code around good data structures is fixable; the r join-errors; prefer typed error enums over opaque/stringly errors. - Unsafe code carries a `SAFETY` comment justifying its invariants. No unhandled concurrency hazards — shared mutable state, TOCTOU, deadlocks. +- **Bound every queue and buffer.** An unbounded channel, work queue, retry backlog, or + per-connection buffer is a memory-DoS — one slow consumer or hostile peer grows it without + limit. Cap depth (with a drop or backpressure policy) and apply size checks before + accumulating bytes; keep metric label sets bounded too (no unbounded-cardinality labels). ### 3. Surface stewardship @@ -105,3 +109,4 @@ line-level checks — do not load them when irrelevant): - **Persistence / on-disk writes / serialization** → §Persistence - **Unit / time / scaling math (ratios, conversions, timestamps)** → §Unit-safety - **Tests, fuzzers, or fault-injection harnesses; perf-shaped changes** → §Test-&-perf-surface +- **CI/CD workflows or composite actions (`.github/workflows`, `action.yml`)** → §CI-workflow-security From 512b65a320245ca51a037bf5f1d52c1491dc2d68 Mon Sep 17 00:00:00 2001 From: vincent-k2026 Date: Wed, 22 Jul 2026 11:55:29 +0000 Subject: [PATCH 2/2] docs: add resource-bounding and CI-workflow-security to review rubric --- .../actions/claude-pr-review/rubric-detail.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/actions/claude-pr-review/rubric-detail.md b/.github/actions/claude-pr-review/rubric-detail.md index 098c822..ff493e9 100644 --- a/.github/actions/claude-pr-review/rubric-detail.md +++ b/.github/actions/claude-pr-review/rubric-detail.md @@ -35,3 +35,19 @@ findings here at `[Minor]`/`[Nit]` unless they cause incorrect behavior, in whic - **Dead or test-only scaffolding is a trapdoor:** a `Default`/`Option`/bootstrap field or method with zero production callers isn't merely unused — it's a latent footgun a future caller can trip. Confirm zero callers, then delete it. + +## CI-workflow-security + +- **No untrusted input in `run:` blocks:** never interpolate a PR title/body/branch or other + attacker-controlled value straight into a shell step — pass it through `env:` and reference + `"$VAR"`, or feed secrets via `--password-stdin`, so a crafted value can't inject commands. +- **Least-privilege the token:** set `persist-credentials: false` on `checkout` when later steps + don't need the token, keep `GITHUB_TOKEN`/job permissions minimal, and pin third-party actions + to a full commit SHA when a broad-scoped token is in play. +- **Teardown must run on failure:** cleanup of enclaves/containers/temp infra belongs on an + `if: always()` / `ERR` / `EXIT` trap path, not a plain step that `set -e` skips after an + earlier failure. Prefer `if: !cancelled()` over `always()` when a cancelled run must not + resurrect work. +- **Guard comment/label triggers:** a workflow keyed on a comment marker or label must check the + actor's association/author — otherwise any outsider triggers the privileged path by echoing + the marker.