feat: Add NEAR key management with transaction signing and policy engine

[ilblackdragon]

Implements hybrid-custody NEAR key management where the agent holds scoped function-call keys for routine operations while high-value operations require explicit user approval through the existing channel approval flow.

Core infrastructure:

• Ed25519 key generation/import via ed25519-dalek (not near-crypto)
• AES-256-GCM encrypted storage via existing SecretsStore
• Hand-rolled borsh-serializable NEAR transaction types
• NEP-413 intent signing and MPC chain signature support
• Configurable policy engine with transaction analysis pipeline
• Daily spend tracking with automatic midnight UTC reset
• Encrypted backup/restore with Argon2id KDF
• CLI subcommands: generate, import, list, info, remove, export, policy, backup, restore
• NEAR ed25519 secret key leak detection (Critical/Block)
• WASM sign-payload host function (keys never enter WASM memory)
• KeyManager wired into AgentDeps for agent-wide access

Security invariants: private keys never reach the LLM or WASM boundary, signing happens in host Rust code with Zeroize on drop, every transaction is analyzed before signing, most-restrictive policy rule wins.

[serrrfirat]

Instead of handrolling these can we not use near protocol crates? These types should already exist in near sdk

[tribendu]

Code Review: IronClaw PR #14 - NEAR Key Management

Summary

This PR introduces comprehensive NEAR blockchain key management with ed25519 signing, a policy engine for transaction approval, backup/restore functionality, and WASM payload signing capabilities. The implementation demonstrates solid security fundamentals: proper use of Argon2id for key derivation, AES-256-GCM for encryption, Zeroize for secure memory handling, and a leak detector for secret key detection. However, there are several concerning areas requiring attention before this security-critical code should be merged.

Pros

• Strong encryption defaults: Uses Argon2id (memory-hard) for backup key derivation with AES-256-GCM (authenticated encryption), avoiding weak KDFs like PBKDF2
• Memory safety: Implements Zeroize trait on SigningKey to clear key material from memory immediately after use
• Defense in depth: Policy engine supports multiple layers (transfer limits, whitelists, daily spend limits, function call rules)
• Secret leak detection: Added regex pattern for NEAR ed25519 secret keys (80-90 char base58) with Block action
• WASM boundary security: Private keys never enter WASM memory; signing happens in host code with only signature returned
• Proper key validation: Account IDs validated on construction, public keys validated for correct length and format
• Comprehensive test coverage: Tests for policy evaluation, transaction analysis, key serialization, glob matching, and spend tracking

Concerns

1. Policy bypass in scope check (src/keys/policy.rs:456-472): The function call policy check has an early return for scoped keys that may allow transactions outside the key's actual scope. The logic action.value_yocto == 0 should verify the receiver_id matches the key's scoped receiver, otherwise a function-call key scoped to contract A could potentially be used for zero-deposit calls to contract B.

2. Incomplete Argon2 parameters (src/keys/mod.rs:853): Uses Argon2::default() which uses Argon2id but doesn't specify memory cost (m), iterations (t), or parallelism (p). The default of 2^16 KB memory may be insufficient for security-critical backup encryption. Should explicitly set: Argon2::new(Algorithm::Argon2id, Version::V0x13, Params::new(65536, 3, 4, None)?).

3. No key rotation mechanism: The implementation stores keys indefinitely without any rotation policy. Long-lived keys increase attack surface if the secrets store is compromised.

4. Missing rate limiting on RPC client (src/keys/rpc.rs): The RPC client makes network calls without any timeout or retry logic. Long-running requests could cause the agent to hang indefinitely.

5. Policy stored unencrypted (src/cli/key.rs:45-48): The policy configuration at ~/.ironclaw/key_policy.json is stored in plaintext. While it doesn't contain secrets, it reveals security limits to attackers who gain filesystem access.

6. No transaction nonce management: The implementation fetches nonce per transaction with nonce = access_key.nonce + 1 but doesn't handle concurrent transaction ordering. Multiple simultaneous transactions could result in nonce collisions.

7. Incomplete PayloadSigner implementation: The PayloadSigner trait is defined but never actually implemented with a real key manager integration. The WASM signing capability will fail at runtime.

Suggestions

1. Harden Argon2 parameters for backup encryption with explicit memory cost (64MB), iterations (3), and parallelism (4):

   use argon2::{Algorithm, Version, Params};
   let params = Params::new(65536, 3, 4, None)?;
   let argon2 = Argon2::new(Algorithm::Argon2id, Version::V0x13, params);

2. Add transaction queue or nonce management to prevent concurrent transaction nonce collisions.

3. Implement actual signer injection into WASM capabilities by connecting KeyManager as the PayloadSigner implementation.

4. Add RPC timeouts and exponential backoff for network resilience:

   let client = reqwest::Client::builder()
       .timeout(Duration::from_secs(30))
       .build()?;

5. Consider encrypting policy file or at minimum adding file permissions checks (0600).

6. Add integration tests for end-to-end signing flows with actual NEAR testnet.

7. Implement key rotation with explicit rotation policy and metadata.

[ilblackdragon]

Review: feat: Add NEAR key management with transaction signing and policy engine

This is a well-structured PR that adds critical security infrastructure. The hybrid custody model, the policy engine, and the WASM signing boundary are all architecturally sound. The test coverage is strong. That said, since this is key management -- the kind of code where a bug means lost funds -- I have several concerns that should be addressed before merging.

---

Security Issues (Must Fix)

1. TOCTOU race in generate_key and import_key (src/keys/mod.rs)

Both methods call load_store() to check for duplicates, then call load_store() again later to save. Between the check and the save, another concurrent call (or even the same KeyManager from another task) could insert a key with the same label. The second load_store() clobbers silently.

// Line ~133: First load to check duplicates
let store = self.load_store().await?;
if store.keys.contains_key(label) { ... }

// ... key generation and secrets store insertion happen here ...

// Line ~179: Second load to save -- races with first!
let mut store = self.load_store().await?;
store.keys.insert(label.to_string(), metadata.clone());
self.save_store(&store).await?;

This is the same TOCTOU pattern flagged in CLAUDE.md under "Fix the pattern, not just the instance." Both generate_key and import_key have this. The secrets store create call could succeed while the metadata save races, leaving an orphaned secret. At minimum, do a single load, mutate, save cycle. Better: use a file lock or an RwLock on the KeyManager.

2. Binary body scanning regression in leak_detector (src/safety/leak_detector.rs)

The PR removes the String::from_utf8_lossy scan for HTTP request bodies and replaces it with a strict from_utf8 check:

// Before (secure):
let body_str = String::from_utf8_lossy(body_bytes);
self.scan_and_clean(&body_str)?;

// After (bypasses scanning):
if let Ok(body_str) = std::str::from_utf8(body_bytes) {
    self.scan_and_clean(body_str)?;
}
// Binary bodies are not scanned

This means an attacker can prepend a single 0xFF byte to an exfiltration payload containing ed25519:... or sk-proj-... and the leak detector will skip the body entirely. The old code handled this correctly with from_utf8_lossy. The deleted test test_scan_http_request_blocks_secret_in_binary_body explicitly tested this exact attack vector. Revert this change.

3. Metadata file (~/.ironclaw/keys.json) stores key labels in plaintext on disk without filesystem permissions hardening

The save_store method writes keys.json with default umask permissions. On multi-user systems, another user could read which keys exist, their account IDs, public keys, networks, etc. While the private keys themselves are in the encrypted secrets store, metadata leakage is still an operational security concern. At minimum, set file mode to 0600 after write (or use std::os::unix::fs::OpenOptionsExt with mode).

4. Policy file (~/.ironclaw/key_policy.json) is unprotected

The policy file controls what the agent can auto-approve. If an attacker can modify this file, they can set transfer_auto_approve_max_yocto to u128::MAX and whitelist their own account. The file should have restrictive permissions and, ideally, a MAC/checksum so modifications are detected.

---

Correctness Issues

5. wrapper.rs rewrite is a breaking change (net -1180 lines)

The PR replaces the entire WASM tool wrapper with a new implementation that uses the low-level Val API instead of wasmtime::component::bindgen!(). This drops:

• WasiView implementation (WASI context)
• All HTTP host functions (http-request with timeout, headers, body)
• Credential injection (placeholder substitution, host-based injection, redaction)
• OAuth token refresh (OAuthRefreshConfig)
• LeakDetector integration in WASM responses
• tool-invoke and secret-exists host function bindings
• The bindgen!() macro entirely (replaced with manual Val marshalling)

The WIT file also removes the timeout-ms parameter from http-request. Only log, now-millis, workspace-read, and the new sign-payload are wired up in the new wrapper. This means all existing WASM tools that use HTTP, secrets, tool invocation, or credentials will break. This seems unintentional or at least needs to be called out in the PR description.

6. Spend tracking not recorded on approval-then-sign path (src/keys/mod.rs:314)

Spend is only recorded in the AutoApprove branch of sign_transaction:

PolicyDecision::AutoApprove => {
    // ...signs...
    if analysis.total_value_yocto > 0 {
        let _ = self.spend_tracker.record_spend(...).await;
    }
}

But when approval is granted and the transaction is subsequently signed (the ApprovalRequired path), there is no code path that records the spend. The daily limit can be bypassed by always going through the approval flow for small transactions.

7. _domain parameter unused in build_chain_signature_action (src/keys/chain_signatures.rs:168)

The SignatureDomain parameter is accepted but ignored. The generated sign call doesn't pass the domain to the MPC contract. This could result in signing with the wrong curve if the contract defaults differ from what the caller expects.

---

Design Feedback

8. Flat file storage for keys metadata and spend tracking

Both keys.json and spend_tracking.json are read-modify-write JSON files. This has no concurrency safety (see #1), no atomicity (a crash mid-write corrupts the file), and doesn't follow the project's Database trait pattern. Per CLAUDE.md: "All new features that touch persistence MUST support both backends." Consider:

• Atomic writes (write to temp file, then rename)
• Or migrate to the Database trait with proper implementations

9. Default policy is too permissive for full-access keys

PolicyConfig::default() sets deny_full_access_operations: false, meaning a full-access key can be used for transfers, function calls, etc. The PR description says "high-value operations require explicit user approval" but the default policy auto-approves zero-value transfers from full-access keys (because transfer_auto_approve_max_yocto: 0 means "up to 0 yocto is auto-approved"). While this technically blocks non-zero transfers, function calls with zero deposit through a full-access key on an arbitrary contract are also auto-approved if the key is scoped. Consider defaulting deny_full_access_operations: true to match the "hybrid custody" philosophy.

10. let _ = self.secrets_store.delete(...) silently ignores deletion errors (src/keys/mod.rs:265)

In remove_key, if the secret deletion fails but the metadata is already removed, the key becomes orphaned in the secrets store with no way to clean it up. At least log the error.

---

Style / Minor

11. let-else refactoring in capabilities.rs and leak_detector.rs

The PR refactors if let ... && ... chains into nested if let / if blocks. This is fine for compatibility, but the commit message doesn't mention it. These should be in a separate commit or at least noted in the PR description since they touch security-critical code paths (endpoint matching, leak detection).

12. format_yocto has precision loss for amounts between 1 milliNEAR and 1 NEAR

let frac = (yocto % ONE_NEAR) / ONE_MILLI_NEAR;

This integer division truncates. E.g., 1.999 NEAR formats as "1.999 NEAR" but 0.001999 NEAR formats as "0.001 NEAR" (the 999 microNEAR is lost). For a financial display, consider at least 6 decimal places.

13. No #[cfg(test)] on InMemorySecretsStore import

The tests in mod.rs use InMemorySecretsStore -- make sure this type is available at test time (it appears to be, but worth a compilation check with --no-default-features).

---

Summary

The core key management design is solid -- encrypted storage, Zeroize on drop, keys never reaching WASM, policy-before-sign pipeline. But the TOCTOU race, the leak detector regression, and the wrapper.rs rewrite breaking existing WASM tools are blockers. The spend tracking gap is a financial correctness bug that should also be fixed before merge.

Recommended disposition: Request changes on items 1, 2, 5, 6 before merging.