diff --git a/src/policy/policy-account-proxy.js b/src/policy/policy-account-proxy.js index 4b2c8db..f627fa8 100644 --- a/src/policy/policy-account-proxy.js +++ b/src/policy/policy-account-proxy.js @@ -31,6 +31,89 @@ const PROTOCOL_GETTERS = [ ['getSdaProtocol', 'sda'] ] +function isProtectedMember (prop) { + return prop === 'keyPair' || (typeof prop === 'string' && prop.startsWith('_')) +} + +/** + * Wraps `subject` in a Proxy that serves `substitutions` in place of the + * subject's own members and treats protected members as absent. + * + * Every trap that can observe a member consults the same two rules, because a + * `get` trap on its own guards only one of several reflective paths: + * + * - `getOwnPropertyDescriptor` does not route through `get`, so without a trap + * `Object.getOwnPropertyDescriptor(account, '_signer').value` hands back the + * raw signer, and — worse — `...(account, 'getSwapProtocol').value(label)` + * hands back the unenforced protocol getter `_registerProtocols` installed + * as an own property, whose methods skip policy evaluation entirely. + * - `has` and `ownKeys` would keep reporting protected members, leaking the + * shape of the underlying account even where the values are hidden. + * + * Substituted members are described with the enforced value rather than + * omitted, so a descriptor read observes the same member `get` returns. + * Descriptors are only reported for members the subject actually owns, which + * keeps prototype methods reading as inherited exactly as on a raw account. + * + * `preventExtensions` is refused because hiding own members is only a legal + * Proxy result while the subject stays extensible. Letting a caller freeze the + * subject would turn every later descriptor read or `ownKeys` call into a + * TypeError. + * + * @param {object} subject - The object to guard. + * @param {Map} substitutions - The members to serve in place of the subject's own. + * @returns {object} The guarded proxy. + */ +function createGuardedProxy (subject, substitutions) { + return new Proxy(subject, { + get (target, prop) { + if (substitutions.has(prop)) return substitutions.get(prop) + + if (isProtectedMember(prop)) return undefined + + const value = Reflect.get(target, prop, target) + + // Bind functions to the underlying target so internal `this.method()` + // calls resolve on the original account, bypassing the proxy. This is + // how nested-call escape works without any async-context tracking. + if (typeof value === 'function') return value.bind(target) + + return value + }, + + getOwnPropertyDescriptor (target, prop) { + if (isProtectedMember(prop)) return undefined + + const descriptor = Reflect.getOwnPropertyDescriptor(target, prop) + + if (descriptor === undefined || !substitutions.has(prop)) return descriptor + + return { + value: substitutions.get(prop), + writable: false, + enumerable: descriptor.enumerable, + configurable: true + } + }, + + has (target, prop) { + if (substitutions.has(prop)) return true + + if (isProtectedMember(prop)) return false + + return Reflect.has(target, prop) + }, + + ownKeys (target) { + return Reflect.ownKeys(target).filter((prop) => !isProtectedMember(prop)) + }, + + preventExtensions () { + return false + } + }) +} + /** * Returns a Proxy that exposes policy-enforced versions of write methods on * the given account. The policy engine itself does not mutate the account @@ -77,7 +160,9 @@ export async function createPolicyEnforcedAccount (account, { blockchain, path, const ctx = { account, readOnlyAccount, blockchain, index, engine } - const enforcedMethods = new Map() + const substitutions = new Map() + + const enforcedOperations = [] // Wrap every method in OPERATIONS that exists on the underlying account, // not just the ones referenced by registered policies. The evaluator @@ -87,66 +172,49 @@ export async function createPolicyEnforcedAccount (account, { blockchain, path, // from moving the same tokens). for (const op of OPERATIONS) { if (typeof account[op] === 'function') { - enforcedMethods.set(op, buildEnforcedMethod(op, account[op].bind(account), ctx)) + enforcedOperations.push(op) + + substitutions.set(op, buildEnforcedMethod(op, account[op].bind(account), ctx)) } } - const enforcedGetters = new Map() - for (const [getterName, type] of PROTOCOL_GETTERS) { if (typeof account[getterName] !== 'function') continue const writeMethods = PROTOCOL_METHODS[type] const originalGetter = account[getterName].bind(account) - enforcedGetters.set(getterName, (label) => { + substitutions.set(getterName, (label) => { const protocol = originalGetter(label) return wrapProtocolInProxy(protocol, writeMethods, ctx) }) } - const simulate = buildSimulateMirror(Array.from(enforcedMethods.keys()), ctx) + substitutions.set('simulate', buildSimulateMirror(enforcedOperations, ctx)) // Late-bound reference to the proxy itself, so the `registerProtocol` // interceptor below can return the proxy instead of the raw account. const handle = { proxy: null } - const proxyHandle = new Proxy(account, { - get (target, prop) { - if (enforcedMethods.has(prop)) return enforcedMethods.get(prop) - if (enforcedGetters.has(prop)) return enforcedGetters.get(prop) - if (prop === 'simulate') return simulate - - // `account.registerProtocol(...)` returns the raw account by design - // (see `_registerProtocols` in wdk.js). Without intercepting - // here, `proxy.registerProtocol(...).sendTransaction(...)` would skip - // enforcement entirely because the caller is no longer holding the - // proxy. Rewrite the return value to the proxy itself. - if (prop === 'registerProtocol' && typeof target.registerProtocol === 'function') { - const fn = target.registerProtocol.bind(target) - - return (...args) => { - fn(...args) - - return handle.proxy - } - } - - const value = Reflect.get(target, prop, target) + // `account.registerProtocol(...)` returns the raw account by design (see + // `_registerProtocols` in wdk.js). Without intercepting, + // `proxy.registerProtocol(...).sendTransaction(...)` would skip enforcement + // entirely because the caller is no longer holding the proxy. Rewrite the + // return value to the proxy itself. + if (typeof account.registerProtocol === 'function') { + const registerProtocol = account.registerProtocol.bind(account) - // Bind functions to the underlying target so internal `this.method()` - // calls resolve on the original account, bypassing the proxy. This is - // how nested-call escape works without any async-context tracking. - if (typeof value === 'function') return value.bind(target) + substitutions.set('registerProtocol', (...args) => { + registerProtocol(...args) - return value - } - }) + return handle.proxy + }) + } - handle.proxy = proxyHandle + handle.proxy = createGuardedProxy(account, substitutions) - return proxyHandle + return handle.proxy } function buildEnforcedMethod (name, boundOriginal, ctx) { @@ -175,25 +243,15 @@ function buildEnforcedMethod (name, boundOriginal, ctx) { } function wrapProtocolInProxy (protocol, opsToWrap, ctx) { - const enforcedMethods = new Map() + const substitutions = new Map() for (const method of opsToWrap) { if (typeof protocol[method] === 'function') { - enforcedMethods.set(method, buildEnforcedMethod(method, protocol[method].bind(protocol), ctx)) + substitutions.set(method, buildEnforcedMethod(method, protocol[method].bind(protocol), ctx)) } } - return new Proxy(protocol, { - get (target, prop) { - if (enforcedMethods.has(prop)) return enforcedMethods.get(prop) - - const value = Reflect.get(target, prop, target) - - if (typeof value === 'function') return value.bind(target) - - return value - } - }) + return createGuardedProxy(protocol, substitutions) } function buildSimulateMirror (methodNames, ctx) { diff --git a/tests/wdk-policy.test.js b/tests/wdk-policy.test.js index f7ae086..5325841 100644 --- a/tests/wdk-policy.test.js +++ b/tests/wdk-policy.test.js @@ -716,6 +716,232 @@ describe('WDK — policy engine', () => { }) }) + // ------------------------------------------------------------------------- + // Internal-reference containment + // ------------------------------------------------------------------------- + + describe('internal-reference containment', () => { + test('key material (keyPair) is not reachable through a governed account', async () => { + getAccountMock.mockResolvedValue(buildAccount(PATH_DEFAULT, { + keyPair: { privateKey: new Uint8Array([1, 2, 3]) } + })) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerPolicy(projectAllowAll('p')) + + const account = await wdk.getAccount('ethereum', 0) + + expect(account.keyPair).toBeUndefined() + }) + + test('internal references (_signer, _provider, _config) are not reachable through a governed account', async () => { + getAccountMock.mockResolvedValue(buildAccount(PATH_DEFAULT, { + _signer: { signTransaction: jest.fn() }, + _provider: { send: jest.fn() }, + _config: { transactionMaxFee: 1n } + })) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerPolicy(projectAllowAll('p')) + + const account = await wdk.getAccount('ethereum', 0) + + expect(account._signer).toBeUndefined() + expect(account._provider).toBeUndefined() + expect(account._config).toBeUndefined() + }) + + test('hiding internal references leaves safe reads and enforced operations working', async () => { + getAccountMock.mockResolvedValue(buildAccount(PATH_DEFAULT, { + _signer: { signTransaction: jest.fn() } + })) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerPolicy(projectAllowAll('p')) + + const account = await wdk.getAccount('ethereum', 0) + + expect(account._signer).toBeUndefined() + expect(account.path).toBe(PATH_DEFAULT) + expect(await account.getBalance()).toBe(DUMMY_BALANCE) + + const result = await account.sendTransaction({ to: RECIPIENT, value: 1n }) + expect(result.hash).toBe(DUMMY_TX_HASH) + }) + + test("a protocol's raw-account back-reference (_account) is not reachable through the protocol proxy", async () => { + const swapInstanceMock = jest.fn().mockResolvedValue(DUMMY_SWAP_RESULT) + + class MySwapProtocol extends SwapProtocol { + constructor (account) { super(); this._account = account } + async swap (opts) { return swapInstanceMock(opts) } + } + + getAccountMock.mockResolvedValue(buildAccount()) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerProtocol('ethereum', 'velora', MySwapProtocol, {}) + .registerPolicy({ + id: 'no-swaps', + name: 'no-swaps', + scope: 'project', + rules: [{ name: 'deny-swap', operation: 'swap', action: 'DENY', conditions: [] }] + }) + + const account = await wdk.getAccount('ethereum', 0) + const swap = account.getSwapProtocol('velora') + + expect(swap._account).toBeUndefined() + + const denied = await catchAsync(() => swap.swap({ tokenIn: 'A', tokenOut: 'B', tokenInAmount: 1n })) + expect(denied.name).toBe('PolicyViolationError') + expect(swapInstanceMock).not.toHaveBeenCalled() + }) + + test('property descriptors do not carry internal references on a governed account', async () => { + getAccountMock.mockResolvedValue(buildAccount(PATH_DEFAULT, { + keyPair: { privateKey: new Uint8Array([1, 2, 3]) }, + _signer: { signTransaction: jest.fn() }, + _provider: { send: jest.fn() } + })) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerPolicy(projectAllowAll('p')) + + const account = await wdk.getAccount('ethereum', 0) + + expect(Object.getOwnPropertyDescriptor(account, '_signer')).toBeUndefined() + expect(Object.getOwnPropertyDescriptor(account, '_provider')).toBeUndefined() + expect(Object.getOwnPropertyDescriptor(account, 'keyPair')).toBeUndefined() + expect(Object.getOwnPropertyDescriptors(account)).not.toHaveProperty('_signer') + }) + + test('membership tests and key enumeration do not reveal internal references on a governed account', async () => { + getAccountMock.mockResolvedValue(buildAccount(PATH_DEFAULT, { + _signer: { signTransaction: jest.fn() }, + _provider: { send: jest.fn() } + })) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerPolicy(projectAllowAll('p')) + + const account = await wdk.getAccount('ethereum', 0) + + expect('_signer' in account).toBe(false) + expect('_provider' in account).toBe(false) + expect('keyPair' in account).toBe(false) + expect(Reflect.ownKeys(account)).not.toContain('_signer') + expect(Reflect.ownKeys(account)).not.toContain('_provider') + expect(Reflect.ownKeys(account)).toContain('path') + expect(Reflect.ownKeys(account)).toContain('sendTransaction') + }) + + test('a descriptor read of a wrapped operation yields the enforced method, not the raw one', async () => { + getAccountMock.mockResolvedValue(buildAccount()) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerPolicy(projectDenyAll('deny')) + + const account = await wdk.getAccount('ethereum', 0) + + const descriptor = Object.getOwnPropertyDescriptor(account, 'sendTransaction') + + const denied = await catchAsync(() => descriptor.value({ to: RECIPIENT, value: 1n })) + + expect(denied.name).toBe('PolicyViolationError') + expect(sendTransactionMock).not.toHaveBeenCalled() + }) + + test('a descriptor read of a protocol getter yields the enforced getter, not the raw one', async () => { + const swapInstanceMock = jest.fn().mockResolvedValue(DUMMY_SWAP_RESULT) + + class MySwapProtocol extends SwapProtocol { + constructor (account) { super(); this._account = account } + async swap (opts) { return swapInstanceMock(opts) } + } + + getAccountMock.mockResolvedValue(buildAccount()) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerProtocol('ethereum', 'velora', MySwapProtocol, {}) + .registerPolicy({ + id: 'no-swaps', + name: 'no-swaps', + scope: 'project', + rules: [{ name: 'deny-swap', operation: 'swap', action: 'DENY', conditions: [] }] + }) + + const account = await wdk.getAccount('ethereum', 0) + + const descriptor = Object.getOwnPropertyDescriptor(account, 'getSwapProtocol') + + const swap = descriptor.value('velora') + + const denied = await catchAsync(() => swap.swap({ tokenIn: 'A', tokenOut: 'B', tokenInAmount: 1n })) + + expect(denied.name).toBe('PolicyViolationError') + expect(swapInstanceMock).not.toHaveBeenCalled() + }) + + test("descriptors, membership tests and key enumeration do not reveal a protocol's raw-account back-reference", async () => { + class MySwapProtocol extends SwapProtocol { + constructor (account) { super(); this._account = account } + async swap () { return DUMMY_SWAP_RESULT } + } + + getAccountMock.mockResolvedValue(buildAccount()) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerProtocol('ethereum', 'velora', MySwapProtocol, {}) + .registerPolicy(projectAllowAll('p')) + + const account = await wdk.getAccount('ethereum', 0) + const swap = account.getSwapProtocol('velora') + + expect(Object.getOwnPropertyDescriptor(swap, '_account')).toBeUndefined() + expect('_account' in swap).toBe(false) + expect(Reflect.ownKeys(swap)).not.toContain('_account') + }) + + test('a governed account cannot be made non-extensible', async () => { + getAccountMock.mockResolvedValue(buildAccount(PATH_DEFAULT, { + _signer: { signTransaction: jest.fn() } + })) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerPolicy(projectAllowAll('p')) + + const account = await wdk.getAccount('ethereum', 0) + + expect(() => Object.preventExtensions(account)).toThrow(TypeError) + expect(() => Object.freeze(account)).toThrow(TypeError) + expect(Object.getOwnPropertyDescriptor(account, '_signer')).toBeUndefined() + expect(Reflect.ownKeys(account)).not.toContain('_signer') + }) + + test('the simulation mirror is reported as a member of a governed account', async () => { + getAccountMock.mockResolvedValue(buildAccount()) + + wdk + .registerWallet('ethereum', WalletManagerMock, {}) + .registerPolicy(projectAllowAll('p')) + + const account = await wdk.getAccount('ethereum', 0) + + expect('simulate' in account).toBe(true) + }) + }) + // ------------------------------------------------------------------------- // PolicyViolationError shape // -------------------------------------------------------------------------