Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/actions/claude-pr-review/rubric-detail.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
5 changes: 5 additions & 0 deletions .github/actions/claude-pr-review/rubric.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Loading