fix(security): require admin auth for MintToken in batch operations (#39)#122
Merged
ameeribro4-sudo merged 1 commit intoJul 24, 2026
Merged
Conversation
…penPeerX#39) `authorize_batch_access` did nothing for `BatchOperation::MintToken`, and the `lib.rs` batch entry points set `caller = None` for a mint-first batch. Together these let anyone mint by submitting a batch whose operation is a `MintToken`, bypassing every KYC and admin guard. Gate any batch that mints behind the contract admin. `authorize_batch_access` now flags `requires_admin` on any `MintToken` op and, once, calls the new `require_batch_admin` helper, which loads the stored admin, enforces `admin.require_auth()`, and re-checks `require_admin` (fail-closed to `admin_req` when no admin is configured). Both `execute_batch_atomic` and `execute_batch_best_effort` route through this helper. - Add regression tests: non-admin mint batch reverts (atomic + best-effort), and mint fails closed when no admin is configured. - Extend scripts/check_kyc_guards.sh to enforce the batch admin gate in CI. Closes OpenPeerX#39 Co-Authored-By: Claude Opus 4.8 <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
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.
Summary
Closes #39.
The batch
MintTokenpath was unauthenticated. Inbatch.rs,authorize_batch_accessdid nothing forBatchOperation::MintToken(=> {}),and in
lib.rsthe batch entry points extractedcalleronly fromSwap/AddLiquidity/RemoveLiquidityops, settingcaller = Nonefor amint-first batch. Together these let anyone mint tokens by submitting a
batch containing a
MintToken, bypassing every KYC and admin guard.This PR makes
MintTokenadmin-only inside batches.Changes
peerx-contracts/counter/batch.rs—authorize_batch_accessnow flagsrequires_adminon anyMintTokenop and, exactly once after the loop,calls the new
require_batch_adminhelper. The helper loads the configuredadmin (
ADMIN_KEY), enforcesadmin.require_auth(), and re-checksrequire_admin, failing closed withadmin_reqwhen no admin is configured.Both
execute_batch_atomicandexecute_batch_best_effortroute throughauthorize_batch_access, so both paths are covered by a single, DRY guard.peerx-contracts/counter/src/kyc_tests.rs— regression tests: anon-admin mint batch reverts on both the atomic and best-effort paths, and a
mint batch fails closed when no admin is configured.
scripts/check_kyc_guards.sh— extended so CI enforces the batch admingate (
require_batch_admin,admin.require_auth(),crate::admin::require_admin(env, &admin)).Acceptance criteria
execute_batch_atomicandexecute_batch_best_effortrequire the adminwhen any op is
MintToken(via the sharedauthorize_batch_access).MintTokenreverts.check_kyc_guards.shupdated (and passing).Verification
scripts/check_kyc_guards.shpasses (this is the hard CI gate).batch.rscompiles cleanly; the added tests type-check cleanly.countercrate as a whole does not yet build locally due to thein-progress soroban-sdk migration already tracked in CI ("user-code compile
errors being triaged post-SDK migration"), which is unrelated to this change.
To validate the fix behaves correctly at runtime regardless, the exact
authorization logic was replicated in an isolated Soroban contract and
executed against the host: an unauthorized (non-admin) mint batch reverts, an
admin-authorized mint batch succeeds, and a mint batch with no admin
configured fails closed — while the previous (empty-arm) logic let an
unauthorized mint through.