Skip to content

Commit 237047e

Browse files
Klowclaude
andcommitted
fix: CodeRabbit review — deep-canonical policyDigest (nested solana was hashed as {}), type-strict enabled gate, never broadcast unsimulated (even unrestricted), cluster-aware ledger entries + explorer links, test env restoration
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent b62e18c commit 237047e

6 files changed

Lines changed: 55 additions & 9 deletions

File tree

mcp-server/src/__tests__/agent_keystore_solana.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import { PublicKey } from '@solana/web3.js';
66

77
import './_setup.js';
88
// Never touch the real OS keychain from tests — force env-var-only resolution.
9+
const PRIOR_DISABLE = process.env.CHAINGPT_DISABLE_KEYCHAIN;
10+
const PRIOR_PASS = process.env.CHAINGPT_AGENT_WALLET_PASSPHRASE;
911
process.env.CHAINGPT_DISABLE_KEYCHAIN = '1';
1012
process.env.CHAINGPT_AGENT_WALLET_PASSPHRASE = 'super-long-passphrase-for-tests-only-1234';
1113

@@ -26,6 +28,8 @@ beforeEach(() => {
2628

2729
afterAll(() => {
2830
delete process.env.CHAINGPT_SOLANA_KEYSTORE_FILE;
31+
if (PRIOR_DISABLE === undefined) delete process.env.CHAINGPT_DISABLE_KEYCHAIN; else process.env.CHAINGPT_DISABLE_KEYCHAIN = PRIOR_DISABLE;
32+
if (PRIOR_PASS === undefined) delete process.env.CHAINGPT_AGENT_WALLET_PASSPHRASE; else process.env.CHAINGPT_AGENT_WALLET_PASSPHRASE = PRIOR_PASS;
2933
rmSync(TMP, { recursive: true, force: true });
3034
});
3135

mcp-server/src/__tests__/agent_policy_solana.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,22 @@ describe('checkSolanaPolicy', () => {
105105
});
106106
});
107107

108+
describe('policyDigest — nested canonicalization', () => {
109+
it('different solana sub-policies produce different digests', async () => {
110+
const { policyDigest } = await import('../lib/agent-policy.js');
111+
const a = { ...base };
112+
const b = { ...base, solana: { ...base.solana!, maxTxLamports: '999' } };
113+
expect(policyDigest(a)).not.toBe(policyDigest(b));
114+
});
115+
});
116+
117+
describe('checkSolanaPolicy — type-strict enabled', () => {
118+
it('a hand-edited string "true" does NOT arm Solana', () => {
119+
const p = { version: 1, killSwitch: false, solana: { enabled: 'true' as unknown as boolean } } as AgentPolicy;
120+
expect(checkSolanaPolicy(intent(), p, NO_SPEND).allowed).toBe(false);
121+
});
122+
});
123+
108124
describe('validatePolicyInput — solana sub-object', () => {
109125
const valid = {
110126
version: 1,

mcp-server/src/lib/agent-activity.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@ export function logActivity(e: ActivityEntry): void {
6161
export type ChainClass = 'evm' | 'solana' | 'all';
6262

6363
function entryClass(e: ActivityEntry): Exclude<ChainClass, 'all'> {
64-
return e.chain === 'solana' ? 'solana' : 'evm';
64+
// 'solana', 'solana-devnet', 'solana-testnet' — all clusters share the class.
65+
// Devnet/testnet sends therefore CONSUME the lamport caps: conservative on
66+
// purpose (a cap that over-counts fails safe; one that under-counts does not).
67+
return e.chain.startsWith('solana') ? 'solana' : 'evm';
6568
}
6669

6770
export function spendStats(windowHours = 24, chainClass: ChainClass = 'all'): { totalWei: bigint; txCount: number; ok: boolean } {

mcp-server/src/lib/agent-policy.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,20 @@ export function loadPolicy(): AgentPolicy {
226226
}
227227

228228
export function policyDigest(p: AgentPolicy): string {
229-
// Stable hash for surfacing in status output. Sort keys for determinism.
230-
const ordered = JSON.stringify(p, Object.keys(p).sort());
231-
return createHash('sha256').update(ordered).digest('hex').slice(0, 16);
229+
// Stable hash for surfacing in status output. Recursively sort keys for
230+
// determinism. (A replacer ARRAY would filter NESTED keys too — the old
231+
// implementation serialized the `solana` sub-object as {} and gave
232+
// different Solana policies identical digests.)
233+
const canonical = (v: unknown): unknown => {
234+
if (Array.isArray(v)) return v.map(canonical);
235+
if (v && typeof v === 'object') {
236+
return Object.fromEntries(
237+
Object.keys(v as object).sort().map((k) => [k, canonical((v as Record<string, unknown>)[k])])
238+
);
239+
}
240+
return v;
241+
};
242+
return createHash('sha256').update(JSON.stringify(canonical(p))).digest('hex').slice(0, 16);
232243
}
233244

234245
export interface TxIntent {
@@ -422,7 +433,7 @@ export function checkSolanaPolicy(
422433
// admin must explicitly opt in. unrestricted does NOT bypass this —
423434
// YOLO mode was granted for the EVM surface; silently arming a second
424435
// chain the admin never enabled violates least surprise.
425-
if (!sol?.enabled) {
436+
if (sol?.enabled !== true) { // type-strict: a hand-edited "true"/1 must not arm a chain
426437
return {
427438
allowed: false,
428439
reason: 'Solana signing is not enabled in the policy. An admin must add `"solana": { "enabled": true, ... }` via the dashboard or a text editor.',

mcp-server/src/tools/agent_wallet.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -853,9 +853,10 @@ async function renderDashboard(res: ServerResponse, address: string, chains: str
853853
const activityRows = activity.length === 0
854854
? `<div class="subtle" style="padding:12px">No transactions yet. When the agent runs <code>chaingpt_agent_wallet_sign_and_send</code> and the policy allows it, entries will appear here.</div>`
855855
: activity.map((a) => {
856-
const isSolana = a.chain === 'solana';
856+
const isSolana = a.chain.startsWith('solana');
857+
const solCluster = isSolana && a.chain !== 'solana' ? `?cluster=${a.chain.slice('solana-'.length)}` : '';
857858
const chain = isSolana ? null : resolveChainWithCustom(a.chain);
858-
const txLink = isSolana ? `https://solscan.io/tx/${a.hash}` : (chain?.explorer ? `${chain.explorer}/tx/${a.hash}` : '#');
859+
const txLink = isSolana ? `https://solscan.io/tx/${a.hash}${solCluster}` : (chain?.explorer ? `${chain.explorer}/tx/${a.hash}` : '#');
859860
const addrLink = isSolana ? '#' : (chain?.explorer ? `${chain.explorer}/address/${a.to}` : '#');
860861
const valueEth = isSolana ? fmtSol(BigInt(a.valueWei)) : fmtEth(BigInt(a.valueWei));
861862
return `<div class="activity-row">

mcp-server/src/tools/agent_wallet_solana.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,18 @@ export async function handleAgentWalletSolanaTool(
232232
return refusalBlock(decision.reason, decision.policyDigest);
233233
}
234234

235-
// 6. Never autonomously broadcast a tx that simulates to failure
235+
// 6a. Never broadcast UNSIMULATED — applies even in unrestricted mode
236+
// (the policy gate allows unrestricted before the lamport checks run,
237+
// so this handler-level refusal is the backstop).
238+
if (!sim.ok) {
239+
return {
240+
content: [{
241+
type: 'text',
242+
text: `⛔ Refused: the transaction could not be simulated (RPC unavailable or fee-payer state unreadable). A policy-fenced agent never broadcasts blind — not even in unrestricted mode. Check SOLANA_RPC_URL and retry.`,
243+
}],
244+
};
245+
}
246+
// 6b. Never autonomously broadcast a tx that simulates to failure
236247
if (sim.ok && sim.err) {
237248
return {
238249
content: [{
@@ -278,7 +289,7 @@ export async function handleAgentWalletSolanaTool(
278289
try {
279290
logActivity({
280291
ts: new Date().toISOString(),
281-
chain: 'solana',
292+
chain: network === 'mainnet' ? 'solana' : `solana-${network}`,
282293
chainId: 0,
283294
from: agentAddress,
284295
to: programIds.join(','),

0 commit comments

Comments
 (0)