Description

The keystore stores two different kinds of data in the same flat string namespace:

fn keypair_key(account: &str) -> String {    account.to_string()}
fn pubkey_key(account: &str) -> String {    format!("{account}.pubkey")}

The private keypair for account alice is stored under storage key alice. The public-key metadata for account alice is stored under storage key alice.pubkey. This becomes unsafe because account names are allowed to contain dots. validate_account_name() permits . and does not reserve the .pubkey suffix. Therefore an account named victim.pubkey has the same private-key storage key as the public-key metadata key for account victim.

The collision looks like this:

pubkey() is intended to be safe and unauthenticated because it should only return public metadata. It validates the account name, loads pubkey_key(account), and returns whatever bytes are stored there:

pub fn pubkey(&self, account: &str) -> Result<Vec<u8>> {    validate_account_name(account)?;    self.store.load(&pubkey_key(account)).map(|z| z.to_vec())}

There is no authentication step in pubkey(). There is also no check that the loaded value is exactly 32 bytes. Consequently, after a colliding account name is imported, pubkey("victim") can return the full 64-byte private keypair for victim.pubkey. This bypasses the intended protection in load_keypair_with_intent(), which authenticates before reading private keypair bytes.

Recommendation

Prefer storing each account as one structured backend record that contains both the private keypair and the public metadata. That removes the flat account versus account.pubkey namespace split entirely, so there is no unauthenticated metadata key that can alias a private keypair record. It also makes account deletion one backend operation instead of two independent cleanup calls, which avoids the stale-public-key state described in the separate delete finding. If separate records must remain, separate private keypair records from public-key metadata records with typed storage keys that cannot be produced from a valid account name. For example:

keypair:<account>pubkey:<account>

Reject account names that use reserved metadata suffixes such as .pubkey. This should happen in validate_account_name() so every import, create, delete, and lookup call shares the same protection. Make pubkey() fail closed if the loaded value is not exactly 32 bytes. This does not replace namespace separation, but it prevents a public-key API from returning private keypair-sized data if storage is corrupted or another collision is introduced.

UX Impact

Separating public metadata from private key records should not add a biometric or password prompt. The fix is namespacing and record shape, so pubkey() cannot address private key material. Users may see reserved account names rejected during create or import, but normal key-use approval should not change.

Description

The macOS keystore builds the helper location directly from the HOME environment variable:

let home = std::env::var("HOME").unwrap_or_else(|_| "/tmp".to_string());let cache_dir = PathBuf::from(home).join(".cache").join("pay");let binary = cache_dir.join("pay.sh");
if binary.exists() {    verify_codesign(&binary)?;    return Ok(binary);}

This only handles the case where HOME is absent. If HOME is present but empty, PathBuf::from(home) is a relative empty path. The helper location becomes .cache/pay/pay.sh under the process current directory. The existing-helper check then accepts that relative executable after only codesign --verify --strict. It does not require the helper to be under the user's real home directory and it does not check ownership or directory permissions before returning the path.

Consequently, a local attacker who can precreate files in a shared working directory can plant an ad-hoc signed .cache/pay/pay.sh. If a victim Pay process later runs from that directory with HOME="", macOS Keychain import executes the planted helper and writes the imported 64-byte keypair to its stdin.

Recommendation

Do not derive the macOS helper location from an unvalidated HOME environment string. Resolve the user's home directory through a trusted OS account API or fail closed when no usable absolute home directory is available.

At minimum, reject empty and relative HOME values before any helper lookup. Also verify that the helper directory is owned by the current user, is not world-writable, is not a symlink and contains the expected Pay helper bytes or a pinned designated signing requirement before executing it.

For example use homeDirectoryForCurrentUser.

UX Impact

Rejecting empty or relative home directories should not add a Touch ID or password prompt. Pay should fail earlier with a setup or environment error. Users running Pay from stripped service environments may need setup guidance to provide a valid user home or a trusted helper location.

Description

Function helper_path() falls back to /tmp, which is unsafe. If someone else has made /tmp/.cache before, they can later rename /tmp/.cache/pay and create a new directory with that name and setup a new executable.

Recommendation

Consider not using /tmp or make the subdirectory direct in /tmp so other users can't change it.

UX Impact

Depending on the chosen solution it might result in an extra question to the user on system that have configuration errors (e.g. missing home).

Description

The macOS keystore stores and reads Keychain secrets by launching a separate executable at:

~/.cache/pay/pay.sh

The code calls this executable a helper. It is not a Rust function. It is a separate Swift program that the Rust backend runs with Command::new(...). That distinction matters because the keystore is trusting a file on disk before it sends wallet key material to that process. When the cache file already exists, helper_path() does not check whether it is the helper that was embedded in the current build. It does not compare the file to a known hash, pinned source, expected Team ID or designated signing requirement. It only verifies that the file has some valid code signature:

if binary.exists() {    verify_codesign(&binary)?;    return Ok(binary);}

verify_codesign() then runs:

codesign --verify --strict <cached-helper>

That check proves the file is code-signed well enough for macOS verification. It does not prove the file is the Pay helper. An attacker running as the same macOS user can put a different executable at ~/.cache/pay/pay.sh, sign it ad hoc and still pass this generic verification step. This becomes sensitive during imports. helper_store() resolves the cached executable and sends the hex-encoded keypair to its stdin:

let binary = helper_path()?;let mut child = Command::new(&binary)    .args(["store", key])    .stdin(std::process::Stdio::piped())    ...

Therefore, if a same-user process plants a replacement executable in the cache before a later import, the keystore can execute the attacker's program and send it the full imported keypair.

The same trust failure applies to other cache-control cases. A replacement helper can be placed directly at the cache path, reached through a symlink, or placed under an unsafe cache directory before helper_path() performs its existing-file fast path. Since the fast path returns after the generic codesign check, it does not enforce the directory permissions branch, reject symlinks or prove that the existing helper bytes match the embedded helper.

Recommendation

Do not trust a mutable cache executable based only on generic code-signature validity.

A possible fix is to ship the helper inside a signed app bundle or fixed install location and verify a strict designated requirement that pins the expected identifier and Team ID. If the helper must remain in the user cache, verify that the cached bytes exactly match the embedded helper for the current build before executing it. At minimum:

• compare the cached helper to a hardcoded SHA-256 hash or embedded helper bytes,
• reject symlinks and hard links,
• verify the owner and permissions of both the cache directory and helper file,
• rewrite mismatched helpers atomically through a temporary file followed by rename,
• invoke /usr/bin/codesign by absolute path,
• reject helpers that do not match the expected hash or designated signing requirement.

After remediation, a pre-existing ad-hoc signed helper in ~/.cache/pay/pay.sh should be rejected or replaced before any key material is sent to it.

UX Impact

Helper verification should happen before macOS asks for biometric or password approval. Pay should verify the helper it is about to run before showing any macOS approval. Users may see an earlier setup or integrity error, but successful operations should keep the same Touch ID or password cadence.

Description

Linux Polkit authorization is selected from the payment amount stored in AuthIntent. That amount is currently derived from strings. For typed payment intents, AuthIntent::authorize_payment() keeps the user-facing message and separately tries to parse the amount into a PaymentLimit:

pub fn authorize_payment(amount: &str, description: &str) -> Self {    Self::AuthorizePayment {        message: format!("authorize payment of {amount} for {description}"),        limit: PaymentLimit::from_amount(amount),    }}

The Linux backend then uses that parsed limit to choose the Polkit action. If parsing failed and the limit is None, Linux falls back to the generic payment action:

AuthIntent::AuthorizePayment { limit, .. } => limit    .map(polkit_payment_limit_action)    .unwrap_or(POLKIT_ACTION_PAYMENT),

That is unsafe because the generic action may be less specific than the real payment amount. For example, a typed amount like $50,000 is a formatted display string. If PaymentLimit::from_amount("$50,000") cannot parse it, the intent loses its amount bucket and Linux receives sh.pay.authorize-payment instead of the above-$50 payment action. The legacy free-form reason parser has a related problem. payment_limit_from_message() finds the first $ and keeps characters only while they are ASCII digits, $, or .:

let amount = message[start..]    .chars()    .take_while(|ch| ch.is_ascii_digit() || *ch == '$' || *ch == '.')    .collect::<String>();

That means the parser stops at commas. A reason such as authorize payment of $50,000 for accessing API is parsed as $50, so Linux can choose the $50 Polkit action even though the text refers to fifty thousand dollars. The same thing happens with smaller formatted values: authorize payment of $1,000 for accessing API is parsed as $1, which maps to sh.pay.authorize-payment-up-to-usd-1. The core issue is that display text is being used as policy input. If that text is formatted, malformed, too large, or otherwise not parsed exactly, Linux can authorize a lower or less-specific Polkit action than the payment amount deserves.

Recommendation

Do not derive security policy from display strings. Represent payment amounts as validated numeric minor units before constructing AuthIntent. The user-facing display string can still be built from that validated value, but the Linux policy bucket should come from structured amount data. If string parsing must remain for compatibility, make it conservative:

• reject commas and locale formatting before intent construction,
• return an error instead of Option for malformed amounts,
• never map parse failure to the generic payment action,
• classify unknown or unsafe values as rejection or AboveUsd50,
• stop using AuthIntent::from_reason() to infer payment limits from free-form prose,
• consider using crate rust_decimal.

UX Impact

No extra prompt should be added. Linux should choose the Polkit action from a validated amount, so malformed values fail or map to the conservative bucket. The user may see a stricter payment prompt, but not an additional biometric or password step.

Description

SyncMode is supposed to be the caller's storage policy for an imported wallet key. ThisDeviceOnly means the key should stay local to the current device. CloudSync means the key may be stored in a backend that syncs through iCloud Keychain, 1Password or a similar cloud-backed service.

/// Controls whether the key syncs to cloud storage.pub enum SyncMode {    /// Key stays on this device only (default).    ThisDeviceOnly,    /// Key syncs to cloud (iCloud Keychain, 1Password, etc.).    CloudSync,}

The import APIs accept that policy, but they discard it. Keystore::import(), Keystore::import_with_reason() and Keystore::import_with_intent() name the parameter _sync. That leading underscore matters because it suppresses the unused-parameter warning. The import then validates the account name and keypair and writes the private keypair plus public-key metadata directly to the configured SecretStore:

self.store.store(&keypair_key(account), keypair_bytes)?;self.store    .store(&pubkey_key(account), &keypair_bytes[32..64])?;

The storage backend never receives the requested sync policy either. SecretStore only receives a key name and bytes:

pub trait SecretStore: Send + Sync {    fn store(&self, key: &str, data: &[u8]) -> Result<()>;    fn load(&self, key: &str) -> Result<Zeroizing<Vec<u8>>>;    fn exists(&self, key: &str) -> bool;    fn delete(&self, key: &str) -> Result<()>;}

As a result, the backend's default behavior wins, regardless of what the caller requested. A caller can request SyncMode::ThisDeviceOnly while using a cloud-syncing backend and the import still succeeds. A caller can also request SyncMode::CloudSync while using a backend that only stores local or non-migrating items, and that import also succeeds.

In both cases, the API gives the caller the impression that Pay honored a key-storage policy. In reality, the implementation neither enforces that policy nor rejects an unsupported one before writing private key material.

Recommendation

Make SyncMode a real storage contract between the caller and the selected backend. The import code should not accept a sync policy unless the backend can actually provide it. At minimum, change the import boundary so it handles the requested mode directly:

• remove the leading underscore from the sync parameter,
• pass SyncMode into the storage layer or an import-specific storage method,
• have each backend declare the modes it supports,
• reject unsupported modes before writing any key material,
• include the selected sync policy in the import approval text where the platform can show it.

For example, SecretStore::store() could be replaced or supplemented with a method that receives storage options:

fn store_with_policy(&self, key: &str, data: &[u8], sync: SyncMode) -> Result<()>;

Backends that cannot implement the requested mode should return an explicit unsupported-mode error. They should not silently use their default persistence behavior. With the current backend model, the smallest safe fix is to reject the combinations that are not currently supported:

match (intent_sync, backend) {    (SyncMode::CloudSync, Backend::Macos | Backend::Linux | Backend::Windows) => {        Err(Error::Unavailable("cloud sync is not supported by this backend".into()))    }    (SyncMode::ThisDeviceOnly, Backend::OnePassword) => {        Err(Error::Unavailable("device-only storage is not supported by 1Password".into()))    }    _ => Ok(()),}

This keeps the existing behavior only for combinations where the backend already matches the requested policy. It also gives callers a clear error before Pay writes private key material. If the product needs cloud sync on macOS, implement that as an explicit CloudSync mode instead of treating it as a fallback. That is a product risk decision: an iCloud-synced key may be recoverable on other devices, but the private key is also present in more places. One implementation shape is to pass the requested sync policy into the helper and store synchronized items with kSecAttrSynchronizable:

func doStore(account: String, hex: String, sync: Bool) {    ...    let s = SecItemAdd([        ...        kSecAttrAccessible as String: sync            ? kSecAttrAccessibleWhenUnlocked            : kSecAttrAccessibleWhenUnlockedThisDeviceOnly,        kSecAttrSynchronizable as String: sync    ] as CFDictionary, nil)}

All macOS query functions that read, update or delete these items should include kSecAttrSynchronizable as String: kSecAttrSynchronizableAny. That lets Pay find the item whether it was stored as synchronized or device-only. The UI and approval text should make the policy clear before import. Device-only keys stay on the current device. Cloud-synced keys may become available on other devices attached to the user's iCloud Keychain.

UX Impact

This can add a clear fail-closed import error. If the backend can not provide the requested sync mode, Pay should stop before writing the key. The normal import biometric or password prompt should not become two prompts, but the prompt text should say whether the key is device-only or cloud-synced. If macOS cloud sync is enabled, the UX should also warn that the key can be present on other iCloud Keychain devices. That is a recoverability feature, but it weakens the current single-device storage boundary.

Description

The 1Password backend imports a wallet by spawning the op CLI. During that import, OnePasswordStore::store() hex-encodes the full keypair and places it directly in the op item create argument list:

let hex = hex_encode(data);
let mut cmd = self.op_cmd();cmd.args([    "item",    "create",    "--category=Crypto Wallet",    &format!("--title={title}"),    &format!("--tags={OP_TAG}"),    &format!("recoveryPhrase[concealed]={}", &*hex),]);

For Pay accounts, data is the 64-byte Solana keypair. The recoveryPhrase[concealed]=... argument therefore contains the full private key material as a 128-character hex string. Command-line arguments are the wrong place to put that value. On Unix-like systems, another process running as the same user can often inspect a live command line through ps, /proc, or similar tooling. That creates a short exposure window while op item create is running. A local process watching command lines during import can read the keypair before 1Password has finished storing it.

This weakens the security boundary users expect from the 1Password backend. The key is meant to move into the password manager, but Pay first exposes it through ordinary process metadata.

Recommendation

Do not pass private key material through command-line arguments. Use a 1Password-supported input mechanism that keeps secrets out of argv. The best option is a supported 1Password SDK or API, because Pay would not need to hand secret material to a CLI process at all. If Pay keeps using op, pass the item data over stdin. op item create --template - supports this shape: Pay can build a JSON template locally, write it to the child process stdin, and keep the keypair out of the process argument list. One possible implementation is:

let json = serde_json::json!({    "title": title,    "category": "Crypto Wallet",    "tags": [OP_TAG],    "fields": [        {            "id": "recoveryPhrase",            "type": "CONCEALED",            "value": hex,        }    ]});
let mut cmd = Command::new("op");cmd.args(["item", "create", "--template", "-"]);cmd.stdin(Stdio::piped());
let mut child = cmd.spawn()?;child    .stdin    .as_mut()    .ok_or_else(|| Error::Backend("failed to open op stdin".into()))?    .write_all(json.to_string().as_bytes())?;

With this shape, the command line only contains the static op item create --template - invocation. The concealed field value travels through stdin instead of argv.

Do not move the keypair into an environment variable as a substitute. Process environments can also be inspected on many systems. A temporary file should only be a last resort, and only if Pay can create it with restrictive permissions, remove it reliably, and avoid shared directories. Also launch op with a sanitized environment and, where possible, an explicit binary path. That does not fix the argv leak by itself, but it reduces the chance that Pay sends secrets to a malicious op selected through PATH.

UX Impact

Moving the secret off argv should not add a biometric or password prompt. The 1Password unlock cadence can stay the same. The user-visible difference is that Pay sends the keypair through a private input channel, so the secret no longer appears in process metadata.

Solana foundation

Acknowledged. Support for 1Password is not used and will be removed.

Cantina

Acknowledged

Description

The keystore already has typed authentication intents such as UseAccount, AuthorizePayment, DeleteAccount, and ImportAccount. Those types are not just labels. They control the local approval text and on Linux they also select the Polkit action. Keystore::load_keypair() is a key-use operation. It should always authenticate as use of an existing account. Instead, it accepts a free-form reason string and asks AuthIntent::from_reason() to infer the security intent from that text:

pub fn load_keypair(&self, account: &str, reason: &str) -> Result<Zeroizing<Vec<u8>>> {    self.load_keypair_with_intent(account, &AuthIntent::from_reason(reason))}

AuthIntent::from_reason() classifies the text by prefix:

if lower.starts_with("authorize payment") {    Self::AuthorizePayment { message, limit }} else if lower.starts_with("delete") {    Self::DeleteAccount(message)} else {    Self::UseAccount(message)}

This is not arbitrary Polkit action injection. The caller cannot supply an action ID directly. The problem is narrower, but still security-relevant: display text can move a key-read operation into one of Pay's built-in privileged intent classes. For example, a keypair load with reason "delete the \"victim\" payment account" is currently classified as AuthIntent::DeleteAccount. The operation is not deleting an account, so the auth gate should receive AuthIntent::UseAccount(...) with that same display text.

The same issue appears with payment-shaped text. A keypair load with reason "authorize payment of $0.0001 for loading the victim keypair" is classified as AuthorizePayment with the lowest payment bucket, even though the operation is still just reading the keypair. On Linux this affects policy, not only wording. polkit_action_for_intent() maps each AuthIntent variant to a different action. Therefore caller-controlled text can make the same key-read operation request the account-use action, a delete-account action, or a payment-limit action.

Recommendation

Do not infer security-sensitive operation types from free-form strings. Make key-read APIs use a fixed typed intent. Keystore::load_keypair() should either be removed in favor of load_keypair_with_intent(), or it should always map the reason string to account use:

AuthIntent::use_account(reason)

Keep typed constructors for privileged operations such as account creation, import, export, deletion, payment authorization, session opening, and gateway fee-payer use. Callers that need those operations should use explicit typed APIs. If compatibility requires keeping AuthIntent::from_reason(), limit it to display-message construction. It should not choose privileged variants or platform policy buckets from caller-controlled prose.

UX Impact

Typed auth intents should not add a second biometric or password prompt. Users should see the same approval moment but the prompt and Linux policy bucket should be tied to the real operation instead of guessed from prose.

Description

The keystore accepts imported key material as a 64-byte Solana keypair, but it only checks the length:

fn validate_keypair(bytes: &[u8]) -> Result<()> {    if bytes.len() != 64 {        return Err(Error::InvalidKeypair(format!(            "expected 64 bytes, got {}",            bytes.len()        )));    }    Ok(())}

For Solana-style Ed25519 keypairs, the first 32 bytes are the private signing seed. The last 32 bytes are expected to be the matching public key. The current validation does not verify that relationship, so it accepts any 64-byte array, including this shape:

secret key A || public key B

After the length check passes, import_with_intent() stores the full 64 bytes as the account keypair. It also stores bytes 32..64 as the account's public-key metadata:

self.store.store(&keypair_key(account), keypair_bytes)?;self.store    .store(&pubkey_key(account), &keypair_bytes[32..64])?;

That lets the keystore record an account whose displayed public key does not come from the imported signing key. pubkey(account) later returns the stored metadata as-is. It does not derive the public key from the private bytes and does not check that the two records match:

pub fn pubkey(&self, account: &str) -> Result<Vec<u8>> {    validate_account_name(account)?;    self.store.load(&pubkey_key(account)).map(|z| z.to_vec())}

A malformed or malicious import can therefore poison account identity metadata. The account can appear to use public key B while the stored private bytes belong to secret key A. Downstream signing code may reject the malformed keypair later, but by then Pay may already have displayed or recorded the wrong account address.

Recommendation

Validate imported keypairs with the same Solana or Ed25519 implementation used for signing. At import time:

• require exactly 64 bytes,
• derive the public key from the first 32 private bytes,
• compare the derived public key with bytes 32..64,
• reject the import if they do not match.

Do not treat caller-supplied public metadata as the source of truth. Derive the public metadata from the validated signing key and store that derived value. When loading existing records, consider revalidating the keypair before returning it. That catches corrupted or externally modified backend data. pubkey() should either derive the public key from a validated keypair record or verify that stored public metadata matches the keypair before returning it.

UX Impact

Keypair consistency checks should not change biometric or password behavior. Pay should reject malformed imports before storing anything. Users may see an import error for bad key files, while valid imports keep the same approval behavior.

Description

AuthIntent::send_sol() builds the local approval message for direct SOL transfers, but it only receives the recipient. It does not receive the amount and it always leaves the payment limit unset:

pub fn send_sol(recipient: &str) -> Self {    Self::AuthorizePayment {        message: format!("authorize sending SOL to {recipient}"),        limit: None,    }}

The caller in pay_core::client::send::send_sol() creates this intent before it parses amount_str into lamports or resolves * into a drain amount. Therefore the keystore approval cannot tell the user whether they are approving 0.1 SOL, 10 SOL, or a transfer that drains the account balance.

On Linux, the missing amount also changes policy selection. Polkit receives the generic payment action instead of an amount-aware action. The user may still approve the prompt, but the approval is not bound to the amount that Pay later signs.

Recommendation

Parse and validate the SOL amount before loading the signer. Build the auth intent from the canonical recipient and the exact amount that will be signed. For fixed sends, the prompt should show the exact SOL amount. For * drain sends, resolve the balance first and make the prompt say that Pay will drain the account minus fees. A known transfer amount should not produce an intent with limit: None.

A safer shape is:

let lamports = resolve_sol_amount(amount_str, &rpc, &sender_pubkey)?;let intent = AuthIntent::send_sol(recipient, lamports);let signer = load_signer_with_intent(active_account_name, &intent)?;

If Linux payment buckets remain USD-denominated, add SOL-specific policy buckets or map SOL sends to a conservative high-value policy when no trusted exchange-rate conversion is available. The important property is that the prompt text and policy decision come from the same amount that Pay will sign.

UX Impact

Adding the SOL amount should not require a second biometric or password prompt. Pay should resolve the amount before loading the signer, then show that amount in the existing approval. Drain sends need clearer wording, but they should still use one approval prompt.

Description

The 1Password auth and storage backends invoke the op CLI by name:

let mut cmd = Command::new("op");

This happens in signout, signin, availability checks and every storage operation. The command is resolved through the current process PATH. The storage operations pass or receive wallet secret material through the selected executable. During import, OnePasswordStore::store() sends the full keypair to op item create. During load, OnePasswordStore::load() trusts op item get --fields=recoveryPhrase --reveal to return the stored key material.

Consequently, a caller or local process that can influence the environment used to launch pay can place a malicious op earlier in PATH. The keystore will execute that binary as the password-manager backend. The malicious binary can capture imported keypairs, return attacker-chosen key material on load, fake success for deletes or creates or bypass the intended 1Password account and vault selection entirely.

Recommendation

Consider taking one of these approaches:

• require an explicitly configured absolute path to op,
• discover op once from a trusted install location and store that path,
• verify the selected binary before use,
• pin or compare a known-good hash for the selected binary when Pay controls the expected op version,
• launch op with a sanitized environment,
• reject execution when op resolves to a user-writable or unexpected location.

Binary verification should use platform-native checks where possible:

• macOS: run codesign -dv --verbose=4 <op-path> and require the Developer ID authority to identify AgileBits Inc.; also run spctl --assess --verbose <op-path> and require an accepted, notarized Developer ID binary.
• Linux: prefer installation through the official 1Password repository, such as apt install 1password-cli, and verify downloaded packages with the vendor signature when installing manually, for example gpg --verify op.sig op.deb with a good signature from 1Password.
• Windows: use Get-AuthenticodeSignature op.exe and require Status: Valid with an AgileBits Inc. signer.

If Pay can avoid the CLI entirely, a supported 1Password SDK or API would reduce this executable-trust surface. At the time of this recommendation, that option should be treated as a design direction rather than an immediate drop-in fix because SDK support appears to be early and does not directly provide a mature Rust integration.

This should be combined with the argv-leak fix. Pinning the binary does not stop argv exposure and moving secrets off argv does not stop a fake binary from receiving secrets.

UX Impact

Pinning or verifying op should not ask the user for another biometric or password check. The user-visible change is setup and failure behavior: Pay should use a trusted op binary or stop before sending secrets to an untrusted executable. A user may need to configure the 1Password CLI path once, but normal unlock prompts should stay on the current 1Password cadence.

Solana foundation

Acknowledged. Support for 1Password is not used and will be removed.

Cantina

Acknowledged

Description

Keystore::import_with_intent() stores one imported account as two backend records. It writes the full 64-byte keypair first, then writes the public-key metadata under a second key:

self.store.store(&keypair_key(account), keypair_bytes)?;self.store    .store(&pubkey_key(account), &keypair_bytes[32..64])?;

There is no rollback if the second write fails. import_with_intent() can return an error even though the private keypair was already committed to the backend. A caller may treat the import as failed and skip account-registry updates, while the keystore still contains orphaned private key material. The 1Password backend has a second replacement hazard. It deletes the old item before creating the new one:

let _ = self.delete(key);

If the delete succeeds and the later op item create fails, the previous account item may be gone. The macOS Keychain helper has the same replacement hazard. Its doStore implementation calls SecItemDelete for the existing generic-password item before it calls SecItemAdd with the new data. If the add fails because the Keychain is locked, interaction is not allowed, access rules changed, or another local Keychain error occurs, the helper reports a store failure after the previous item has already been removed.

In all of these cases, the API result no longer matches the durable state of the keystore.

Recommendation

Make account import atomic from the caller's point of view. Consider storing one structured account record that contains both the keypair and the public metadata. That removes the split-write failure case and makes account creation, replacement, and deletion one backend operation. If separate records must remain, stage the private key under a temporary record and commit the import only after every required write succeeds. If any step fails, remove the temporary private record before returning an error. For example:

let tmp = format!("{}.tmp", keypair_key(account));self.store.store(&tmp, keypair_bytes)?;
if let Err(err) = self.store.store(&pubkey_key(account), &keypair_bytes[32..64]) {    let _ = self.store.delete(&tmp);    return Err(err);}
self.store.store(&keypair_key(account), keypair_bytes)?;let _ = self.store.delete(&tmp);

For 1Password, avoid delete-before-create. First check whether the Pay item already exists, for example with op item list --tags <OP_TAG> --format json, then filter by the exact pay/<key> title and the selected vault/account. If an exact item exists, update it in place with op item edit instead of deleting it. Prefer editing by item ID rather than title so duplicate or ambiguous titles do not change the wrong item. If no exact item exists, create a new item. In both cases, read the item back and verify the stored value before reporting success.

The 1Password update should still use the safer secret transport from the argv-leak finding. Do not move key material into op item edit arguments. Use a supported SDK/API where practical, or pass a JSON template through stdin if the CLI supports that for edits.

For macOS Keychain, prefer SecItemUpdate for replacement. If the data model must change, add and verify the replacement item before removing the old item. If a backend cannot provide an atomic replace operation, return a clear partial-write error instead of leaving the account half-created or half-deleted.

UX Impact

Atomic import should not change the biometric or password cadence. The import should either finish cleanly or report that no account was written. Users may see a clearer failure or repair message after a backend write error, but they should not have to approve the same import twice.

Description

AuthIntent::open_session() creates a fixed approval message:

pub fn open_session() -> Self {    Self::OpenSession("authorize opening a pay session".to_string())}

The intent has no fields for the session deposit, currency or mint, target network, operator, token account, cap or expiry. The pull-mode session client uses this generic intent before it builds the delegation transactions. After the signer is loaded, the code uses the server-supplied operator and mint plus the caller-supplied deposit to build the init and update delegation transactions.

Consequently, the local keystore approval is not bound to the session terms that will be signed. On macOS and Windows the user sees only a generic session-opening prompt. On Linux, Polkit receives only the static sh.pay.open-session action and no amount bucket. This is weaker than the payment intent model, where the amount and description are carried into AuthIntent::authorize_payment().

For example:

1. A user opens a pull-mode Pay session with a protected mainnet account.
2. The session challenge or surrounding flow supplies an unexpected operator, mint, network, or larger deposit.
3. open_pull_session_header() loads the wallet with AuthIntent::open_session().
4. The OS prompt only says "authorize opening a pay session".
5. After approval, Pay builds delegation transactions using the actual operator, mint and deposit.
6. The user has approved a generic session unlock, but not the concrete delegation terms that will be signed.

Recommendation

Make session-opening auth intents structured. Include the exact session terms that affect user risk: approved amount, currency or mint, network, operator, token account or server identity and expiry or cap.

Build the prompt and Linux policy bucket from those structured fields instead of a fixed string. If a platform cannot display all fields in the OS prompt, add a Pay-controlled confirmation immediately before authentication. Reject or require an explicit stronger confirmation for sessions whose terms cannot be displayed safely.

UX Impact

The fix should not add a second biometric or password prompt. It should make the existing approval text more specific. Users may see a longer confirmation or an earlier fail-closed error when session terms are missing, malformed, or too large to display safely.

Solana Foundation

Acknowledged. Will be deferred, see security_report.md

Cantina

Acknowledged

Description

The macOS backed backend invokes several tools by name. If binaries with the same name are placed earlier in the PATH then these binaries will be used. The binaries are:

• security
• swiftc
• codesign

Recommendation

Consider using an explicitly configured absolute path to the binaries.

Description

Keystore::delete_with_intent() removes the private keypair record first, then treats the public-key metadata cleanup as best effort:

self.store.delete(&keypair_key(account))?;let _ = self.store.delete(&pubkey_key(account));Ok(())

The result of the second delete is discarded. If the keypair delete succeeds but the .pubkey delete fails, delete_with_intent() still returns Ok(()). That leaves the keystore in a split state. exists(account) returns false because the private keypair is gone, while pubkey(account) can still return the old public-key metadata.

Recommendation

Do not ignore public metadata deletion failures. Prefer deleting a single structured account record. If separate records remain, either:

• return an error when either delete fails,
• perform a best-effort cleanup but return a structured partial-delete warning,
• or retry/repair stale metadata on the next lookup.

pubkey() should also fail closed when the private keypair record no longer exists, unless the storage model intentionally treats public metadata as independent from account existence.

UX Impact

Delete cleanup should not add another biometric or password step. If only part of the delete succeeds, Pay should say that clearly instead of reporting a clean deletion. The user-facing change is better cleanup and recovery guidance, not a new unlock flow.

Description

The macOS Touch ID helper receives a structured LocalAuthentication error, but it sends only the localized description string back to Rust:

LAContext().evaluatePolicy(.deviceOwnerAuthenticationWithBiometrics, localizedReason: reason) { ok, e in    if !ok { authErr = e?.localizedDescription ?? "denied" }    sema.signal()}

Rust then classifies the failure as AuthDenied only when the helper text is exactly denied or contains the English substring cancel:

let err = extract_error(&output.stderr);if is_user_cancel(&err) {    Err(Error::AuthDenied(err))} else {    Err(Error::Backend(err))}

That makes auth error classification depend on localized, human-readable text instead of the structured LAError reason. On a non-English macOS installation, or if Apple changes the wording, a real user cancellation can be surfaced as a generic backend error.

Recommendation

Return structured LocalAuthentication error information from the helper, such as the error domain and code or a small explicit enum. Map user cancellation, system cancellation, app cancellation and failed user verification to the appropriate typed keystore error. Keep biometric availability, enrollment, lockout, policy and noninteractive states as distinct backend or unavailable errors with actionable guidance.

UX Impact

No extra macOS biometric or password prompt should be added. The helper should return structured LocalAuthentication errors so Pay can tell cancellation, lockout and setup failures apart. Users get clearer recovery messages without another Touch ID dialog.

Description

The 1Password auth gate is documented in code as signing out and signing back in on every access, with the goal of forcing biometric or password authentication through the 1Password desktop app integration. The implementation does not enforce that reset. OnePasswordAuth::signout runs op signout, but discards the process result:

fn signout(&self) {    let mut cmd = Command::new("op");    cmd.arg("signout");    if let Some(acct) = &self.account {        cmd.arg(format!("--account={acct}"));    }    let _ = cmd.output(); // best-effort}

OnePasswordAuth::authenticate then calls signin even if signout failed:

fn authenticate(&self, _intent: &crate::AuthIntent) -> crate::Result<()> {    self.signout();    self.signin()}

The 1Password CLI documentation says op signin is idempotent and prompts only when the selected account is not already authenticated. A successful op signin after a failed op signout can therefore mean only that an existing CLI session is still usable. It does not necessarily prove that the user approved the current Pay operation. This weakens the intended per-operation authorization model for 1Password-backed accounts. If Pay relies on signing out first to force a fresh local prompt a failed signout has to stop the operation. Otherwise export, delete, import and signing operations can proceed under an existing 1Password CLI session.

Recommendation

Make signout return a Result and propagate failures when signout is part of the security model for forcing a fresh prompt. If Pay intentionally trusts existing 1Password CLI sessions, remove the misleading best-effort signout behavior and document that the 1Password backend authorizes through the session model op is already using. In that model, the product should not claim that every keystore access forces a fresh biometric or password prompt.

UX Impact

If Pay requires fresh 1Password approval, this fix can make UX stricter. Users may see biometric or password prompts more often and commands should fail when Pay cannot reset the session. The lighter alternative is to document that Pay trusts the current 1Password session and stop promising a fresh prompt every time.

Solana foundation

Acknowledged. Support for 1Password is not used and will be removed.

Cantina

Acknowledged

Description

WindowsHelloAuth::authenticate maps several Windows Hello setup and availability states to Error::AuthDenied:

UserConsentVerificationResult::DeviceBusy => {    Err(Error::AuthDenied("Windows Hello: device busy".into()))}UserConsentVerificationResult::DisabledByPolicy => Err(Error::AuthDenied(    "Windows Hello: disabled by policy".into(),)),UserConsentVerificationResult::NotConfiguredForUser => Err(Error::AuthDenied(    "Windows Hello: not configured — set up in Settings first".into(),)),_ => Err(Error::AuthDenied("Windows Hello: auth failed".into())),

Microsoft's UserConsentVerificationResult values distinguish cancellation from states such as no device present, not configured for the user, disabled by policy or device busy. Those conditions are setup, policy, availability or retryable failures. They do not prove that the user intentionally denied the current Pay operation.

Consequently, callers can treat Windows Hello setup or availability failures as user refusal and a payment or signing caller that maps AuthDenied to user cancellation cannot distinguish that from a missing verifier, a policy block or a temporarily busy device.

Recommendation

Map only true cancellation and failed user verification to AuthDenied. Return structured unavailable, setup, policy and retryable errors for DeviceNotPresent, NotConfiguredForUser, DisabledByPolicy, and DeviceBusy. Update callers to show distinct guidance for setup required, disabled by policy, verifier unavailable, temporary busy state, and user cancellation.

UX Impact

Better Windows Hello error mapping should not add a new biometric or password prompt. The same authentication attempt should happen, but setup, policy, busy, and cancellation results should be shown differently. The user-facing change is better recovery guidance, not stricter authentication.

Description

The shared account-name validator accepts both uppercase and lowercase ASCII letters. Therefore Default and default are both valid logical Pay account names. The Windows backend derives every Credential Manager target by formatting the raw key into pay.sh/<key>:

impl SecretStore for WindowsCredentialStore {    fn store(&self, key: &str, data: &[u8]) -> Result<()> {        cred_write(&to_wide(&format!("pay.sh/{key}")), data)    }
    fn load(&self, key: &str) -> Result<Zeroizing<Vec<u8>>> {        cred_read(&to_wide(&format!("pay.sh/{key}")))    }
    fn exists(&self, key: &str) -> bool {        cred_exists(&to_wide(&format!("pay.sh/{key}")))    }
    fn delete(&self, key: &str) -> Result<()> {        cred_delete(&to_wide(&format!("pay.sh/{key}")))    }}

The Windows Credential Manager documentation states that generic credential TargetName values are case-insensitive. It also states that CredWriteW creates a credential or replaces the existing credential with the same target and type. Consequently, two Pay account names that differ only by case can map to the same Windows credential target even though the rest of the application can treat them as distinct account names.

Recommendation

Canonicalize account names before deriving keystore targets. The simplest mitigation is to reject account names that are not already lowercase ASCII, so every backend uses the same account identity model. If preserving display case is important, store display names separately from canonical account IDs. Use the canonical lowercase ID for account-file keys and every backend storage target.

UX Impact

Canonicalizing Windows account names should not trigger a new Windows Hello biometric or password prompt. The fix is account-name handling: reject or canonicalize confusing names before they reach Credential Manager. Users may see an earlier validation error, not a new authentication step.

Description

Keystore::delete_with_intent deletes the private keypair record first and only tries to delete the public-key metadata record if the first delete succeeds:

self.store.delete(&keypair_key(account))?;let _ = self.store.delete(&pubkey_key(account));Ok(())

On Windows, WindowsCredentialStore::delete forwards CredDeleteW failures directly as backend errors:

fn delete(&self, key: &str) -> Result<()> {    cred_delete(&to_wide(&format!("pay.sh/{key}")))}
fn cred_delete(target: &[u16]) -> Result<()> {    unsafe { CredDeleteW(PCWSTR(target.as_ptr()), CRED_TYPE_GENERIC, 0) }        .map_err(|e| Error::Backend(format!("CredDeleteW failed: {e}")))}

Microsoft documents CredDeleteW as failing with ERROR_NOT_FOUND when the target credential is absent, see https://learn.microsoft.com/en-us/windows/win32/api/wincred/nf-wincred-creddeletew. Therefore a Windows account whose main pay.sh/<account> credential has already been removed is not idempotently deletable through the keystore. The facade returns before attempting <account>.pubkey cleanup. Consequently a failure on a missing Windows credential prevents both public-key cleanup and higher-level account removal.

Recommendation

Make Windows credential deletion idempotent for missing credentials. WindowsCredentialStore::delete should treat ERROR_NOT_FOUND as success, while preserving meaningful errors such as ERROR_NO_SUCH_LOGON_SESSION or invalid-parameter failures. Also consider changing delete_with_intent so missing-keypair conditions still allow best-effort public-key metadata cleanup. If any part of cleanup fails, return a structured partial-delete error that tells the caller which records may still exist.

UX Impact

Making Windows cleanup idempotent should not add a Windows Hello biometric or password prompt. Cleanup should continue when a credential is already missing and then report any real leftover state. The user-facing change is a clearer delete result, not another unlock.

Description

hex_decode() only checks that the input has an even byte length, then slices the UTF-8 string with byte indexes:

(0..hex.len())    .step_by(2)    .map(|i| {        u8::from_str_radix(&hex[i..i + 2], 16)            .map_err(|e| Error::InvalidKeypair(format!("hex: {e}")))    })    .collect()

Rust string slices must start and end on UTF-8 character boundaries. Because this code uses byte offsets, an even-byte non-ASCII string can pass the length check and still panic before from_str_radix() has a chance to return a normal error. The path is reachable when backend or helper output is malformed. For example, a compromised helper, corrupted 1Password field or unexpected backend value can crash the process instead of returning Error::InvalidKeypair.

OnePasswordStore::load is one concrete source. It reads op item get --fields=recoveryPhrase --reveal, converts stdout with String::from_utf8_lossy(...).trim(), and passes the result into hex_decode(). A manually edited, corrupted, or unexpected 1Password recoveryPhrase value can therefore trigger the same panic during account loading.

Recommendation

Treat hex as an ASCII byte format instead of slicing the UTF-8 string. If Pay keeps the local decoder, use hex.as_bytes(), validate that every byte is an ASCII hex digit and return Error::InvalidKeypair for malformed input. For example:

fn hex_value(b: u8) -> Option<u8> {    match b {        b'0'..=b'9' => Some(b - b'0'),        b'a'..=b'f' => Some(b - b'a' + 10),        b'A'..=b'F' => Some(b - b'A' + 10),        _ => None,    }}

Then process hex.as_bytes().chunks_exact(2) and reject non-ASCII bytes through the normal error path. Alternatively, replace the local decoder with the maintained hex crate:

hex::decode(hex.trim())    .map_err(|e| Error::InvalidKeypair(format!("hex: {e}")))

hex::decode treats hex as a byte encoding, accepts uppercase and lowercase hex, and returns structured decode errors such as odd length or invalid hex character instead of panicking on UTF-8 boundaries. The crate is available at:

• crates.io: https://crates.io/crates/hex
• docs.rs: https://docs.rs/hex
• GitHub: https://github.com/KokaKiwi/rust-hex

If Pay takes this route, add hex as a direct pay-keystore dependency rather than relying on the transitive copy already present in the workspace lockfile.

UX Impact

Rejecting bad hex is an input-validation change, so the biometric or password prompt should not move. Bad hex should be rejected as invalid input before sensitive keystore work continues. The user should see a normal error instead of a panic or confusing backend failure.

Description

Keystore::import() is the public convenience API for importing an existing 64-byte keypair. It authenticates with AuthIntent::create_account(account):

pub fn import(&self, account: &str, keypair_bytes: &[u8], _sync: SyncMode) -> Result<()> {    self.import_with_intent(        account,        keypair_bytes,        _sync,        &AuthIntent::create_account(account),    )}

The crate already defines a separate import-specific intent:

pub fn import_account(account: &str) -> Self {    Self::ImportAccount(format!("import the \"{account}\" payment account"))}

On Linux, CreateAccount and ImportAccount map to different Polkit actions. Therefore the convenience import API can request the create-account policy and prompt text for an import operation.

For example:

1. An integration calls Keystore::import() to import an existing keypair.
2. The keystore builds AuthIntent::create_account(account).
3. The auth backend receives a create-account intent even though the operation is an import.
4. On Linux, Polkit sees the create-account action rather than the import-keypair action.
5. A policy that distinguishes import from create cannot enforce that distinction for this API.

Recommendation

Change Keystore::import() to authenticate with AuthIntent::import_account(account). Keep AuthIntent::create_account() for generated accounts only. Add a regression that asserts the convenience import API records ImportAccount, not CreateAccount.

UX Impact

The import intent fix should reuse the existing biometric or password approval. The same import approval can be used, but the prompt text should say that an existing key is being imported rather than a new account being created. This improves consent wording without changing prompt frequency.

Description

The macOS auth backend passes the user-facing approval reason to the Swift helper as a command-line argument:

let output = Command::new(&binary)    .args(["authenticate", &message])    .output()

That message can contain payment metadata. For example, AuthIntent::authorize_payment() formats the amount and description into the prompt text:

message: format!("authorize payment of {amount} for {description}")

Command-line arguments are visible through normal process metadata on many systems while the child process is running. Therefore a local same-user observer can inspect the helper process and recover details such as payment amounts, API domains, recipients or account names included in the approval text.

This is mostly a privacy issue for a wallet/payment keystore because approval prompts can reveal what service the user is paying, how much they are authorizing, or which account operation they are performing.

Recommendation

The simplest fix is to keep only the helper subcommand in argv and send the user-facing reason over stdin, an anonymous pipe or another IPC channel that is not exposed through process metadata. The helper can then read the reason from that private channel before passing it to LocalAuthentication.

The cleaner long-term fix is to remove the helper command-line boundary. Pay could link the LocalAuthentication code as a Swift module or library, preferably statically where practical so prompt text never has to cross argv. A Rust-native implementation is also reasonable: use security-framework for Keychain/Security.framework access and objc2-local-authentication for Touch ID / LocalAuthentication. That route also makes secret memory handling easier to reason about, because Rust supports explicit zeroization patterns more naturally than Swift.

Relevant crates: security-framework (https://crates.io/crates/security-framework, https://docs.rs/security-framework, https://github.com/kornelski/rust-security-framework), security-framework_sys (https://docs.rs/security-framework-sys/latest/security_framework_sys) and objc2-local-authentication (https://crates.io/crates/objc2-local-authentication, https://docs.rs/objc2-local-authentication, https://github.com/madsmtm/objc2).

Also normalize and bound prompt components before display so remote descriptions, account names or other caller-controlled fields cannot inject confusing control characters into the prompt.

UX Impact

Moving the macOS reason out of argv should not create another biometric or password prompt. The same macOS approval should appear, but Pay should pass the prompt reason through a private channel instead of argv. The visible text may be shorter after sanitization, but the Touch ID or password timing should not change.

Description

The generic keystore import logic stores two records:

self.store.store(&keypair_key(account), keypair_bytes)?;self.store    .store(&pubkey_key(account), &keypair_bytes[32..64])?;

The generic pubkey() API later loads the .pubkey record:

self.store.load(&pubkey_key(account)).map(|z| z.to_vec())

The 1Password backend does not store that record. It treats any key ending in .pubkey as public metadata and returns success without creating an item:

if key.ends_with(".pubkey") {    return Ok(());}

As a result, a successful 1Password import can leave the generic pubkey() API unable to retrieve the account's public key. pubkey("victim") tries to load pay/victim.pubkey, but the 1Password store never created that item.

Recommendation

Make public-key retrieval a first-class backend capability instead of overloading SecretStore::load().

Consider keeping the private key material in 1Password and store the public key in Pay-managed public metadata. The public key is not secret, so pubkey() does not need to read the 1Password item or load the concealed keypair just to derive it.

Reasonable implementation options include:

• cache the 32-byte public key or wallet address in Pay's account metadata when the account is created or imported,
• store a separate non-secret public-key item under a non-colliding namespace, while recognizing that reading it through 1Password may still require the normal 1Password unlock/session,
• implement a backend-specific load_pubkey() that reads public metadata without requesting the concealed recoveryPhrase,
• or change the generic storage model to one structured account record with explicit public metadata.

Whichever shape is chosen, update and delete the public metadata together with the account so it cannot drift. Do not silently skip .pubkey writes while keeping a generic pubkey() implementation that expects those writes to exist.

UX Impact

pubkey() should not need a new biometric or password prompt just to read public metadata. If Pay keeps the public key in its own public account metadata, the call can avoid touching 1Password entirely. If the implementation instead reads public fields from 1Password, the normal 1Password biometric or password unlock may still appear because 1Password controls access to the vault item.

Solana foundation

Acknowledged. Support for 1Password is not used and will be removed.

Cantina

Acknowledged

Description

The keystore stores one logical account as two independent backend records: the full keypair under <account> and the public-key cache under <account>.pubkey. Import writes those records in two separate calls:

self.store.store(&keypair_key(account), keypair_bytes)?;self.store    .store(&pubkey_key(account), &keypair_bytes[32..64])?;

Delete removes the same records in two separate calls:

self.store.delete(&keypair_key(account))?;let _ = self.store.delete(&pubkey_key(account));Ok(())

There is no per-account lock, version check, or backend transaction around those multi-record mutations. Therefore two same-account operations can both return success while leaving a state that no single successful operation should produce.

One interleaving is concurrent imports. Import A writes keypair A and pauses before writing pubkey A. Import B writes keypair B and pubkey B. Import A then resumes and writes pubkey A. The final keypair belongs to B, while pubkey(account) returns A's public key.

A second interleaving is import racing with delete. Import writes the keypair and pauses before writing the public-key cache. Delete removes the keypair and any current public-key cache. The paused import then writes the public-key cache. Both calls can report success, while exists(account) is false and pubkey(account) still returns the imported public key.

Recommendation

Serialize same-account mutations in the keystore facade, or replace the two-record model with one atomic account record containing both keypair and public metadata. If separate records remain, make import and delete transactional from the caller's point of view. A successful operation should leave matching records and a failed or interrupted operation should repair or roll back partial state. pubkey(account) should also verify that the cache corresponds to the current keypair, or the cache should carry a version/check value tied to the keypair record.

UX Impact

Serializing same-account writes should not ask for another biometric or password approval. Pay should serialize same-account writes or store one atomic account record. Users may see one operation wait for another or get a clear partial-state error, but they should not be asked to approve the same mutation twice.

Description

Keystore::exists is the one account-facing keystore method in this group that reaches the backend before validating the account name. Import, public-key lookup, delete, and keypair load all apply the shared account-name check first, but exists derives the backend key directly from the caller's input:

pub fn exists(&self, account: &str) -> bool {    self.store.exists(&keypair_key(account))}

The shared validator rejects empty names and characters outside ASCII alphanumerics, ., _, and -:

if !name    .bytes()    .all(|b| b.is_ascii_alphanumeric() || b == b'.' || b == b'_' || b == b'-'){    return Err(Error::InvalidKeypair(...));}

This means exists("bad/name") can still call into the selected backend even though the real mutating and loading operations would reject the same account name before storage access. That matters because exists is naturally used as a preflight. Account creation checks ks.exists(name) before import_with_intent validates the name, and legacy cleanup logic probes backends with ks.exists(name) before the validated delete path is reached.

Therefore, exists does not enforce the same account namespace boundary as the rest of the keystore facade. Invalid names should be rejected before they are converted into backend keys.

Recommendation

Validate account names before any backend access. The cleanest shape is a fallible API such as try_exists(&self, account: &str) -> Result<bool>, then move overwrite and cleanup checks to that API. If the existing boolean wrapper stays for compatibility, it should validate first and return false for invalid names or be clearly documented as lossy and unsuitable for security-sensitive preflights.

UX Impact

Account-name validation should happen before storage access, so biometric and password behavior should not change. Invalid account names should fail before any backend lookup. The visible UX change is an earlier validation error instead of a backend-specific not-found result or prompt.

Description

The keystore storage interface exposes existence checks as a plain boolean. The 1Password backend implements that boolean by running op item get pay/<key> --format=json and returning true only when the command exits successfully. Every command failure becomes false. That means OnePasswordStore::exists cannot distinguish a genuinely missing item from authentication failure, desktop app unavailability, account-selection errors, permission errors, duplicate or ambiguous item titles, rate limits, or other backend failures.

This matters because pay account new uses ks.exists(name) && !force as the no-force overwrite guard. For 1Password, this check runs before write authentication. If the account already exists but the preflight op item get command fails, create_account treats the account as absent, generates a new keypair, authenticates, and imports it. The 1Password store then performs its replacement behavior by deleting the old item before creating the new one.

Therefore, a backend error during exists can incorrectly allow a no-force account creation to enter replacement at all.

Recommendation

Do not represent overwrite-sensitive backend checks as a lossy boolean. Change the storage API or add a 1Password-specific preflight, so callers can distinguish present, absent and unknown/error. pay account new should skip the no-force guard only when the backend returns a confirmed not-found result. Authentication failures, desktop app errors, account or vault selection failures, permission errors, duplicate/ambiguous item responses, rate limits and command execution errors should fail closed with a clear message.

For 1Password, parse known not-found responses explicitly. Treat all other op item get failures as errors rather than absence.

UX Impact

This should fail closed before replacement when 1Password cannot confirm whether the account exists. Usually that means a clearer 1Password error, not a new biometric or password prompt. If Pay chooses an authenticated preflight, the user may see the normal 1Password unlock before Pay decides whether --force is required.

Solana foundation

Acknowledged. Support for 1Password is not used and will be removed.

Cantina

Acknowledged

Description

The keystore API carries a typed AuthIntent through import, delete, load, signing, export, payment, session and gateway fee-payer operations. Those intents preserve operation-specific context such as the account name, payment amount, payment description, export/delete action, session opening or gateway fee-payer use.

The 1Password auth gate discards that context. OnePasswordAuth::authenticate accepts _intent, signs out, then runs op signin. It never uses intent.message(), intent.prompt_message() or the typed intent variant. Therefore the 1Password-backed approval only proves that the user authenticated to the 1Password CLI. It does not show the Pay operation being approved.

Recommendation

Do not treat a generic 1Password CLI sign-in as operation-specific Pay consent.

If the 1Password CLI cannot display custom prompt text for op signin, add a Pay-controlled confirmation step immediately before sensitive 1Password-backed operations. That confirmation should display the exact AuthIntent message, including payment amount, resource description, account action, export/delete operation, session opening, or gateway fee-payer use.

If the product intentionally relies only on generic 1Password user presence, document that limitation clearly and avoid describing the 1Password backend as providing per-operation approval.

UX Impact

This can add one Pay confirmation step if 1Password cannot show the operation details itself. That is a real UX cost, but it gives the user the account, amount, or action before approval. It does not have to force an extra biometric or password prompt, the 1Password app should still own the actual unlock.

Solana foundation

Acknowledged. Support for 1Password is not used and will be removed.

Cantina

Acknowledged

Description

The Linux keystore backend exposes a synchronous interface over async Secret Service and Polkit APIs. Polkit::authenticate, Polkit::is_available, SecretServiceStore::is_available and every SecretServiceStore operation call a shared run(...) helper. That helper builds a new current-thread Tokio runtime and immediately calls block_on for the supplied future.

That is unsafe when the Linux keystore is called from code that is already running inside a Tokio runtime. Tokio panics when a runtime is blocked from within an asynchronous execution context. Instead of returning a structured keystore or config error, the process can abort.

This is reachable from the server signer setup path. server start creates the main Tokio runtime and then runs signer resolution inside rt.block_on(...). The resolve_signer helper is explicitly documented as needing to be called from within the main async runtime. When operator.signer.backend = account resolves a GNOME Keyring-backed account on Linux, it calls the synchronous account signer loader. That reaches Keystore::gnome_keyring(), then load_keypair_with_intent, which calls both the Linux Polkit auth gate and Secret Service store through the nested run(...) helper.

The result is a Linux availability bug for async caller paths, including server startup with a GNOME Keyring account signer. A recoverable startup or keystore failure can become a panic.

Recommendation

Do not create and block on a new Tokio runtime inside the Linux backend when the caller may already be inside a runtime.

Prefer exposing async Linux backend methods and having async callers await them. If the public keystore interface must stay synchronous, move the Linux async work onto a dedicated blocking boundary before calling Runtime::block_on, such as a standard thread or a carefully isolated blocking task that is not itself executing inside the runtime being blocked.

For server startup, ensure Linux account signer loading either uses an async keystore path or runs the synchronous keystore facade outside the active async runtime. Keystore and auth failures should return typed Result errors rather than panicking.

UX Impact

Fixing the Tokio panic should not add another biometric or password check. The Linux backend should return a normal error instead of panicking when called from an async context. Users may see a clearer Secret Service or collection-unlock failure, but the unlock cadence should stay the same.

Description

The Linux Secret Service backend unlocks the pay collection, performs the requested storage operation, and then relocks the collection before returning. In store, the backend saves the result of store_item(...), but it checks col.lock().await before returning that saved result:

let result = store_item(&col, &key, &data).await;col.lock().await.map_err(ss_err)?;result

If Secret Service successfully creates or replaces the item and the later relock call fails, store returns the relock error even though the keypair record was already persisted. The shared Keystore::import_with_intent facade then stops before writing the .pubkey record, so the caller sees an import failure while the backend may contain a keypair-only account. load has the same result-ordering shape. It performs the read, saves that result, then checks col.lock().await before returning the saved value:

let result = async {    ...    Ok(Zeroizing::new(item.get_secret().await.map_err(ss_err)?))}.await;col.lock().await.map_err(ss_err)?;result

If the read succeeds and relocking fails, the caller gets only the relock error. That is less damaging than a masked write or delete, but it is the same cleanup-result ordering bug.

delete has the same cleanup-ordering problem. It deletes matching items from the pay collection, then relocks that collection before deleting matching items from the default collection:

for item in col.search_items(attrs(&key)).await.map_err(ss_err)? {    item.delete().await.map_err(ss_err)?;}col.lock().await.map_err(ss_err)?;
if let Ok(default) = ss.get_default_collection().await {    ...}

If the relock fails after one or more items were deleted, the method reports an error and returns before the default-collection cleanup. Callers can therefore react to a failed delete even though durable secret state has already changed.

The Linux-specific problem is that post-operation cleanup is allowed to decide what the caller sees after the backend operation has already happened.

Consequently, Linux Secret Service-backed accounts can enter confusing partial states after an apparent failed operation. For imports, a relock failure after the keypair write can leave a private keypair record without the matching public-key metadata record. For deletes, a relock failure can leave only part of the intended cleanup complete. The issue does not expose private keys or bypass authentication by itself, but it makes caller-visible errors unreliable as a statement about whether storage changed.

Recommendation

Handle Secret Service operations as two separate outcomes: the primary operation and the relock cleanup. Always attempt the relock, but do not let it hide the primary operation result.

For store, the minimum safe shape is to save both results and combine them after cleanup:

let result = store_item(&col, &key, &data).await;let lock_result = col.lock().await.map_err(ss_err);result.and(lock_result)

Use the same ordering for load, but preserve the loaded value:

let result = read_item(&col, &key).await;let lock_result = col.lock().await.map_err(ss_err);result.and_then(|value| lock_result.map(|_| value))

The important point is the ordering: first complete the read or write, then attempt relock, then combine the two results. That preserves the real read or write error if the primary operation failed, instead of replacing it with a later cleanup error.

For successful mutations, a relock error should be reported as cleanup/partial-success state, not as a plain store or delete failure. During import, Pay should either reconcile the partially written account or return an error that clearly says the keypair may already have been stored. During delete, the backend should continue best-effort cleanup, including default-collection cleanup, even if relocking the pay collection fails; then it can return a structured partial-delete error if any step failed.

An RAII-style guard can help make relocking hard to forget, but it should not be the only error-reporting mechanism because Drop cannot return a failure. If a guard is used, keep it best-effort and still capture the explicit lock_result in the operation so callers know whether cleanup failed.

UX Impact

Relock reporting should not require a second biometric or password interaction. If relocking fails after a store or delete already happened, Pay should report that partial success plainly. Users may need cleanup guidance, but they should not be asked to unlock again just to learn what happened.

Description

The Linux Secret Service backend does not apply one consistent namespace policy for the custom pay collection and the default Secret Service collection. load requires the custom pay collection and only reads from that collection:

let col = get_collection(&ss).await.ok_or_else(|| {    Error::Backend("pay keyring not found — run `pay setup` first".to_string())})?;...let items = col.search_items(attrs(&key)).await.map_err(ss_err)?;

exists searches the default collection only when the pay collection is absent. If the pay collection exists, exists searches only pay:

let Some(col) = get_collection(&ss).await else {    let Ok(default) = ss.get_default_collection().await else {        return false;    };    return default.search_items(attrs(&key)).await...};col.search_items(attrs(&key)).await...

delete is broader. It deletes matching items from the pay collection when present and then also searches the default collection:

if let Some(col) = get_collection(&ss).await {    ...}if let Ok(default) = ss.get_default_collection().await {    ...}

These APIs do not describe one coherent storage namespace. A legacy item in the default collection can make exists("default") return true when no pay collection exists, but a later load("default") still fails because load never reads the default collection. Once the pay collection exists, the same default-collection item becomes invisible to exists, even though delete would try to remove it.

Recommendation

Make the Linux Secret Service namespace policy explicit and consistent. If default-collection migration is supported, exists, load and delete should search the same collection set in the same order. A successful migration should rewrite or move the item into the custom pay collection. If default-collection migration is not supported, remove the fallback from exists and provide explicit migration or cleanup guidance.

UX Impact

No new biometric or password prompt should be added. Linux users may see clearer migration or cleanup guidance when an item is found in the default collection. Existing Secret Service unlock prompts can still occur, but the fix should not add a second Pay-level password gate.

Description

The keystore validates imported keypairs before writing them, but it does not validate data loaded back from the storage backend. Keystore::pubkey is documented as returning a 32-byte public key, yet it returns whatever bytes the backend stores under <account>.pubkey:

pub fn pubkey(&self, account: &str) -> Result<Vec<u8>> {    validate_account_name(account)?;    self.store.load(&pubkey_key(account)).map(|z| z.to_vec())}

load_keypair_with_intent is documented as loading the full 64-byte keypair, but it also returns the backend value without checking its length:

pub fn load_keypair_with_intent(    &self,    account: &str,    intent: &AuthIntent,) -> Result<Zeroizing<Vec<u8>>> {    validate_account_name(account)?;    self.auth.authenticate(intent)?;    self.store.load(&keypair_key(account))}

This means a corrupted, manually edited, legacy or malicious backend record can be treated as a successful keystore load even when it is not a 32-byte public key or a 64-byte keypair. Real backends do not guarantee that every record was produced by the current crate version. 1Password items can be edited, platform credential records can be stale or migrated and local same-user code can sometimes alter backend data through the platform's native tools.

Recommendation

Validate backend data at the keystore boundary. load_keypair_with_intent should reject any loaded value whose length is not exactly 64 bytes. pubkey should reject any loaded value whose length is not exactly 32 bytes. Return a structured backend-corruption error so callers can show recovery guidance. Do not make every CLI or library caller remember to revalidate lengths after a successful keystore read.

UX Impact

Length checks should happen after the normal backend read, without changing biometric or password timing. Malformed backend records should be rejected after load with a clear corruption or recovery error. Users should not see an additional unlock just because Pay validates record lengths.

Description

The Linux auth gate first maps each typed AuthIntent to a specific Polkit action. Payment approvals can map to amount-scoped actions such as sh.pay.authorize-payment-up-to-usd-005, while account creation, import, export, deletion, session opening, gateway fee-payer use and account use each have separate action IDs.

Polkit::authenticate does not fail closed when the selected specific action is missing. If polkit_authenticate(action) reports a missing action, the code retries with the legacy generic action sh.pay.unlock-keypair.

This fallback changes the authorization that the user and policy engine see. A missing amount-scoped payment action can become a generic "use account" approval. A missing delete, import, export, session or gateway fee-payer action can also be retried as the same generic legacy action. The current policy file defines both the specific actions and the legacy action, so a stale or partially installed policy can silently downgrade the prompt instead of telling the user to update the policy.

This does not remove authentication entirely. The user still has to satisfy Polkit for sh.pay.unlock-keypair. The problem is that the approval no longer matches the requested operation or amount scope.

Recommendation

Fail closed when a specific Polkit action is missing. Return the existing setup error and tell the user to install or update the policy instead of retrying a broader legacy action. If compatibility fallback is still required, restrict it to operations that are security-equivalent to sh.pay.unlock-keypair. Do not use the legacy action for amount-scoped payments, account creation, import, export, deletion, session opening, or gateway fee-payer use.

UX Impact

This may make Linux fail closed more often, but it should not add a second prompt. If a specific Polkit action is missing, Pay should ask the user to install the updated policy instead of falling back to a broader prompt. Whether that one Polkit prompt uses a password or biometric factor remains a local system policy decision.

Description

Gateway fee-payer mode is an intended feature where the gateway operator's wallet co-signs payment transactions to sponsor Solana network fees for users. In that setup, users can complete paid API requests without holding enough SOL for transaction fees themselves; the gateway's configured fee-payer account covers those fees while the user still pays the API charge. The problem is the approval boundary around that feature: AuthIntent::use_gateway_fee_payer() builds one hard-coded approval for every gateway fee-payer use:

pub fn use_gateway_fee_payer() -> Self {    Self::UseGatewayFeePayer("use your pay account as the gateway fee payer".to_string())}

That message tells the user the account will be used as a fee payer, but not the terms of that authority. The intent has no fields for the selected network, server or provider spec, operator recipient, RPC endpoint, fee cap, lifetime, request volume or pull-mode operator role. It also carries no payment or fee limit for Linux policy selection. This matters because server startup loads the signer once with this generic intent and then reuses the resulting signer. The same fee_payer_signer is placed into each MPP server configuration. Pull-mode session setup can also reuse it as the operator signer for the open-channel batcher.

Therefore, a protected-account prompt approves a long-lived server role without naming the server role's concrete limits. A local test gateway, a mainnet public gateway and a pull-mode operator setup can all produce the same approval text.

Recommendation

Make gateway fee-payer approvals structured.

AuthIntent::use_gateway_fee_payer should include the exact terms that affect user risk: network, server or provider identity, fee-payer recipient/operator public key, whether pull-mode operator signing is enabled, maximum fee budget, expected lifetime and whether the signer will be cached for server-wide reuse.

Server startup should resolve those terms before loading the signer. If terms are missing or ambiguous, fail before the auth prompt. For long-running gateways, require an explicit cap or session lifetime and renew approval when the cap is exhausted or the lifetime expires.

UX Impact

The fix should not require an extra biometric or password prompt for a normal startup. It should make the existing prompt more specific. Users may see a longer confirmation message or an additional Pay-controlled confirmation when a gateway wants broad or long-lived fee-payer authority.

Solana Foundation

Acknowledged. Will be deferred, see security_report.md

Cantina

Acknowledged

Description

1Password strings might be language dependant, so interpreting might not work correctly. They might also changes with version updates.

Recommendation

Test the strings in different platform languages and for different versions.

Also consider using an sdk.

UX Impact

The UX should be improved in the situation where an error occurs.

Solana foundation

Acknowledged. Support for 1Password is not used and will be removed.

Cantina

Acknowledged

Description

Several variables are not zeroized after use, to it leaves sensitive data in process memory.

Recommendation

Consider changing the code in the following way to zeroize.

1Password

fn load(&self, key: &str) -> Result<Zeroizing<Vec<u8>>> {    ...-   let hex = Zeroizing::new(String::from_utf8_lossy(&output.stdout).trim().to_string());+   let stdout = Zeroizing::new(output.stdout);+   let hex = Zeroizing::new(String::from_utf8_lossy(&stdout).trim().to_string());    hex_decode(&hex).map(Zeroizing::new)}

macOS

fn helper_run(args: &[&str]) -> Result<String> {    ...    if output.status.success() {-       Ok(String::from_utf8_lossy(&output.stdout).to_string())+       let stdout = Zeroizing::new(output.stdout);+       Ok(String::from_utf8_lossy(&stdout).to_string())    }    ...    }

Windows

fn cred_read(target: &[u16]) -> Result<Zeroizing<Vec<u8>>> {    ...- let blob = unsafe {+ let blob = Zeroizing::new(unsafe {      let c = &*ptr;      slice::from_raw_parts(c.CredentialBlob, c.CredentialBlobSize as usize).to_vec()- };+ });  unsafe { CredFree(ptr.cast()) };- Ok(Zeroizing::new(blob))+ Ok(blob)    ...}

UX Impact

No impact on the UX.

Description

fn exists() uses op get. However this also retrieves sensisitive information.

Recommendation

Consider using op item list.

UX Impact

The UX might be improved because less approvals have to be given.

Solana foundation

Acknowledged. Support for 1Password is not used and will be removed.

Cantina

Acknowledged

Description

Checks on error texts or zbus are not robust. These texts might be different in different languages and different versions.

Recommendation

Check if zbus::Error::MethodError can be used, also see zbus documentation.

This can be tested in the following way:

.map_err(|e| match &e {            zbus::Error::MethodError(name, _, _)                if name.contains("NoSuchAction")                || name.contains("Error.Failed") =>            {                missing_action_error(action)            }            _ => Error::Backend(format!("polkit: {e}")),        })?;

UX Impact

The UX should be improved in the situation where an error occurs.

Description

Errors with set_permissions() are ignored, however these errors can occur on for example on LAN systems.

The access to the binary relies on this so it should not be ignored.

Recommendation

Consider throwing an error if the set_permissions() fails.

UX Impact

A previously undetected security issue might throw an error.

Description

Developer ID Application is the first signer to be used, but these certificates are for distribution outside the App Store and require Apple Notarization. So its more logical to put Apple Development first, because these certificates are for local/team development.

Recommendation

Consider switching the order in the following way:

-for prefix in ["Developer ID Application", "Apple Development"] {+for prefix in ["Apple Development", "Developer ID Application"] {

UX Impact

No impact on the UX.

Description

The function verify_codesign() uses --strict which on some macOS versions enforces the presence of a secure timestamp. However codesign_binary() doesn't use --timestamp. This can cause --strict verification to fail on non-ad-hoc signatures.

Recommendation

Consider adding --timestamp when signing with a real identity.

let sign = Command::new("codesign")    .args(["-s", &identity, "-f", "--entitlements"])+   .args(if identity != "-" { Some("--timestamp") } else { None })    ...;

UX Impact

The UX should be improved in the situation where verify would previously fail.

Description

If an error occurs with write_all() then the child process might stay running, taking up resources.

Recommendation

Consider killing the child process when an error occurs:

child.kill().ok();child.wait().ok();

UX Impact

No impact on the UX.

Description

/usr/bin/security is a more powerfull command that might accidentally remove too much. It is not clear why it is used.

Recommendation

Doublecheck why it is used. If it used to access keys that are made with a previous version of the app, then ksecattraccessgroup could perhaps be used to allow access to it.

let accessGroup = "\(teamID).com.solana.pay"let query: [String: Any] = [    kSecClass as String: kSecClassGenericPassword,    kSecAttrService as String: "pay.sh",    kSecAttrAccount as String: account,    kSecAttrAccessGroup as String: accessGroup]

Description

Authentication might fail in the following situations:

• Touch ID is not enrolled
• Touch ID is disabled
• Too many failed biometric attempts
• The user taps "Use Password" in the Touch ID dialog

Recommendation

Consider adding a fallback to passcode, after checking this is desirable from a security point of view.

-    LAContext().evaluatePolicy(.deviceOwnerAuthenticationWithBiometrics, ...)) +    LAContext().evaluatePolicy(.deviceOwnerAuthentication, ...)

UX Impact

The UX might will be improved when Touch ID doesn't work.

Description

GetForegroundWindow() returns the window with keyboard focus at the time of the call. This can be any window on the desktop, which could be confusing.

Recommendation

Consider checking if the foreground window is ours. If not we skip it and fall back to None, which triggers the UWP path (UserConsentVerifier::RequestVerificationAsync) that shows the prompt without a parent window.

let foreground = unsafe { GetForegroundWindow() };+    let mut foreground_pid = 0u32;+    unsafe { GetWindowThreadProcessId(foreground, Some(&mut foreground_pid)) };-    if !foreground.is_invalid() {+    if foreground_pid == std::process::id() {        return Some(foreground);}

UX Impact

The UX might be improved because less confusion prompts are given.