diff --git a/Cargo.lock b/Cargo.lock index e5e5e43..739831a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -136,6 +136,7 @@ dependencies = [ "rpassword", "serde", "serde_json", + "tempfile", "thiserror 1.0.69", "tokio", "toml 0.9.12+spec-1.1.0", diff --git a/README.md b/README.md index 61e9794..2818b3e 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,8 @@ Environment variables consumed at startup: | `ALDABRA_KOIOS_BASE` | no | public Koios for the chosen network | Override to point at a self-hosted Koios. | | `ALDABRA_PASSPHRASE` | yes | — | Unlocks `mnemonic.age`. Source from a docker secret or systemd `EnvironmentFile` — never commit it. | | `ALDABRA_BOOTSTRAP` | no | (unset) | Set to `new` or `import` to enter bootstrap mode on next launch. | +| `ALDABRA_MAX_SEND_LOVELACE` | no | mainnet 10 ADA / preprod\|preview 100 tADA | Hard cap on lovelace flowing to any non-self destination. Enforced by every tool that signs or submits; pass `force=true` per-tool to override. | +| `ALDABRA_SAFE_READS_ROOT` | no | `$ALDABRA_DATA/scripts/` | Sandbox dir for the `reference_script_path` / `policy_cbor_path` tool args. Files outside this dir (canonical) are refused. Created with 0o700 at startup if missing. | ## MCP tools diff --git a/crates/aldabra-mcp/Cargo.toml b/crates/aldabra-mcp/Cargo.toml index 0aba77f..8904c58 100644 --- a/crates/aldabra-mcp/Cargo.toml +++ b/crates/aldabra-mcp/Cargo.toml @@ -42,3 +42,8 @@ age = { workspace = true } toml = { workspace = true } rpassword = { workspace = true } zeroize = { workspace = true } + +[dev-dependencies] +# CRIT-1/CRIT-2 audit fix tests (2026-05-12): need a real tx CBOR +# fixture + temp-dir sandbox root. +tempfile = "3" diff --git a/crates/aldabra-mcp/src/config.rs b/crates/aldabra-mcp/src/config.rs index 1e00692..c908ecb 100644 --- a/crates/aldabra-mcp/src/config.rs +++ b/crates/aldabra-mcp/src/config.rs @@ -37,6 +37,14 @@ pub struct Config { /// faucet-replaceable). Override via TOML or /// `ALDABRA_MAX_SEND_LOVELACE`. pub max_send_lovelace: u64, + /// Sandbox root for filesystem reads via `*_path` tool args + /// (`reference_script_path`, `policy_cbor_path`). Tools must + /// canonicalize both this root and the user-supplied path, then + /// refuse any path whose canonical form does not live under this + /// root. Default `$ALDABRA_DATA/scripts/`. Override via TOML or + /// `ALDABRA_SAFE_READS_ROOT`. Created with 0o700 at startup if + /// missing — the daemon user is the only intended writer. + pub safe_reads_root: PathBuf, } /// Network-aware default for `max_send_lovelace`. See the docstring on @@ -60,6 +68,8 @@ struct FileConfig { index: Option, #[serde(default)] max_send_lovelace: Option, + #[serde(default)] + safe_reads_root: Option, } fn parse_network(s: &str) -> Result { @@ -178,6 +188,12 @@ impl Config { .unwrap_or_else(|| default_max_send_for(network)), }; + let safe_reads_root: PathBuf = std::env::var("ALDABRA_SAFE_READS_ROOT") + .ok() + .map(PathBuf::from) + .or_else(|| file_cfg.safe_reads_root.as_ref().map(PathBuf::from)) + .unwrap_or_else(|| data_dir.join("scripts")); + Ok(Self { network, koios_base, @@ -186,6 +202,7 @@ impl Config { index, data_dir, max_send_lovelace, + safe_reads_root, }) } } diff --git a/crates/aldabra-mcp/src/main.rs b/crates/aldabra-mcp/src/main.rs index ff2d3c4..f9343cd 100644 --- a/crates/aldabra-mcp/src/main.rs +++ b/crates/aldabra-mcp/src/main.rs @@ -56,9 +56,41 @@ async fn run() -> Result<()> { account = cfg.account, index = cfg.index, data_dir = %cfg.data_dir.display(), + safe_reads_root = %cfg.safe_reads_root.display(), "aldabra starting" ); + // CRIT-2 audit fix (2026-05-12): make sure the sandbox root exists + // and is daemon-only readable. Tools use canonicalize() against + // this dir to validate `*_path` args, which requires the dir to + // be a real directory on disk. Idempotent: create_dir_all is a + // no-op when the dir already exists. + // + // Adversarial-review fix (2026-05-12): chmod is `?`'d not swallowed. + // If the filesystem refuses chmod (noexec, selinux, broken mount), + // we'd otherwise silently fall back to the umask default (commonly + // 0o755) — making the security comment "daemon-only readable" a + // lie. Fail loudly instead so the operator sees + investigates. + if !cfg.safe_reads_root.exists() { + std::fs::create_dir_all(&cfg.safe_reads_root).map_err(|e| { + anyhow::anyhow!( + "create safe_reads_root {}: {e}", + cfg.safe_reads_root.display() + ) + })?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&cfg.safe_reads_root, std::fs::Permissions::from_mode(0o700)) + .map_err(|e| { + anyhow::anyhow!( + "chmod 0o700 safe_reads_root {}: {e}", + cfg.safe_reads_root.display() + ) + })?; + } + } + // CLI mode flags — all out-of-band, before the MCP transport // takes over stdio. // @@ -132,6 +164,7 @@ async fn run() -> Result<()> { stake_key, cfg.max_send_lovelace, cfg.data_dir.clone(), + cfg.safe_reads_root.clone(), ); let server = service .serve(stdio()) diff --git a/crates/aldabra-mcp/src/tools.rs b/crates/aldabra-mcp/src/tools.rs index 18b7d16..0969652 100644 --- a/crates/aldabra-mcp/src/tools.rs +++ b/crates/aldabra-mcp/src/tools.rs @@ -23,7 +23,7 @@ //! - `Result` lets us surface chain / build errors //! as MCP tool-call errors instead of crashing the daemon. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::Arc; use aldabra_chain::{ChainBackend, KoiosClient}; @@ -78,6 +78,67 @@ use aldabra_dao::discovery::{ }; use aldabra_dao::reader::{DaoReader, KoiosDaoReader}; +/// CRIT-2 audit fix (2026-05-12): reject any `*_path` arg whose +/// canonical form does not live under the configured sandbox root. +/// Without this guard, a prompt-injection that gets the LLM to call a +/// tool with `reference_script_path: "/var/lib/aldabra/root-xprv.age"` +/// would have the daemon happily read the encrypted key blob. Even +/// though hex decoding will fail, the previous error message included +/// the byte offset of the first non-hex character — a small but real +/// information leak on the file's binary structure. This guard + +/// content-stripped error messages close both surfaces. +/// +/// `safe_reads_root` and `user_path` are both canonicalized (resolves +/// symlinks, requires the file to exist). The user path must +/// `starts_with` the root; equality is allowed but rare in practice. +/// +/// Adversarial-review fix (2026-05-12): also reject regular files +/// whose link count > 1. `canonicalize` resolves symlinks but NOT +/// hardlinks — a hardlink IS the file (same inode, multiple directory +/// entries). Without this check, an attacker with daemon-uid write +/// access could plant a hardlink inside the sandbox pointing at the +/// inode of `$ALDABRA_DATA/root-xprv.age` and exfiltrate key bytes +/// through the read path. No legitimate use case puts hardlinks in +/// a script-CBOR sandbox; refusing them outright is conservative +/// and easy. +fn assert_inside_sandbox(safe_reads_root: &Path, user_path: &str) -> Result { + let user_canon = std::fs::canonicalize(user_path) + .map_err(|e| format!("resolve path: {e} (path must exist + be readable)"))?; + let root_canon = std::fs::canonicalize(safe_reads_root).map_err(|e| { + format!( + "sandbox root '{}' is not accessible — restart the daemon to recreate it: {e}", + safe_reads_root.display() + ) + })?; + if !user_canon.starts_with(&root_canon) { + return Err(format!( + "path is outside the sandbox root '{}' — only files under that directory may be read \ + via *_path args (set ALDABRA_SAFE_READS_ROOT to widen)", + root_canon.display() + )); + } + // Hardlink rejection. `metadata` follows the post-canonicalize path + // directly to the file (no symlink traversal needed since canonicalize + // already resolved them). nlink > 1 means another directory entry + // points at the same inode; we can't verify the other entry is also + // inside the sandbox without an exhaustive filesystem scan, so we + // reject conservatively. + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let meta = + std::fs::metadata(&user_canon).map_err(|e| format!("stat path inside sandbox: {e}"))?; + if meta.is_file() && meta.nlink() > 1 { + return Err( + "path is a hardlinked file — refusing to read because additional directory \ + entries to the same inode may live outside the sandbox" + .into(), + ); + } + } + Ok(user_canon) +} + /// Resolve a reference-script bytestring from EITHER an inline hex /// argument OR a file path inside the container. Caller passes both /// raw options; this fn enforces the "at most one" rule and reads @@ -88,7 +149,16 @@ use aldabra_dao::reader::{DaoReader, KoiosDaoReader}; /// rearrangement somewhere between client and stdio reader. Reading /// from a file inside the container bypasses the JSON-RPC arg path /// entirely. +/// +/// CRIT-2 audit fix (2026-05-12): the path arg is sandboxed via +/// [`assert_inside_sandbox`] against the daemon's configured +/// `safe_reads_root` (default `$ALDABRA_DATA/scripts/`). Decode-error +/// messages no longer include the file path or byte-offset context — +/// only "could not decode as hex" — to avoid leaking structural +/// information about non-hex files (e.g. encrypted key blobs that an +/// attacker might point the tool at). fn resolve_ref_script_bytes( + safe_reads_root: &Path, cbor_hex: Option<&str>, path: Option<&str>, ) -> Result>, String> { @@ -103,17 +173,20 @@ fn resolve_ref_script_bytes( })?)) } (None, Some(p)) => { - let raw = std::fs::read_to_string(p) - .map_err(|e| format!("read reference_script_path '{p}': {e}"))?; + let canon = assert_inside_sandbox(safe_reads_root, p)?; + let raw = std::fs::read_to_string(&canon) + .map_err(|e| format!("read reference_script_path: {e}"))?; let cleaned: String = raw.chars().filter(|c| !c.is_whitespace()).collect(); if cleaned.is_empty() { - return Err(format!( - "reference_script_path '{p}' contained no hex characters" - )); + return Err("reference_script_path contained no hex characters".into()); } - Ok(Some(hex_decode(&cleaned).map_err(|e| { - format!("decode reference_script_path '{p}' contents: {e}") - })?)) + // CRIT-2: do NOT include the hex_decode error's position- + // offset detail or the file's contents in the user-visible + // error. A failed decode is a yes/no signal; anything more + // narrows the attacker's guesses about file structure. + hex_decode(&cleaned) + .map(Some) + .map_err(|_| "reference_script_path contents are not valid hex".to_string()) } (None, None) => Ok(None), } @@ -130,7 +203,10 @@ fn resolve_ref_script_bytes( /// surfacing as "odd length" hex decode errors and blocking debug- /// build minting policies. Reading from a file inside the container /// bypasses the JSON-RPC arg path entirely. +/// +/// CRIT-2 audit fix (2026-05-12): see [`resolve_ref_script_bytes`]. fn resolve_policy_cbor_bytes( + safe_reads_root: &Path, cbor_hex: Option<&str>, path: Option<&str>, ) -> Result, String> { @@ -141,15 +217,15 @@ fn resolve_policy_cbor_bytes( hex_decode(&cleaned).map_err(|e| format!("decode policy_cbor_hex: {e}")) } (None, Some(p)) => { - let raw = std::fs::read_to_string(p) - .map_err(|e| format!("read policy_cbor_path '{p}': {e}"))?; + let canon = assert_inside_sandbox(safe_reads_root, p)?; + let raw = std::fs::read_to_string(&canon) + .map_err(|e| format!("read policy_cbor_path: {e}"))?; let cleaned: String = raw.chars().filter(|c| !c.is_whitespace()).collect(); if cleaned.is_empty() { - return Err(format!( - "policy_cbor_path '{p}' contained no hex characters" - )); + return Err("policy_cbor_path contained no hex characters".into()); } - hex_decode(&cleaned).map_err(|e| format!("decode policy_cbor_path '{p}' contents: {e}")) + hex_decode(&cleaned) + .map_err(|_| "policy_cbor_path contents are not valid hex".to_string()) } (None, None) => Err("must set exactly one of policy_cbor_hex / policy_cbor_path".into()), } @@ -249,9 +325,15 @@ struct WalletInner { /// quotas. None = public tier. Sourced from env only — never from /// disk; never logged. koios_bearer: Option, + /// CRIT-2 audit fix (2026-05-12): sandbox root for `*_path` tool + /// args. Files outside this directory (canonical) are refused to + /// keep prompt-injection from steering reads at the encrypted + /// key material under `$ALDABRA_DATA/`. + safe_reads_root: PathBuf, } impl WalletService { + #[allow(clippy::too_many_arguments)] pub fn new( network: Network, address: String, @@ -261,6 +343,7 @@ impl WalletService { stake_key: StakeKey, max_send_lovelace: u64, data_dir: PathBuf, + safe_reads_root: PathBuf, ) -> Self { let bearer_ref = koios_bearer.as_deref(); Self { @@ -279,6 +362,7 @@ impl WalletService { dao_reader: KoiosDaoReader::with_bearer(koios_base.clone(), bearer_ref), koios_base, koios_bearer, + safe_reads_root, }), } } @@ -315,6 +399,59 @@ impl WalletService { } Ok(()) } + + /// CRIT-1 audit fix (2026-05-12): scan a Conway-era tx CBOR and + /// enforce the wallet's `max_send_lovelace` cap against the sum + /// of lovelace going to any address that isn't this wallet's own. + /// This is the chokepoint that defends `wallet_sign_partial` and + /// `wallet_submit_signed_tx` against an unsigned-tx → + /// sign-partial → submit drain via prompt injection. Each of + /// those tools individually had `lovelace > 0` validation but no + /// blast-radius limit. + /// + /// "Non-self" is computed by comparing the wallet's bech32 address + /// (decoded to raw bytes via pallas-addresses, then hexed) against + /// each output's `address_hex` in the tx body. Outputs whose + /// address matches are treated as change/self-send and excluded. + /// This is intentionally conservative on the same wallet's other + /// accounts/indexes — they'll be flagged as non-self until/unless + /// the wallet learns additional addresses to consider its own. + /// Force `=true` overrides; mirror the `enforce_value_cap` contract. + fn enforce_cap_on_cbor(&self, cbor_bytes: &[u8], force: bool) -> Result<(), String> { + let total = sum_non_self_lovelace(cbor_bytes, &self.inner.address)?; + self.enforce_value_cap(total, force) + } +} + +/// Decode a Conway-era tx CBOR and sum lovelace across every output +/// whose address differs from the supplied wallet bech32. Free +/// function so the cap-check logic is unit-testable without standing +/// up a full WalletService. +/// +/// Adversarial-review fix (2026-05-12): `try_fold` + `checked_add` +/// instead of `.sum::()`. The naive `.sum()` wraps on overflow +/// in release builds — a prompt-injection could craft a CBOR with +/// outputs summing past `u64::MAX`, wrap to a small value, and +/// silently pass the cap check. Real-world unreachable (Cardano max +/// supply is ~45B ADA = 4.5e16 lovelace, ~400x below `u64::MAX`) but +/// trivial to harden. +fn sum_non_self_lovelace(cbor_bytes: &[u8], wallet_bech32: &str) -> Result { + use pallas_addresses::Address; + let wallet_addr = + Address::from_bech32(wallet_bech32).map_err(|e| format!("decode wallet address: {e}"))?; + let wallet_bytes = wallet_addr.to_vec(); + let mut wallet_hex = String::with_capacity(wallet_bytes.len() * 2); + for b in &wallet_bytes { + wallet_hex.push_str(&format!("{:02x}", b)); + } + let summary = aldabra_core::summarize_tx(cbor_bytes) + .map_err(|e| format!("decode tx for cap check: {e}"))?; + summary + .outputs + .iter() + .filter(|o| o.address_hex != wallet_hex) + .try_fold(0u64, |acc, o| acc.checked_add(o.lovelace)) + .ok_or_else(|| "non-self lovelace sum overflows u64 — refusing to sign/submit".into()) } #[derive(Debug, Deserialize, schemars::JsonSchema)] @@ -450,6 +587,12 @@ pub struct UnsignedSendArgs { /// [`SendArgs::reference_script_kind`]. #[serde(default)] pub reference_script_kind: Option, + /// CRIT-1 audit fix (2026-05-12): bypass the configured + /// `max_send_lovelace` hard cap. Required to build an unsigned + /// tx that would otherwise drain the wallet past the cap once + /// signed + submitted. Mirrors `wallet_send.force`. + #[serde(default)] + pub force: bool, } #[derive(Debug, Deserialize, schemars::JsonSchema)] @@ -458,6 +601,14 @@ pub struct SubmitSignedArgs { /// cold-signer that consumed the unsigned CBOR returned by /// `wallet_send_unsigned`. pub signed_cbor_hex: String, + /// CRIT-1 audit fix (2026-05-12): bypass the configured + /// `max_send_lovelace` hard cap. The submit path scans the tx + /// CBOR and sums lovelace going to any non-self address; pass + /// `force=true` to push a tx that exceeds that cap (cold-signed + /// multi-sig flows where a treasury moves real value + /// intentionally). + #[serde(default)] + pub force: bool, } #[derive(Debug, Deserialize, schemars::JsonSchema)] @@ -528,6 +679,12 @@ pub struct PlutusMintUnsignedArgs { pub ex_units_mem: Option, #[serde(default)] pub ex_units_steps: Option, + /// CRIT-1 audit fix (2026-05-12): bypass the configured + /// `max_send_lovelace` hard cap on `dest_lovelace`. Required for + /// large script-bootstrap mints (governor / stakes / proposal + /// outputs that carry sizeable min-utxo + datum overhead). + #[serde(default)] + pub force: bool, } #[derive(Debug, Deserialize, schemars::JsonSchema)] @@ -564,6 +721,11 @@ pub struct MintUnsignedArgs { /// optional metadata for downstream signers. #[serde(default)] pub disclosed_signer_pkh_hex: Option, + /// CRIT-1 audit fix (2026-05-12): bypass the configured + /// `max_send_lovelace` hard cap on `dest_lovelace`. Mirrors + /// `wallet_mint.force`. + #[serde(default)] + pub force: bool, } #[derive(Debug, Deserialize, schemars::JsonSchema)] @@ -683,6 +845,14 @@ pub struct SignPartialArgs { /// partially signed by another party. The wallet's payment key /// gets appended as an additional VKeyWitness. pub cbor_hex: String, + /// CRIT-1 audit fix (2026-05-12): bypass the configured + /// `max_send_lovelace` hard cap. Before signing, the tool sums + /// lovelace across every non-self output in the CBOR (anything + /// not addressed to this wallet) and rejects if the total + /// exceeds the cap. Pass `force=true` for cold-signed multi-sig + /// flows where a treasury moves real value intentionally. + #[serde(default)] + pub force: bool, } #[derive(Debug, Deserialize, schemars::JsonSchema)] @@ -890,6 +1060,7 @@ impl WalletService { None => None, }; let ref_script_bytes = resolve_ref_script_bytes( + &self.inner.safe_reads_root, reference_script_cbor_hex.as_deref(), reference_script_path.as_deref(), )?; @@ -965,11 +1136,18 @@ impl WalletService { reference_script_cbor_hex, reference_script_path, reference_script_kind, + force, }: UnsignedSendArgs, ) -> Result { if lovelace == 0 { return Err("lovelace must be > 0".into()); } + // CRIT-1 audit fix (2026-05-12): apply the same cap as wallet_send + // so the unsigned-build chain isn't a quiet bypass of the daemon's + // value-safety policy. Defense-in-depth — the sign / submit + // tools also re-check via enforce_cap_on_cbor. + self.enforce_value_cap(lovelace, force)?; + let utxos = self .inner .chain @@ -998,6 +1176,7 @@ impl WalletService { None => None, }; let ref_script_bytes = resolve_ref_script_bytes( + &self.inner.safe_reads_root, reference_script_cbor_hex.as_deref(), reference_script_path.as_deref(), )?; @@ -1040,9 +1219,18 @@ impl WalletService { )] async fn wallet_submit_signed_tx( &self, - #[tool(aggr)] SubmitSignedArgs { signed_cbor_hex }: SubmitSignedArgs, + #[tool(aggr)] SubmitSignedArgs { + signed_cbor_hex, + force, + }: SubmitSignedArgs, ) -> Result { let bytes = hex_decode(&signed_cbor_hex).map_err(|e| format!("decode: {e}"))?; + // CRIT-1 audit fix (2026-05-12): scan the tx for outputs going + // to non-self addresses and enforce the cap on the sum. Closes + // the prompt-injection drain path where an attacker convinces + // the LLM to hand-craft + sign + submit a tx that bypasses the + // cap on the individual wallet_send tool. + self.enforce_cap_on_cbor(&bytes, force)?; self.inner .chain .submit_tx(&bytes) @@ -1640,6 +1828,7 @@ impl WalletService { policy, metadata, disclosed_signer_pkh_hex, + force, }: MintUnsignedArgs, ) -> Result { if quantity == 0 { @@ -1650,6 +1839,9 @@ impl WalletService { "dest_lovelace {dest_lovelace} below 1 ADA min for asset-bearing UTXO" )); } + // CRIT-1 audit fix (2026-05-12): cap the unsigned-mint output's + // dest_lovelace, mirroring wallet_mint. + self.enforce_value_cap(dest_lovelace, force)?; // Resolve PolicySpec — caller-supplied JSON or wallet default. let policy_spec: PolicySpec = match policy { @@ -1725,6 +1917,7 @@ impl WalletService { required_input_refs, ex_units_mem, ex_units_steps, + force, }: PlutusMintUnsignedArgs, ) -> Result { if mint_assets.is_empty() { @@ -1735,9 +1928,15 @@ impl WalletService { "dest_lovelace {dest_lovelace} below 1 ADA min for asset-bearing UTXO" )); } + // CRIT-1 audit fix (2026-05-12): cap the Plutus-mint + // dest_lovelace, mirroring wallet_mint. + self.enforce_value_cap(dest_lovelace, force)?; - let policy_cbor = - resolve_policy_cbor_bytes(policy_cbor_hex.as_deref(), policy_cbor_path.as_deref())?; + let policy_cbor = resolve_policy_cbor_bytes( + &self.inner.safe_reads_root, + policy_cbor_hex.as_deref(), + policy_cbor_path.as_deref(), + )?; let redeemer_cbor = hex_decode(&redeemer_cbor_hex).map_err(|e| format!("decode redeemer: {e}"))?; let policy_ver = match policy_version.trim().to_ascii_lowercase().as_str() { @@ -1871,9 +2070,15 @@ impl WalletService { )] async fn wallet_sign_partial( &self, - #[tool(aggr)] SignPartialArgs { cbor_hex }: SignPartialArgs, + #[tool(aggr)] SignPartialArgs { cbor_hex, force }: SignPartialArgs, ) -> Result { let bytes = hex_decode(&cbor_hex).map_err(|e| format!("decode: {e}"))?; + // CRIT-1 audit fix (2026-05-12): scan the unsigned/partial tx + // for outputs going to non-self addresses and enforce the cap + // on the sum BEFORE appending a witness. Closes the prompt- + // injection drain path through the unsigned-build → sign → + // submit chain (each step individually had no cap-awareness). + self.enforce_cap_on_cbor(&bytes, force)?; let updated = add_witness(&self.inner.payment_key, &bytes).map_err(|e| format!("sign: {e}"))?; let mut hex = String::with_capacity(updated.len() * 2); @@ -3676,6 +3881,7 @@ impl WalletService { }: EscrowDepositUnsignedArgs, ) -> Result { let validator_cbor = resolve_validator_required( + &self.inner.safe_reads_root, validator_script_cbor_hex.as_deref(), validator_script_path.as_deref(), )?; @@ -3741,6 +3947,7 @@ impl WalletService { }: EscrowAgreeUnsignedArgs, ) -> Result { let validator_cbor = resolve_validator_required( + &self.inner.safe_reads_root, validator_script_cbor_hex.as_deref(), validator_script_path.as_deref(), )?; @@ -3825,6 +4032,7 @@ impl WalletService { }: EscrowVetoUnsignedArgs, ) -> Result { let validator_cbor = resolve_validator_required( + &self.inner.safe_reads_root, validator_script_cbor_hex.as_deref(), validator_script_path.as_deref(), )?; @@ -3886,6 +4094,7 @@ impl WalletService { }: EscrowSettleUnsignedArgs, ) -> Result { let validator_cbor = resolve_validator_required( + &self.inner.safe_reads_root, validator_script_cbor_hex.as_deref(), validator_script_path.as_deref(), )?; @@ -3960,6 +4169,7 @@ impl WalletService { }: EscrowRefundTimeoutUnsignedArgs, ) -> Result { let validator_cbor = resolve_validator_required( + &self.inner.safe_reads_root, validator_script_cbor_hex.as_deref(), validator_script_path.as_deref(), )?; @@ -4055,10 +4265,11 @@ fn decode_pkh28(s: &str, field: &str) -> Result<[u8; 28], String> { /// Required (not optional) — every escrow spend tool needs the script. /// Wraps `resolve_ref_script_bytes` adding the "must be set" gate. fn resolve_validator_required( + safe_reads_root: &Path, cbor_hex: Option<&str>, path: Option<&str>, ) -> Result, String> { - let resolved = resolve_ref_script_bytes(cbor_hex, path)?; + let resolved = resolve_ref_script_bytes(safe_reads_root, cbor_hex, path)?; resolved.ok_or_else(|| { "validator_script_cbor_hex or validator_script_path must be set — escrow spend \ requires the V3 validator UPLC bytes (extract from aiken-escrow/plutus.json's \ @@ -4612,3 +4823,281 @@ mod server_info_tests { ); } } + +#[cfg(test)] +mod sandbox_tests { + use super::*; + use std::fs; + use std::io::Write; + + /// CRIT-2 audit fix (2026-05-12): files inside the sandbox root resolve + /// cleanly via canonicalization. + #[test] + fn path_inside_sandbox_is_accepted() { + let root = tempfile::tempdir().expect("tempdir"); + let target = root.path().join("policy.cbor"); + fs::write(&target, "deadbeef").expect("write fixture"); + let resolved = + assert_inside_sandbox(root.path(), target.to_str().unwrap()).expect("inside ok"); + assert!(resolved.starts_with(root.path().canonicalize().unwrap())); + } + + /// Asking the tool to read a file OUTSIDE the sandbox must error — + /// that's the entire prompt-injection containment. Without this, + /// `policy_cbor_path: "/var/lib/aldabra/root-xprv.age"` would have + /// silently exfiltrated key material via decode-error replies. + #[test] + fn path_outside_sandbox_is_rejected() { + let root = tempfile::tempdir().expect("tempdir-root"); + let other = tempfile::tempdir().expect("tempdir-other"); + let target = other.path().join("naughty.cbor"); + fs::write(&target, "deadbeef").expect("write fixture"); + let err = assert_inside_sandbox(root.path(), target.to_str().unwrap()) + .expect_err("must reject outside-root path"); + assert!( + err.contains("outside the sandbox root"), + "expected sandbox error, got: {err}" + ); + } + + /// Traversal via `..` must NOT escape the sandbox even when the + /// destination file does exist on disk. canonicalize() resolves + /// the up-traversal so the comparison still catches it. + #[test] + fn dotdot_traversal_is_rejected() { + let parent = tempfile::tempdir().expect("tempdir-parent"); + let sandbox = parent.path().join("sandbox"); + fs::create_dir(&sandbox).expect("mkdir sandbox"); + let sibling = parent.path().join("secret.age"); + fs::write(&sibling, "AAAA").expect("write fixture"); + // Path the LLM would try: `/../secret.age` + let trav = sandbox.join("..").join("secret.age"); + let err = assert_inside_sandbox(&sandbox, trav.to_str().unwrap()) + .expect_err("must reject ..-traversal"); + assert!( + err.contains("outside the sandbox root"), + "expected sandbox error, got: {err}" + ); + } + + /// Adversarial-review fix: hardlinked files inside the sandbox + /// must be refused. `canonicalize` resolves symlinks but NOT + /// hardlinks — a hardlink IS the file (same inode, different + /// directory entry). Without this, an attacker with daemon-uid + /// write access could `link()` the encrypted key blob's inode + /// into the sandbox and exfiltrate the bytes. + #[cfg(unix)] + #[test] + fn hardlinked_file_inside_sandbox_is_rejected() { + let root = tempfile::tempdir().expect("tempdir"); + // Create a "secret" outside the sandbox. + let secret = root.path().join("secret.bin"); + fs::write(&secret, "00aaaa").expect("write secret"); + // Carve a sandbox under the same tempdir (same filesystem + // so hardlinks across the boundary are allowed by the OS). + let sandbox = root.path().join("sandbox"); + fs::create_dir(&sandbox).expect("mkdir sandbox"); + let planted = sandbox.join("planted.hex"); + // The attack: hardlink the secret's inode into the sandbox. + fs::hard_link(&secret, &planted).expect("hardlink"); + // Sanity check: both names point at the same inode. + let m1 = fs::metadata(&secret).unwrap(); + let m2 = fs::metadata(&planted).unwrap(); + { + use std::os::unix::fs::MetadataExt; + assert_eq!(m1.ino(), m2.ino(), "hardlink should share inode"); + assert!(m1.nlink() >= 2, "nlink should be ≥ 2 after hardlinking"); + } + let err = assert_inside_sandbox(&sandbox, planted.to_str().unwrap()) + .expect_err("must reject hardlink"); + assert!( + err.contains("hardlinked file"), + "expected hardlink rejection, got: {err}" + ); + } + + /// The path arg references a file that doesn't exist; we want a + /// clear error that doesn't leak filesystem layout. + #[test] + fn nonexistent_path_errors_cleanly() { + let root = tempfile::tempdir().expect("tempdir"); + let bogus = root.path().join("does-not-exist"); + let err = assert_inside_sandbox(root.path(), bogus.to_str().unwrap()) + .expect_err("missing file must error"); + // Generic "resolve path" prefix; no byte-content leak. + assert!(err.starts_with("resolve path"), "got: {err}"); + } + + /// `resolve_policy_cbor_bytes` rejects non-hex contents WITHOUT + /// leaking the byte position. Previous wording included the + /// hex_decode error verbatim ("invalid hex char at 12"), which is a + /// small but real information leak about file structure. Now + /// returns a constant message. + #[test] + fn nonhex_file_yields_constant_message() { + let root = tempfile::tempdir().expect("tempdir"); + // Even length, all non-hex bytes — exercises the "invalid hex + // char" branch of hex_decode without tripping the length + // check or read_to_string's UTF-8 validation. + let mut f = fs::File::create(root.path().join("bogus.txt")).expect("create bogus"); + f.write_all(b"ghijghij").expect("write text"); + let err = resolve_policy_cbor_bytes( + root.path(), + None, + Some(root.path().join("bogus.txt").to_str().unwrap()), + ) + .expect_err("invalid hex must error"); + assert_eq!(err, "policy_cbor_path contents are not valid hex"); + } +} + +#[cfg(test)] +mod cap_tests { + use super::*; + use aldabra_core::{ + build_unsigned_payment_extras, derive_base_address, derive_payment_key, derive_stake_key, + hex_decode, Mnemonic, Network, + }; + + const ABANDON_ART: &str = concat!( + "abandon abandon abandon abandon abandon abandon ", + "abandon abandon abandon abandon abandon abandon ", + "abandon abandon abandon abandon abandon abandon ", + "abandon abandon abandon abandon abandon art", + ); + + fn build_cbor_to(to_address: &str, lovelace: u64) -> (Vec, String) { + let root = Mnemonic::from_phrase(ABANDON_ART) + .unwrap() + .into_root_key() + .unwrap(); + let _payment = derive_payment_key(&root, 0, 0); + let _stake = derive_stake_key(&root, 0); + let wallet_addr = derive_base_address(&root, Network::Preprod, 0, 0).unwrap(); + let inputs = vec![aldabra_core::InputUtxo { + tx_hash_hex: "deadbeef".repeat(8), + output_index: 0, + lovelace: 1_000_000_000, + assets: Default::default(), + }]; + let built = build_unsigned_payment_extras( + Network::Preprod, + &inputs, + &wallet_addr, + to_address, + lovelace, + &[], + None, + None, + &aldabra_core::ProtocolParams::default(), + ) + .expect("build unsigned"); + let cbor = hex_decode(&built.cbor_hex).expect("hex decode"); + (cbor, wallet_addr) + } + + /// CRIT-1: a tx whose non-self output sits below the cap returns 0 + /// total when the destination IS self (change-only); above cap the + /// sum reflects the actual outbound lovelace. + #[test] + fn sum_non_self_excludes_self_outputs() { + // Send to the wallet's OWN address — every output is "self"; + // total should be 0 (any "send" is a self-move). + let root = Mnemonic::from_phrase(ABANDON_ART) + .unwrap() + .into_root_key() + .unwrap(); + let wallet_addr = derive_base_address(&root, Network::Preprod, 0, 0).unwrap(); + let (cbor, _) = build_cbor_to(&wallet_addr, 5_000_000); + let total = sum_non_self_lovelace(&cbor, &wallet_addr).expect("decode"); + assert_eq!( + total, 0, + "self-send should contribute 0 to non-self total, got {total}" + ); + } + + /// CRIT-1: a tx sending lovelace to an UNRELATED address has its + /// destination output reflected in the non-self sum, so the + /// enforce_value_cap chokepoint can refuse to sign/submit it. + #[test] + fn sum_non_self_counts_outbound_outputs() { + // Send to a different derivation (account=0, index=1) of the + // same wallet — this is intentionally treated as non-self + // because the wallet only learns its own primary address. + let root = Mnemonic::from_phrase(ABANDON_ART) + .unwrap() + .into_root_key() + .unwrap(); + let other = derive_base_address(&root, Network::Preprod, 0, 1).unwrap(); + let wallet_addr = derive_base_address(&root, Network::Preprod, 0, 0).unwrap(); + let (cbor, _) = build_cbor_to(&other, 50_000_000); + let total = sum_non_self_lovelace(&cbor, &wallet_addr).expect("decode"); + assert!( + total >= 50_000_000, + "outbound send should count toward total, got {total}" + ); + } + + /// Adversarial-review fix: `.sum::()` wraps on overflow in + /// release builds. The cap check must NEVER silently roll over a + /// u64 — a prompt-injection could craft CBOR with outputs summing + /// past u64::MAX, wrap to a small value, pass the check. Now uses + /// `checked_add` and returns Err on overflow. We test this via the + /// free function with hand-constructed `OutputSummary` mock data + /// rather than building a real CBOR (Cardano min-utxo + max-supply + /// constraints make a real overflow tx impossible to build). + /// + /// This test exercises `OutputSummary` from `aldabra-core::inspect` + /// to confirm the type's `lovelace: u64` field is what we expect, + /// and that the cap check refuses to silently roll over. A future + /// refactor could expose a `compute_total` helper taking a slice + /// of OutputSummary so we can test overflow in isolation; for now + /// the test relies on documenting the invariant via a regression + /// case in the free function below. + #[test] + fn sum_non_self_handles_overflow_safely() { + // Build two synthetic OutputSummary mocks via the public type. + // If their lovelace sum exceeds u64::MAX, the helper must + // return an error rather than wrap. + use aldabra_core::OutputSummary; + let synthetic = [ + OutputSummary { + address_hex: "attacker_1".to_string(), + lovelace: u64::MAX - 5, + assets: vec![], + has_inline_datum: false, + has_reference_script: false, + }, + OutputSummary { + address_hex: "attacker_2".to_string(), + lovelace: 100, + assets: vec![], + has_inline_datum: false, + has_reference_script: false, + }, + ]; + // Replicate the production filter+fold logic to confirm the + // overflow path returns None / Err and never wraps. The + // production fn applies the same fold against `summary.outputs`. + let result: Option = synthetic + .iter() + .try_fold(0u64, |acc, o| acc.checked_add(o.lovelace)); + assert!( + result.is_none(), + "checked_add must refuse to roll over u64::MAX, got: {result:?}" + ); + } + + /// Garbage CBOR yields a clean error rather than a panic — the + /// sign/submit chokepoint must handle malformed input gracefully. + #[test] + fn sum_non_self_rejects_garbage() { + let root = Mnemonic::from_phrase(ABANDON_ART) + .unwrap() + .into_root_key() + .unwrap(); + let wallet_addr = derive_base_address(&root, Network::Preprod, 0, 0).unwrap(); + let err = sum_non_self_lovelace(b"not cbor at all", &wallet_addr).expect_err("garbage"); + assert!(err.contains("decode tx"), "got: {err}"); + } +}