feat(audit): hot-key custody rule — the deploy key deploys and holds NOTHING (#81)

.claude-plugin/marketplace.json

@@ -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.33.0",
+      "version": "0.34.0",
       "author": { "name": "Rain Open Source Software Ltd" },
       "keywords": ["audit", "security", "code-review", "test-coverage", "correctness", "hazard", "solidity"]
     }

.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "audit",
   "displayName": "Audit",
-  "version": "0.33.0",
+  "version": "0.34.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",

skills/audit/SKILL.md

@@ -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.33.0
+version: 0.34.0
 ---
 
 # Codebase Audit (multi-dimension, declared scope)
@@ -225,6 +225,12 @@ Exclude auto-generated files (bindings, build artifacts, `*.pointers.sol` and si
   ```
 
   The proposed test follows the **Solidity/Foundry test rules** above (specific post-state assertions here; specific revert expectations on any negative path it also exercises). This does not contradict **Deploy-pin completeness & release historicals**' keep-tests-OFFLINE mandate: that mandate covers the frozen record's self-consistency checks, which re-derive locally, while a pre-flight dry-run of an on-chain mutation is irreducibly a fork test — the same accepted shape as that rule's item (b) reality check, which fork-verifies the *resulting deployment*; this rule fork-verifies the *script execution* that produces the change. Severity: **MEDIUM** by default; **HIGH** when the script moves funds or mutates authorization (owners, thresholds, roles, authoriser wiring) — exactly where an unrehearsed op lands worst. The missing fork test files under dimension 2; the unverified state-changing script is also Hazard cat. 7 (a manual / out-of-band operational step), with the pranked fork dry-run as the structural mitigation.
+- **Hot-key custody: the deploy key deploys and holds NOTHING.** The broadcasting key of a deploy/operational script — the CI deploy key, any EOA a workflow signs with — is a **hot wallet**, the most exposed key in the system, so authority must never route through it, even transiently. Governance (the Safe / timelock / owner multisig) is admin/owner **from block one**: an initializer argument to the deterministic clone for an **instance / per-token proxy**, a hardcoded constant for an **implementation** (singleton, beacon, deployer), which per **Deterministic production deployments** takes NO constructor arguments and bakes its config into fixed init bytecode. **From block one is literal — the seating is ATOMIC with creation**: an instance still initializable in a later transaction than the one that deployed it is front-runnable into seating an attacker, the same defect with a worse principal, so the deploy and the initialize are one transaction (a factory call that does both, or init data the proxy constructor consumes). The deploy key signs the deploy transaction and holds none of the authority the deployed contracts expose. Everything above is normative on its own; the three flags below are the shapes the **hot-key** defect takes, not a checklist that exhausts the rule — an instance a stranger can still initialize trips it with no key compromised and no flag matched.
+  - **Transient hot-key authority.** Any deploy/init path where the broadcaster acquires authority even momentarily: `initialAdmin` / `owner` taken from the calling context instead of an explicit governance argument (`msg.sender` where the broadcaster calls directly, `tx.origin` where a factory sits between and `msg.sender` would be the factory) or otherwise set to the deployer, an Ownable constructor followed by a `transferOwnership` step, a grant-then-renounce ceremony, an admin handoff scheduled "later". The window is the finding regardless of whether the handoff eventually happened — a hot-key compromise inside it owns everything the authority gates — and the ceremony itself is extra out-of-band choreography (Hazard cat. 7) that can be forgotten, half-run, or reordered; where such a ceremony nonetheless ships, it is a state-changing authorization mutation and carries the pranked-fork rule's dry-run and n+1 rehearsal, but its *existence* is this rule's finding first. Live shape: `S01-Issuer/st0x.deploy`'s June 2026 authoriser clone deploy initialized `initialAdmin = <deploy key>` — the hot key held **all seven `_ADMIN` roles for ~18 days** until a 3×`grantRole` + 7×`renounceRole` batch, decoded from the key's own tx history, handed the hierarchy to the token-owner Safe; the corrected deploy path (`S01-Issuer/st0x.deploy#250`) passes the Safe as `initialAdmin` so the key never enters the role graph. **Severity by what the authority gates: HIGH** where it gates value, minting, upgrades, or permissions. The proposed fix is always governance-from-block-one — never a faster or better-rehearsed ceremony.
+  - **Handoffs are per-chain claims.** Where the transient shape already shipped, evidence the handoff completed on ONE chain proves nothing about the others — each chain's clone/beacon/contract went through its own ceremony, or silently did not (`st0x.deploy`'s Base and Ethereum clones each needed their own grant-and-renounce batch, and nothing enforced that both ran). Flag a handoff asserted globally from single-chain evidence; the check is per-chain live verification — a fork assertion per supported network (the network set taken from wherever the repo itself enumerates it, per **Deploy-pin completeness & release historicals**) that the intended principal holds the authority on THAT chain. **MEDIUM**, escalating on the first flag's scale for any chain where the handoff in fact never ran.
+  - **Invariant surfaces assert the NEGATIVE.** An invariant lib / prod fork suite that asserts expected principals hold their expected authority but never asserts the broadcast key — and every retired key — holds **nothing** is incomplete: it proves arrival, not absence, and a stray grant to the hot key passes it silently. Require the negative assertion alongside the positive grants, per chain: for each live contract, the deploy key and every retired key hold **none of the authority that contract exposes** — the slot set read off the contract's own surface rather than a fixed list, because whatever list a suite hardcodes, the next authority pattern is off it (e.g. AccessControl roles and `DEFAULT_ADMIN_ROLE`, an Ownable `owner` / `pendingOwner`, a proxy's admin slot, a Safe's owners and threshold, a timelock's proposer / executor / canceller, any bespoke `onlyX` address the contract reads). Reference shape: the deployer-holds-nothing assertion in `S01-Issuer/st0x.deploy#250`'s `LibAuthoriserInvariants`, composed into the same `assertAll` the prod fork tests run. **MEDIUM** for the missing negative check, escalating on the first flag's scale where a stray grant turns out to be live.
+
+  **Non-findings:** a deploy key whose only on-chain acts are deploy transactions meeting the seating requirements above — the key holds no authority the deployed contracts expose at any point, and creation leaves no window in which anyone could seat some; authority the protocol necessarily records about the deployer that is structurally inert — recorded but not exposed, so no path gates on it (a factory logging `msg.sender` as clone-event metadata); and a documented, deliberate single-key governance design whose key is NOT the broadcaster (a repo whose declared owner IS an EOA other than the deploy key has a key-custody design to review under Security, not a transient window under this rule — the exemption is for WHO governance is, never for how it was seated; naming the broadcasting key itself the standing owner is this rule at its strongest, not an exemption from it). Sender-independence of the ADDRESS is not sender-independence of the AUTHORITY: a Zoltu CREATE2 address is a pure function of its init code and says nothing about what the constructor or initializer writes into storage, so a broadcast that seats `initialAdmin = msg.sender` is the first flag above however deterministic the address it lands at.
 - **ERC-165 interface discovery.** `supportsInterface` is a discovery contract consumers trust to route/validate, so hold it to four rules:
   - **Use OZ `ERC165` + `super`, never a hand-rolled base check** (code-quality / correctness): a contract MUST inherit OpenZeppelin's `ERC165` and delegate the base case to `super.supportsInterface(interfaceId)`, adding only its own id(s) — `interfaceId == type(IFooV1).interfaceId || super.supportsInterface(interfaceId)`. Declaring `is IERC165` and writing the base membership check literally (`interfaceId == type(IERC165).interfaceId`) re-implements library logic and drifts from canonical (it's how the `0xffffffff` sentinel or a future base change gets silently mishandled) — a finding **even when it happens to be correct today**.
   - **No vacuous advertisement** (hazard: dead surface): a contract that implements `supportsInterface` but advertises **only** `type(IERC165).interfaceId` (or nothing beyond it) is vacuous — *every* ERC-165 contract returns true for `IERC165`, so it conveys nothing. Either advertise a real contract-specific interface, or remove the dead ERC-165 surface.