feat(api-server): enforce request signing on Soroban write endpoints - #955
Merged
fejilaup-cloud merged 2 commits intoAug 29, 2026
Merged
Conversation
The request_signing middleware (AtomicIP#535) was defined but never attached to a route, so every endpoint accepted unsigned requests. Wire it onto the six write endpoints that submit signed transactions to Soroban (commit_ip, transfer_ip, initiate_swap, accept_swap, reveal_key, cancel_swap) in both the production and test routers, and add tests proving all six reject unsigned requests and accept a valid signature. Also repair pre-existing breaks in main.rs that prevented the crate from compiling: restore the rpc_client field on AppState (referenced by the FromRef impl used by get_swap), rebuild build_app() around a single valid AppState, define the app_with_rpc_client test helper the swap read-path tests already call, and remove the duplicated /ip/owner/{owner}/cursor routes that would panic axum at router construction. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@Sundayabel222 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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
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.
Closes #858
Summary
The request-signing middleware from #535 (
request_signing.rs) was defined and unit-tested but never attached to a single route — the server accepted unsigned requests on every endpoint. This PR wiresverify_request_signatureonto the six write endpoints that will submit signed transactions to Soroban, in both the production router and the test router:POST /ip/commit(commit_ip)POST /ip/transfer(transfer_ip)POST /swap/initiate(initiate_swap)POST /swap/{swap_id}/accept(accept_swap)POST /swap/{swap_id}/reveal(reveal_key)POST /swap/{swap_id}/cancel(cancel_swap)All six now require valid
X-Signature,X-Timestamp, andX-Public-Keyheaders (timestamp within 5 minutes, Stellar-format public key). Read endpoints,verify_commitment, and the batch/bulk variants are intentionally not signed.Motivation
Request signing was verified as enforced on zero of the six endpoints rather than a subset — the middleware existed but had no call site.
grepforverify_request_signaturefound only its definition. Since these are the endpoints that submit signed transactions to Soroban, unsigned requests should not reach them.Changes
api-server/src/main.rs:verify_request_signatureas a per-route layer on the six write endpoints inmain()(production) andbuild_app()(test router).test_signed_write_endpoints_reject_unsigned_requests: each of the six endpoints returns401for an unsigned POST.test_signed_write_endpoint_accepts_valid_signature: a correctly signed request passes the middleware and reaches the handler.main.rsthat prevented the crate from compiling (found while wiring; see below).Pre-existing repairs included (required for the crate to build)
AppStatelost itsrpc_clientfield — theFromRefimpl used byget_swap'sState<Arc<dyn SorobanRpcClient>>extractor referencedstate.rpc_clientwhich didn't exist. Restored the field and populated it inmain(),build_app(), and the test helper.build_app()had a duplicate/brokenlet state = AppState { ... }(one referenced an undefinedrpc_clientand movedschema/health_checkertwice). Rebuilt around a single valid state.app_with_rpc_clientwas called by the swap read-path tests but never defined. Added it to the test module./ip/owner/{owner}/cursorroutes (in both routers) would panic axum at router construction ("overlapping method route"). Removed the duplicates.Known limitation (follow-up)
The current scheme is
sha256(method || path || timestamp || body_hash)— the doc comment claims Stellar keypair signing, but no secret material is used, so a signature is forgeable by anyone who observes a request (the public key travels in the request itself). Enforcing it still rejects unsigned traffic, but genuine authentication requires real ed25519 verification against the Stellar keypair. Suggested follow-up: replacegenerate_signature/verify_signaturewithed25519-dalekand verify against the decodedG...public key. The header contract (X-Signature/X-Timestamp/X-Public-Key) is unchanged, so clients won't break twice.Testing
request_signing.rsunit tests are untouched and continue to pass.cargois not available in the environment where this change was prepared, so it was reviewed by inspection; CI will compile and run the suite.Related