[SEC-131] Enforce local Ethereum transaction expenditure policy - #531
Conversation
Change-Id: I416216011bf58f90a1ca1ef9d9f195609095e8f2
Change-Id: Ide86fccb3cda8ea3f777199bddee1e7c705064af
Change-Id: I2b3698563780ea37f0e1836f0f4793ce75fc6404
Change-Id: Iea03b91f06f9cff21432e44267594404a3445fef
Change-Id: I7f67f6d889805c959b6d87a3c316c517dd682955
heifner
left a comment
heifner
left a comment
There was a problem hiding this comment.
Reviewed the full diff. The core of this change is right — the RLP/QUANTITY encoding fixes and the expenditure-policy plumbing are solid (details at the bottom). Findings below, most severe first.
A framing note that applies throughout: a misconfigured nodeop should log and exit at startup rather than continue, and invalid options should never be silently ignored. Two of the findings below are cases where this PR moves away from that.
1. HIGH — the new chain-id guard makes the underwriter exit the node on a valid config
plugins/outpost_ethereum_client_plugin/src/outpost_ethereum_client_plugin.cpp:467
The new entry->chain_id != chain_id guard in create_outpost_client fires on a config the rest of the plugin treats as legitimate, and the failure is terminal.
The chain:
- The underwriter's wiring loop iterates
outpost_endpoints(operator config), whileoutpost_external_chain_idsis populated byread_outpost_registry()fromsysio.chains::chains. read_outpost_registrydeliberately skips inactive chains (underwriter_plugin.cpp:1094).- So a configured chain whose registry row is inactive or absent yields
ext_id = 0— a caseunderwriter_plugin.cpp:869-870explicitly documents as "harmless — no leg references it until it is active". entry->chain_idis now always non-zero (the 3-field path rejects zero at:155; the 4-field path parses an operator literal).- The guard throws
ethereum_transaction_policy_exception, which derives fromfc::exception(ethereum_transaction_policy.hpp:50). underwriter_plugin.cpp:902catches it →gate_state = wiring_failed.
Two amplifiers:
- It shuts the node down.
wiring_failedis terminal (sync_detail.hpp:55,is_terminal_failure), andquit_if_startup_failed_terminally()callsapp().quit(). Under a restarting supervisor the registry state is unchanged on restart, so this is a crash loop. - One chain takes down all of them. The
trywraps the entire wiring loop and thecatchreturns before cron registration, so a single configured-but-inactive EVM chain aborts wiring for every healthy chain in the process.
It is reachable through a green preflight: check 2 (:477-517) validates active ⊆ configured, so a configured superset passes. The triggering window is a normal new-chain rollout, where the flag is configured before activchain runs. Master had no guard here at all — chain_id went straight to the constructor — so this is entirely new behavior.
Note also that outpost_solana_client_plugin::create_outpost_client has no equivalent check, so identical registry state is fatal for an EVM chain and harmless for an SVM one.
Suggested fix — keep fail-fast, but move the detection to the layer that can explain it. ext_id = 0 is not operator config; it is manufactured at underwriter_plugin.cpp:876-879 from missing depot state, and the guard reports it as configuration_chain_id_mismatch with observed=0, allowed=<id>, naming neither the chain nor the real problem. Preflight is already the config-vs-depot-state validator: check 2 compares the same two maps in the opposite direction. Adding the inverse check there — a configured chain with no active registry row — gives an actionable message ("chain X is configured but has no active sysio.chains row — run activchain or remove the flag"), reuses the existing 15s bounded grace (preflight_retry_grace_ms) before failing terminally, and fails before any client is built so it cannot half-wire other chains.
With that in place create_outpost_client never receives 0, and the guard at :467 can stay exactly as written as a true invariant assertion — and should be mirrored onto the Solana path.
The ext_id = 0 "harmless" comment at :869-870 should be deleted as part of this. That is the upside of the strict behavior: today a typo'd chain code in --underwriter-eth-outpost is silently inert forever (never active, so never selected, so never diagnosed). Fail-fast catches it.
2. HIGH — configured chain id is no longer checked against the endpoint
libraries/libfc/src/network/ethereum/ethereum_client.cpp:240, plugins/outpost_ethereum_client_plugin/src/outpost_ethereum_client_plugin.cpp:295
Master validated an explicit chain id against the endpoint's eth_chainId and refused startup on mismatch. That check is removed for both file clients and 4-field legacy clients, and get_chain_id() now returns the configured value unconditionally.
Concretely: chain_id: 1 configured against a Sepolia rpc_url — a one-character typo, or a mainnet block copy-pasted into a multi-chain file — now starts clean. The client signs mainnet-domain EIP-1559 transactions and broadcasts them to Sepolia, where the signed payload is public and directly replayable on Ethereum mainnet, spending real funds from the operator key.
"Don't trust the RPC for the signing domain" is the right principle and I'd keep it — but it doesn't require discarding the agreement check. A misconfigured chain-id/endpoint pair is an invalid option, and should log and exit at startup rather than run. Restoring this as a fatal startup check preserves both properties.
3. MEDIUM — one shared 5s budget across all legacy chain-id resolutions
plugins/outpost_ethereum_client_plugin/src/outpost_ethereum_client_plugin.cpp:255-258
A single fc::task::deadline_scope is now created before the resolution loop, where create_validated_client previously scoped it per client. The scope installs an absolute thread-local deadline and clamps nested calls to the earliest one, so clients share one budget rather than each getting one.
With three three-field clients, a first endpoint that takes 4.8s leaves clients 2 and 3 under 0.2s; both fail with "Unable to resolve chain id" and abort startup despite healthy endpoints. Moving the scope inside resolve_legacy_chain_id, or budgeting N × timeout, fixes it.
4. MEDIUM — three-field clients now hard-require a reachable RPC at plugin_initialize
plugins/outpost_ethereum_client_plugin/src/outpost_ethereum_client_plugin.cpp:297
resolve_legacy_chain_id issues eth_chainId during init and fails the whole plugin when the endpoint is unreachable. The test change confirms the inversion: startup_accepts_three_field_client_without_remote_chain_id_check (using connection_closing_http_server, asserting no throw) became startup_resolves_three_field_client_chain_id_from_rpc against a live server. A node restarted before its Ethereum RPC is up previously booted fine on a three-field spec and now aborts.
Failing fast is the right instinct here, so this is a design question rather than a clear defect: an unreachable endpoint is a transient dependency rather than an invalid option. Worth considering the same bounded-grace-then-exit shape preflight already uses, instead of aborting on the first connection failure.
5. LOW — duplicate chain_id across file clients is not rejected
libraries/opp/client_config/src/client_config_loader.cpp:224
The loader rejects duplicate client_id (:207) but nothing rejects two clients sharing a chain_id. get_client_by_chain_id (:353) returns nullptr on the ambiguity, so batch_operator_plugin.cpp:601 emits a wlog and skips that outpost on every refresh, forever — a config typo becomes a permanent silent no-relay condition with a successful startup. Since the file format is new, rejecting duplicates alongside the existing client_id check is cheap and matches the fail-fast-on-bad-config posture.
6. LOW — RPC decode faults are reported as spend-policy rejections
libraries/libfc/src/network/ethereum/ethereum_client.cpp:229
parse_rpc_quantity throws ethereum_transaction_policy_exception, so a malformed or non-canonical eth_getTransactionCount / eth_gasPrice / eth_estimateGas, or a block missing baseFeePerGas, is caught by create_default_tx / get_gas_config and logged at elog as "Ethereum transaction policy rejected" with reason_code=rpc_quantity_invalid (:410, :431, :451, :531). An operator debugging a non-compliant RPC provider sees high-severity policy-rejection lines and hunts an expenditure-config problem that does not exist. A distinct log path or exception type for transport/decode faults would separate them from genuine cap violations.
Verified correct — no action
- The
encode_uint56 → 248shift is inside theis_same_v<T, fc::uint256>branch only: no UB foruint64_t, and it fixes real truncation above 2^64. parse_legacy_chain_idcarries its overflow guard.to_data_from_params(..., true)is behavior-equivalent to the deleted inline prefixing.- The
estimate_gaschange fromto_hex(rlp::encode_uint(gc.max_fee_per_gas))toformat_rpc_quantity(gc.tip)/format_rpc_quantity(gc.max_fee_per_gas)fixes both an RLP-vs-QUANTITY encoding bug and a wrong-field bug. is_safe_network_host_implrenaming keeps all internal call sites resolving through the header declaration.libraries/opp/CMakeLists.txt'sGLOB_RECURSE proto/*.protodoes not reachclient_config/proto, so the generation-isolation claim holds.
Change-Id: Ibaf6c5e63b642ac9e2fb42539ea35a774fe099a3
|
@heifner Thanks for the detailed review. I addressed all six findings in follow-up commit
The affected Clang 18 Release targets and focused suites pass locally, and current-head Linux, Apple Silicon, and OPP bundle CI are all green. The PR description has also been refreshed for the complete current diff. Please take another look when you have a chance. |
heifner
left a comment
heifner
left a comment
There was a problem hiding this comment.
Re-reviewed ba0b3eb94d against the previously reviewed 270f03e2c0, plus the full diff vs master. All six findings are addressed, and the two HIGH fixes are structurally right rather than patched over. Two new LOW items, both diagnosability.
Verification of the six findings
1. HIGH — chain-id guard exits the node on a valid config → FIXED, at the right layer
find_endpoint_coverage_gap (routing_detail.hpp:120-131) now runs both directions with std::optional<int> per side, and preflight emits the actionable message (underwriter_plugin.cpp:493-498) and returns false — so it reuses the 15s preflight_retry_grace_ms before going terminal, exactly the suggested shape. ext_id is now outpost_external_chain_ids.at(chain_code) with the 0 fallback and the "harmless" comment deleted, and read_outpost_registry populates that map unconditionally (:1104), so the two maps always share a key set.
I checked the other caller: batch_operator_plugin.cpp:601-615 derives entry from get_client_by_chain_id(op.chain_id), so entry->chain_id == chain_id holds by construction there and a missing client is a wlog + skip. The guard at :506 is now genuinely unreachable from valid config on both paths.
On mirroring the guard onto the Solana path — correctly not done: solana_client_entry_t (outpost_solana_client_plugin.hpp:59-64) carries no chain_id, so there is nothing to compare against. That sub-item of my original review was wrong.
2. HIGH — configured chain id no longer checked against the endpoint → FIXED
add_client(..., rpc_chain_id_validation::required) for file clients and explicit four-field legacy; the SYS_ASSERT at :250 carries both values. get_chain_id() still returns the configured value, so "don't trust the RPC for the signing domain" is preserved while the agreement check is back — which is what I was asking for. Covered by file_configuration_rejects_rpc_chain_id_mismatch and startup_rejects_explicit_chain_id_mismatch.
3. MEDIUM — one shared 5s budget → FIXED, and the budget is real
deadline_scope moved inside resolve_rpc_chain_id (:177). I verified it actually bounds in-flight calls rather than just the retry loop: http_client.cpp:141-153 folds fc::task::current_deadline() into the request deadline, and timeout_options::inherit_task_deadline defaults to true (http_client.hpp:148; only file downloads opt out).
4. MEDIUM — three-field clients hard-require a reachable RPC → addressed as a design decision
200ms→1s backoff inside the 5s budget, transient transport retried, exhaustion fails closed. startup_retries_transient_chain_id_transport_failure proves recovery from a reset first connection. Reasonable. Noting only that the grace is 5s where preflight uses 15s.
5. LOW — duplicate chain_id across file clients → FIXED
client_config_loader.cpp:232-236, placed after the range check and before the has_transaction_policy() continue, so policy-less clients are covered too. Test added.
6. LOW — RPC decode faults reported as spend-policy rejections → FIXED
is_rpc_quantity_rejection plus a separate Ethereum RPC response rejected log path that correctly drops allowed= (meaningless for a decode fault).
I also re-confirmed the uint256 cap math is unchanged and still correct — the MAX/gas_limit, MAX/2, and MAX/6 pre-division guards each bound their multiplication exactly.
New findings
LOW — resolve_rpc_chain_id discards every transport exception, so a startup abort has no cause
plugins/outpost_ethereum_client_plugin/src/outpost_ethereum_client_plugin.cpp:180-185
The retry lambda catches fc::exception / std::exception and returns std::nullopt with no logging. After ~7 silent attempts the operator gets one line:
Unable to resolve or validate chain id for outpost Ethereum client 'X' within the bounded RPC startup grace
DNS failure, TLS error, 401 Unauthorized, and connection-refused are indistinguishable, and the endpoint isn't named. Master was equally opaque, but it made one attempt and only for four-field specs; finding 4's fix makes a reachable RPC a hard startup requirement for three-field clients too, so this path is now materially more likely to be what an operator is staring at during a restart.
A wlog per failed attempt — or retaining the last exception and folding its message into the final throw — closes it. fc::http::sanitized_endpoint(fc::url(url)) is already used in this file for exactly that.
LOW — the surviving configuration_chain_id_mismatch still names neither the chain nor the client
plugins/outpost_ethereum_client_plugin/src/outpost_ethereum_client_plugin.cpp:506-511
With finding 1 fixed, the guard is reachable only on genuine misconfiguration: a --underwriter-eth-outpost <code>,<client-id>,<addr> whose client is bound to a different chain. Terminal exit is right there. But what the underwriter logs is reason_code=configuration_chain_id_mismatch field=chain_id observed=<registry id> allowed=<client id>, wrapped in "Ethereum transaction policy rejected" — no chain code, no client id, and it reads as a spend-policy problem.
Both eth_client_id and chain_code are parameters of the enclosing function. This is the residual half of my original finding 1's "names neither the chain nor the real problem".
Note for the runbook, not a defect
The inverse coverage check is bidirectional and terminal, as requested. The consequence worth writing down somewhere: deactchain on the depot now means every underwriter still carrying that chain's flag fails preflight and exits at its next restart. Ordering becomes activchain → add flag → restart on rollout, and remove flag → restart → deactchain on decommission.
Reviewed statically; I did not rebuild — the Clang 18 Release run and current-head CI cover that.
Change-Id: I02eac918ea7fac5376d673e2c45dedb6ad3b3d48
heifner
left a comment
heifner
left a comment
There was a problem hiding this comment.
Both LOW findings from the last round are fixed in 7145d9a71b, and the tests are better than what I asked for. Approving — the three items below are nits and a follow-up, none of them blocking.
Verification
LOW-A — swallowed transport cause → FIXED
resolve_rpc_chain_id now carries endpoint= (via fc::http::sanitized_endpoint) and a last_failure= category into the throw. The last_failure selection is the subtle part and it's right: timeout_total only overwrites when nothing more specific was recorded (:226-231), so the deadline firing at the end doesn't mask the real first cause (dns, tls_handshake, http_status:401), and retry_budget_exhausted covers the case where no attempt threw at all. sanitized_chain_id_failure_cause only ever emits the failure-kind name plus an optional three-digit status — never message content — with fc_exception:<name> as the fallback.
The test is the part worth calling out: startup_rejects_unavailable_rpc_after_bounded_grace now feeds http://operator:super-secret@host/rpc?token=secret and asserts both that endpoint=<sanitized> and last_failure=io appear and that super-secret / token=secret do not. That pins the sanitization contract rather than just the diagnostic.
LOW-B — unnamed chain/client on the mismatch → FIXED
create_outpost_client:557-566 names all four values (chain=ETH, client_id, registry_chain_id, client_chain_id). Changing the type from ethereum_transaction_policy_exception to chain::plugin_config_exception goes past what I asked for and is the right call — this was never a spend-policy rejection — and the test asserting detail.find("transaction policy") == npos locks that in. It still derives from fc::exception, so the underwriter's catch still lands on wiring_failed. Deleting the now-unused configuration_chain_id_mismatch enum member is correct cleanup; I confirmed no stragglers.
I also sanity-checked the new includes: magic_enum, failure_kind_name, and sanitized_endpoint all arrive via http_client.hpp, and <algorithm> / <ranges> were already present.
No new defects.
Three nits, none blocking
1. The reason code is now a bare string literal
configuration_chain_id_mismatch_reason (outpost_ethereum_client_plugin.cpp:39) is hand-spelled, where every other reason code in this subsystem comes from a typed enum via reason_code_name / client_config_reason_name. client_config_reason already lives next door with chain_id_invalid and chain_id_duplicate; a chain_id_registry_mismatch member there plus client_config_reason_name(...) keeps it typed and renames through the compiler. The plugin already includes that header and uses that function in plugin_initialize's catch, so it's a small change.
2. sanitized_chain_id_failure_cause reconstructs a typed enum by string-searching prose
http_client.cpp:132-138 throws "Outbound HTTP {}: {}" where the first field is already failure_kind_name(failure.kind) — the typed kind exists at the throw site, gets flattened to a message, and is recovered here by scanning for all 18 markers. It's correct today (the trailing : disambiguates connect from timeout_connect) and it degrades gracefully, but it is coupled to another library's log format: change that format string and every cause silently becomes fc_exception:..., with no test failing outside this plugin.
The durable fix is for the transport to expose failure_kind on the thrown exception so consumers read the enum instead of parsing prose. Follow-up, not this PR.
3. Legacy specs still bypass validate_rpc_url
File clients get scheme + is_safe_network_host validation (client_config_loader.cpp:111-128); --outpost-ethereum-client gets only !url.empty() (:370-374). Not a regression — master had neither — but it leaves the PR's "transport-valid HTTP(S) hosts" guarantee applying to only one of the two configuration paths.
Closing it would also clean up a small attribution loss this commit introduced: fc::url(url) moved above the try at :212, and for a three-field legacy spec that is the first code to parse the URL, so a malformed one now throws a bare "Unable to parse URL scheme" out of plugin_initialize with no client id attached — where master's create_validated_client catch produced a client-identified failure. I checked fc::url: its parse errors use static messages and never echo the URL, so there is no credential exposure here, only lost attribution. Exporting validate_rpc_url and calling it from load_legacy_clients closes both.
Reviewed statically across 270f03e2c0 → ba0b3eb94d → 7145d9a71b; I did not rebuild — the Clang 18 Release run and CI cover that.
|
Tracked the three non-blocking approval-review notes as follow-up Jira tasks: SEC-147 (typed configuration mismatch reason), SEC-148 (typed outbound HTTP failure kinds), and SEC-149 (legacy Ethereum RPC URL validation). These remain non-blocking for SEC-131. |
Change-Id: I27e1312d354993091e5857847e9ceccbb7292ac3
Change-Id: Id05311057427ac365bec3fbdfc0fb3e2f99b9464
Change-Id: Id17e276f84607ee9e5085f12eb70a6c24f18b418
|
@heifner, follow-up is ready for re-review at |
heifner
left a comment
heifner
left a comment
There was a problem hiding this comment.
Re-reviewed a7e5153a60 against the previously approved 7145d9a71b. The delta is the startup-diagnostic test stabilization, four documentation corrections, and a master merge. Re-approving; one docs-only nit below.
Scope of the delta
The combined diff on the merge commit is empty, so it carried no conflict resolutions, and the master side (sysio.epoch, sysio.msgch, the attestation/types protos, nodeop_chainbase_allocation_test.py) touches none of this PR's files. The branch now contains all of origin/master including #534, whose files overlap this PR's most heavily. So the only reviewed-code changes since the approval are the test and the four doc/help-text strings.
Test stabilization — correct, and it keeps the contract
Accepting either connect or io in startup_rejects_unavailable_rpc_after_bounded_grace is the right call rather than papering over a defect: against connection_closing_http_server the reset can land before or after the handshake completes, so both are legitimate categories for the same server behavior.
The part that matters is that it did not loosen into a tautology. The match stays anchored on the last_failure= prefix, so timeout_connect still does not satisfy it despite containing connect, and I checked the enum — connect and io are the only members with those exact spellings, and no other member is a prefix-extension of either. A regression to retry_budget_exhausted or fc_exception:... still fails the case, and the sanitized endpoint= assertion plus both credential-absence assertions are untouched. That was the contract I cared about last round and it is intact.
Reaching the spellings through fc::http::failure_kind_name instead of literals is a small improvement over what I reviewed — a rename in the transport now propagates into this test through the compiler.
Documentation corrections — all four verified against the code
docs/outpost-client-plugins.mdpreviously claimed startup does not calleth_chainIdfor file-configured clients. That went stale when finding 2 was fixed last round;add_clientis invoked withrpc_chain_id_validation::requiredon the file path, so the new wording is accurate. Good catch — it was asserting the opposite of what the code does.- The
--outpost-ethereum-clienthelp text matchesload_legacy_clients, which selectsrequiredfor a four-field spec andnot_requiredfor three fields. outpost_ethereum_client_plugin::create_outpost_client's@param chain_codenow readssysio.chains::chains, which is correct —sysio.chains.hppdeclares the[[sysio::table("chains")]] chain_row. The previoussysio.epoch::outpostswas wrong.- Dropping the hardcoded
--outpost-ethereum-clientfrom thebatch_operatorskip warning and from both help strings is right now that a client can equally come from the configuration file. The warning an operator sees no longer names an option they may not be using.
Nit — docs, not blocking
docs/outpost-client-plugins.md still describes the three-field form as resolving eth_chainId "under a shared five-second deadline". That was accurate when I filed finding 3, but the fix moved the deadline_scope inside resolve_rpc_chain_id, so each client now gets its own independent five-second budget — the comment on chain_id_resolution_retry_options states exactly that. Since this pass corrected the two neighboring sentences in the same paragraph, "shared" is worth changing to per-client in the same edit.
While there: the bounded grace is now attached only to the three-field form in that paragraph, but with file and four-field clients verifying against the endpoint, every client form requires a reachable RPC at startup. Worth one clause so an operator sizing a restart window does not read it as a three-field-only cost.
Prior round
The three nits from the approval are tracked as SEC-147, SEC-148, and SEC-149. Agreed those are non-blocking for SEC-131 — the second in particular wants the transport to carry failure_kind on the exception, which is not this PR's to change.
Reviewed statically across 7145d9a71b → a7e5153a60; I did not rebuild.
Change-Id: Ic80a99c56d6de04a24c6bb38994bc849905b32d5
Change-Id: Ic1316e4a9712463b1c2bc6f6a83c26aeb47a340e
|
@heifner, addressed the docs-only nit in |
heifner
left a comment
heifner
left a comment
There was a problem hiding this comment.
Re-reviewed at 1556ff4d74. The only change this branch owns since the previous approval is the startup-budget wording in docs/outpost-client-plugins.md; the remaining tree delta is master's net_plugin work arriving through a merge that resolved nothing.
The new wording matches the code. deadline_scope is constructed inside resolve_rpc_chain_id, so each probe opens its own five-second budget instead of sharing one across the client loop. All three client forms do require a reachable endpoint at startup: file clients and four-field legacy clients go through rpc_chain_id_validation::required, and three-field legacy clients call resolve_rpc_chain_id directly to obtain the chain id, after which add_client correctly skips the redundant second probe.
Re-approving.
2 participants
Summary
gas_limit * max_fee_per_gas + valueJsonStringToMessage; reject unknown fields and then apply semantic validation for safe unique client/chain IDs, Ethereum providers, HTTP(S) endpoints, and complete positive policy limits--outpost-ethereum-clientcompatibilityeth_chainId; four-field legacy and file clients verify the endpoint against the locally authoritative signing chain IDnulltransaction policy toUINT256_MAXcompatibility caps; production operators must configure reviewed finite limitssysio.chainsrows and emit actionable configuration diagnostics for missing/inactive chains and chain/client mismatchesConfiguration
{ "schema_version": 1, "clients": [ { "connection": { "client_id": "eth-mainnet", "signature_provider_id": "eth-01", "rpc_url": "https://rpc.example.invalid" }, "chain_id": 1, "transaction_policy": { "max_priority_fee_per_gas_wei": "2000000000", "max_fee_per_gas_wei": "100000000000", "max_gas_limit": "2000000", "max_total_native_cost_wei": "250000000000000000" } } ] }Standard ProtoJSON numeric spellings are accepted for
schema_versionandchain_id. File clients must have unique client and chain IDs. The file and legacy client options cannot be combined.File-configured, four-field legacy, and three-field legacy clients all require a reachable RPC endpoint during startup. Each client receives an independent five-second budget for chain-ID verification or resolution.
Enforcement details
max_fee_per_gas = 2 * base_fee_per_gas + max_priority_fee_per_gasand gas headroom(estimate * 6) / 5with checked arithmeticValidation
1556ff4d74is clean, synchronized with currentmaster, and passesgit diff --check; its review follow-up is documentation-onlyresolve_rpc_chain_idcall receives an independent five-second deadline and all three client forms require startup RPC reachabilitya7e5153a60passed the Clang 18 Release default build, 41 Ethereum client plugin tests, batch-operator tests, underwriter tests, and local full parallel Release validationioorconnect) through the typed failure-kind API while continuing to verify endpoint sanitization and credential redactioncontracts/sysio.*, wire-ethereum, or wire-solanaRelated work
Jira: SEC-131 / WSA-225