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
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"name": "audit",
"source": "./",
"description": "Multi-pass codebase audit: a strictly-sequential pipeline of file-scoped review passes (process, security, test coverage, documentation, code quality, correctness/intent, hazard surface) plus triage, with findings reported (not fixed) and tracked as GitHub issues.",
"version": "0.28.0",
"version": "0.29.0",
"author": { "name": "Rain Open Source Software Ltd" },
"keywords": ["audit", "security", "code-review", "test-coverage", "correctness", "hazard", "solidity"]
}
Expand Down
2 changes: 1 addition & 1 deletion .claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "audit",
"displayName": "Audit",
"version": "0.28.0",
"version": "0.29.0",
"description": "Multi-pass codebase audit (process, security, test coverage, documentation, code quality, correctness/intent verification, hazard surface) plus triage. Passes run strictly sequentially; file-scoped subagents within each pass read one file in full and report findings (not fixes), tracked as GitHub issues.",
"author": {
"name": "Rain Open Source Software Ltd",
Expand Down
9 changes: 7 additions & 2 deletions skills/audit/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
name: audit
description: Codebase audit — seven review dimensions (process, security, test coverage, documentation, code quality, correctness/intent, hazard surface) plus triage. Runs at a DECLARED scope — `whole-repo` (every source file across all languages), `pr:<number>` (the diff plus its ramifications: callees, callers, siblings, and every claim the PR makes) or `paths:<globs>` — and is never diff-only at any of them. Reports problems (never fixes them, never "works correctly"), severity-rates each, attaches a concrete proposed fix, and tracks whole-repo findings as GitHub issues; triage then re-validates each finding against live source and applies fixes TDD-style. Triggers on "audit this codebase", "security review", "full audit", "review the whole repo for bugs/coverage/docs/quality/correctness/hazards", "review this PR's diff plus its ramifications", "find what's wrong before an external audit".
version: 0.28.0
version: 0.29.0
---

# Codebase Audit (multi-dimension, declared scope)
Expand Down Expand Up @@ -81,7 +81,7 @@ Every dimension runs at every scope unless stated otherwise here. "Runs" narrows
- **3. Documentation** — enumerate every public item the change **adds, or whose signature or behaviour it alters**, one by one, and check each is documented and accurate: exhaustive over that set, no sampling. Add the docs *elsewhere* that describe the changed behaviour and are now stale — interface NatSpec, README, the PR body, the linked issue. A repo-wide undocumented-API census is out of scope.
- **4. Code quality** — split by check; no check is dropped. The per-file ones (#2, #3, #6, #7, #8, #9, #12, #13 — #13 reading the bin's own `Cargo.toml` for the clap dependency is cross-referencing, not a widening — plus the Shared rules' Solidity naming and convention rules) apply to the in-scope files. The repo-global ones **read wide and report narrow**: #1 must still establish the convention from the widest scope observable — that is the whole point of #1 — but reports only where the change deviates; #10's wrapper-chain detection must still see the whole script→tool graph but reports only a wrapper the change adds or feeds; #5 and #11 fire only where the change touches a dependency pin or version, since an unrelated stale pin is not this review's business; #4 means the build warnings the change itself introduces. One explicit **non**-degradation: #6's completeness clause (enumerate the bare imports across `src/`, `test/` **AND** `script/`) is a **whole-repo** obligation — a scoped run reports the bare imports the change adds and must NOT claim the category is discharged.
- **5. Correctness / intent** — does not degrade in kind. It is the dimension a change review most needs and the one `raindex#2778` was missed by: every limb applies to each named item the change adds or alters. The **Claims** ramification limb is an *addition* under a scoped run that `whole-repo` has no analogue for — the PR body and the linked issue are themselves claims to verify against source.
- **6. Hazard surface** — the one dimension whose unit is not the file, so it needs its own rule. First decide which categories the change **touches**: does it add a second copy of a fact (cat. 1), add or reorder a step whose order is unenforced (2), add a convention or derived constant with no check (3), add configuration that must agree elsewhere (4), touch a generated artifact (5), change something another repo or language consumes (6), add a manual step (7), add a default resting on an unpinned property (8), or change a deploy path or release record (9)? Run **those** categories at their **full cross-file breadth** — a category scan that stops at the diff cannot see the second source of truth, which is the entire reason this dimension partitions by category rather than by file. Do not run the untouched categories, and do not silently widen to all of them. The nine stay **non-exhaustive** at every scope, as the dimension says: a shape the change itself creates that fits the framing question is in scope as an emergent category even where none of the nine names it. The four-part naming requirement is unchanged, and part (3), the realistic scenario, must be one the change makes possible or likelier.
- **6. Hazard surface** — the one dimension whose unit is not the file, so it needs its own rule. First decide which categories the change **touches**: does it add a second copy of a fact (cat. 1), add or reorder a step whose order is unenforced (2), add a convention or derived constant with no check (3), add configuration that must agree elsewhere (4), touch a generated artifact (5), change something another repo or language consumes (6), add a manual step (7), add a default resting on an unpinned property (8), change a deploy path or release record (9), or change published-package content or its publish wiring (10)? Run **those** categories at their **full cross-file breadth** — a category scan that stops at the diff cannot see the second source of truth, which is the entire reason this dimension partitions by category rather than by file. Do not run the untouched categories, and do not silently widen to all of them. The ten stay **non-exhaustive** at every scope, as the dimension says: a shape the change itself creates that fits the framing question is in scope as an emergent category even where none of the ten names it. The four-part naming requirement is unchanged, and part (3), the realistic scenario, must be one the change makes possible or likelier.
- **Triage** — a `whole-repo` stage in practice: it is a human-in-the-loop loop over **filed audit issues**. A scoped run's findings go to its caller and the fix belongs in the change under review, so there is nothing for triage to loop over. Where a scoped run did file issues (see below), triage them exactly as written.

### Output per scope
Expand Down Expand Up @@ -309,6 +309,11 @@ Categories (starting points, NOT exhaustive — anything fitting the framing que
7. **Manual / out-of-band operational steps** — anything depending on a human remembering to act at the right time ("after bumping X, regenerate pointers manually" → better: a CI check failing on mismatch; "rotate the mint key every 90 days" → better: an on-chain timelock). Each is a finding; the report must propose the structural change removing the step.
8. **Defaults that encode brittle assumptions** — defaults/fallbacks/convenience APIs that work today because of a property the system happens to have but would silently break if it changes (a `balanceOf` view assuming total supply < 2^128 that silently wraps; a storage-slot derivation hardcoding an OZ namespace string that breaks on the next OZ major; a fixture using `block.timestamp = 1` masking a sign-comparison bug). For each: what property does it depend on, and is that property pinned by a test/type or implicit? If implicit, file it.
9. **Non-deterministic / unpinnable deployments** — a production deploy path whose resulting address is not deterministic (nonce-dependent `new` / CREATE, or a deployer/impl whose constructor args vary its init bytecode) cannot be codehash-pinned, cross-repo address-pinned, upgrade-lock-checked, or reproducibly verified — so config silently drifts and deploys can't be independently reproduced. The structural fix is a deterministic deployer (Zoltu CREATE2 over fixed bytecode; CREATE2 clones for instances) with all config hardcoded as constants; the full prescriptive rule lives in the Domain rules' **Deterministic production deployments** — here the finding is the unpinnable-address hazard itself (no pin / verification / reproducibility). HIGH when it blocks pinning or verification of a production artifact. A *missing, incomplete or non-frozen* release record is the same hazard for *past* releases — **no record at all** (hand-maintained address/codehash/creation-code literals, or no pins whatsoever), a pin missing creation/runtime bytecode, a dependency-versioned suffix, or a "frozen" tag whose bytecode re-derives from current source. Note the deploy path being deterministic does NOT discharge this: a Zoltu deploy with nothing frozen per release is still unreproducible for every release already published. See the Domain rules' **Deploy-pin completeness & release historicals**. The repo's release *lifecycle* is part of the same surface: frozen per-tag snapshots released under the library lifecycle (`rainix-autopublish`) desync `[package].version` from the frozen `DEPLOY_TAG` on every merge, leaving the `version == DEPLOY_TAG` identity test permanently red — classifier, flags, and the split fix live in the Domain rules' **Release-lifecycle split**.
10. **Broken or stale package publish pipeline (soldeer autopublish)** — the *producer-side* counterpart of the Quality dimension's #11 (that check audits this repo's dependency pins as a *consumer*; this one audits what this repo *publishes*). Trigger: the repo is a soldeer-published package — a `[package] name` in `foundry.toml`, a workflow passing `soldeer-package:`, or a registry entry under the repo's package name. The hazard: if the merge→registry pipeline silently breaks or falls behind, downstream consumers pin a **stale** version and never receive new deploy constants / bytecode — with **no red anywhere**. The canonical correct answer is the merge-driven `rainix-autopublish.yaml` reusable (`on: push: branches: [main]`, `with: soldeer-package: <name>`), whose `soldeer-gate` hashes the package's **normalized source** (source-only, `src/generated/` excluded, the `foundry.toml` version line blanked) against the latest published revision and enforces the **next-version invariant**: `[package].version` is the NEXT, unpublished version, strictly ahead of the registry. Four checks — causes first, then the symptom:
- **Dead / stale publish wiring** — a publish workflow whose `uses:` references a rainix reusable that no longer exists at the pinned ref (e.g. the removed tag-triggered `publish-soldeer.yaml`), or a tag-triggered publish that has not fired across the last several content-changing merges. Live case: `S01-Issuer/st0x.deploy`'s `st0x-deploy` package sat at 0.1.1 through two deploy releases (0.1.2 orchestrator, 0.1.3 corp-actions) because the workflow referenced a removed reusable — no red anywhere until `S01-Issuer/st0x.deploy#239` migrated it to autopublish. **MEDIUM**, **HIGH** when the unpublished delta carries a security/correctness fix or deploy-record change consumers pin against. Fix = the merge-driven `rainix-autopublish` wiring above.
- **Cross-org `secrets: inherit`** — a workflow calling a reusable owned by a **different org** (e.g. a non-`rainlanguage` repo calling `rainlanguage/rainix/...`) with `secrets: inherit`. GitHub disallows this and the run dies at **startup** (`startup_failure` — before any job or log a reader would think to check), so the publish silently never runs. Flag `secrets: inherit` on any reusable `uses:` whose owner org differs from the calling repo's; secrets must be passed **explicitly** (`S01-Issuer/st0x.deploy#240` is the shape of the fix). **MEDIUM.**
- **Missing `[package]` identity** — a soldeer-published repo whose `foundry.toml` lacks `[package] name` / `version`, which `rainix-autopublish`'s content gate + auto-bump read (an empty version breaks the gate). **MEDIUM.**
- **Registry behind source / un-bumped next-version** — the observable symptom the three causes above produce, and the one check that also runs **pre-merge**. Query the registry (`https://api.soldeer.xyz/api/v1/revision?project_name=<name>`) for the latest published revision, then: *on main*, flag a latest published revision that predates the last content-changing merge (the package has fallen behind); *under `pr:<n>`*, flag a diff that changes published content (`src/`, excluding `src/generated/`) while `[package].version` still **equals** the published latest — the merge is already doomed, because the post-merge gate enforces the invariant a round trip late. Live case (2026-07-30): `rainlanguage/rain.deploy#21` merged new public API at `[package] version 0.1.4` with 0.1.4 already published; the post-merge `Package Release` run ([30564517433](https://github.com/rainlanguage/rain.deploy/actions/runs/30564517433)) failed at `soldeer-gate` — "`foundry.toml [package].version (0.1.4) is not ahead of the published revision (0.1.4)`" — main went red, and bump PR `rainlanguage/rain.deploy#24` had to land before 0.1.5 shipped. The gate is the *backstop*, loud but a merge too late; the audit's job is to catch the missing bump **in the PR**. Where feasible, compare the published revision's normalized-content hash against current source (the same normalization the gate uses) and flag real drift — never a bare version-string echo. **MEDIUM** for both shapes (silent consumer staleness pending; guaranteed-red main + blocked publish). Fix = bump `[package].version` in the same PR that changes published content. Needs a network query — if the registry is unreachable, note the check was **skipped** rather than reporting a false "current" (the same honesty rule as Quality #11).

**Every Pass-6 finding names all four parts** (incomplete = revise before filing): (1) **the hazard** — what mistake the shape makes more likely; (2) **the shape that creates it** — the duplicate / ordering / manual step / brittle default; (3) **the realistic scenario** where it silently lands in production; (4) **the structural fix** — generator+check, enumeration test, or redesign. Pass-6 severity: HIGH = corrupts user state/money in a realistic scenario (wrong bridge address, stale codehash admitting a malicious impl, missed key rotation); MEDIUM = silently breaks a feature for some users (website missing a token, indexer crashes on an unknown event); LOW = breaks a dev workflow not production; INFO = a convention kept by discipline today that could be cheaply enforced.

Expand Down
Loading