Return HTTP 451 for banned Safes - #2966
Merged
Uxío (Uxio0) merged 5 commits intoJul 30, 2026
Merged
Conversation
Safes with SafeContract.banned were only excluded from indexing. Now: - Add BannedSafeMixin: every Safe-scoped endpoint (v1, v2, messages, 4337) returns 451 Unavailable For Legal Reasons for a banned Safe - Don't publish queue events for banned Safes - Exclude banned Safes from owners endpoints - Cache the banned address set in memory (BANNED_SAFES_CACHE_TTL, 5m default) and use it for event filtering and tx processing
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a74710c6e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
Uxío (Uxio0)
marked this pull request as draft
July 29, 2026 15:12
- Exclude banned Safes from module lookups too - Recheck the ban when the events commit callback runs, so a Safe banned inside a still open transaction is also enforced - Reuse get_banned_addresses() for the owners/modules exclusions
SafeContract is written frequently, so clearing the cache on every save would make it useless. Accept that bans propagate within BANNED_SAFES_CACHE_TTL in every process: - Remove the cache clearing signal - Filter banned payloads when the event is scheduled instead of on commit
Set a custom SafeAutoSchema as DEFAULT_SCHEMA_CLASS that adds the 451 response to the OpenAPI schema of every view using BannedSafeMixin, instead of documenting it by hand on each view
Uxío (Uxio0)
marked this pull request as ready for review
July 30, 2026 10:01
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8111691cbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
Felipe Alvarado (falvaradorodriguez)
approved these changes
Jul 30, 2026
Uxío (Uxio0)
deleted the
uxio/pla-1785-txs-service-banned-safes-should-return-http-451-instead-of
branch
July 30, 2026 10:44
Clóvis Neto (clovisdasilvaneto)
pushed a commit
to safe-global/safe-client-gateway
that referenced
this pull request
Aug 20, 2026
The Transaction Service now answers every Safe-scoped endpoint with HTTP 451 Unavailable For Legal Reasons when a Safe is banned from the indexer, reporting the reason under `detail`. `HttpErrorFactory` only reads `message` from an upstream error body, so these responses reached clients as a 451 carrying the generic `An error occurred`. Add `mapBannedSafeError`, which rewrites a 451 payload before the factory reads it, and apply it in the three datasources that call the Transaction Service: `TransactionApi`, `SafeBalancesApi` and `ExportApi`. The mapping is scoped to those datasources rather than placed inside `HttpErrorFactory` because 451 only carries this meaning for the Transaction Service; every other upstream keeps forwarding its own message unchanged. The `should forward a %s error` cases in the Transaction Service specs drew a random 4xx/5xx status that could land on 451, so they now exclude it through a single shared helper and 451 gets its own dedicated case. Refs: WA-2969 See: safe-global/safe-transaction-service#2966
Clóvis Neto (clovisdasilvaneto)
added a commit
to safe-global/safe-client-gateway
that referenced
this pull request
Aug 21, 2026
) * feat: handle HTTP 451 banned-Safe responses with a custom message The Transaction Service now answers every Safe-scoped endpoint with HTTP 451 Unavailable For Legal Reasons when a Safe is banned from the indexer, reporting the reason under `detail`. `HttpErrorFactory` only reads `message` from an upstream error body, so these responses reached clients as a 451 carrying the generic `An error occurred`. Add `mapBannedSafeError`, which rewrites a 451 payload before the factory reads it, and apply it in the three datasources that call the Transaction Service: `TransactionApi`, `SafeBalancesApi` and `ExportApi`. The mapping is scoped to those datasources rather than placed inside `HttpErrorFactory` because 451 only carries this meaning for the Transaction Service; every other upstream keeps forwarding its own message unchanged. The `should forward a %s error` cases in the Transaction Service specs drew a random 4xx/5xx status that could land on 451, so they now exclude it through a single shared helper and 451 gets its own dedicated case. Refs: WA-2969 See: safe-global/safe-transaction-service#2966 * refactor: address review feedback on banned-Safe handling Follow-up to the review on #3370. Let the 451 status decide before any payload shape is read: `mapError` now guards with `isBannedSafeError` up front, so a 451 that also carried a `nonFieldErrors` array — a shape the Transaction Service does not currently pair with 451, but one CGW does not control — still gets the banned-Safe message. Extract the "random 4xx/5xx status excluding a given code" loop, duplicated across three specs, into `errorStatusCodeExcluding` in the shared test faker. Cover the second `SafeBalancesApi` catch site at route level: there is no datasource spec for `SafeBalancesApi`, so `/collectibles` had no banned-Safe coverage at all. Generate the upstream `detail` text with faker rather than hard-coding it — nothing asserts on it, since the mapper discards it. The `detail` key stays literal: that it is not `message` is the whole point of the mapper. Refresh the "One error funnel per layer" canonical example, which quoted the `SafeBalancesApi` catch blocks this change rewrote. Refs: WA-2969 * test(balances): cover getBalance's banned-Safe error funnel `getBalance` bypasses the cache and calls `INetworkService` directly, and its only caller is the no-fee relayer validation path, so no route reaches its catch block. With no datasource spec for `SafeBalancesApi`, the `mapBannedSafeError` wiring at that third catch site was untested. Add a focused spec for the funnel: a 451 yields the dedicated banned-Safe message, and any other error status still forwards the upstream one. The real `HttpErrorFactory` is used rather than a mock so the assertion lands on the `DataSourceError` a caller actually sees. Scoped to the error funnel on purpose — `getBalance`'s success and token-not-found paths are pre-existing behaviour this change does not touch. Refs: WA-2969 * test(balances): cover getBalances and getCollectibles banned-Safe funnels All three SafeBalancesApi read methods apply the same mapBannedSafeError, but only getBalance had isolated coverage; its two siblings were exercised solely through their integration specs. Parameterise the spec over the three methods so each catch site is pinned directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VsYyzsAtDgmiFzrcaNtphA * test: construct real Response objects instead of casting in new specs Every NetworkResponseError this PR adds built its response with a `{ status } as Response` cast. A real `new Response(null, { status })` carries the same status with no cast, so use it for the 13 sites the PR introduces; pre-existing casts elsewhere are left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VsYyzsAtDgmiFzrcaNtphA --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong? 👾
Fixes PLA-1785
SafeContract.bannedonly stopped indexing. A banned Safe kept serving (stale) data on every endpoint, could still propose transactions, appeared in/owners/{address}/safes/and kept emitting queue events. EU sanctions (Regulation (EU) 2026/1848, effective 2026-08-23) require the identified Safes to not be served at all.How was it fixed? 🎯
BannedSafeMixinapplied to every Safe-scoped endpoint (v1, v2, messages, 4337): requests for a banned Safe return451 Unavailable For Legal Reasonswith body{"detail": "Safe is unavailable for legal reasons"}. The check is a single indexed DB lookup (not cached), so a ban is enforced immediately. It runs before the view cache, so a 451 is never cached or served from cache.send_payloads_on_commit()chokepoint that filters banned payloads./v1|v2/owners/{address}/safes/. Checked withEXPLAIN (ANALYZE, BUFFERS): the GINowners @>index scan stays the driver, the exclusion only adds a hashed subplan on the tiny partial banned index.BANNED_SAFES_CACHE_TTL, default 5 minutes) following theTokenService.get_trusted_token_addressespattern. Used by event filtering andSafeTxProcessor(replaces the per-batch query). Cleared onSafeContractsave/delete in the same process, other processes refresh on TTL.getSafeand multisig-transactions endpoints so CGW can rely on it.Out of scope (agreed): hash-keyed detail endpoints (
/multisig-transactions/{safe_tx_hash}/, messages by hash, 4337 by hash) don't check the ban.New tests are isolated in
TestBannedSafeViewsclasses per app, so this behavior can be adjusted easily if regulatory requirements change.