From 2f2b7af81b2dea3c1a98fc2868306cd401dfd4fe Mon Sep 17 00:00:00 2001 From: twzrd-sol Date: Wed, 26 Aug 2026 15:23:39 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat(spend-control):=20counterparty=20polic?= =?UTF-8?q?y=20=E2=80=94=20payee/network/asset=20allow-deny=20lists?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SpendLimits already constrains how much an agent may pay. This adds optional, default-off allow/deny lists for who it may pay and on which network/asset, evaluated on the existing check() path. - SpendLimits gains allowedPayees/blockedPayees/allowedNetworks/allowedAssets (string[], optional). Same ownership model as the existing spend windows. - check(estimatedCost, counterparty?) takes an optional second param; existing single-arg callers are unaffected. - Denial reuses the existing refusal path via a new CheckResult.blockedByPolicy field rather than widening the public SpendWindow union, which is a time-window concept, not a "why blocked" enum. - blockedPayees wins over allowedPayees when a payee is on both. - Fails closed if a policy is configured but check() isn't given the matching counterparty field. - setPolicy()/clearPolicy() mirror setLimit()/clearLimit(). Also fixes FileSpendControlStorage.load(), which reconstructs limits from a hardcoded key allowlist — any new SpendLimits field would silently vanish on the next load/restart even though save() writes it out fine. Extended the same explicit-and-validated loading pattern to the four new fields. --- src/index.ts | 2 + src/spend-control.test.ts | 202 +++++++++++++++++++++++++++++++++++++- src/spend-control.ts | 123 ++++++++++++++++++++++- 3 files changed, 325 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index c93a6813..b5b7a8b5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2451,7 +2451,9 @@ export { } from "./spend-control.js"; export type { SpendWindow, + PolicyList, SpendLimits, + CounterpartyInfo, SpendRecord, SpendingStatus, CheckResult, diff --git a/src/spend-control.test.ts b/src/spend-control.test.ts index 30fb5b7f..f3bfb340 100644 --- a/src/spend-control.test.ts +++ b/src/spend-control.test.ts @@ -2,7 +2,10 @@ * SpendControl tests — limits, recording, window expiry, persistence. */ -import { describe, it, expect } from "vitest"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { describe, it, expect, vi, afterEach } from "vitest"; import { SpendControl, InMemorySpendControlStorage, formatDuration } from "./spend-control.js"; function createControl(nowMs = Date.now()) { @@ -236,6 +239,203 @@ describe("SpendControl", () => { }); }); +describe("counterparty policy", () => { + describe("payee allowlist/blocklist", () => { + it("has no effect when not configured", () => { + const { control } = createControl(); + expect(control.check(0.01, { payTo: "0xanything" }).allowed).toBe(true); + expect(control.check(0.01).allowed).toBe(true); + }); + + it("allows a payee in the allowlist", () => { + const { control } = createControl(); + control.setPolicy("allowedPayees", ["0xgood"]); + expect(control.check(0.01, { payTo: "0xgood" }).allowed).toBe(true); + }); + + it("blocks a payee not in the allowlist", () => { + const { control } = createControl(); + control.setPolicy("allowedPayees", ["0xgood"]); + const result = control.check(0.01, { payTo: "0xother" }); + expect(result.allowed).toBe(false); + expect(result.blockedByPolicy).toBe("allowedPayees"); + }); + + it("blocks a payee on the blocklist", () => { + const { control } = createControl(); + control.setPolicy("blockedPayees", ["0xbad"]); + const result = control.check(0.01, { payTo: "0xbad" }); + expect(result.allowed).toBe(false); + expect(result.blockedByPolicy).toBe("blockedPayees"); + }); + + it("passes a payee not on the blocklist", () => { + const { control } = createControl(); + control.setPolicy("blockedPayees", ["0xbad"]); + expect(control.check(0.01, { payTo: "0xfine" }).allowed).toBe(true); + }); + + it("blocklist wins when a payee is on both lists", () => { + const { control } = createControl(); + control.setPolicy("allowedPayees", ["0xboth"]); + control.setPolicy("blockedPayees", ["0xboth"]); + const result = control.check(0.01, { payTo: "0xboth" }); + expect(result.allowed).toBe(false); + expect(result.blockedByPolicy).toBe("blockedPayees"); + }); + + it("fails closed when policy is configured but no payTo is given", () => { + const { control } = createControl(); + control.setPolicy("allowedPayees", ["0xgood"]); + const result = control.check(0.01); + expect(result.allowed).toBe(false); + expect(result.blockedByPolicy).toBe("allowedPayees"); + }); + + it("clearPolicy removes a configured list", () => { + const { control } = createControl(); + control.setPolicy("allowedPayees", ["0xgood"]); + control.clearPolicy("allowedPayees"); + expect(control.check(0.01, { payTo: "0xanything" }).allowed).toBe(true); + }); + + it("does not set blockedBy (SpendWindow) for a policy denial", () => { + const { control } = createControl(); + control.setLimit("perRequest", 1000); + control.setPolicy("blockedPayees", ["0xbad"]); + const result = control.check(0.01, { payTo: "0xbad" }); + expect(result.allowed).toBe(false); + expect(result.blockedByPolicy).toBe("blockedPayees"); + expect(result.blockedBy).toBeUndefined(); + }); + }); + + describe("network allowlist", () => { + it("has no effect when not configured", () => { + const { control } = createControl(); + expect(control.check(0.01, { network: "anything" }).allowed).toBe(true); + }); + + it("allows a network in the allowlist", () => { + const { control } = createControl(); + control.setPolicy("allowedNetworks", ["base"]); + expect(control.check(0.01, { network: "base" }).allowed).toBe(true); + }); + + it("blocks a network not in the allowlist", () => { + const { control } = createControl(); + control.setPolicy("allowedNetworks", ["base"]); + const result = control.check(0.01, { network: "solana" }); + expect(result.allowed).toBe(false); + expect(result.blockedByPolicy).toBe("allowedNetworks"); + }); + + it("fails closed when configured but no network is given", () => { + const { control } = createControl(); + control.setPolicy("allowedNetworks", ["base"]); + const result = control.check(0.01); + expect(result.allowed).toBe(false); + expect(result.blockedByPolicy).toBe("allowedNetworks"); + }); + }); + + describe("asset allowlist", () => { + it("allows an asset in the allowlist", () => { + const { control } = createControl(); + control.setPolicy("allowedAssets", ["USDC"]); + expect(control.check(0.01, { asset: "USDC" }).allowed).toBe(true); + }); + + it("blocks an asset not in the allowlist", () => { + const { control } = createControl(); + control.setPolicy("allowedAssets", ["USDC"]); + const result = control.check(0.01, { asset: "SOL" }); + expect(result.allowed).toBe(false); + expect(result.blockedByPolicy).toBe("allowedAssets"); + }); + }); + + describe("setPolicy validation", () => { + it("rejects an empty list", () => { + const { control } = createControl(); + expect(() => control.setPolicy("allowedPayees", [])).toThrow(); + }); + + it("rejects non-string or empty-string entries", () => { + const { control } = createControl(); + // @ts-expect-error deliberately invalid entry type, for a runtime validation test + expect(() => control.setPolicy("allowedPayees", [123])).toThrow(); + expect(() => control.setPolicy("allowedPayees", [""])).toThrow(); + }); + }); + + describe("amount checks still run after policy passes", () => { + it("still enforces perRequest once payee policy passes", () => { + const { control } = createControl(); + control.setPolicy("allowedPayees", ["0xgood"]); + control.setLimit("perRequest", 0.1); + const result = control.check(0.5, { payTo: "0xgood" }); + expect(result.allowed).toBe(false); + expect(result.blockedBy).toBe("perRequest"); + }); + }); +}); + +describe("FileSpendControlStorage persistence", () => { + let tmpHome: string | undefined; + const originalHome = process.env.HOME; + + afterEach(() => { + if (tmpHome) fs.rmSync(tmpHome, { recursive: true, force: true }); + tmpHome = undefined; + if (originalHome !== undefined) process.env.HOME = originalHome; + else delete process.env.HOME; + }); + + it("round-trips policy lists, not just spend limits, across save/load", async () => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "clawrouter-spend-")); + process.env.HOME = tmpHome; + vi.resetModules(); + const mod = await import("./spend-control.js"); + const storage = new mod.FileSpendControlStorage(); + + storage.save({ + limits: { + perRequest: 0.5, + allowedPayees: ["0xgood"], + blockedPayees: ["0xbad"], + allowedNetworks: ["base"], + allowedAssets: ["USDC"], + }, + history: [], + }); + + const loaded = storage.load(); + expect(loaded?.limits.perRequest).toBe(0.5); + expect(loaded?.limits.allowedPayees).toEqual(["0xgood"]); + expect(loaded?.limits.blockedPayees).toEqual(["0xbad"]); + expect(loaded?.limits.allowedNetworks).toEqual(["base"]); + expect(loaded?.limits.allowedAssets).toEqual(["USDC"]); + }); + + it("drops malformed policy entries instead of throwing", async () => { + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), "clawrouter-spend-")); + process.env.HOME = tmpHome; + vi.resetModules(); + const mod = await import("./spend-control.js"); + const storage = new mod.FileSpendControlStorage(); + const spendingFile = path.join(tmpHome, ".openclaw", "blockrun", "spending.json"); + fs.mkdirSync(path.dirname(spendingFile), { recursive: true }); + fs.writeFileSync( + spendingFile, + JSON.stringify({ limits: { allowedPayees: ["ok", 123, ""] }, history: [] }), + ); + + const loaded = storage.load(); + expect(loaded?.limits.allowedPayees).toBeUndefined(); + }); +}); + describe("formatDuration", () => { it("formats seconds", () => { expect(formatDuration(30)).toBe("30s"); diff --git a/src/spend-control.ts b/src/spend-control.ts index 6402f080..187503f4 100644 --- a/src/spend-control.ts +++ b/src/spend-control.ts @@ -24,11 +24,34 @@ const DAY_MS = 24 * HOUR_MS; export type SpendWindow = "perRequest" | "hourly" | "daily" | "session"; +/** + * Counterparty/network/asset allow-or-deny lists. Default-off: a list only + * takes effect once configured via setPolicy(). `allowedPayees`/`blockedPayees` + * are both supported (block always wins if both are set); network and asset + * are allowlist-only, matching what a caller can realistically enumerate. + */ +export type PolicyList = "allowedPayees" | "blockedPayees" | "allowedNetworks" | "allowedAssets"; + export interface SpendLimits { perRequest?: number; hourly?: number; daily?: number; session?: number; + allowedPayees?: string[]; + blockedPayees?: string[]; + allowedNetworks?: string[]; + allowedAssets?: string[]; +} + +/** + * Counterparty details for a pending payment, passed to check() alongside + * the estimated cost. Exact string match — callers are responsible for any + * chain-specific normalization (e.g. EVM checksum casing). + */ +export interface CounterpartyInfo { + payTo?: string; + network?: string; + asset?: string; } export interface SpendRecord { @@ -56,6 +79,7 @@ export interface SpendingStatus { export interface CheckResult { allowed: boolean; blockedBy?: SpendWindow; + blockedByPolicy?: PolicyList; remaining?: number; reason?: string; resetIn?: number; @@ -87,6 +111,21 @@ export class FileSpendControlStorage implements SpendControlStorage { limits[key] = val; } } + for (const key of [ + "allowedPayees", + "blockedPayees", + "allowedNetworks", + "allowedAssets", + ] as const) { + const val = rawLimits[key]; + if ( + Array.isArray(val) && + val.length > 0 && + val.every((v) => typeof v === "string" && v.length > 0) + ) { + limits[key] = val; + } + } const history: SpendRecord[] = []; if (Array.isArray(rawHistory)) { @@ -182,11 +221,93 @@ export class SpendControl { this.save(); } + setPolicy(list: PolicyList, values: string[]): void { + if ( + !Array.isArray(values) || + values.length === 0 || + values.some((v) => typeof v !== "string" || v.length === 0) + ) { + throw new Error("Policy list must be a non-empty array of non-empty strings"); + } + this.limits[list] = [...values]; + this.save(); + } + + clearPolicy(list: PolicyList): void { + delete this.limits[list]; + this.save(); + } + getLimits(): SpendLimits { return { ...this.limits }; } - check(estimatedCost: number): CheckResult { + check(estimatedCost: number, counterparty?: CounterpartyInfo): CheckResult { + const payeePolicySet = + (this.limits.blockedPayees && this.limits.blockedPayees.length > 0) || + (this.limits.allowedPayees && this.limits.allowedPayees.length > 0); + if (payeePolicySet) { + if (counterparty?.payTo === undefined) { + return { + allowed: false, + blockedByPolicy: this.limits.blockedPayees?.length ? "blockedPayees" : "allowedPayees", + reason: "Payee policy is configured but no payTo was provided to check()", + }; + } + if (this.limits.blockedPayees?.includes(counterparty.payTo)) { + return { + allowed: false, + blockedByPolicy: "blockedPayees", + reason: `Payee is blocked by policy: ${counterparty.payTo}`, + }; + } + if ( + this.limits.allowedPayees && + this.limits.allowedPayees.length > 0 && + !this.limits.allowedPayees.includes(counterparty.payTo) + ) { + return { + allowed: false, + blockedByPolicy: "allowedPayees", + reason: `Payee is not in the configured allowlist: ${counterparty.payTo}`, + }; + } + } + + if (this.limits.allowedNetworks && this.limits.allowedNetworks.length > 0) { + if (counterparty?.network === undefined) { + return { + allowed: false, + blockedByPolicy: "allowedNetworks", + reason: "Network policy is configured but no network was provided to check()", + }; + } + if (!this.limits.allowedNetworks.includes(counterparty.network)) { + return { + allowed: false, + blockedByPolicy: "allowedNetworks", + reason: `Network is not in the configured allowlist: ${counterparty.network}`, + }; + } + } + + if (this.limits.allowedAssets && this.limits.allowedAssets.length > 0) { + if (counterparty?.asset === undefined) { + return { + allowed: false, + blockedByPolicy: "allowedAssets", + reason: "Asset policy is configured but no asset was provided to check()", + }; + } + if (!this.limits.allowedAssets.includes(counterparty.asset)) { + return { + allowed: false, + blockedByPolicy: "allowedAssets", + reason: `Asset is not in the configured allowlist: ${counterparty.asset}`, + }; + } + } + const now = this.now(); if (this.limits.perRequest !== undefined) { From 0bf27f9eb29469fd92ef626c247462bd7f31165b Mon Sep 17 00:00:00 2001 From: twzrd-sol Date: Thu, 27 Aug 2026 20:56:26 +0000 Subject: [PATCH 2/2] fix: harden policy list against reference leaks and key confusion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues from CodeRabbit's review of the counterparty-policy PR: - setPolicy()/clearPolicy() took a PolicyList-typed param but never validated it at runtime, so a caller passing a SpendWindow name (e.g. "perRequest") would silently overwrite or delete a monetary limit instead of a policy list — both share the same underlying object with no runtime tag check. Added isPolicyList() and reject unknown keys. - getLimits() and getStatus() shallow-copied `this.limits`, so the new array-valued fields were shared by reference. A caller mutating the returned array mutated live internal policy state directly, bypassing setPolicy()'s validation and save(). Added cloneLimits(), which deep- copies the four policy arrays, and used it everywhere a SpendLimits crosses a public boundary: getLimits(), getStatus(), and InMemorySpendControlStorage's load()/save() (which already cloned SpendRecord history per-record for this exact reason, just never needed to for limits before this PR). --- src/spend-control.test.ts | 35 +++++++++++++++++++++++++++++++++++ src/spend-control.ts | 37 +++++++++++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/spend-control.test.ts b/src/spend-control.test.ts index f3bfb340..a079b0b3 100644 --- a/src/spend-control.test.ts +++ b/src/spend-control.test.ts @@ -367,6 +367,41 @@ describe("counterparty policy", () => { expect(() => control.setPolicy("allowedPayees", [123])).toThrow(); expect(() => control.setPolicy("allowedPayees", [""])).toThrow(); }); + + it("rejects a SpendWindow name passed as a policy list, and does not touch that limit", () => { + const { control } = createControl(); + control.setLimit("perRequest", 0.5); + // @ts-expect-error deliberately invalid list, for a runtime validation test + expect(() => control.setPolicy("perRequest", ["0xgood"])).toThrow(); + expect(control.getLimits().perRequest).toBe(0.5); + }); + + it("clearPolicy rejects a SpendWindow name and does not clear that limit", () => { + const { control } = createControl(); + control.setLimit("hourly", 1.0); + // @ts-expect-error deliberately invalid list, for a runtime validation test + expect(() => control.clearPolicy("hourly")).toThrow(); + expect(control.getLimits().hourly).toBe(1.0); + }); + }); + + describe("defensive copies", () => { + it("mutating the array returned by getLimits() does not affect live policy", () => { + const { control } = createControl(); + control.setPolicy("allowedPayees", ["0xgood"]); + const limits = control.getLimits(); + limits.allowedPayees?.push("0xsneaky"); + expect(control.check(0.01, { payTo: "0xsneaky" }).allowed).toBe(false); + expect(control.getLimits().allowedPayees).toEqual(["0xgood"]); + }); + + it("mutating the array returned by getStatus().limits does not affect live policy", () => { + const { control } = createControl(); + control.setPolicy("blockedPayees", ["0xbad"]); + const status = control.getStatus(); + status.limits.blockedPayees?.push("0xalsogood"); + expect(control.check(0.01, { payTo: "0xalsogood" }).allowed).toBe(true); + }); }); describe("amount checks still run after policy passes", () => { diff --git a/src/spend-control.ts b/src/spend-control.ts index 187503f4..90df0b80 100644 --- a/src/spend-control.ts +++ b/src/spend-control.ts @@ -32,6 +32,17 @@ export type SpendWindow = "perRequest" | "hourly" | "daily" | "session"; */ export type PolicyList = "allowedPayees" | "blockedPayees" | "allowedNetworks" | "allowedAssets"; +const POLICY_LISTS: readonly PolicyList[] = [ + "allowedPayees", + "blockedPayees", + "allowedNetworks", + "allowedAssets", +]; + +function isPolicyList(value: string): value is PolicyList { + return (POLICY_LISTS as readonly string[]).includes(value); +} + export interface SpendLimits { perRequest?: number; hourly?: number; @@ -43,6 +54,18 @@ export interface SpendLimits { allowedAssets?: string[]; } +/** Defensive copy: the four policy fields are arrays, so a shallow `{...limits}` still shares them by reference. */ +function cloneLimits(limits: SpendLimits): SpendLimits { + const clone: SpendLimits = { ...limits }; + for (const key of POLICY_LISTS) { + const val = limits[key]; + if (val !== undefined) { + clone[key] = [...val]; + } + } + return clone; +} + /** * Counterparty details for a pending payment, passed to check() alongside * the estimated cost. Exact string match — callers are responsible for any @@ -175,7 +198,7 @@ export class InMemorySpendControlStorage implements SpendControlStorage { load(): { limits: SpendLimits; history: SpendRecord[] } | null { return this.data ? { - limits: { ...this.data.limits }, + limits: cloneLimits(this.data.limits), history: this.data.history.map((r) => ({ ...r })), } : null; @@ -183,7 +206,7 @@ export class InMemorySpendControlStorage implements SpendControlStorage { save(data: { limits: SpendLimits; history: SpendRecord[] }): void { this.data = { - limits: { ...data.limits }, + limits: cloneLimits(data.limits), history: data.history.map((r) => ({ ...r })), }; } @@ -222,6 +245,9 @@ export class SpendControl { } setPolicy(list: PolicyList, values: string[]): void { + if (!isPolicyList(list)) { + throw new Error(`Unknown policy list: ${String(list)}`); + } if ( !Array.isArray(values) || values.length === 0 || @@ -234,12 +260,15 @@ export class SpendControl { } clearPolicy(list: PolicyList): void { + if (!isPolicyList(list)) { + throw new Error(`Unknown policy list: ${String(list)}`); + } delete this.limits[list]; this.save(); } getLimits(): SpendLimits { - return { ...this.limits }; + return cloneLimits(this.limits); } check(estimatedCost: number, counterparty?: CounterpartyInfo): CheckResult { @@ -421,7 +450,7 @@ export class SpendControl { const dailySpent = this.getSpendingInWindow(now - DAY_MS, now); return { - limits: { ...this.limits }, + limits: cloneLimits(this.limits), spending: { hourly: hourlySpent, daily: dailySpent,