fix(rust/mpp/charge): address audit#150
Merged
Merged
Conversation
Adds AUDIT-ASSESSMENT.md as a running ledger of the 2026-05-26 Solana MPP audit findings, with one decision per issue. #38 (primary recipient in splits, medium): keep as-is. Recipient appearing in splits is a legitimate use case. The drain attack specifically needs ataCreationRequired=true on that split — a narrower guard for that misconfig will be added separately if/when we revisit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Parsed-credential verification matched System Program transfers on parsed.type + info.lamports + info.destination alone, with no programId check and no source check. In fee-sponsored mode this opens a path where the server (fee payer) bankrolls the value of the charge: the runtime is happy as long as the source signs, and the parsed verifier could not tell that the source was the fee payer rather than the client. Mirror the lower-level path (verify_sol_transfer_instructions): - require parsed_program_id() == System Program - read info.source and reject source == fee_payer Thread fee_payer: Option<&str> through verify_sol_transfers into the parsed-credential code path. The non-fee-sponsored mode passes None, so behavior there is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Parsed-credential SPL verification matched transferChecked instructions on mint + token amount + derived destination ATA only. info.source and info.authority were never read, so in fee-sponsored mode the server's signature could be re-used to fund the value transfer out of its own ATA — the SPL analogue of audit #32. Mirror the lower-level path (verify_spl_transfer_instructions): - read info.authority and reject when authority == fee_payer - read info.source and reject when source == ATA derived from (fee_payer, token_program, mint). Required even when the authority is a delegate/multisig. Thread fee_payer: Option<&str> through verify_spl_transfers into the parsed-credential code path. Non-fee-sponsored callers pass None and behavior there is unchanged. The audit also suggested deriving the expected source ATA from the signer and rejecting anything else (with a flag for delegates). We didn't take that route — the model intentionally allows delegate/multisig flows, and the fee-payer exclusion is the narrower fix that closes the actual drain shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec §7.2: if tokenProgram is omitted from a challenge, the receiver MUST determine it by fetching the mint and inspecting its owner — NOT by falling back to the legacy Token Program. The server was doing the latter via default_token_program_for_currency() for any currency outside the hardcoded stablecoin list, so a challenge generated against an arbitrary Token-2022 mint would go out with the wrong tokenProgram and either be rejected at verify time or silently fail. Resolve the token program once at Mpp::new and cache it on the struct: - SOL → None (no token program). - Known stablecoin symbol/mint → static table. - Arbitrary mint address → fetch the mint account, validate that the owner is Token Program or Token-2022 Program, error otherwise. Doing this at boot rather than per-challenge avoids RPC fan-out on the hot path and makes the server fail fast if its configured currency is unreachable. Challenge generation now emits the cached value directly. The parsed-credential verifier prefers the embedded tokenProgram, then the cached value, and errors if both are missing — never the old guess. Adds is_known_stablecoin_mint() to make the static-table boundary explicit, and warns in the doctring of default_token_program_for_currency that it is a static path only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec §13.3 says a client that is asked to pay in an unknown mint must verify the mint is a known token before signing. We currently only check the mint is owned by Token Program or Token-2022, which lets a malicious server slip in a Token-2022 mint with transfer hooks. Hooks run arbitrary program code inside the transfer CPI, and the server does not simulate inner instructions in pull mode. A pure allowlist would break arbitrary mints, which we just leaned into on the server side at boot. Split the gate on the actual threat axis instead: hookless vanilla Token stays open; arbitrary Token-2022 needs an explicit opt-in. Known stablecoins (USDC, USDT, USDG, PYUSD, CASH) are unaffected. - BuildChargeTransactionOptions::allow_unknown_token_2022 (bool, default false) — opt-in for the raw builder. - SelectChargeChallengeOptions::allow_unknown_token_2022 — same for challenge selection. select_charge_challenge skips candidates whose tokenProgram is Token-2022 or missing when the currency is not a known mint; vanilla Token Program passes. - build_credential_header_with_options exposes the opt-in to callers that don't drop to build_charge_transaction_with_options. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) In fee-sponsored pull mode the server signs the transaction before broadcasting, so the priority fee is paid out of the merchant's wallet. The general cap of 5_000_000 ulamports/CU was fine for client-paid mode but allowed an attacker to spend up to ceil(5_000_000 * 200_000 / 1_000_000) = 1_000_000 lamports of merchant SOL per "valid" charge — ~200x the base fee. Add a separate, tighter cap that applies only when the server is the fee payer: MAX_COMPUTE_UNIT_PRICE_MICROLAMPORTS_FEE_SPONSORED = 10_000 Worst-case priority fee at the new cap is ~2_000 lamports per request, ~20% of a per-signature base fee. Honest clients still have headroom to bump priority during congestion, without being able to drain the merchant. validate_compute_budget_instruction now takes a fee_sponsored flag; the allowlist passes fee_payer.is_some(), which is Some precisely when the server is acting as the fee payer. Client-paid mode keeps the 5_000_000 ceiling — no merchant funds at risk there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mpp::new accepted any string as the HMAC-SHA256 key that binds challenge IDs — including the empty string, "key", or other trivial values. A weak key lets an attacker forge challenges, so the gate has to live at server boot, before any challenge is issued. Enforce a 32-byte minimum (MIN_SECRET_KEY_BYTES) via validate_secret_key(). 32 bytes matches NIST SP 800-107 guidance for HMAC-SHA256 (key length >= hash output length). The check applies to both code paths — explicit Config.secret_key and the MPP_SECRET_KEY env var — so neither becomes a back door. Updated the docstring on Config.secret_key to require >=32 bytes of cryptographically-random data and point at `openssl rand -base64 32`. Bumped every test secret across the crate to satisfy the new minimum; "key" literals in unit tests now reuse the existing TEST_SECRET constant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The client builder auto-created Associated Token Accounts for every split recipient when no server fee-payer was set, ignoring ataCreationRequired. A hostile server could attach N dust splits to a challenge and force the client to fund N × ~0.002 SOL of ATA rent. Spec draft-solana-charge-00 §7.2 says the client MUST include the ATA-create instruction WHEN ataCreationRequired is true; it doesn't authorize creation otherwise. The §9.5 ban on fee-payer- funded ATA creation for unmarked recipients is narrower than the audit suggested, but the silent rent drain on the client in client-paid mode is the actual threat. Drop the implicit `fee_payer.is_none()` branch — the explicit flag is now the only signal in both modes. Servers that need client-funded ATAs must set ataCreationRequired: true per split. Servers that forget the flag will see the receiving transfer fail on-chain instead of silently charging the client — a clearer failure mode. Server-side expected_ata_creation_policy is left as-is; it's spec-conformant (the spec only restricts fee-payer-funded creation) and tightening it would break integrators with legitimate auto-create flows on their own clients. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
charge_challenge_with_options HMAC-signed a caller-supplied
ChargeRequest directly. Nothing checked amount, currency,
recipient, network, decimals, tokenProgram, or splits — a buggy
caller could produce a cryptographically-valid challenge with
malformed or off-route contents.
Add Mpp::validate_charge_request and call it at the top of the
function. It enforces:
- amount parses as u64
- currency matches self.currency (case-insensitive)
- recipient is Some and parses as Pubkey
- methodDetails.network, .decimals, .tokenProgram (if present)
match self.* — including the boot-resolved self.token_program
from audit #28
- each split has a parseable Pubkey recipient and u64 amount
fee_payer / feePayerKey are deliberately not pinned: the high-
level charge_with_options already supports a per-request fee-
payer override via ChargeOptions, and the orthogonal
feePayer=true-with-no-signer issue is its own audit item.
Callers that legitimately need to issue challenges for an
unrelated route still have PaymentChallenge::with_secret_key_full
as a public escape hatch — the trusted construction path the
audit recommendation refers to.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec §9.5 forbids fee-payer-funded ATA creation for the top-level recipient. validate_charge_options now rejects any split whose recipient duplicates the server-configured recipient and sets ataCreationRequired=true — the misconfig shape that, in fee-sponsored mode, lets the recipient close/recreate its own ATA to keep draining server-funded rent. Legitimate splits that name the primary recipient without the flag stay allowed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Auto-pay integrations pipe server-issued challenges straight into the signer, which makes the server's challenge fields effectively unauthorized control over the user's wallet. Adds two opt-in gates on BuildChargeTransactionOptions — max_amount_base_units and expected_network — and an always-on refusal to sign an expired challenge in build_credential_header. Default behaviour is unchanged for callers who already validate the challenge upstream. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After the 30-poll await_pull_confirmation loop times out, do one definitive get_signature_status check. Lagging or load-balanced RPCs can fail to observe a signature within 6 s while the tx is actually on-chain — without this, the verifier would return network_error and the already-reserved replay-store entry would block any retry. On-chain runtime failures are surfaced as transaction_failed rather than timeout. Genuinely missing txs keep the original timeout error. Four-case interpretation pulled into a pure helper for unit testing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d (audit #2) The simple verify_credential API decoded the ChargeRequest from the echoed challenge and verified the payment against that. The HMAC tier and pinned-fields tier protected the cross-server case, but nothing pinned the route's amount or other per-route economics — a server issuing challenges for multiple priced routes would accept a $1 credential against a $100 route. Documentation alone is a soft control, so the unsafe method is removed and every caller is now forced through verify_credential_with_expected with an explicit expected ChargeRequest built from the route's static configuration. Tier-1 (HMAC) and tier-2 (pinned-fields) tests migrate to the lower- level public `verify(&cred, &request)` API. Integration tests gain a tiny `expected_charge` helper that mirrors how integrators should build the expected from their own static config rather than from the credential. axum integration was already on verify_credential_with_expected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
verify_credential_with_expected previously compared only amount, currency, recipient between the credential's decoded request and the expected request. Other payment-constraining fields (externalId, description, methodDetails.network/decimals/tokenProgram/feePayer/ feePayerKey/splits) flowed into settlement unchecked. Now extracts a compare_expected_to_request helper that compares every field exhaustively, with recent_blockhash deliberately excluded (per-challenge state, not per-route policy). Splits are compared element-wise (order-sensitive). Fixes a latent bug in payment_link_server.rs where the expected was built without the description that the user-facing challenge carried — the strict comparison would have rejected every honest credential. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ts (audit #39) The integer branch did 10u128.pow(decimals) * value with neither input bound and neither operation checked. Hostile or buggy callers could panic the process (debug builds) or silently wrap (release). Adds MAX_DECIMALS = 18 (well below the 39 cliff where 10.pow actually overflows u128, ample headroom over Solana's 0-9 SPL convention), rejects decimals above the cap at the function entry, and replaces the pow/mul with checked_pow/checked_mul that return an explicit overflow error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#30) Three callsites summed split amounts via .sum::<u64>(), which panics in debug and silently wraps in release. Replaces the pattern with a shared checked_sum_split_amounts helper that uses try_fold + checked_add and returns None on overflow, mapped to each callsite's existing error type. While here: centralizes the previously-hardcoded split count cap as MAX_SPLITS = 8 in protocol/solana.rs and updates the two callsites (client pre-build, server pre-broadcast) plus the TooManySplits thiserror display string so the cap has a single source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
diagnose_balances used 10u64.pow(methodDetails.decimals) to render a UI-amount divisor for failure diagnostics. decimals is Option<u8>, so values >= 20 panic in debug builds or wrap in release — and this function runs *after* settlement already failed, so a panic here loses the diagnostic and crashes the thread. Extracts a to_ui_amount(base_units, decimals) -> Option<f64> helper that uses checked_pow and returns None when the divisor can't be represented. diagnose_balances silently omits the token-balance hint in that case; the fee-payer SOL diagnostic still runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) diagnose_balances hardcoded programs::TOKEN_PROGRAM when deriving the payer's ATA, so for Token-2022 mints (PYUSD, USDG on Token-2022, CASH, USDC variants) the diagnostic produced the wrong address and could silently lie about the payer's balance. Audit #28 already resolves the token program at boot and embeds it on every SPL challenge as methodDetails.tokenProgram. The diagnostic now reads that value. When it's missing (lower-level ChargeRequest construction edge case), the token-balance hint is skipped; the fee-payer SOL diagnostic still runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#9) parse_www_authenticate decoded the `request` parameter (base64url) and parsed it as JSON without applying the MAX_TOKEN_LEN cap that parse_authorization and parse_receipt already enforced on their inputs. A large WWW-Authenticate value could drive proportionally larger base64 + JSON parsing work than the other parsers allowed. Adds a length check on `request` immediately after extraction, matching the same constant and error style as the existing parsers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…missing (audit #42) Spec §7.2: methodDetails.decimals MUST be present when currency is a mint address. Two callsites used unwrap_or(6), silently defaulting non-6-decimal SPL flows to a wrong divisor. Client build_spl_instructions now errors when decimals is None — the SDK produces signed transactions, and a silent wrong-decimals transfer is the worst failure mode. Server diagnose_balances silently omits the token-balance diagnostic when decimals is missing (best-effort post- failure diagnostic, same pattern as the audit #8 and #13 fixes for the same function); fee-payer SOL diagnostic still runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec §7.2 requires feePayerKey when feePayer is true. Mpp::new accepted
Config{fee_payer: true, fee_payer_signer: None} without complaint and
charge_with_options then emitted spec-violating challenges (feePayer
true without feePayerKey).
Two gates close it: Mpp::new rejects the boot misconfig, and
validate_charge_options rejects the per-call override case where
Config.fee_payer is false but ChargeOptions.fee_payer is true.
Two pre-existing tests constructed Mpps in the exact misconfig shape;
updated them to provide a signer so they now exercise the
spec-compliant path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DEFAULT_REALM = "MPP Payment" meant two services sharing MPP_SECRET_KEY and both keeping the default participated in one shared credential namespace — cross-service credential replay. Replaces the constant with derive_default_realm(recipient): SHA-256 the recipient pubkey, take the first 4 bytes as u32 mod 10^8, format as "App Id - #<digits>". Two services with the same secret but different recipients now automatically get different realms, so HMAC IDs differ and cross-service replay fails. Operators can still override with an explicit realm; explicitly providing an empty string is now rejected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Spec §7.2 requires network to be one of mainnet/devnet/localnet. default_rpc_url silently mapped anything else (typos, "mainnet-beta", "testnet") to mainnet-beta — and two copies of the function had been drifting independently. Adds validate_network() in protocol/solana.rs (the single source of truth) and calls it from Mpp::new so misconfigured network strings fail at boot. Removes the duplicate default_rpc_url in server/charge.rs; the private callsite now uses the canonical public one. Canonicalizes on "mainnet" — "mainnet-beta" is an RPC hostname only, not a slug. Client matches_network default switches from "mainnet-beta" to DEFAULT_NETWORK (= "mainnet") per spec, and a handful of test fixtures that used "mainnet-beta" as a slug are updated to "mainnet". The actual Solana RPC URL hostname stays as-is. Session-flow and x402-crate references to "mainnet-beta" are out of scope for this finding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
validate_charge_options only ran additional split checks when at least one split had ataCreationRequired=true. For all other splits, charge_with_options embedded them into methodDetails with no parseability check, no positive-amount check, no dedup, and no count cap at challenge issuance — invalid splits surfaced only when the transaction reached the chain. Extracts validate_splits() in protocol/solana.rs as the single source of truth: enforces count <= MAX_SPLITS, recipient parses as a Pubkey, amount parses as u64 and is > 0, aggregate fits in u64, no duplicate recipients. Called from both validate_charge_options (per-call) and validate_charge_request (the lower-level charge_challenge_with_options path), with the per-split loop in the latter removed. Application-level recipient allowlists are intentionally out of scope — domain policy, not protocol validation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t scope The audit's threat (signer drains itself / drops below rent via SOL transfer) requires the SOL system_instruction::transfer code path, which the product doesn't expose to end users — signers transfer stablecoins only. Insufficient SOL for fees is caught by the runtime; server fee-payer monitoring is a separate spec §13.6 concern. Prototype shipped briefly (skip_balance_check opt-out + pre-sign get_balance) but reverted once we walked the threat cases together. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
verify(&credential, &request) recomputed HMAC from credential.challenge.request but settled against the caller-supplied request. A direct caller could authenticate one request shape and verify settlement against a different one — HMAC said "issued" but about something else. After the HMAC tier-1 check, verify now decodes the credential's authenticated request and calls compare_expected_to_request against the supplied request (the same audit #1 helper used by verify_credential_with_expected). Any divergence fails with the same per-field mismatch errors. verify_credential_with_expected's existing up-front compare still runs — it now fires twice, both succeed, defense-in-depth. Tier-2 unit tests construct request as-if-decoded-from-credential so they keep passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…17) The audit's server-side concern (verify reaching the Solana charge path on a non-solana/non-charge challenge) is already closed by verify_pinned_fields, which runs unconditionally from verify and is covered by the tier2_rejects_tampered_method / tier2_rejects_non_charge_intent tests. The client-side gap is real: build_credential_header_with_options accepted any PaymentChallenge handed to it. Callers who skipped select_charge_challenge (which filters via is_solana_charge_challenge_name) got no protection. Adds the same gate at the credential-builder entry point. The lower-level build_charge_transaction_with_options is unchanged — it takes already-decoded fields and has no notion of method/intent; the trust boundary belongs at the PaymentChallenge entry point. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Push-mode credentials match on-chain transactions to challenges by shape only — any matching-shape tx can satisfy any matching-shape challenge. Spec §13.5 acknowledges this trade-off and accepts "first accepted presentation wins" as the base-flow model; it also considers the audit's recommended memo-carrying-challenge-id mitigation and explicitly leaves it as MAY, not MUST, citing on-chain correlation metadata as the privacy cost. We follow the spec base flow but reduce surface: Config.accept_push_mode defaults to false. Servers that don't actively need push mode stay safe without an explicit decision; servers that want it opt in. The new gate runs before B34 (which still narrows the fee-sponsored case). interop_server sets accept_push_mode: push_mode so the interop suite still exercises push mode end-to-end when it's the mode under test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five informational findings batched together — all about how strictly we interpret input strings or how clearly we document the contracts. #44 / #45: parse_units now rejects empty inputs, more-than-one decimal points ("1.2.3" used to silently parse as 123 because split_once only splits on the first dot), empty integer/fraction parts (".5", "5.", "."), and any non-ASCII-digit character on either side of the dot. The pre-existing parse_units_zero_decimals_with_dot test, which asserted "1." == "1", is replaced with one that asserts rejection. #27: resolve_mint docstring corrected — the function returns Some(input) passthrough for unknown symbols, not just Some(mint_address). #14: SelectChargeChallengeOptions docstrings now make the precedence explicit — currency_preferences takes priority over currency when both are set. #34: the "ataCreationRequired requires an SPL token mint address" check now directly tests `Pubkey::from_str(&request.currency).is_err()` instead of the oblique "currency != expected_mint.to_string()" check. Same outcome, clearer intent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…(CI green) Audit #1 made verify_credential_with_expected compare methodDetails (network, decimals, tokenProgram) exhaustively. The integration test helper expected_charge only set top-level fields, so every test failed with "methodDetails.network mismatch" once the server embedded its config-derived methodDetails in every challenge. expected_charge now takes network + decimals + token_program and populates them in a MethodDetails. All 8 callsites updated (6 SOL on localnet/9/None, 2 USDC on localnet/6/TOKEN_PROGRAM). sol_charge_expired_challenge_rejected: audit #10 added a client-side "refuse to sign expired" gate, so build_credential_header is where expiry is rejected now. Test updated to expect the client-side rejection. The server-side expiry path remains as defense-in-depth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…out) Audit #24 made the Rust server reject HMAC secrets shorter than 32 bytes. Three interop secret literals were stuck at the pre-audit length: - harness/test/e2e.test.ts:279 — "mpp-interop-secret-key" (22 B) - harness/test/e2e.test.ts:501 — "mpp-interop-secret-key-server-b" (31 B) - rust/crates/mpp/src/bin/interop_server.rs:59 default (22 B) All three padded to 35 B. CI checks that explicitly spin up the rust server ("Interop: TypeScript harness" and "Interop: Swift client to TypeScript + Rust servers") now clear the boot gate. Per-language checks that don't run the rust server weren't affected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Format-only changes — no behaviour difference. Several recent audit fixes left lines at the rustfmt fence (find_sol_transfer test asserts, challenge_to_html call). The CI "Rust format check" step requires clean rustfmt output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…21) Per the spec: 9 splits is illegal (MAX_SPLITS=8). The interop server now calls validate_splits in parse_state, so a misconfigured server fails to boot rather than starting and rejecting at challenge-issue time. Aligns with "the harness failing is a feature — let's align the approach on languages." The harness scenario "charge-splits-too-many" assumes the server responds with 402 and asserts on the response. The harness has no notion of "expected adapter-exit-before-readiness" today, so this commit also restricts the scenario to the SDKs that still runtime-reject — rust is dropped from serverIds. Re-include rust once the harness gains a way to assert on startup-failure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # harness/test/e2e.test.ts # rust/crates/mpp/src/bin/harness_server.rs # rust/crates/mpp/src/client/charge.rs # rust/crates/mpp/src/protocol/solana.rs # rust/crates/mpp/src/server/charge.rs # rust/crates/mpp/tests/charge_integration.rs
`SiwxExtension::from_extensions_value` deserialized the `sign-in-with-x`
object as a flat struct, but the x402 spec (and live servers like Venice)
nest the challenge fields under `info` with a sibling `supportedChains`
array. The flat parse failed, so spec-compliant SIWX challenges were
silently dropped and clients fell through to per-call payment instead of
authenticating with wallet credits.
Parse via a nested `{ info, supportedChains }` wire shape and convert to
the flat in-memory `SiwxExtension`; `as_extensions_value` emits the same
nested shape so it round-trips. Adds a regression test using Venice's real
`info`-nested challenge.
Greptile SummaryThis PR addresses 45 security audit findings for the Solana MPP Rust SDK, systematically hardening challenge issuance, credential verification, and client transaction building. Nearly every finding has been fixed with a corresponding test.
Confidence Score: 4/5Safe to merge with one P1 fix recommended: case-sensitive currency comparison in Thorough, well-documented remediation of 45 audit findings with strong test coverage. One P1 identified (currency case-sensitivity inconsistency in
Important Files Changed
Reviews (3): Last reviewed commit: "chore(deps): bump ws override to ^8.21.0..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- rustfmt wraps the long challenge_binding_secret test literals (CI format check) - remove the duplicated #[allow(clippy::too_many_arguments)] on build_spl_instructions left by the main merge (Greptile P2)
`pnpm audit --production` (CI Audit job) flags a high-severity DoS in transitive `ws` 8.20.1 (<8.21.0), pulled via @solana/kit's rpc-subscriptions websocket channel. The existing pnpm override already pinned `ws` (to ^8.20.1); bump it to ^8.21.0 and refresh the lockfile (now ws@8.21.0). Repo-wide advisory, not specific to this branch.
Collaborator
Author
|
(PR reviewed off-band by auditing team) |
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.
No description provided.