test(mpp): cover session on-chain channel-binding rejects#242
Open
EfeDurmaz16 wants to merge 106 commits into
Open
test(mpp): cover session on-chain channel-binding rejects#242EfeDurmaz16 wants to merge 106 commits into
EfeDurmaz16 wants to merge 106 commits into
Conversation
…o fix/python-security-hardening
…o fix/python-security-hardening
…o fix/python-security-hardening
…o fix/python-security-hardening
…o fix/python-security-hardening
…o fix/python-security-hardening
…o fix/python-security-hardening
…o fix/mpp-subscription-hardening
Adversarial review: dropping the reversed release order (canonical key deleted first) passed the whole suite while opening the stranding race the docstring warns about, and nothing asserted that a losing claim performs zero deletes through the full adapter path. Both are pinned, and the legacy-marker block tests now assert the signature_consumed code and that the old worker's marker survives untouched.
MppAdapter re-raises the store policy violation as the public ConfigurationError (a PayKitError, not a PaymentError), so the boot probe's except PaymentError was dead: the rejection escaped as a bare traceback that only matched the fail-closed signature by luck of the message landing in the stderr ring buffer. Catch the exact public type and correct the docstring to the real exception and message.
The generic Node job omits boot-policy.test.ts because that suite requires the TypeScript charge server to fail closed, which ships in the TypeScript subscription leaf, not this Python leaf. Document that the Go and TypeScript probes return to CI there, so a reader does not read the omission as lost coverage.
server.New now enforces the core.SharedStore/IsShared() contract on any injected replay Store: an injected store must affirmatively implement SharedStore and report IsShared() == true on every network, closing the gap where a non-shared custom store passed the prior concrete-type check. AllowUnsafeMemoryStore is an explicit per-server development opt-in. It only ever authorizes the internally created process-local MemoryStore when Config.Store is nil; it never blesses an injected unshared store, and it is rejected outright on mainnet. A nil store without the opt-in fails closed on all networks, including localnet. The high-level paykit MPP adapter threads ReplayStore and the opt-in through and validates lazily at first serverFor(), so the guard surfaces as a challenge/settle error rather than at client construction.
Adds a ReplayStoreValidator that fails closed unless an injected store affirmatively declares a durable/shared replay capability via DurableStore or ReplayStoreCapability (and every declared capability method returns true). The MPP Adapter now enforces it: an injected store must pass the validator, allowUnsafeMemoryStore only permits an internally created process-local MemoryStore, that opt-in is rejected on mainnet, and a missing store fails closed on every network including localnet. Threads allowUnsafeMemoryStore through MppConfig, the middleware, and the Laravel/Symfony wiring, and updates the cross-SDK PHP harness to opt into the process-local store explicitly.
Introduces an atomic charge replay store distinct from the session store. mpp/server/store.ts adds ReplayStore plus resolveReplayStore, which fails closed unless an injected store reports isShared and isDurable or the explicit allowUnsafeMemoryStore flag authorizes an SDK-created process-local store; mainnet plus that flag is rejected. solana.charge and the push-mode path now reserve the consumed-signature marker with putIfAbsent after every fallible settlement check and raise VerificationFailedError (mapped to 402) on collision, so a broadcast ambiguity or timeout never burns a legitimate retry and only one concurrent caller obtains a receipt. pay-kit/replay-store.ts bridges the store into pay-kit with an identity-based production brand (declareProductionReplayStore) that copying capability booleans onto an unknown store cannot forge. configure() resolves and validates the store, and assertReplayStorePolicy revalidates it at every adapter boundary. The MPP adapter forwards the atomic view to both charge and subscription construction so subscription consumed-markers also upgrade to atomic putIfAbsent. Session-store durability policy (Session.ts, session/store.ts) and the subscription store threading stay with the subscription delivery.
Session store: mark createMemorySessionStore() with an enumerable
global-symbol brand so a shallow spread copy ({ ...store }) still reports
as process-local via isMemorySessionStore and is rejected off-localnet. A
symbol key stays invisible to JSON.stringify and Object.keys regardless of
the enumerable flag, so nothing leaks into serialized output; the prior
non-enumerable brand was dropped by spread and could be smuggled onto
mainnet as durable. Export isMemorySessionStore from the server entrypoint
so pay-kit adapters can detect it across the package boundary.
Subscription store: require isShared and isDurable outside localnet and
route consumed-reservation cleanup through releaseConsumedBestEffort so a
failed delete keeps the reservation instead of throwing.
Adds session-store brand tests (factory detection, JSON/Object.keys
invisibility, spread fail-closed, non-memory-store rejection) and extends
the subscription-server tests.
…lnet Generalize the config store gate to requiresSettlementReplayStore, covering both mpp and x402 settlement paths, and materialize a single shared in-memory replay store only on the devnet opt-in (PAY_KIT_ALLOW_INMEMORY_REPLAY_STORE=1). The session adapter rejects an explicit createMemorySessionStore() off-localnet via isMemorySessionStore so process-local session state cannot reach mainnet, and the x402 path is held to the same policy. Error copy generalized from "MPP replayStore" to "replayStore". Extends config, mpp-adapter, mpp-session, and paykit tests, including a new x402 config case.
Drive the TypeScript fail-closed store contract through a real createPayKit + requirePayment fixture (paykit-boot.ts) and require a covered probe only once its SDK carries the PAY_KIT_ALLOW_INMEMORY_REPLAY_STORE guard marker in-tree (detected by git-grepping the SDK source). Go and TypeScript ship the guard here so they run; Python asserts-SKIP with a loud note until its remediation lands, after which the same grep auto-promotes it to a required probe. This keeps the leaf green without hardcoding "python is red". Wires the ordered @solana/mpp -> @solana/pay-kit build and a dedicated fixture typecheck into CI behind a build-pay-kit input, excludes the fixture from the mpp-only baseline typecheck, and bumps @solana/kit to ^6.10.0 (lockfile fan-out only: @solana/* to 6.10, undici-types transitive, commander dropped).
The low-level session() method silently fell back to a process-local in-memory SessionStore on any network, including mainnet. A direct @solana/mpp consumer that bypasses the pay-kit adapter therefore lost every open channel on restart and never shared channel state across workers, letting cumulative vouchers double-spend. resolveSessionStore now mirrors the replay-store policy: a durable-shared store is honored everywhere, localnet may use a process-local store freely, devnet only behind the PAY_KIT_ALLOW_INMEMORY_REPLAY_STORE (or allowUnsafeMemoryStore) dev opt-in, and mainnet forbids it outright. A mainnet session() with no store or a memory store throws a ConfigurationError. Adds the isMemorySessionStore marker and SessionStoreDurability type consumed by the adapter layer, plus regression tests covering the mainnet-reject, devnet-opt-in, localnet, and durable-store paths.
Repo text uses no em-dash; two remediation comment lines and two earlier subscription comment lines carried them.
Reconcile #238 (MPP subscription) onto #237's nominal replay-store policy, which the maintainer chose as the single config-layer model. #238's flag-based policy is removed; all its unique behaviour is preserved on top of #237's model. config.ts: - Delete #238's validateReplayStore + requiresSettlementReplayStore. - Keep #237's assertReplayStorePolicy as the single replay policy (isAtomicReplayStore / isProductionReplayStore / declareProductionReplayStore / isAuthorizedUnsafeMemoryReplayStore). - x402 config coverage stays adapter-only: the x402 store cannot satisfy the putIfAbsent production marker and #237's own tests require x402-only configs to accept a legacy Store.Store, so the mpp-only gate is left in place and x402 replay is fenced by the x402 adapter (#236). Never silently accepts a non-durable x402 mainnet store. - Preserve #238's validators, inMemoryStoresAllowed, freezePayKitConfig / freezePayKitSigner snapshot, 32-byte challenge-secret check, expiresIn date check, and demo-signer-by-address detection (now also enforced at the assertReplayStorePolicy boundary). validatePayKitConfig runs the replay/signer policy before the challenge-secret length so mainnet/store misconfig reports first. subscription reconciliation: - Subscription adapter requires a durable shared store via #237's nominal isProductionReplayStore rather than isShared/isDurable flags; the devnet in-memory opt-in (createUnsafeMemoryReplayStore, isAuthorizedUnsafe) is rejected for subscription activation and never accepted on mainnet. - The atomic view (createAtomicReplayStoreView) now exposes reserve() aliasing putIfAbsent, so #238's reserve-based subscription server runs unchanged against the operator's putIfAbsent production store. subscription-replay-store.ts is removed. session store: keep #237's low-level durability guard (sessionStoreDurability) and adopt #238's more robust global-registry enumerable isMemorySessionStore brand (survives package-copy and shallow-spread). tests: rewrite #238 tests that asserted the deleted validateReplayStore messages to the nominal assertReplayStorePolicy model; every replay/session/subscription money-path test still asserts fail-closed. Broaden the harness fail-closed signature to accept #237's Go "forbidden on mainnet" wording.
# Conflicts: # .github/workflows/ci.yml # harness/test/boot-policy.test.ts
Read the on-chain Channel account at the confirmed slot and bind session authoritative state (deposit, splits, grace period) to it. Session state is never advanced or trusted for an unconfirmed or economically mismatched channel. No sibling PR touches the Rust session server; this layer is unique to #239 and applies cleanly on the settled base.
Add #239's unique fail-closed binding: a session open/top-up is rejected unless the on-chain Channel account (fetched via getAccountInfo at the confirmed slot) confirms the claimed deposit, mint, payee, authorized signer, grace period and distribution hash, and a signature-only open is bound to its canonical broadcast transaction. - server/session/on-chain.ts: verifyChannelAccountState, verifySignatureOnlyOpenTransaction, GetAccountInfoRpc, TopUpTransactionRpc. - server/session/wire-tx.ts + store.ts: openSignature/salt/settlement fields carried on ChannelState for the binding. - server/Session.ts: wire the verify calls into open/top-up/routes; export SettlementRpc. - tests: session-state-binding, session-signature-only-open, plus the #239 additions to session-on-chain / session-server-on-chain / session-store. Dedup against stacked siblings #237/#238 (already in the base): keep their session-store durability guard (resolveSessionStore + allowUnsafeMemoryStore) and replay-store policy; drop #239's independent re-implementation of the same concern (requireProductionSessionSafety + allowUnsafeEphemeralStoreOffLocalnet + the adapter wiring), which conflicted with those siblings' regression tests. Superseded session-store-durability assertions were removed from the #239 test files; the mainnet fail-closed control is unchanged (stronger via #237).
Reconcile #239's on-chain Channel-account state binding onto the base's solana-go v2 migration and hardened open/top-up verification (#214). Adds the authoritative state-binding layer that no sibling or the base delivers: fetchAndBindChannelAccount, NewOpenStateTxVerifier, bindConfirmedOpenChannel, validateBoundOpenChannel, NewTopUpStateTxVerifier and confirmedTransactionSlot. Session open/top-up state is bound to the on-chain Channel account read at the confirmed slot; an unconfirmed or economically mismatched channel is rejected (fail closed). Base store-safety (env-gated) is preserved; the config-field guard reconciles to honor the same PAY_KIT_ALLOW_INMEMORY_REPLAY_STORE opt-in. Ports #239's v1 solana-go usage to v2 and keeps base's structural verifiers + concurrency/replay hardening; drops #239's duplicate top-up-fence and legacy verifier redefinitions superseded by the base.
Add PR #239's unique behavior on top of sibling #228: the state-aware open verifier that fetches the on-chain Channel account at the confirmed slot and binds deposit, splits, grace period and PDA seeds to on-chain reality, so a payload's claimed economics are never persisted as facts. - session_onchain: BoundChannel, fetch_and_bind_channel_account, new_open_state_tx_verifier (+ signature-only confirm/bind helpers, _session_distribution_hash, _expected_session_grace_period). The structural open path reuses #228's verify_open_tx via the recipients contract. - confirm_transaction_signature now returns the confirmed slot and rejects a present-but-malformed slot, so the account read is pinned with min_context_slot; a status that omits the slot still confirms. - session: SessionConfig.verify_open_state_tx / open_tx_submitter seams, VerifiedOpenFacts, and process_open fail-closed: off localnet a payment-channel open requires the state verifier and persists only account-derived facts. - session_method: new_session installs the state verifier off localnet; _handle_open propagates the challenge recentSlot for signature-only push. - session_store: ChannelState.salt / open_signature round-trip. - rpc: SolanaRpc.get_account_info min_context_slot + typed get_transaction. - tests: add test_session_state_binding.py (adapted to #228's ProductionChannelStore policy, which is kept as-is). #228's structural open, top-up fence, channel-store policy and replay code are kept unchanged; their tests continue to pass.
Give the shared test operator a real signing key (sessionTestOperatorKey backs sessionTestRecipient) and co-sign the rentPayer slot so signature-only open fixtures reproduce a fully signed confirmed transaction. Base #214 verifies open signatures cryptographically, so the prior dummy-signature fixtures no longer pass. Also restores base's env-gated nil-store default (a merge artifact had re-added an unconditional off-localnet rejection that ignored the opt-in).
Base #214 hardened open verification (real wire-bound signatures, no placeholder acceptance) and the session store policy became env-gated. Update the stale Go test fixtures/assertions to the reconciled behavior: - flip TestNewOpenTxVerifierKeepsStructuralPlaceholderAcceptance to TestNewOpenTxVerifierRejectsPlaceholderSignature: the seam now rejects the pending placeholder ("must be a real transaction signature") - assert the real confirmed signature and derived PDA in the signature-only open test rather than the dummy reference the caller supplied - thread the derived channel PDA into the top-up test and reflect the post-top-up deposit in the authoritative on-chain account - match the reconciled off-localnet store-policy message, and enforce the unmarked-store rejection at the runtime safety gate (where it now lives) rather than at construction - rename the idle-close no-signer test and assert the no-op: closeOnIdle bails before touching the channel when no merchant signer is configured Test-only; no production source changed.
… path prepareOpenTx (SubmitOpenTx, the gasless server-submit flow) verified the open with allowUnsignedFeePayer=false, rejecting the placeholder fee-payer signature before the co-sign step the same function performs. The Go client signs only as the channel payer and leaves the operator slot for the server, so the server rejected its own client's open. Use the same verifyOpenTx(..., true) pre-verification handleOpen already uses: the payer signature is still verified and the post-co-sign zero-signature guard keeps broadcast fail-closed.
…resh test doubles confirm_transaction_signature returns int|None (a confirmed status may omit the slot), so _fetch_and_verify_signature_only_open's tuple[int, ...] annotation was unsound; the slot flows into min_context_slot (int|None), so widen the return. Refresh the _Rpc/_OpenConfig test doubles to the current RpcClient (search_transaction_history kwarg) and OpenVerifierConfig (settlement_window) protocols so pyright's structural check passes (13 errors -> 0).
…aths Add rejection-branch tests for validateBoundOpenChannel, resolveSessionTokenProgram, NewOpenStateTxVerifier/verifySignatureOnlyOpen, fetchAndBindChannelAccount, and the top-up verifiers (each asserts the concrete fail-closed error, not just coverage). Raises Go aggregate coverage 90.3% -> 91.1%, back over the 91.0 floor.
…ard for radar Prettier the mpp server index/session store re-exports. Mark the localnet-only MemoryChannelStore fallback nosemgrep: enforce_channel_store_policy above already rejects an absent/in-memory store off-localnet without the opt-in, which the radar rule's inline-guard pattern cannot see.
…ture The epoch-addressed channel PDA seeds include open_slot, so the session challenge now pre-fetches recentSlot alongside recentBlockhash and the open builder fails closed without it. The defect-verifier fixture predates that contract and crashed at construction instead of reaching its rejection assertions; supply the challenge slot so all five value-binding rejections (and the accept controls) exercise their intended paths.
semgrep only honors a nosemgrep comment on the match line or the line immediately before it; the wrapped two-line comment left the marker one line too far and the blocking radar scan still flagged the guarded localnet-only MemoryChannelStore fallback.
process_topup fetches the real channel account and rejects when its status/mint/payee/rent_payer/authorized_signer/payer diverge from the session config or the stored channel, but the existing *_mismatch tests only exercised the pass side of those checks (they reach the later grace_period / distribution_hash / resulting-deposit rejects). Add six unit tests that seed the SessionAccountRpc mock with a channel diverging on exactly one binding field and assert each reject fires and leaves the stored deposit untouched. Retires the reject-coverage gap documented in the mpp/server/session.rs coverage exemption.
|
Too many files changed for review. ( Bypass the limit by tagging |
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
1 participant
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.
Follow-up to the #216 re-delivery cascade. Retires the reject-coverage gap documented in the
mpp/server/session.rsentry ofscripts/check-rust-coverage.py(added when that file was exempted from the per-file coverage floor).What
SessionServer::process_topupfetches the real channel account off-chain and rejects when its binding fields diverge from the session config or the stored channel:OPENmintmismatchpayeemismatchrent_payermismatchauthorized_signerdoes not match the stored channelpayerdoes not match the stored channelThe existing
process_topup_rejects_*tests only exercised the pass side of these checks (they seed a fully-matching channel and reach the latergrace_period/distribution_hash/ resulting-deposit rejects), so the divergent-binding rejects themselves were unasserted despite being reachable through the existingSessionAccountRpcmock.Tests added (6)
Each seeds the mock RPC with a channel that matches on every field the checks reach before the one under test (so the intended reject fires first), drives
process_topup, asserts the specific rejection, and asserts the stored deposit is left untouched. Modeled onprocess_topup_rejects_grace_period_mismatch. Therent_payercase builds the channel explicitly becauseencoded_channeltiesrent_payertopayee.Test-only; no production change.
Merge notes
split/pr216-ci-harness-mega(ci(harness): radar layers + anti-false-green gates + cross-SDK protocol conformance #219), same base as the re-delivery leaves.fix/mpp-session-state-hardening(fix(mpp): bind session state to confirmed channels #239), which ownssession.rs. It does not touch the coverage gate config (owned by the harness leaf), so on its own thecore / Rust testscoverage gate stays red exactly as the other rust-touching leaves do; it converges green at ci(harness): radar layers + anti-false-green gates + cross-SDK protocol conformance #219 integration where the harness leaf's exemption lands. Once merged, the "Known reject-coverage gap" caveat in thempp/server/session.rsexemption can be dropped.