Skip to content

feat(safe): add guard lifecycle scripts - #93

Merged
makemake-kbo merged 5 commits into
masterfrom
makemake/eng-4054-featguard-script-to-deploy-and-remove-guard
Jul 27, 2026
Merged

feat(safe): add guard lifecycle scripts#93
makemake-kbo merged 5 commits into
masterfrom
makemake/eng-4054-featguard-script-to-deploy-and-remove-guard

Conversation

@makemake-kbo

Copy link
Copy Markdown
Contributor

Summary

  • make CredibleSafeGuard fail open when registry reads revert, exhaust their bounded gas budget, or return malformed data
  • add Foundry scripts to deploy the guard and generate checksummed Safe Transaction Builder JSON for installation and removal
  • validate Safe/guard addresses and Safe guard ERC-165 support before generating a batch
  • document the Safe{Wallet} import, review, signing, replacement, and removal workflow

Testing

  • FOUNDRY_PROFILE=safe-guard forge test --offline -vv — 20 passed
  • forge test --offline --match-path test/protection/safe/CredibleSafeGuard.t.sol -vv — 29 passed
  • FOUNDRY_PROFILE=safe-guard forge build --offline
  • forge fmt --check
  • git diff --check

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Jul 24, 2026

Copy link
Copy Markdown

ENG-4054

Comment thread examples/safe-guard/script/GenerateSafeGuardBatch.s.sol
Comment thread src/protection/safe/CredibleSafeGuard.sol
Comment thread examples/safe-guard/script/DeployCredibleSafeGuard.s.sol
Comment thread src/protection/safe/CredibleSafeGuard.sol
…ry probes

Address review feedback on the guard lifecycle scripts:

- DeployCredibleSafeGuard now validates the registry before broadcasting:
  reverts RegistryHasNoCode for a codeless address (e.g. a typo resolving to
  an EOA, which would otherwise deploy a permanently-fail-open guard) and
  RegistryReadFailed unless both isCredibleBlock and lastCredibleBlock return
  a well-formed 32-byte word. Kept out of deploy() so constructor validation
  is preserved for in-memory mocks.

- CredibleSafeGuard tests gain two adversarial registry doubles: a gas
  guzzler whose reads burn far more than the 50k probe budget, and a
  returndata bomb returning >32 bytes with a leading "credible" word. New
  tests prove the bounded staticcall fails open gracefully on both reads
  without propagating OOG or copying unbounded returndata.
…decision

Per repo-owner sign-off on the enforcement-boundary review comment: keep the
existing fail-open behavior exactly as-is (no control-flow change) but make it
an explicit, documented product decision rather than an implicit side effect.

Expand the contract-level NatSpec and annotate the enforcement decision points
(_checkCredibleBlock, _tryIsCredibleBlock, _tryLastCredibleBlock) to state:
- the guard fails OPEN when a registry read reverts / has no code / returns
  malformed or non-32-byte data / exceeds the 50k gas budget;
- the rationale: never permanently brick the Safe's owner path on an
  unavailable/broken/misconfigured registry, degrading to no protection;
- the security tradeoff plainly: a broken/absent/malicious registry allows all
  owner transactions through;
- that the deploy-time validateRegistry check (codeless/EOA rejection + read
  verification before broadcast) is the mitigation, since the registry address
  is immutable.
Comment thread examples/safe-guard/script/DeployCredibleSafeGuard.s.sol Outdated
Comment thread src/protection/safe/CredibleSafeGuard.sol Outdated
…egistry

The deploy script's validateRegistry only checked that isCredibleBlock
returned exactly 32 bytes, so a registry answering abi.encode(uint256(2))
passed deploy-time validation while the guard's runtime decode
(_tryIsCredibleBlock, value > 1) rejects it as unreadable -- silently
deploying a permanently-fail-open guard. Decode the word as a canonical
bool (reject > 1) up front, mirroring the runtime check, and add a
regression test with a mock registry returning uint256(2).
A broken registry returning a lastCredibleBlock beyond the current block
(e.g. type(uint256).max) previously kept fail-open disabled and reverted
every owner transaction until the chain reached that height, effectively
bricking the Safe. Such a value is impossible for a sound registry, so
treat it as an unreadable response and fail open, consistent with the
guard's registry-failure policy.

Updates the unit test that previously preserved the bricking behavior,
adds max-uint and equal-to-current cases, and adds a real-Safe E2E
fail-open test for a future reported height.
Comment thread examples/safe-guard/script/DeployCredibleSafeGuard.s.sol
Comment thread examples/safe-guard/script/DeployCredibleSafeGuard.s.sol
@makemake-kbo
makemake-kbo merged commit 6fdc01c into master Jul 27, 2026
10 checks passed
@makemake-kbo
makemake-kbo deleted the makemake/eng-4054-featguard-script-to-deploy-and-remove-guard branch July 27, 2026 09:55
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.

2 participants