From 8defd77e9e07dca6683fdf937eb761dc6fb3674f Mon Sep 17 00:00:00 2001 From: Jason Park Date: Fri, 11 Sep 2026 17:12:27 +0000 Subject: [PATCH 1/9] fix: reject vaults with non-canonical permissions --- .github/workflows/ci.yml | 2 +- .../migration.sql | 3 + backend/prisma/schema.prisma | 1 + backend/prisma/schema.sqlite.prisma | 1 + backend/scripts/seed-fixtures.ts | 1 + backend/src/indexer.ts | 62 +++++- backend/src/mina-client.ts | 64 ++++++- backend/src/routes.ts | 126 +++++++++---- .../tests/indexer-archive-discovery.test.ts | 54 +++++- .../src/tests/indexer-autosubscribe.test.ts | 56 ++++++ backend/src/tests/indexer-dropped-tx.test.ts | 6 +- .../src/tests/indexer-event-decode.test.ts | 4 +- backend/src/tests/indexer-reorg.test.ts | 2 +- backend/src/tests/routes-api.test.ts | 17 +- backend/src/tests/routes-subscribe.test.ts | 95 +++++++++- backend/src/tests/vault-security.test.ts | 54 ++++++ backend/src/vault-security.ts | 111 +++++++++++ contracts/src/MinaGuard.ts | 26 +-- contracts/src/guard-permissions.ts | 64 +++++++ contracts/src/index.ts | 6 + docs/backend-audit-guide.md | 41 ++-- docs/contracts-audit-guide.md | 31 ++- docs/security-audit-guide.md | 13 +- docs/ui-audit-guide.md | 10 +- e2e/ui/seed.ts | 1 + ui/app/accounts/[address]/page.tsx | 28 ++- ui/app/transactions/[id]/page.tsx | 61 +++++- ui/app/transactions/new/page.tsx | 45 ++++- ui/hooks/useVaultSecurity.ts | 35 ++++ ui/lib/api.ts | 177 ++++++++++++++++++ ui/lib/types.ts | 2 + 31 files changed, 1075 insertions(+), 124 deletions(-) create mode 100644 backend/prisma/migrations/20260911000000_verify_contract_permissions/migration.sql create mode 100644 backend/src/tests/vault-security.test.ts create mode 100644 backend/src/vault-security.ts create mode 100644 contracts/src/guard-permissions.ts create mode 100644 ui/hooks/useVaultSecurity.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fb660482..3d527019 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,7 +66,7 @@ jobs: CI: true DATABASE_URL: postgresql://postgres:postgres@localhost:5432/minaguard - name: Run backend tests - # All ten suites against a fresh Postgres (135 tests). Test files that + # All backend suites against a fresh Postgres. Test files that # mock.module mina-client re-register the real module in afterAll — # bun module mocks are process-global, and CI's file order differs # from local, so a leaked stub broke mina-client-tx-status here. diff --git a/backend/prisma/migrations/20260911000000_verify_contract_permissions/migration.sql b/backend/prisma/migrations/20260911000000_verify_contract_permissions/migration.sql new file mode 100644 index 00000000..adcce0fc --- /dev/null +++ b/backend/prisma/migrations/20260911000000_verify_contract_permissions/migration.sql @@ -0,0 +1,3 @@ +ALTER TABLE "Contract" +ADD COLUMN "permissionsVerified" BOOLEAN NOT NULL DEFAULT false; + diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index 4fc36227..bff3460c 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -23,6 +23,7 @@ model Contract { address String @unique parent String? ready Boolean @default(false) + permissionsVerified Boolean @default(false) discoveredAt DateTime @default(now()) discoveredAtBlock Int? lastSyncedAt DateTime? diff --git a/backend/prisma/schema.sqlite.prisma b/backend/prisma/schema.sqlite.prisma index cfa81431..dfe80624 100644 --- a/backend/prisma/schema.sqlite.prisma +++ b/backend/prisma/schema.sqlite.prisma @@ -23,6 +23,7 @@ model Contract { address String @unique parent String? ready Boolean @default(false) + permissionsVerified Boolean @default(false) discoveredAt DateTime @default(now()) discoveredAtBlock Int? lastSyncedAt DateTime? diff --git a/backend/scripts/seed-fixtures.ts b/backend/scripts/seed-fixtures.ts index b26663b3..37268ab0 100644 --- a/backend/scripts/seed-fixtures.ts +++ b/backend/scripts/seed-fixtures.ts @@ -110,6 +110,7 @@ async function seedVault(spec: VaultSpec, walletAddresses: string[]): Promise 0 + ) { + console.warn( + `[indexer] refusing ${address}: non-canonical permissions (${security.permissionMismatches.join( + ', ' + )})` + ); + } continue; } const created = await prisma.contract.create({ - data: { address, discoveredAtBlock: deployBlock }, + data: { + address, + discoveredAtBlock: deployBlock, + permissionsVerified: true, + }, }); await this.backfillContract(created.id, address); @@ -426,6 +438,38 @@ export class MinaGuardIndexer { fromHeight: number, toHeight: number ): Promise { + const tracked = await prisma.contract.findUnique({ + where: { id: contractId }, + select: { permissionsVerified: true }, + }); + if (!tracked?.permissionsVerified) { + const security = await fetchVaultSecurityStatus( + address, + this.config.minaguardVkHash + ); + if (!security.accountFound) return; + if (!security.safe) { + await prisma.contract.update({ + where: { id: contractId }, + data: { ready: false, permissionsVerified: false }, + }); + console.warn( + `[indexer] refusing ${address}: ${ + security.verificationKeyMatches + ? `non-canonical permissions (${security.permissionMismatches.join( + ', ' + )})` + : 'verification key mismatch' + }` + ); + return; + } + await prisma.contract.update({ + where: { id: contractId }, + data: { permissionsVerified: true }, + }); + } + const rawEvents = await fetchDecodedContractEvents(address, fromHeight, toHeight); // o1js fetchEvents returns events within a single tx in *reverse* emission diff --git a/backend/src/mina-client.ts b/backend/src/mina-client.ts index 9075c708..b594214d 100644 --- a/backend/src/mina-client.ts +++ b/backend/src/mina-client.ts @@ -1,7 +1,11 @@ import { Mina, PublicKey, fetchAccount, UInt32 } from 'o1js'; -import { MinaGuard } from 'contracts'; +import { GUARD_PERMISSION_KINDS, MinaGuard } from 'contracts'; import type { Pool } from 'pg'; import type { BackendConfig, } from './config.js'; +import { + validatePermissionVector, + type PermissionKindVector, +} from './vault-security.js'; const EMPTY_PUBLIC_KEY = PublicKey.empty().toBase58(); @@ -308,6 +312,64 @@ export async function fetchVerificationKeyHash(address: string): Promise { + const pub = PublicKey.fromBase58(address); + const accountResult = await fetchAccount({ publicKey: pub }); + const account = accountResult.account as any; + if (!account) { + return { + accountFound: false, + verificationKeyHash: null, + verificationKeyMatches: false, + permissionKinds: {}, + expectedPermissionKinds: GUARD_PERMISSION_KINDS, + permissionMismatches: [], + safe: false, + }; + } + + const verificationKeyHash = + account.zkapp?.verificationKey?.hash?.toString() ?? + account.verificationKey?.hash?.toString() ?? + null; + const verificationKeyMatches = + verificationKeyHash !== null && + (expectedVerificationKeyHash === null || + verificationKeyHash === expectedVerificationKeyHash); + const { permissionKinds, mismatches } = validatePermissionVector( + account.permissions + ); + + return { + accountFound: true, + verificationKeyHash, + verificationKeyMatches, + permissionKinds, + expectedPermissionKinds: GUARD_PERMISSION_KINDS, + permissionMismatches: mismatches, + safe: verificationKeyMatches && mismatches.length === 0, + }; +} + /** Reads the current on-chain MinaGuard state needed by the backend/indexer. * Values are normalized for persistence: the empty-pubkey sentinel used by * root contracts for `parent` is flattened to null so callers don't need to diff --git a/backend/src/routes.ts b/backend/src/routes.ts index bf27d1f0..bc7c86c2 100644 --- a/backend/src/routes.ts +++ b/backend/src/routes.ts @@ -1,11 +1,16 @@ import { Router } from 'express'; import { z } from 'zod'; import { PublicKey, fetchAccount } from 'o1js'; +import { GUARD_PERMISSION_KINDS } from 'contracts'; import { prisma } from './db.js'; import { deleteContract, type MinaGuardIndexer } from './indexer.js'; import type { BackendConfig } from './config.js'; -import { fetchLatestBlockHeight, fetchVerificationKeyHash, fetchZkappTxStatus } from './mina-client.js'; +import { + fetchLatestBlockHeight, + fetchVaultSecurityStatus, + fetchZkappTxStatus, +} from './mina-client.js'; import { serializeProposalRecord, type ContractState } from './proposal-record.js'; import { acquireLightnetAccount, @@ -89,10 +94,47 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi res.json(result); })); + /** Performs a live VK + complete permission-vector check for any address. + * CREATE_CHILD approvers use this even when the child is intentionally not + * present in the accepted-contract index. */ + router.get( + '/api/accounts/:address/security', + addressParamsMiddleware, + safe(async (req, res) => { + if (!config) { + res.status(503).json({ error: 'Backend config unavailable' }); + return; + } + const { address } = addressParamsSchema.parse(req.params) as AddressParams; + // The deterministic UI harness deliberately has no chain endpoint. It + // may synthesize only the canonical snapshot for a fixture that was + // explicitly seeded as permission-verified; production never takes + // this branch because INDEXER_DISABLED is a test-only setting. + if (config.indexerDisabled) { + const fixture = await prisma.contract.findUnique({ + where: { address }, + select: { permissionsVerified: true }, + }); + const accepted = fixture?.permissionsVerified === true; + res.json({ + accountFound: accepted, + verificationKeyHash: accepted ? config.minaguardVkHash : null, + verificationKeyMatches: accepted, + permissionKinds: accepted ? GUARD_PERMISSION_KINDS : {}, + expectedPermissionKinds: GUARD_PERMISSION_KINDS, + permissionMismatches: accepted ? [] : Object.keys(GUARD_PERMISSION_KINDS), + safe: accepted, + }); + return; + } + res.json(await fetchVaultSecurityStatus(address, config.minaguardVkHash)); + }) + ); + /** Lists tracked contracts with derived config + aggregate counts. */ router.get('/api/contracts', safe(async (_req, res) => { const contracts = await prisma.contract.findMany({ - where: { ready: true }, + where: { ready: true, permissionsVerified: true }, orderBy: { discoveredAt: 'desc' }, include: { _count: { @@ -133,7 +175,7 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi }, }); - if (!contract || !contract.ready) { + if (!contract || !contract.ready || !contract.permissionsVerified) { res.status(404).json({ error: 'Contract not found' }); return; } @@ -150,7 +192,7 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi const { address } = addressParamsSchema.parse(req.params) as AddressParams; const children = await prisma.contract.findMany({ - where: { parent: address, ready: true }, + where: { parent: address, ready: true, permissionsVerified: true }, orderBy: { discoveredAt: 'asc' }, }); @@ -175,10 +217,10 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi const contract = await prisma.contract.findUnique({ where: { address }, - select: { id: true, ready: true }, + select: { id: true, ready: true, permissionsVerified: true }, }); - if (!contract || !contract.ready) { + if (!contract || !contract.ready || !contract.permissionsVerified) { res.status(404).json({ error: 'Contract not found' }); return; } @@ -199,10 +241,10 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi const contract = await prisma.contract.findUnique({ where: { address }, - select: { id: true, ready: true }, + select: { id: true, ready: true, permissionsVerified: true }, }); - if (!contract || !contract.ready) { + if (!contract || !contract.ready || !contract.permissionsVerified) { res.status(404).json({ error: 'Contract not found' }); return; } @@ -261,10 +303,10 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi const contract = await prisma.contract.findUnique({ where: { address }, - select: { id: true, ready: true }, + select: { id: true, ready: true, permissionsVerified: true }, }); - if (!contract || !contract.ready) { + if (!contract || !contract.ready || !contract.permissionsVerified) { res.status(404).json({ error: 'Contract not found' }); return; } @@ -315,9 +357,9 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi const contract = await prisma.contract.findUnique({ where: { address }, - select: { id: true }, + select: { id: true, ready: true, permissionsVerified: true }, }); - if (!contract) { + if (!contract || !contract.ready || !contract.permissionsVerified) { res.status(404).json({ error: 'Contract not found' }); return; } @@ -348,10 +390,10 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi const contract = await prisma.contract.findUnique({ where: { address }, - select: { id: true, ready: true }, + select: { id: true, ready: true, permissionsVerified: true }, }); - if (!contract || !contract.ready) { + if (!contract || !contract.ready || !contract.permissionsVerified) { res.status(404).json({ error: 'Contract not found' }); return; } @@ -391,10 +433,10 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi const contract = await prisma.contract.findUnique({ where: { address }, - select: { id: true, ready: true }, + select: { id: true, ready: true, permissionsVerified: true }, }); - if (!contract || !contract.ready) { + if (!contract || !contract.ready || !contract.permissionsVerified) { res.status(404).json({ error: 'Contract not found' }); return; } @@ -577,33 +619,49 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi fromBlockNum = fromBlock; } - const existing = await prisma.contract.findUnique({ where: { address } }); - if (existing) { - res.json(existing); - return; - } - - // VK lookup only on the manual path. The auto-subscribe path races the - // deploy tx (account still in mempool → VK null), so we skip it; the - // unready rescan validates once the deploy lands. + // Live account authentication only on the manual path. The auto-subscribe + // path races the deploy tx (account still in mempool), so it creates an + // unverified row; syncSingleContract performs this same fail-closed check + // before it can mark that row ready. + let permissionsVerified = false; if (fromBlockNum !== null) { - const verificationKeyHash = await fetchVerificationKeyHash(address); - if (!verificationKeyHash) { + const security = await fetchVaultSecurityStatus( + address, + config.minaguardVkHash + ); + if (!security.accountFound || !security.verificationKeyHash) { res.status(404).json({ error: 'Account not found on-chain or not a zkApp' }); return; } - // Reject a VK from a different MinaGuard release — its proofs fail - // on-chain. No-op when minaguardVkHash is unset. Mirrors indexer.ts. - if ( - config?.minaguardVkHash && - verificationKeyHash !== config.minaguardVkHash - ) { + if (!security.verificationKeyMatches) { res.status(400).json({ error: 'Contract verification key does not match this app version. ' + 'It was likely deployed with a different MinaGuard release.', }); return; } + if (security.permissionMismatches.length > 0) { + res.status(400).json({ + error: `Contract has non-canonical MinaGuard permissions: ${security.permissionMismatches.join( + ', ' + )}`, + }); + return; + } + permissionsVerified = true; + } + + const existing = await prisma.contract.findUnique({ where: { address } }); + if (existing) { + const accepted = + permissionsVerified && !existing.permissionsVerified + ? await prisma.contract.update({ + where: { id: existing.id }, + data: { permissionsVerified: true }, + }) + : existing; + res.json(accepted); + return; } // Safety margin on the default path: the UI calls subscribe right @@ -617,7 +675,7 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi Math.max(0, (await fetchLatestBlockHeight(config)) - SUBSCRIBE_MARGIN); const created = await prisma.contract.create({ - data: { address, discoveredAtBlock }, + data: { address, discoveredAtBlock, permissionsVerified }, }); res.json(created); diff --git a/backend/src/tests/indexer-archive-discovery.test.ts b/backend/src/tests/indexer-archive-discovery.test.ts index bb482cf3..8528fc13 100644 --- a/backend/src/tests/indexer-archive-discovery.test.ts +++ b/backend/src/tests/indexer-archive-discovery.test.ts @@ -4,8 +4,18 @@ import type { BackendConfig } from '../config.js'; import { prisma } from '../db.js'; import { MinaGuardIndexer } from '../indexer.js'; import { stubMinaClient } from './stub-mina-client.js'; +import { GUARD_PERMISSION_KINDS } from 'contracts'; const VK_HASH = '22592591136635241954458728867125272730912271761728581931779127524287952990537'; +const securityStatus = (verificationKeyHash = VK_HASH) => ({ + accountFound: true, + verificationKeyHash, + verificationKeyMatches: verificationKeyHash === VK_HASH, + permissionKinds: GUARD_PERMISSION_KINDS, + expectedPermissionKinds: GUARD_PERMISSION_KINDS, + permissionMismatches: [], + safe: verificationKeyHash === VK_HASH, +}); const archiveConfig = { minaEndpoint: 'http://stub', @@ -84,7 +94,7 @@ describe('archive discovery: happy path', () => { return candidates; }, // All 3 candidates verify with the matching MinaGuard VK on-chain. - fetchVerificationKeyHash: async () => VK_HASH, + fetchVaultSecurityStatus: async () => securityStatus(), // Backfill is a no-op (no events emitted yet). fetchDecodedContractEvents: async () => [], })); @@ -127,8 +137,8 @@ describe('archive discovery: happy path', () => { { address: matching, deployBlock: 200 }, { address: mismatched, deployBlock: 300 }, ], - fetchVerificationKeyHash: async (addr: string) => - addr === matching ? VK_HASH : 'different-vk-hash', + fetchVaultSecurityStatus: async (addr: string) => + securityStatus(addr === matching ? VK_HASH : 'different-vk-hash'), fetchDecodedContractEvents: async () => [], })); @@ -139,6 +149,34 @@ describe('archive discovery: happy path', () => { const contracts = await prisma.contract.findMany(); expect(contracts.map((c) => c.address)).toEqual([matching]); }); + + test('skips a canonical VK whose on-chain send permission is Either', async () => { + const address = PrivateKey.random().toPublicKey().toBase58(); + stubMinaClient(() => ({ + fetchGenesisConstants: async () => ({ + genesisTimestampMs: 0, + slotDurationMs: 90000, + }), + fetchLatestBlockHeightFromArchive: async () => 500, + fetchBestChainHeaders: async () => [], + discoverCandidateAddressesFromArchive: async () => [ + { address, deployBlock: 200 }, + ], + fetchVaultSecurityStatus: async () => ({ + ...securityStatus(), + permissionKinds: { ...GUARD_PERMISSION_KINDS, send: 'Either' }, + permissionMismatches: ['send'], + safe: false, + }), + fetchDecodedContractEvents: async () => [], + })); + + const indexer = new MinaGuardIndexer(archiveConfig); + await indexer.start(); + indexer.stop(); + + expect(await prisma.contract.findUnique({ where: { address } })).toBeNull(); + }); }); describe('archive discovery: per-iteration failure isolation', () => { @@ -161,9 +199,9 @@ describe('archive discovery: per-iteration failure isolation', () => { { address: bad, deployBlock: 200 }, { address: ok2, deployBlock: 300 }, ], - fetchVerificationKeyHash: async (addr: string) => { + fetchVaultSecurityStatus: async (addr: string) => { if (addr === bad) throw new Error('simulated daemon hiccup on VK fetch'); - return VK_HASH; + return securityStatus(); }, fetchDecodedContractEvents: async () => [], })); @@ -207,7 +245,7 @@ describe('archive discovery: per-iteration failure isolation', () => { { address: bad, deployBlock: 100 }, { address: ok, deployBlock: 200 }, ], - fetchVerificationKeyHash: async () => VK_HASH, + fetchVaultSecurityStatus: async () => securityStatus(), fetchDecodedContractEvents: async (addr: string) => { if (addr === bad) { backfillCallCountForBad += 1; @@ -261,7 +299,7 @@ describe('archive discovery: backfill range', () => { fetchLatestBlockHeightFromArchive: async () => 50_000, fetchBestChainHeaders: async () => [], discoverCandidateAddressesFromArchive: async () => [{ address: addr, deployBlock: 12_345 }], - fetchVerificationKeyHash: async () => VK_HASH, + fetchVaultSecurityStatus: async () => securityStatus(), fetchDecodedContractEvents: async (_addr: string, from: number, to: number) => { backfillCalls.push({ from, to }); return []; @@ -302,7 +340,7 @@ describe('archive discovery: cursor progression', () => { calls.push({ from, to }); return []; }, - fetchVerificationKeyHash: async () => VK_HASH, + fetchVaultSecurityStatus: async () => securityStatus(), fetchDecodedContractEvents: async () => [], })); diff --git a/backend/src/tests/indexer-autosubscribe.test.ts b/backend/src/tests/indexer-autosubscribe.test.ts index ac1c4bee..b13db0a1 100644 --- a/backend/src/tests/indexer-autosubscribe.test.ts +++ b/backend/src/tests/indexer-autosubscribe.test.ts @@ -4,6 +4,7 @@ import type { BackendConfig } from '../config.js'; import { prisma } from '../db.js'; import { MinaGuardIndexer } from '../indexer.js'; import { stubMinaClient } from './stub-mina-client.js'; +import { GUARD_PERMISSION_KINDS } from 'contracts'; const liteConfig = { minaEndpoint: 'http://stub', @@ -42,6 +43,17 @@ async function setCursor(height: number) { beforeEach(async () => { await clearAll(); + stubMinaClient(() => ({ + fetchVaultSecurityStatus: async () => ({ + accountFound: true, + verificationKeyHash: 'vk-hash-stub', + verificationKeyMatches: true, + permissionKinds: GUARD_PERMISSION_KINDS, + expectedPermissionKinds: GUARD_PERMISSION_KINDS, + permissionMismatches: [], + safe: true, + }), + })); }); afterEach(() => { @@ -413,6 +425,50 @@ describe('tick: rescanUnreadyContracts', () => { const updated = await prisma.contract.findUniqueOrThrow({ where: { id: contract.id } }); expect(updated.ready).toBe(true); + expect(updated.permissionsVerified).toBe(true); + }); + + test('never readies a vault with the canonical VK but weakened permissions', async () => { + const address = PrivateKey.random().toPublicKey().toBase58(); + const contract = await prisma.contract.create({ + data: { address, ready: false, discoveredAtBlock: 50 }, + }); + await setCursor(100); + + let fetchedEvents = false; + stubMinaClient(() => ({ + fetchGenesisConstants: async () => ({ + genesisTimestampMs: 0, + slotDurationMs: 90000, + }), + fetchLatestBlockHeight: async () => 100, + fetchBestChainHeaders: async () => [], + fetchVaultSecurityStatus: async () => ({ + accountFound: true, + verificationKeyHash: 'vk-hash-stub', + verificationKeyMatches: true, + permissionKinds: { ...GUARD_PERMISSION_KINDS, send: 'Either' as const }, + expectedPermissionKinds: GUARD_PERMISSION_KINDS, + permissionMismatches: ['send'], + safe: false, + }), + fetchDecodedContractEvents: async () => { + fetchedEvents = true; + return [makeExecutionEvent('must-not-ingest', 60)]; + }, + })); + + await runSingleTick(liteConfig); + + const updated = await prisma.contract.findUniqueOrThrow({ + where: { id: contract.id }, + }); + expect(updated.ready).toBe(false); + expect(updated.permissionsVerified).toBe(false); + expect(fetchedEvents).toBe(false); + expect( + await prisma.eventRaw.count({ where: { contractId: contract.id } }) + ).toBe(0); }); test('uses discoveredAtBlock as the lower bound of the rescan range', async () => { diff --git a/backend/src/tests/indexer-dropped-tx.test.ts b/backend/src/tests/indexer-dropped-tx.test.ts index f2cb6f6e..f8228d71 100644 --- a/backend/src/tests/indexer-dropped-tx.test.ts +++ b/backend/src/tests/indexer-dropped-tx.test.ts @@ -45,7 +45,11 @@ async function seedPendingProposal( txHash: string, ): Promise { const contract = await prisma.contract.create({ - data: { address: PrivateKey.random().toPublicKey().toBase58(), ready: true }, + data: { + address: PrivateKey.random().toPublicKey().toBase58(), + ready: true, + permissionsVerified: true, + }, }); const proposal = await prisma.proposal.create({ data: { diff --git a/backend/src/tests/indexer-event-decode.test.ts b/backend/src/tests/indexer-event-decode.test.ts index 97a0472d..56c86d8f 100644 --- a/backend/src/tests/indexer-event-decode.test.ts +++ b/backend/src/tests/indexer-event-decode.test.ts @@ -56,7 +56,9 @@ async function ingest( const address = opts.address ?? PrivateKey.random().toPublicKey().toBase58(); const toHeight = opts.toHeight ?? 20; const contract = await prisma.contract.create({ - data: { address, discoveredAtBlock: 1 }, + // Permission admission is covered separately; these tests exercise only + // event decoding after a vault has passed that boundary. + data: { address, discoveredAtBlock: 1, permissionsVerified: true }, }); stubMinaClient(() => ({ fetchDecodedContractEvents: async () => events, diff --git a/backend/src/tests/indexer-reorg.test.ts b/backend/src/tests/indexer-reorg.test.ts index 405e75ff..7665221a 100644 --- a/backend/src/tests/indexer-reorg.test.ts +++ b/backend/src/tests/indexer-reorg.test.ts @@ -303,7 +303,7 @@ describe('reconstruction after rollback', () => { const ownerB = PrivateKey.random().toPublicKey().toBase58(); const contract = await prisma.contract.create({ - data: { address, discoveredAtBlock: 4 }, + data: { address, discoveredAtBlock: 4, permissionsVerified: true }, }); const indexer = new MinaGuardIndexer(stubConfig); diff --git a/backend/src/tests/routes-api.test.ts b/backend/src/tests/routes-api.test.ts index 5bffdaaa..0210895c 100644 --- a/backend/src/tests/routes-api.test.ts +++ b/backend/src/tests/routes-api.test.ts @@ -58,14 +58,19 @@ async function clearDatabase() { async function seedDatabase() { await prisma.contract.createMany({ data: [ - { address: contractAddress, ready: true }, - { address: otherContractAddress, ready: true }, + { address: contractAddress, ready: true, permissionsVerified: true }, + { address: otherContractAddress, ready: true, permissionsVerified: true }, // Two subaccounts of `contractAddress`. childTwo has multi-sig disabled // (e.g. after a destroy) so the API exposes both states. - { address: childOneAddress, parent: contractAddress, ready: true }, - { address: childTwoAddress, parent: contractAddress, ready: true }, - { address: invalidStateContractAddress, ready: true }, - { address: invalidStateChildAddress, parent: invalidStateContractAddress, ready: true }, + { address: childOneAddress, parent: contractAddress, ready: true, permissionsVerified: true }, + { address: childTwoAddress, parent: contractAddress, ready: true, permissionsVerified: true }, + { address: invalidStateContractAddress, ready: true, permissionsVerified: true }, + { + address: invalidStateChildAddress, + parent: invalidStateContractAddress, + ready: true, + permissionsVerified: true, + }, ], }); diff --git a/backend/src/tests/routes-subscribe.test.ts b/backend/src/tests/routes-subscribe.test.ts index a95a0cb1..ff2652ac 100644 --- a/backend/src/tests/routes-subscribe.test.ts +++ b/backend/src/tests/routes-subscribe.test.ts @@ -7,6 +7,7 @@ import { prisma } from '../db.js'; import type { MinaGuardIndexer } from '../indexer.js'; import { stubMinaClient } from './stub-mina-client.js'; import { createApiRouter } from '../routes.js'; +import { GUARD_PERMISSION_KINDS } from 'contracts'; let server: Server; let baseUrl = ''; @@ -17,6 +18,15 @@ const subscribedAddress = PrivateKey.random().toPublicKey().toBase58(); const liteConfig = { indexerMode: 'lite' } as unknown as BackendConfig; const fullConfig = { indexerMode: 'full' } as unknown as BackendConfig; +const safeSecurity = { + accountFound: true, + verificationKeyHash: 'vk-hash-stub', + verificationKeyMatches: true, + permissionKinds: GUARD_PERMISSION_KINDS, + expectedPermissionKinds: GUARD_PERMISSION_KINDS, + permissionMismatches: [], + safe: true, +}; async function clearDatabase() { await prisma.approval.deleteMany(); @@ -74,7 +84,7 @@ beforeAll(async () => { // the "not a zkApp" test overrides this to null. stubMinaClient(() => ({ fetchLatestBlockHeight: async () => 0, - fetchVerificationKeyHash: async () => 'vk-hash-stub', + fetchVaultSecurityStatus: async () => safeSecurity, })); ({ server, baseUrl } = await startServer(liteConfig)); @@ -87,7 +97,7 @@ afterEach(async () => { mock.restore(); stubMinaClient(() => ({ fetchLatestBlockHeight: async () => 0, - fetchVerificationKeyHash: async () => 'vk-hash-stub', + fetchVaultSecurityStatus: async () => safeSecurity, })); }); @@ -155,7 +165,7 @@ describe('POST /api/subscribe', () => { fetchLatestCalls += 1; return 9999; }, - fetchVerificationKeyHash: async () => 'vk-hash-stub', + fetchVaultSecurityStatus: async () => safeSecurity, })); const res = await post('/api/subscribe', { address: subscribedAddress, fromBlock: 0 }); @@ -170,7 +180,13 @@ describe('POST /api/subscribe', () => { test('rejects explicit fromBlock when address is not a deployed zkApp (manual add-existing path)', async () => { stubMinaClient(() => ({ fetchLatestBlockHeight: async () => 0, - fetchVerificationKeyHash: async () => null, + fetchVaultSecurityStatus: async () => ({ + ...safeSecurity, + accountFound: false, + verificationKeyHash: null, + verificationKeyMatches: false, + safe: false, + }), })); const res = await post('/api/subscribe', { address: subscribedAddress, fromBlock: 0 }); @@ -187,9 +203,9 @@ describe('POST /api/subscribe', () => { let vkCalls = 0; stubMinaClient(() => ({ fetchLatestBlockHeight: async () => 0, - fetchVerificationKeyHash: async () => { + fetchVaultSecurityStatus: async () => { vkCalls += 1; - return null; + return safeSecurity; }, })); @@ -209,6 +225,32 @@ describe('POST /api/subscribe', () => { expect(stored?.discoveredAtBlock).toBe(500); }); + test('rejects a canonical VK with a weakened send permission', async () => { + stubMinaClient(() => ({ + fetchVaultSecurityStatus: async () => ({ + ...safeSecurity, + permissionKinds: { + ...GUARD_PERMISSION_KINDS, + send: 'Either', + }, + permissionMismatches: ['send'], + safe: false, + }), + })); + + const res = await post('/api/subscribe', { + address: subscribedAddress, + fromBlock: 0, + }); + expect(res.status).toBe(400); + expect((await res.json()).error).toContain('send'); + expect( + await prisma.contract.findUnique({ + where: { address: subscribedAddress }, + }) + ).toBeNull(); + }); + test('rejects negative fromBlock', async () => { const res = await post('/api/subscribe', { address: subscribedAddress, fromBlock: -1 }); expect(res.status).toBe(400); @@ -351,7 +393,9 @@ describe('ready flag visibility', () => { test('GET /api/contracts hides unready rows', async () => { const readyAddress = PrivateKey.random().toPublicKey().toBase58(); const unreadyAddress = PrivateKey.random().toPublicKey().toBase58(); - await prisma.contract.create({ data: { address: readyAddress, ready: true } }); + await prisma.contract.create({ + data: { address: readyAddress, ready: true, permissionsVerified: true }, + }); await prisma.contract.create({ data: { address: unreadyAddress, ready: false } }); const res = await fetch(`${baseUrl}/api/contracts`); @@ -362,6 +406,32 @@ describe('ready flag visibility', () => { expect(addresses).not.toContain(unreadyAddress); }); + test('GET /api/contracts hides ready rows without verified permissions', async () => { + const verifiedAddress = PrivateKey.random().toPublicKey().toBase58(); + const unverifiedAddress = PrivateKey.random().toPublicKey().toBase58(); + await prisma.contract.create({ + data: { + address: verifiedAddress, + ready: true, + permissionsVerified: true, + }, + }); + await prisma.contract.create({ + data: { + address: unverifiedAddress, + ready: true, + permissionsVerified: false, + }, + }); + + const res = await fetch(`${baseUrl}/api/contracts`); + expect(res.status).toBe(200); + const body = (await res.json()) as Array<{ address: string }>; + const addresses = body.map((contract) => contract.address); + expect(addresses).toContain(verifiedAddress); + expect(addresses).not.toContain(unverifiedAddress); + }); + test('GET /api/contracts/:address returns 404 for unready rows', async () => { const unreadyAddress = PrivateKey.random().toPublicKey().toBase58(); await prisma.contract.create({ data: { address: unreadyAddress, ready: false } }); @@ -375,9 +445,16 @@ describe('ready flag visibility', () => { const readyChild = PrivateKey.random().toPublicKey().toBase58(); const unreadyChild = PrivateKey.random().toPublicKey().toBase58(); - await prisma.contract.create({ data: { address: parentAddress, ready: true } }); await prisma.contract.create({ - data: { address: readyChild, parent: parentAddress, ready: true }, + data: { address: parentAddress, ready: true, permissionsVerified: true }, + }); + await prisma.contract.create({ + data: { + address: readyChild, + parent: parentAddress, + ready: true, + permissionsVerified: true, + }, }); await prisma.contract.create({ data: { address: unreadyChild, parent: parentAddress, ready: false }, diff --git a/backend/src/tests/vault-security.test.ts b/backend/src/tests/vault-security.test.ts new file mode 100644 index 00000000..50ab1e9e --- /dev/null +++ b/backend/src/tests/vault-security.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'bun:test'; +import { Permissions } from 'o1js'; +import { + GUARD_PERMISSION_KINDS, + GUARD_PERMISSION_NAMES, + GUARD_PERMISSIONS, +} from 'contracts'; +import { + permissionKindVector, + permissionMismatches, + validatePermissionVector, +} from '../vault-security.js'; + +describe('canonical MinaGuard permissions', () => { + it('accepts every field of the canonical vector', () => { + const result = validatePermissionVector(GUARD_PERMISSIONS); + + expect(result.mismatches).toEqual([]); + expect(result.permissionKinds).toEqual(GUARD_PERMISSION_KINDS); + expect(Object.keys(result.permissionKinds).sort()).toEqual( + [...GUARD_PERMISSION_NAMES].sort() + ); + }); + + it('rejects the hidden send: Either withdrawal permission', () => { + const altered = { + ...GUARD_PERMISSIONS, + send: Permissions.proofOrSignature(), + }; + + expect(validatePermissionVector(altered).mismatches).toEqual(['send']); + }); + + it('fails closed when any permission field is absent', () => { + const actual = permissionKindVector(GUARD_PERMISSIONS); + delete actual.access; + + expect(permissionMismatches(actual)).toEqual(['access']); + }); + + it('rejects a non-canonical verification-key transaction version', () => { + const altered = { + ...GUARD_PERMISSIONS, + setVerificationKey: { + ...GUARD_PERMISSIONS.setVerificationKey, + txnVersion: GUARD_PERMISSIONS.setVerificationKey.txnVersion.add(1), + }, + }; + + expect(validatePermissionVector(altered).mismatches).toEqual([ + 'setVerificationKey', + ]); + }); +}); diff --git a/backend/src/vault-security.ts b/backend/src/vault-security.ts new file mode 100644 index 00000000..421b9707 --- /dev/null +++ b/backend/src/vault-security.ts @@ -0,0 +1,111 @@ +import { + GUARD_PERMISSION_KINDS, + GUARD_PERMISSION_NAMES, + GUARD_PERMISSIONS, + type GuardPermissionName, +} from 'contracts'; + +export type PermissionKind = + | 'None' + | 'Either' + | 'Proof' + | 'Signature' + | 'Impossible'; + +export type PermissionKindVector = Partial< + Record +>; + +type BoolLike = boolean | { toBoolean(): boolean }; +type PermissionLike = { + constant?: BoolLike; + signatureNecessary?: BoolLike; + signatureSufficient?: BoolLike; +}; + +function readBool(value: BoolLike | undefined): boolean | null { + if (typeof value === 'boolean') return value; + if (value && typeof value.toBoolean === 'function') return value.toBoolean(); + return null; +} + +/** Converts o1js' three-bit AuthRequired representation to its protocol name. */ +export function permissionKind(permission: unknown): PermissionKind | null { + const value = permission as PermissionLike | undefined; + const constant = readBool(value?.constant); + const necessary = readBool(value?.signatureNecessary); + const sufficient = readBool(value?.signatureSufficient); + if (constant === null || necessary === null || sufficient === null) + return null; + + if (constant && necessary && !sufficient) return 'Impossible'; + if (constant && !necessary && sufficient) return 'None'; + if (!constant && !necessary && !sufficient) return 'Proof'; + if (!constant && necessary && sufficient) return 'Signature'; + if (!constant && !necessary && sufficient) return 'Either'; + return null; +} + +/** Normalizes all permission fields for API clients and audit diagnostics. */ +export function permissionKindVector( + permissions: unknown +): PermissionKindVector { + if (!permissions || typeof permissions !== 'object') return {}; + const source = permissions as Record; + const result: PermissionKindVector = {}; + + for (const name of GUARD_PERMISSION_NAMES) { + const raw = + name === 'setVerificationKey' + ? (source[name] as { auth?: unknown } | undefined)?.auth ?? source[name] + : source[name]; + const kind = permissionKind(raw); + if (kind !== null) result[name] = kind; + } + return result; +} + +/** Returns every missing or non-canonical permission field. */ +export function permissionMismatches( + actual: PermissionKindVector +): GuardPermissionName[] { + return GUARD_PERMISSION_NAMES.filter( + (name) => actual[name] !== GUARD_PERMISSION_KINDS[name] + ); +} + +function transactionVersion(value: unknown): string | null { + if (value === null || value === undefined) return null; + if (typeof value === 'object' && 'toString' in value) { + return String((value as { toString(): string }).toString()); + } + return String(value); +} + +/** + * Compares the complete o1js permission vector, including `access` and the + * setVerificationKey transaction-version guard. + */ +export function validatePermissionVector(permissions: unknown): { + permissionKinds: PermissionKindVector; + mismatches: GuardPermissionName[]; +} { + const permissionKinds = permissionKindVector(permissions); + const mismatches = permissionMismatches(permissionKinds); + const actualVkVersion = transactionVersion( + (permissions as { setVerificationKey?: { txnVersion?: unknown } } | null) + ?.setVerificationKey?.txnVersion + ); + const expectedVkVersion = transactionVersion( + GUARD_PERMISSIONS.setVerificationKey.txnVersion + ); + + if ( + actualVkVersion !== expectedVkVersion && + !mismatches.includes('setVerificationKey') + ) { + mismatches.push('setVerificationKey'); + } + + return { permissionKinds, mismatches }; +} diff --git a/contracts/src/MinaGuard.ts b/contracts/src/MinaGuard.ts index d7de20e7..ee9ed596 100644 --- a/contracts/src/MinaGuard.ts +++ b/contracts/src/MinaGuard.ts @@ -5,7 +5,6 @@ import { method, Field, PublicKey, - Permissions, MerkleMapWitness, Poseidon, Bool, @@ -15,7 +14,7 @@ import { UInt32, UInt64, } from 'o1js'; - +import { GUARD_PERMISSIONS } from './guard-permissions.js'; import { MAX_OWNERS, @@ -285,6 +284,14 @@ export class MinaGuard extends SmartContract { /** * Configures account permissions and emits a deploy discovery event. * + * SECURITY, authenticate the deployed account: deploy() is ordinary + * transaction-construction code, not part of MinaGuard's proved circuit. A + * creator can deploy the canonical verification key while changing this + * signature-authorized AccountUpdate's permissions. Never recognize or use + * a MinaGuard account based on its verification key alone. Clients and + * indexers MUST compare every on-chain permission against GUARD_PERMISSIONS + * before displaying, funding, proposing, approving, or executing for it. + * * SECURITY, initialize atomically: deploy() only publishes the account and its * proof-authorized permissions; it does NOT set governance. Between deploy() * and setup()/reserveForParent() the guard is uninitialized, and those init @@ -298,20 +305,7 @@ export class MinaGuard extends SmartContract { */ async deploy() { await super.deploy(); - this.account.permissions.set({ - ...Permissions.default(), - editState: Permissions.proof(), - send: Permissions.proof(), - receive: Permissions.none(), - setDelegate: Permissions.proof(), - setPermissions: Permissions.impossible(), - setVerificationKey: Permissions.VerificationKey.impossibleDuringCurrentVersion(), - setZkappUri: Permissions.impossible(), - setTokenSymbol: Permissions.impossible(), - incrementNonce: Permissions.impossible(), - setVotingFor: Permissions.impossible(), - setTiming: Permissions.impossible(), - }); + this.account.permissions.set(GUARD_PERMISSIONS); this.emitEvent('deployed', { guardAddress: this.address, diff --git a/contracts/src/guard-permissions.ts b/contracts/src/guard-permissions.ts new file mode 100644 index 00000000..57ddf646 --- /dev/null +++ b/contracts/src/guard-permissions.ts @@ -0,0 +1,64 @@ +import { Permissions } from 'o1js'; + +/** + * The only account-permission vector supported by MinaGuard. + * + * A verification-key match does not authenticate permissions because both are + * installed by the signature-authorized deployment AccountUpdate. Every + * component that accepts a vault must also compare its on-chain permissions + * against this complete vector. + */ +export const GUARD_PERMISSIONS = { + editState: Permissions.proof(), + send: Permissions.proof(), + receive: Permissions.none(), + setDelegate: Permissions.proof(), + setPermissions: Permissions.impossible(), + setVerificationKey: + Permissions.VerificationKey.impossibleDuringCurrentVersion(), + setZkappUri: Permissions.impossible(), + editActionState: Permissions.proof(), + setTokenSymbol: Permissions.impossible(), + incrementNonce: Permissions.impossible(), + setVotingFor: Permissions.impossible(), + setTiming: Permissions.impossible(), + access: Permissions.none(), +}; + +export const GUARD_PERMISSION_NAMES = [ + 'editState', + 'send', + 'receive', + 'setDelegate', + 'setPermissions', + 'setVerificationKey', + 'setZkappUri', + 'editActionState', + 'setTokenSymbol', + 'incrementNonce', + 'setVotingFor', + 'setTiming', + 'access', +] as const; + +export type GuardPermissionName = (typeof GUARD_PERMISSION_NAMES)[number]; + +/** JSON/GraphQL representation used by online clients for field-by-field checks. */ +export const GUARD_PERMISSION_KINDS: Record< + GuardPermissionName, + 'None' | 'Either' | 'Proof' | 'Signature' | 'Impossible' +> = { + editState: 'Proof', + send: 'Proof', + receive: 'None', + setDelegate: 'Proof', + setPermissions: 'Impossible', + setVerificationKey: 'Impossible', + setZkappUri: 'Impossible', + editActionState: 'Proof', + setTokenSymbol: 'Impossible', + incrementNonce: 'Impossible', + setVotingFor: 'Impossible', + setTiming: 'Impossible', + access: 'None', +}; diff --git a/contracts/src/index.ts b/contracts/src/index.ts index 69fa8980..fc300cc7 100644 --- a/contracts/src/index.ts +++ b/contracts/src/index.ts @@ -32,6 +32,12 @@ export { } from './constants.js'; export { OwnerWitness, PublicKeyOption, computeOwnerChain, assertOwnerMembership, addOwnerToCommitment, removeOwnerFromCommitment } from './list-commitment.js'; +export { + GUARD_PERMISSIONS, + GUARD_PERMISSION_NAMES, + GUARD_PERMISSION_KINDS, + type GuardPermissionName, +} from './guard-permissions.js'; export { ownerKey } from './utils.js'; diff --git a/docs/backend-audit-guide.md b/docs/backend-audit-guide.md index ee7f89b8..69034274 100644 --- a/docs/backend-audit-guide.md +++ b/docs/backend-audit-guide.md @@ -63,8 +63,10 @@ In full mode, the source of candidate addresses is itself pluggable via `DISCOVE | `daemon` (default) | bestChain scan over daemon GraphQL | ~290 blocks (transition-frontier cap) | none | | `archive` | direct SQL against the Mina archive postgres | unbounded (from genesis) | `ARCHIVE_DB_*` connection env vars + `MINAGUARD_VK_HASH` (the SQL filters on the VK hash to keep results bounded — config load fails fast without it) | -Both backends funnel their candidates through the same dedup / VK re-verification / backfill -path (`processCandidateAddresses`). The archive backend also reads the latest chain height from +Both backends funnel their candidates through the same dedup / full on-chain security check / +backfill path (`processCandidateAddresses`). That check requires both the expected verification key +and the complete canonical `GUARD_PERMISSIONS` vector; verification-key equality alone does not +authenticate the signature-authorized deployment permissions. The archive backend also reads the latest chain height from postgres (`fetchLatestBlockHeightFromArchive`) instead of archive-node-api, which has been observed to return generic errors mid-block. @@ -111,10 +113,12 @@ has teeth on heights where MinaGuard activity landed — which is exactly where ### Contract discovery and readiness -A `Contract` row exists in one of two states: +A `Contract` row has two independent admission flags: - **`ready = false`** — address is known (discovered or subscribed) but no MinaGuard event has been ingested yet. Hidden from most read routes. - **`ready = true`** — flipped on first event ingestion in `syncSingleContract`. Any event other than `setup`/`setupOwner` proves the contract actually initialized on-chain. +- **`permissionsVerified = false`** — the complete on-chain permission vector has not passed the canonical comparison. Hidden from all contract-scoped read routes and ineligible to become ready. +- **`permissionsVerified = true`** — every permission, including `access` and the `setVerificationKey` transaction-version guard, matched `GUARD_PERMISSIONS`. `ready` exists because a `Contract` row can be inserted speculatively — a user subscribing before the deploy tx lands, or `applyProposalEvent` eagerly inserting a child on a CREATE_CHILD proposal @@ -122,11 +126,11 @@ before `executeSetupChild` actually runs. Read routes filter on `ready = true` s rows don't surface as ghost UI entries, while the unready-rescan loop keeps polling their address range until real events land and promote them. -Three ways to become tracked: +Every path fails closed on the same permission check before a contract can become usable: -- **Full mode, daemon discovery**: `discoverCandidateAddresses` scans recent bestChain blocks, `fetchVerificationKeyHash` confirms it's a zkApp, and the hash is optionally matched against `MINAGUARD_VK_HASH`. Backfill window is `max(0, indexedHeight - 300)` — a safe margin around the ~290-block bestChain horizon, which is guaranteed to cover the deploy since that horizon is the only place daemon discovery could have seen it. -- **Full mode, archive discovery**: `discoverCandidateAddressesFromArchive` queries the archive postgres for account updates that installed MinaGuard's VK (applied zkapp commands in non-orphaned blocks). Because this can surface contracts deployed at arbitrary historical heights, the backfill lower bound is `indexStartHeight` (default 0). The on-chain VK re-fetch via the daemon still runs per new candidate: it catches the edge case where the archive shows a VK install that has since been upgraded on-chain. The query includes `pending` blocks so fresh deploys are discoverable before finalization; orphaned pending deploys are cleaned up by `rollbackAboveFork`, which deletes `Contract` rows by `discoveredAtBlock` on every reorg rollback. The residual risk is the same as any reorg deeper than the ~290-block detection window: operator intervention. -- **Lite mode subscribe**: user calls `POST /api/subscribe { address, fromBlock? }`. `fromBlock` omitted = `latestHeight - 5` (margin to cover a block landing mid-request), with **no on-chain check at all** — the auto-subscribe after a fresh deploy races the tx landing, so the address may still be in the mempool. `fromBlock` supplied = trusted explicit lower bound, and this manual path is the **only** one that runs a VK lookup (`routes.ts`): the address must already resolve to a deployed zkApp (a *missing* VK → HTTP 404, guarding against typos backfilling forever) and, when `MINAGUARD_VK_HASH` is set, a *mismatched* VK → HTTP 400. +- **Full mode, daemon discovery**: `discoverCandidateAddresses` scans recent bestChain blocks. `fetchVaultSecurityStatus` then fetches the account and rejects it unless the VK matches (when configured) and every permission is canonical. Backfill window is `max(0, indexedHeight - 300)` — a safe margin around the ~290-block bestChain horizon, which is guaranteed to cover the deploy since that horizon is the only place daemon discovery could have seen it. +- **Full mode, archive discovery**: `discoverCandidateAddressesFromArchive` queries the archive postgres for account updates that installed MinaGuard's VK (applied zkapp commands in non-orphaned blocks). Because this can surface contracts deployed at arbitrary historical heights, the backfill lower bound is `indexStartHeight` (default 0). The live account security check still runs per new candidate, catching either later VK drift or a canonical VK installed alongside weakened permissions. The query includes `pending` blocks so fresh deploys are discoverable before finalization; orphaned pending deploys are cleaned up by `rollbackAboveFork`, which deletes `Contract` rows by `discoveredAtBlock` on every reorg rollback. The residual risk is the same as any reorg deeper than the ~290-block detection window: operator intervention. +- **Lite mode subscribe**: user calls `POST /api/subscribe { address, fromBlock? }`. `fromBlock` omitted = `latestHeight - 5` (margin to cover a block landing mid-request). This subscribe-before-deploy path cannot inspect an account still in the mempool, so it creates an unverified, unready row; `syncSingleContract` performs the full live check before ingesting events or marking it ready. `fromBlock` supplied = trusted explicit lower bound and performs the check immediately: a missing/non-zkApp account returns HTTP 404, while a wrong VK or any non-canonical permission returns HTTP 400. The `rescanUnreadyContracts` loop re-scans `[discoveredAtBlock, latestHeight]` every tick until events land. First event flips `ready = true` and the contract joins the forward sweep. @@ -141,7 +145,7 @@ events land. First event flips `ready = true` and the contract joins the forward 4. **Dedupe by fingerprint** (`address::type::blockHeight::txHash::payload`). `EventRaw.fingerprint` is unique; second writer is a no-op. 5. **Upsert BlockHeader** for the event's `(height, blockHash, parentHash)`. First writer wins; mismatches across events at the same height get caught by the next tick's reorg detector. 6. **Insert EventRaw** and dispatch to the appropriate `apply*` handler. -7. **Flip `ready`** if any event was ingested. +7. **Flip `ready`** if any event was ingested, after `permissionsVerified` has passed. ### Data model @@ -163,7 +167,7 @@ block it became valid at. Current state is the latest row; reorg rollback is a s **Identity / pointer.** -- **`Contract`** — `(address, parent?, ready, discoveredAtBlock, ...)`. Identity + latest-synced metadata. `parent` set from `setup.parent` (null/EMPTY for root guards). +- **`Contract`** — `(address, parent?, ready, permissionsVerified, discoveredAtBlock, ...)`. Identity + latest-synced metadata. `parent` set from `setup.parent` (null/EMPTY for root guards). Read APIs require both admission flags. - **`Proposal`** — `(contractId, proposalHash, ...)`, unique per `@@unique([contractId, proposalHash])`. Identity + propose-time fields (`proposer`, `toAddress`, `tokenId`, `txType`, `data`, `nonce`, `configNonce`, `expirySlot`, `guardAddress`, `destination`, `childAccount`, `memo`/`memoHash`/`executionMemoHash`, `createdAtBlock`), plus last-submitted approve/execute tx hashes and error fields for UI polling. `ProposalReceiver` child rows carry per-slot receivers from `receiver` events (padded empties skipped); for governance proposals slot 0 is mirrored onto `Proposal.toAddress`. **There is no stored status column** — status is derived at read time (see [Proposal status](#proposal-status)). - **`IndexerCursor`** — key/value rows. `indexed_height` is the forward-sweep cursor. `archive_discovered_height` is the archive-discovery high-water mark, tracked separately so that switching `DISCOVERY_BACKEND` from `daemon` to `archive` triggers a from-genesis sweep instead of inheriting the (much narrower) daemon cursor position. @@ -224,10 +228,11 @@ lookups positively succeed (a genuine `pending` from `fetchZkappTxStatus` — an treated as absent — **and** a real mempool set, `null` on network failure). Verify neither lookup failing can misclassify an included tx as dropped, since that flag releases the UI signer lock. -**4. VK-hash filtering.** Discovery filters candidates by `MINAGUARD_VK_HASH` (required for archive, -optional for daemon), and re-verifies the on-chain VK per candidate. Confirm a wrong/stale hash -degrades to "discovers nothing" rather than "tracks arbitrary zkApps", and that the network-specific -hash (`testnet=`/`mainnet=` in `contracts/.vk-hash`) matches the target chain. +**4. Deployment authentication.** Discovery filters candidates by `MINAGUARD_VK_HASH` (required for +archive, optional for daemon), then validates the live VK and every permission against +`GUARD_PERMISSIONS`. Confirm that a canonical VK with even one altered field (especially +`send: proofOrSignature`) never becomes `permissionsVerified` or `ready`, and that legacy rows are +re-checked rather than grandfathered. ### Failure semantics @@ -310,14 +315,18 @@ URL characters don't need percent-encoding. ### API routes -Contract-scoped read routes only surface contracts with `ready = true` (at least one MinaGuard -event ingested); speculative rows — subscribed-before-deploy addresses, eagerly inserted children -— 404 until real events land. +Contract-scoped read routes only surface contracts with `ready = true` and +`permissionsVerified = true`; speculative or unsafe rows return 404. + +The deterministic UI harness has no chain. Only when `INDEXER_DISABLED=true`, the security endpoint +synthesizes a canonical result for fixtures explicitly seeded with `permissionsVerified=true`. +This branch is test-only and must never be enabled in a deployment that accepts real vaults. | Route | Purpose | |---|---| | `GET /health` | Process liveness (`{ ok, now }`). | | `GET /api/indexer/status` | In-memory indexer status: `running`, `lastRunAt`, `lastSuccessfulRunAt`, `latestChainHeight`, `latestSlot`, `indexedHeight`, `lastError`, `discoveredContracts`, `indexerMode`. | +| `GET /api/accounts/:address/security` | Live VK and complete permission-vector comparison for diagnostics and the chainless deterministic UI harness. Production UI checks the configured Mina node directly. | | `GET /api/tx-status?hash=` | Looks up a submitted zkApp tx hash on bestChain (used for CREATE proposals with no `Proposal` row yet). `{ status, reason? }`, status ∈ `pending`/`included`/`failed`/`unknown` — `unknown` means the lookup failed, not confirmed absent. | | `GET /api/contracts` | Lists tracked (`ready`) contracts merged with latest `ContractConfig` snapshot + `_count.owners`/`proposals`/`events`. Ordered `discoveredAt desc`. | | `GET /api/contracts/:address` | One tracked contract, same enriched shape. `404` if not found or not ready. | diff --git a/docs/contracts-audit-guide.md b/docs/contracts-audit-guide.md index 1764862c..513b072a 100644 --- a/docs/contracts-audit-guide.md +++ b/docs/contracts-audit-guide.md @@ -236,7 +236,10 @@ Defined in `constants.ts`: ### On-chain multi-step flow **Deploy.** `deploy()` sets account permissions (see [Permissions](#permissions)) and emits -a `DeployEvent` with the contract address for indexer discovery. +a `DeployEvent` with the contract address for indexer discovery. `deploy()` is transaction-building +code, not a proved method: the deployment signature authenticates the installed values, while the +MinaGuard verification key does not. A vault MUST NOT be recognized from its verification-key hash +alone; online consumers must also compare every stored permission with `GUARD_PERMISSIONS`. **Setup.** `setup(threshold, numOwners, initialOwners)` — one-time root-guard initialization. @@ -460,15 +463,29 @@ Set in `deploy()`: | `setPermissions` | `impossible()` | Prevents permission downgrade attacks | | `setVerificationKey` | `impossibleDuringCurrentVersion()` | Pins the verification key for the lifetime of the current version | | `setZkappUri` | `impossible()` | Metadata cannot be rewritten | +| `editActionState` | `proof()` | Actions can only be edited by proof | | `setTokenSymbol` | `impossible()` | Token symbol cannot be rewritten | | `incrementNonce` | `impossible()` | Proof-authorized AUs don't set a nonce precondition | | `setVotingFor` | `impossible()` | Not used | | `setTiming` | `impossible()` | Not used | - -All other permissions use `Permissions.default()`. The one-shot deploy key that sets these is -**powerless afterward**: every state/fund knob requires a proof and every permission knob is -`impossible`, so a leaked deploy key has no post-deploy authority (this is what makes the UI's -in-browser ephemeral key safe — see [`ui-audit-guide.md`](./ui-audit-guide.md) focus point 5). +| `access` | `none()` | No additional access gate | + +The canonical vector is defined once as `GUARD_PERMISSIONS` in +`contracts/src/guard-permissions.ts`. If that exact vector was installed, the one-shot deploy key is +powerless afterward: every state/fund knob requires a proof and every permission knob is +`impossible`. + +That statement is conditional on checking the stored vector. `deploy()` is not part of the proved +circuit, and its AccountUpdate is authorized by the vault account signature. A creator can use the +canonical MinaGuard verification key while changing `send` to `proofOrSignature()`, retain the +deployment key, and later withdraw by signature. Therefore a verification-key match alone does not +identify a safe MinaGuard vault. The backend and every online client MUST compare all on-chain +permission fields (including `access` and the `setVerificationKey` transaction version) with +`GUARD_PERMISSIONS` before displaying, funding, proposing, approving, or executing for an account. +The browser must obtain the actual vector directly from its configured Mina node and compare it +with a build-time canonical value, rather than trusting an indexer to supply both sides. +The offline CLI cannot perform this authentication because its bundle is supplied by an untrusted +online producer. --- @@ -545,7 +562,7 @@ re-authorizes state/fund movement outside a proof, and that the deployed VK matc | Anyone can execute | Execution is permissionless once threshold is met | | MINA receivable | `receive: Permissions.none()` allows deposits without proof | | State changes proof-only | `editState: Permissions.proof()` — no signature fallback | -| Permission downgrade prevented | `setPermissions: Permissions.impossible()` | +| Permission downgrade prevented after canonical deployment | `setPermissions: Permissions.impossible()`; online consumers first verify the complete stored vector against `GUARD_PERMISSIONS` | | Verification key immutable | `setVerificationKey: impossibleDuringCurrentVersion` | | Bounded circuit size | `MAX_OWNERS = 20`, `MAX_RECEIVERS = 9` | diff --git a/docs/security-audit-guide.md b/docs/security-audit-guide.md index 700020ef..d5e613b6 100644 --- a/docs/security-audit-guide.md +++ b/docs/security-audit-guide.md @@ -29,9 +29,14 @@ MinaGuard is a **non-custodial** hierarchical multisig vault. Funds held by a gu contract move only when a proposal reaches its owner-signature threshold and a proven contract method executes it. No server in the system holds a key that can move vault funds: owner signatures are produced in the owners' own wallets (Auro extension or -Ledger via WebHID) or on an air-gapped machine via the offline CLI, and the contract's -account permissions (`send: proof()`, `editState: proof()`, `setPermissions: impossible()`) -rule out any non-proof path to the balance or state. +Ledger via WebHID) or on an air-gapped machine via the offline CLI. For an authenticated +MinaGuard deployment, the account permissions (`send: proof()`, `editState: proof()`, +`setPermissions: impossible()`) rule out any non-proof path to the balance or state. +Verification-key equality alone does not authenticate those signature-installed permissions: +the backend and online UI must also verify the complete stored permission vector against +`GUARD_PERMISSIONS` before accepting the vault. The UI fetches this snapshot directly from its +configured Mina node and compares it with a build-time canonical vector; it does not trust the +indexer to report either side of the comparison. ### Trusted computing base @@ -129,7 +134,7 @@ This table maps each claim to its enforcement point and primary test coverage (a | Parent can always recover child funds | `executeReclaimToParent` / `executeDestroy` deliberately skip the `childMultiSigEnabled` check — disabling a child never strands its balance | `child.test.ts` | | Parent state drift voids REMOTE approvals | child pins parent state via AccountUpdate preconditions | `child.test.ts` | | Governance preserves `0 < threshold ≤ numOwners ≤ MAX_OWNERS` | `setup()`, `executeOwnerChange()`, `executeThresholdChange()` all assert the bounds — the vault can be neither locked (threshold unreachable) nor unbounded | `setup.test.ts`, `governance.test.ts` | -| No permission downgrade / VK swap | `setPermissions: impossible()`, `setVerificationKey: impossibleDuringCurrentVersion()` set in `deploy()` | `setup.test.ts` | +| No hidden signature authority / later permission downgrade | Backend and online UI reject any stored vector other than `GUARD_PERMISSIONS`; once accepted, `setPermissions: impossible()` and `setVerificationKey: impossibleDuringCurrentVersion()` prevent later changes | `vault-security.test.ts`, `routes-subscribe.test.ts`, `indexer-archive-discovery.test.ts`, `indexer-autosubscribe.test.ts` | Off-chain, one invariant matters for the trust argument above: **clients recompute the hash they sign from the fields they display and verify it equals the selected proposal's identity** — diff --git a/docs/ui-audit-guide.md b/docs/ui-audit-guide.md index 152f40b9..7dee3e10 100644 --- a/docs/ui-audit-guide.md +++ b/docs/ui-audit-guide.md @@ -100,8 +100,12 @@ reclaimable. installs whatever key compile produced, while afterwards a swap only yields proofs that fail on-chain. Unset ⇒ skipped, like the backend's `minaguardVkHash`. - **The backend is not trusted for integrity.** Data from the backend is used to construct - transactions and display information. Security-critical operations, such as proposal creation, approval, - and execution, are performed on-chain. Transactions are also submitted directly to the node. + transactions and display information. Before exposing vault actions, the browser queries the configured + Mina node directly and compares the account's verification key and every stored permission against its + built-in MinaGuard policy. Missing fields, RPC failures, or any mismatch fail closed. This check also runs + against a proposed child before CREATE_CHILD approval or execution. Security-critical operations, such as + proposal creation, approval, and execution, are performed on-chain. Transactions are also submitted + directly to the node. - **Interactions with the chain.** Interactions with the chain, like transactions submitted, reach the node directly. Note, however, that: - Transactions submitted through Auro wallet reach the node endpoint defined by Auro. @@ -134,6 +138,8 @@ Assuming that the frontend (UI) is not compromised, the interactions are the fol - **Backend (indexer).** Read-only, *untrusted*. The indexer is used to retrieve on-chain data and events. The indexer cannot affect critical operations. For example, consider a propose-approve-execute flow: + - The UI does not rely on the indexer's `permissionsVerified` flag as its trust anchor. It independently + reads the account's verification key and complete permission vector from Mina before enabling actions. - Proposal is created in the UI and submitted directly to the node. The contract acts as the trust anchor here. - Owners see the proposal data (controlled by the indexer) and may choose to approve. A diff --git a/e2e/ui/seed.ts b/e2e/ui/seed.ts index a2d9ad29..189a1c3f 100644 --- a/e2e/ui/seed.ts +++ b/e2e/ui/seed.ts @@ -62,6 +62,7 @@ async function seedVault(spec: VaultSpec): Promise { address: spec.address, parent: spec.parent ?? null, ready: true, + permissionsVerified: true, discoveredAtBlock: 1, discoveredAt: at(-10 * spec.listOrder), }, diff --git a/ui/app/accounts/[address]/page.tsx b/ui/app/accounts/[address]/page.tsx index 01cb893e..aee0d4bb 100644 --- a/ui/app/accounts/[address]/page.tsx +++ b/ui/app/accounts/[address]/page.tsx @@ -19,6 +19,7 @@ import TxTypeIcon from '@/components/TxTypeIcon'; import { fetchBalance, fetchChildren } from '@/lib/api'; import ConnectNotice from '@/components/ConnectNotice'; import Link from 'next/link'; +import { useVaultSecurity } from '@/hooks/useVaultSecurity'; import { clearPendingTx, getPendingTx, @@ -59,8 +60,12 @@ export default function AccountPage() { return allContractOwners.get(multisig.address)?.includes(wallet.address) ?? false; }, [wallet.address, multisig, allContractOwners]); const childMultiSigEnabled = multisig?.childMultiSigEnabled !== false; - const localProposalsEnabled = isOwner && (isRoot || childMultiSigEnabled); - const childActionsEnabled = isRoot && isOwner; + const liveSecurity = useVaultSecurity(multisig?.address ?? null); + const permissionsVerified = + multisig?.permissionsVerified === true && liveSecurity === 'safe'; + const localProposalsEnabled = + permissionsVerified && isOwner && (isRoot || childMultiSigEnabled); + const childActionsEnabled = permissionsVerified && isRoot && isOwner; const hasChildren = useMemo( () => contracts.some((c) => c.parent === multisig?.address), [contracts, multisig?.address], @@ -69,7 +74,9 @@ export default function AccountPage() { () => new Set(['allocateChild', 'reclaimChild', 'destroyChild', 'enableChildMultiSig']), [], ); - const localDisabledReason = !isOwner + const localDisabledReason = !permissionsVerified + ? 'Vault permissions have not passed the canonical security check' + : !isOwner ? 'You are not an owner of this Vault' : !childMultiSigEnabled ? 'Multi-sig disabled by the Vault' @@ -146,6 +153,13 @@ export default function AccountPage() { /> ) : multisig && multisig.address === urlAddress ? (
+ {!permissionsVerified && ( +
+ {liveSecurity === 'checking' + ? 'Checking the complete on-chain permission vector. Transaction actions remain blocked.' + : 'Unsafe Vault: its complete on-chain permission vector has not been verified as canonical. Do not fund or use this account; transaction actions are blocked.'} +
+ )}

Vault Address

@@ -237,7 +251,13 @@ export default function AccountPage() { (null); + const [childPermissionCheck, setChildPermissionCheck] = useState< + 'checking' | 'match' | 'mismatch' | null + >(null); useEffect(() => { if (!proposal || proposal.txType !== 'createChild') { setChildConfigCheck(null); @@ -146,6 +157,29 @@ export default function TransactionDetailPage() { return () => { cancelled = true; }; }, [proposal, proposalHash, multisig, proposalsAddress]); + // Child-targeting approvals must authenticate the deployed child account + // itself. Its VK can be canonical while its signature-authorized deployment + // update installed a creator withdrawal permission. + useEffect(() => { + if (!proposal?.childAccount) { + setChildPermissionCheck(null); + return; + } + if (proposal._localPending) return; + let cancelled = false; + setChildPermissionCheck('checking'); + void fetchVaultSecurityStatus(proposal.childAccount).then((status) => { + if (!cancelled) { + setChildPermissionCheck( + isCanonicalVaultSecurity(status) ? 'match' : 'mismatch' + ); + } + }); + return () => { + cancelled = true; + }; + }, [proposal?.txType, proposal?.childAccount, proposal?._localPending]); + // For ADD_OWNER: recompute the canonical post-add owner commitment from the // indexed owner list and compare it to the signed proposal.data. A mismatch // means execution would store an owner order no client can reconstruct, so @@ -272,6 +306,11 @@ export default function TransactionDetailPage() { [proposals, proposal?.proposalHash, threshold], ); const contractLock = useContractTxLock(multisig?.address ?? null, proposalsForLock); + const parentPermissionCheck = useVaultSecurity(multisig?.address ?? null); + const permissionsSafe = + multisig?.permissionsVerified === true && parentPermissionCheck === 'safe'; + const childPermissionsSafe = + !proposal?.childAccount || childPermissionCheck === 'match'; const canApprove = !!proposal && !isLocalPending && @@ -279,6 +318,8 @@ export default function TransactionDetailPage() { isOwner && !hasApproved && !isConfigStale && + permissionsSafe && + childPermissionsSafe && // Block approval when the displayed SubVault config provably does not hash // to the signed proposal.data (config-swap). Only a computed mismatch // blocks — 'checking'/'unavailable' don't, to avoid gating on indexer lag. @@ -294,6 +335,8 @@ export default function TransactionDetailPage() { proposal.status === 'pending' && proposal.approvalCount >= threshold && !isConfigStale && + permissionsSafe && + childPermissionsSafe && !executeInFlight && !contractLock.locked && !insufficientBalance; @@ -302,6 +345,7 @@ export default function TransactionDetailPage() { !isLocalPending && proposal.status === 'pending' && isOwner && + permissionsSafe && proposal.nonce !== null && !isDeleteProposal(proposal) && // CREATE_CHILD uses the reserved nonce=0 sentinel, which the current @@ -618,6 +662,21 @@ export default function TransactionDetailPage() {
)} + {(!permissionsSafe || childPermissionCheck === 'mismatch') && ( +
+

Unsafe permission vector

+

+ {!permissionsSafe + ? parentPermissionCheck === 'checking' + ? 'The complete on-chain Vault permission check is still running.' + : 'This Vault has not passed the complete canonical permission check.' + : 'The proposed SubVault has a missing or non-canonical on-chain permission field.'}{' '} + Approval, execution, deletion, and offline bundle creation are + blocked. +

+
+ )} + {childConfigCheck === 'unavailable' && (

SubVault config could not be verified

diff --git a/ui/app/transactions/new/page.tsx b/ui/app/transactions/new/page.tsx index 4bab4497..9c8b29b2 100644 --- a/ui/app/transactions/new/page.tsx +++ b/ui/app/transactions/new/page.tsx @@ -14,8 +14,14 @@ import { import TxTypeIcon from '@/components/TxTypeIcon'; import { createOnchainProposal } from '@/lib/multisigClient'; import { assertValidMinaAddress, buildOfflineProposeBundle } from '@/lib/offline-signing'; -import { fetchChildren, fetchContract } from '@/lib/api'; +import { + fetchChildren, + fetchContract, + fetchVaultSecurityStatus, + isCanonicalVaultSecurity, +} from '@/lib/api'; import { useContractTxLock } from '@/hooks/useContractTxLock'; +import { useVaultSecurity } from '@/hooks/useVaultSecurity'; import { savePendingTx } from '@/lib/storage'; import { DownloadCLILink, OfflineSigningFlow, UploadSignedResponse } from '@/components/OfflineSigningFlow'; @@ -41,6 +47,9 @@ function NewTransactionPageInner() { const isRoot = !!multisig && !multisig.parent; const contractLock = useContractTxLock(multisig?.address ?? null, proposals); + const liveSecurity = useVaultSecurity(multisig?.address ?? null); + const permissionsSafe = + multisig?.permissionsVerified === true && liveSecurity === 'safe'; // Available tx types: LOCAL on every guard; subaccount actions only on roots. // CREATE_CHILD is shown on roots so the action is discoverable here, but it @@ -155,6 +164,11 @@ function NewTransactionPageInner() { const handleSubmit = async (data: NewProposalInput) => { if (!wallet.address || !multisig) return; + if (!permissionsSafe) { + throw new Error( + 'Vault permissions have not passed the canonical security check' + ); + } const contractAddress = multisig.address; const proposerAddress = wallet.address; @@ -163,7 +177,15 @@ function NewTransactionPageInner() { let createdHash: string | null = null; await startOperation('Submitting proposal on-chain...', async (onProgress) => { - const fresh = await fetchContract(contractAddress); + const [fresh, security] = await Promise.all([ + fetchContract(contractAddress), + fetchVaultSecurityStatus(contractAddress), + ]); + if (!fresh?.permissionsVerified || !isCanonicalVaultSecurity(security)) { + throw new Error( + 'Vault permissions have not passed the canonical security check' + ); + } const configNonce = fresh?.configNonce ?? fallbackConfigNonce; const result = await createOnchainProposal({ contractAddress, @@ -219,6 +241,12 @@ function NewTransactionPageInner() {

Connect your wallet and select a contract to create proposals.

+ ) : !permissionsSafe ? ( +
+ {liveSecurity === 'checking' + ? 'Checking the complete on-chain permission vector. Proposal creation remains blocked.' + : 'Unsafe Vault: its complete on-chain permission vector has not been verified as canonical. Proposal creation is blocked.'} +
) : multisig.ownersCommitment == null ? (

Contract not initialized. Run Setup first before creating proposals.

@@ -342,7 +370,18 @@ function NewTransactionPageInner() { throw new Error('Signer address is not an owner of this multisig'); } const input = getFormInputRef.current!(); - const fresh = await fetchContract(multisig!.address); + const [fresh, security] = await Promise.all([ + fetchContract(multisig!.address), + fetchVaultSecurityStatus(multisig!.address), + ]); + if ( + !fresh?.permissionsVerified || + !isCanonicalVaultSecurity(security) + ) { + throw new Error( + 'Vault permissions have not passed the canonical security check' + ); + } const configNonce = fresh?.configNonce ?? multisig!.configNonce ?? 0; return buildOfflineProposeBundle({ contractAddress: multisig!.address, diff --git a/ui/hooks/useVaultSecurity.ts b/ui/hooks/useVaultSecurity.ts new file mode 100644 index 00000000..961b2e40 --- /dev/null +++ b/ui/hooks/useVaultSecurity.ts @@ -0,0 +1,35 @@ +'use client'; + +import { useEffect, useState } from 'react'; +import { fetchVaultSecurityStatus, isCanonicalVaultSecurity } from '@/lib/api'; + +export type VaultSecurityCheck = 'checking' | 'safe' | 'unsafe' | null; + +/** + * Performs the UI's own live, field-by-field vault admission check. + * Missing accounts, missing fields, and network failures all fail closed. + */ +export function useVaultSecurity(address: string | null): VaultSecurityCheck { + const [check, setCheck] = useState(null); + + useEffect(() => { + if (!address) { + setCheck(null); + return; + } + + let cancelled = false; + setCheck('checking'); + void fetchVaultSecurityStatus(address).then((status) => { + if (!cancelled) { + setCheck(isCanonicalVaultSecurity(status) ? 'safe' : 'unsafe'); + } + }); + + return () => { + cancelled = true; + }; + }, [address]); + + return check; +} diff --git a/ui/lib/api.ts b/ui/lib/api.ts index 6861bbfc..4c7efca6 100644 --- a/ui/lib/api.ts +++ b/ui/lib/api.ts @@ -8,6 +8,7 @@ import { normalizeDestination, normalizeTxType, } from '@/lib/types'; +import { getMinaGuardConfig } from '@/lib/endpoints'; const API_BASE = process.env.NEXT_PUBLIC_API_BASE_URL ?? 'http://localhost:3001'; @@ -29,6 +30,181 @@ export async function fetchContract(address: string): Promise>; + expectedPermissionKinds: Partial>; + permissionMismatches: string[]; + safe: boolean; +} + +const PERMISSION_FIELD_NAMES = [ + 'editState', + 'send', + 'receive', + 'setDelegate', + 'setPermissions', + 'setVerificationKey', + 'setZkappUri', + 'editActionState', + 'setTokenSymbol', + 'incrementNonce', + 'setVotingFor', + 'setTiming', + 'access', +] as const; +type PermissionFieldName = (typeof PERMISSION_FIELD_NAMES)[number]; + +/** + * Browser-side trust anchor. Keep this serialized form in lockstep with the + * o1js GUARD_PERMISSIONS constant; unlike API-supplied expected values, it + * cannot be changed by a compromised indexer response. + */ +const EXPECTED_PERMISSION_KINDS: Record = { + editState: 'Proof', + send: 'Proof', + receive: 'None', + setDelegate: 'Proof', + setPermissions: 'Impossible', + setVerificationKey: 'Impossible', + setZkappUri: 'Impossible', + editActionState: 'Proof', + setTokenSymbol: 'Impossible', + incrementNonce: 'Impossible', + setVotingFor: 'Impossible', + setTiming: 'Impossible', + access: 'None', +}; + +// o1js@3.0.0-mesa.final's current transaction version, committed by +// impossibleDuringCurrentVersion(). The backend compares the UInt32 directly. +const EXPECTED_SET_VK_TXN_VERSION = '4'; + +/** + * Fetches and validates the account directly from the configured Mina node. + * The browser does not trust the indexer to report either the actual or the + * expected permission vector. The deterministic UI harness is the sole + * exception because it intentionally runs without a chain. + */ +export async function fetchVaultSecurityStatus( + address: string +): Promise { + if (process.env.NEXT_PUBLIC_E2E_TEST === 'true') { + return getJson(`/api/accounts/${address}/security`); + } + + const query = `query($publicKey: PublicKey!) { + account(publicKey: $publicKey) { + verificationKey { hash } + permissions { + editState + send + receive + setDelegate + setPermissions + setVerificationKey { auth txnVersion } + setZkappUri + editActionState + setTokenSymbol + incrementNonce + setVotingFor + setTiming + access + } + } + }`; + + try { + const response = await fetch(getMinaGuardConfig().minaEndpoint, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + cache: 'no-store', + body: JSON.stringify({ query, variables: { publicKey: address } }), + }); + if (!response.ok) return null; + const body = (await response.json()) as { + data?: { + account?: { + verificationKey?: { hash?: string | null } | null; + permissions?: Record | null; + } | null; + }; + errors?: unknown; + }; + if (body.errors) return null; + const account = body.data?.account; + if (!account) { + return { + accountFound: false, + verificationKeyHash: null, + verificationKeyMatches: false, + permissionKinds: {}, + expectedPermissionKinds: EXPECTED_PERMISSION_KINDS, + permissionMismatches: [...PERMISSION_FIELD_NAMES], + safe: false, + }; + } + + const raw = account.permissions ?? {}; + const setVerificationKey = raw.setVerificationKey as + | { + auth?: unknown; + txnVersion?: unknown; + } + | undefined; + const permissionKinds: Partial> = {}; + for (const name of PERMISSION_FIELD_NAMES) { + const value = + name === 'setVerificationKey' ? setVerificationKey?.auth : raw[name]; + if (typeof value === 'string') permissionKinds[name] = value; + } + const permissionMismatches = PERMISSION_FIELD_NAMES.filter( + (name) => permissionKinds[name] !== EXPECTED_PERMISSION_KINDS[name] + ); + if ( + String(setVerificationKey?.txnVersion ?? '') !== + EXPECTED_SET_VK_TXN_VERSION && + !permissionMismatches.includes('setVerificationKey') + ) { + permissionMismatches.push('setVerificationKey'); + } + + const verificationKeyHash = account.verificationKey?.hash ?? null; + const expectedVkHash = process.env.NEXT_PUBLIC_MINAGUARD_VK_HASH; + const verificationKeyMatches = + verificationKeyHash !== null && + (!expectedVkHash || verificationKeyHash === expectedVkHash); + return { + accountFound: true, + verificationKeyHash, + verificationKeyMatches, + permissionKinds, + expectedPermissionKinds: EXPECTED_PERMISSION_KINDS, + permissionMismatches, + safe: verificationKeyMatches && permissionMismatches.length === 0, + }; + } catch { + return null; + } +} + +/** UI-side, field-by-field permission check. Fails closed on missing fields. */ +export function isCanonicalVaultSecurity( + status: VaultSecurityStatus | null +): boolean { + return ( + status !== null && + status.accountFound && + status.verificationKeyMatches && + status.safe && + PERMISSION_FIELD_NAMES.every( + (name) => status.permissionKinds[name] === EXPECTED_PERMISSION_KINDS[name] + ) + ); +} + /** Lists direct subaccounts of a parent contract. */ export async function fetchChildren(parentAddress: string): Promise { const data = await getJson>>( @@ -210,6 +386,7 @@ async function getJson(path: string): Promise { function toContractSummary(input: Record): ContractSummary { return { address: asString(input.address) ?? '', + permissionsVerified: input.permissionsVerified === true, ownersCommitment: asNullableString(input.ownersCommitment), threshold: asNullableNumber(input.threshold), numOwners: asNullableNumber(input.numOwners), diff --git a/ui/lib/types.ts b/ui/lib/types.ts index 1bedf4c4..51783fe6 100644 --- a/ui/lib/types.ts +++ b/ui/lib/types.ts @@ -80,6 +80,8 @@ export interface OwnerRecord { /** Contract summary returned by backend contract listing endpoints. */ export interface ContractSummary { address: string; + /** True only after the backend compared every on-chain permission field. */ + permissionsVerified: boolean; ownersCommitment: string | null; threshold: number | null; numOwners: number | null; From 14246da85fcb6f94f3c9ae4d5095da10ccc0fd33 Mon Sep 17 00:00:00 2001 From: Jason Park Date: Fri, 11 Sep 2026 17:53:38 +0000 Subject: [PATCH 2/9] fix: validate raw Mina permission response --- backend/src/indexer.ts | 4 +-- backend/src/mina-client.ts | 43 ++++++++++++++++++------ backend/src/routes.ts | 4 +-- backend/src/tests/vault-security.test.ts | 26 ++++++++++++++ backend/src/vault-security.ts | 10 ++++++ 5 files changed, 73 insertions(+), 14 deletions(-) diff --git a/backend/src/indexer.ts b/backend/src/indexer.ts index bc46043d..e988c2d0 100644 --- a/backend/src/indexer.ts +++ b/backend/src/indexer.ts @@ -307,7 +307,7 @@ export class MinaGuardIndexer { const security = await fetchVaultSecurityStatus( address, - this.config.minaguardVkHash + this.config ); if (!security.safe) { if ( @@ -445,7 +445,7 @@ export class MinaGuardIndexer { if (!tracked?.permissionsVerified) { const security = await fetchVaultSecurityStatus( address, - this.config.minaguardVkHash + this.config ); if (!security.accountFound) return; if (!security.safe) { diff --git a/backend/src/mina-client.ts b/backend/src/mina-client.ts index b594214d..6dd2380e 100644 --- a/backend/src/mina-client.ts +++ b/backend/src/mina-client.ts @@ -330,11 +330,37 @@ export interface VaultSecurityStatus { */ export async function fetchVaultSecurityStatus( address: string, - expectedVerificationKeyHash: string | null + config: BackendConfig ): Promise { - const pub = PublicKey.fromBase58(address); - const accountResult = await fetchAccount({ publicKey: pub }); - const account = accountResult.account as any; + const query = `query($publicKey: PublicKey!) { + account(publicKey: $publicKey) { + verificationKey { hash } + permissions { + editState + send + receive + setDelegate + setPermissions + setVerificationKey { auth txnVersion } + setZkappUri + editActionState + setTokenSymbol + incrementNonce + setVotingFor + setTiming + access + } + } + }`; + const response = await graphqlRequest<{ + account?: { + verificationKey?: { hash?: string | null } | null; + permissions?: Record | null; + } | null; + }>(query, config.minaEndpoint, config.minaFallbackEndpoint, { + publicKey: address, + }); + const account = response.account; if (!account) { return { accountFound: false, @@ -347,14 +373,11 @@ export async function fetchVaultSecurityStatus( }; } - const verificationKeyHash = - account.zkapp?.verificationKey?.hash?.toString() ?? - account.verificationKey?.hash?.toString() ?? - null; + const verificationKeyHash = account.verificationKey?.hash ?? null; const verificationKeyMatches = verificationKeyHash !== null && - (expectedVerificationKeyHash === null || - verificationKeyHash === expectedVerificationKeyHash); + (config.minaguardVkHash === null || + verificationKeyHash === config.minaguardVkHash); const { permissionKinds, mismatches } = validatePermissionVector( account.permissions ); diff --git a/backend/src/routes.ts b/backend/src/routes.ts index bc7c86c2..f67a3008 100644 --- a/backend/src/routes.ts +++ b/backend/src/routes.ts @@ -127,7 +127,7 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi }); return; } - res.json(await fetchVaultSecurityStatus(address, config.minaguardVkHash)); + res.json(await fetchVaultSecurityStatus(address, config)); }) ); @@ -627,7 +627,7 @@ export function createApiRouter(indexer: MinaGuardIndexer, config?: BackendConfi if (fromBlockNum !== null) { const security = await fetchVaultSecurityStatus( address, - config.minaguardVkHash + config ); if (!security.accountFound || !security.verificationKeyHash) { res.status(404).json({ error: 'Account not found on-chain or not a zkApp' }); diff --git a/backend/src/tests/vault-security.test.ts b/backend/src/tests/vault-security.test.ts index 50ab1e9e..b304fe69 100644 --- a/backend/src/tests/vault-security.test.ts +++ b/backend/src/tests/vault-security.test.ts @@ -31,6 +31,32 @@ describe('canonical MinaGuard permissions', () => { expect(validatePermissionVector(altered).mismatches).toEqual(['send']); }); + it('accepts the raw permission strings returned by Mina GraphQL', () => { + const raw = { + ...GUARD_PERMISSION_KINDS, + setVerificationKey: { + auth: GUARD_PERMISSION_KINDS.setVerificationKey, + txnVersion: GUARD_PERMISSIONS.setVerificationKey.txnVersion.toString(), + }, + }; + + expect(validatePermissionVector(raw).mismatches).toEqual([]); + }); + + it('does not confuse raw setVerificationKey: None with Impossible', () => { + const raw = { + ...GUARD_PERMISSION_KINDS, + setVerificationKey: { + auth: 'None', + txnVersion: GUARD_PERMISSIONS.setVerificationKey.txnVersion.toString(), + }, + }; + + expect(validatePermissionVector(raw).mismatches).toEqual([ + 'setVerificationKey', + ]); + }); + it('fails closed when any permission field is absent', () => { const actual = permissionKindVector(GUARD_PERMISSIONS); delete actual.access; diff --git a/backend/src/vault-security.ts b/backend/src/vault-security.ts index 421b9707..75e77a9e 100644 --- a/backend/src/vault-security.ts +++ b/backend/src/vault-security.ts @@ -31,6 +31,16 @@ function readBool(value: BoolLike | undefined): boolean | null { /** Converts o1js' three-bit AuthRequired representation to its protocol name. */ export function permissionKind(permission: unknown): PermissionKind | null { + if ( + permission === 'None' || + permission === 'Either' || + permission === 'Proof' || + permission === 'Signature' || + permission === 'Impossible' + ) { + return permission; + } + const value = permission as PermissionLike | undefined; const constant = readBool(value?.constant); const necessary = readBool(value?.signatureNecessary); From c28fc8789ad987e91c0a88cbaa8a1f1ad28da747 Mon Sep 17 00:00:00 2001 From: Jason Park Date: Fri, 11 Sep 2026 18:06:07 +0000 Subject: [PATCH 3/9] fix: normalize empty verification key filter --- backend/src/config.ts | 5 ++++- backend/src/tests/config.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 backend/src/tests/config.test.ts diff --git a/backend/src/config.ts b/backend/src/config.ts index 1b5b4193..277df038 100644 --- a/backend/src/config.ts +++ b/backend/src/config.ts @@ -101,7 +101,10 @@ export function loadConfig(): BackendConfig { archiveFallbackEndpoint: process.env.ARCHIVE_FALLBACK_ENDPOINT ?? null, indexPollIntervalMs: numericEnv('INDEX_POLL_INTERVAL_MS', 15000), indexStartHeight: numericEnv('INDEX_START_HEIGHT', 0), - minaguardVkHash: process.env.MINAGUARD_VK_HASH ?? null, + // An empty value is the documented E2E/dev sentinel for disabling the VK + // filter. Normalize it to the same representation as an unset variable so + // downstream security checks do not try to match every vault against "". + minaguardVkHash: process.env.MINAGUARD_VK_HASH || null, indexerMode, indexerDisabled: process.env.INDEXER_DISABLED === 'true', fixedLatestSlot: diff --git a/backend/src/tests/config.test.ts b/backend/src/tests/config.test.ts new file mode 100644 index 00000000..3fb056f4 --- /dev/null +++ b/backend/src/tests/config.test.ts @@ -0,0 +1,22 @@ +import { afterEach, describe, expect, it } from 'bun:test'; +import { loadConfig } from '../config.js'; + +const originalEnv = { ...process.env }; + +afterEach(() => { + for (const key of Object.keys(process.env)) { + if (!(key in originalEnv)) delete process.env[key]; + } + Object.assign(process.env, originalEnv); +}); + +describe('backend configuration', () => { + it('normalizes an empty verification-key hash to an unset filter', () => { + process.env.DATABASE_URL = 'postgresql://localhost/minaguard'; + process.env.MINA_ENDPOINT = 'http://localhost:8080/graphql'; + process.env.ARCHIVE_ENDPOINT = 'http://localhost:8282'; + process.env.MINAGUARD_VK_HASH = ''; + + expect(loadConfig().minaguardVkHash).toBeNull(); + }); +}); From 7143111e986f3eb5350a5c7fd93cb89698c67094 Mon Sep 17 00:00:00 2001 From: Jason Park Date: Mon, 14 Sep 2026 06:22:47 +0000 Subject: [PATCH 4/9] fix: enforce permission checks for offline actions --- e2e/ui/display.test.ts | 99 +++++++++++++ ui/app/transactions/[id]/page.tsx | 209 +++++++++++++++++---------- ui/components/OfflineSigningFlow.tsx | 16 +- 3 files changed, 250 insertions(+), 74 deletions(-) diff --git a/e2e/ui/display.test.ts b/e2e/ui/display.test.ts index bea0b779..5e35e3d4 100644 --- a/e2e/ui/display.test.ts +++ b/e2e/ui/display.test.ts @@ -10,6 +10,7 @@ import { TREASURY, OPS_CHILD, PERSONAL, + OWNER_2, RECIPIENT, TREASURY_STATE, PROPOSALS, @@ -127,3 +128,101 @@ test('expired proposal has no approve/execute buttons', async ({ page }) => { await expect(page.getByText('expired', { exact: true }).first()).toBeVisible({ timeout: 10_000 }); await expectNoActionButtons(page, [/approve proposal/i, /execute proposal/i]); }); + +test('unsafe CREATE_CHILD target blocks online and offline approval', async ({ page }) => { + // Re-shape the pending fixture as a remote CREATE_CHILD proposal while + // leaving the indexed parent canonical. This models the finding's malicious + // creator deploying an unsafe child outside the supported client. + await page.route( + new RegExp(`/api/contracts/${TREASURY}/proposals(?:\\?.*)?$`), + async (route) => { + const response = await route.fetch(); + const proposals = (await response.json()) as Array>; + await route.fulfill({ + response, + json: proposals.map((proposal) => + proposal.proposalHash === PROPOSALS.pendingTransfer + ? { + ...proposal, + txType: 'createChild', + destination: 'remote', + childAccount: OPS_CHILD, + receivers: [], + } + : proposal, + ), + }); + }, + ); + await page.route( + `**/api/contracts/${TREASURY}/proposals/${PROPOSALS.pendingTransfer}/approvals`, + (route) => route.fulfill({ json: [] }), + ); + await page.route(`**/api/accounts/${OPS_CHILD}/security`, (route) => + route.fulfill({ + json: { + accountFound: true, + verificationKeyHash: 'canonical-vk', + verificationKeyMatches: true, + permissionKinds: { send: 'Either' }, + expectedPermissionKinds: { send: 'Proof' }, + permissionMismatches: ['send'], + safe: false, + }, + }), + ); + + await openProposal(page, PROPOSALS.pendingTransfer); + await expect(page.getByText('Unsafe permission vector')).toBeVisible({ + timeout: 10_000, + }); + await expect( + page.getByRole('button', { name: /approve proposal/i }), + ).not.toBeVisible(); + + await page.getByRole('button', { name: 'Offline', exact: true }).click(); + await expect( + page.getByText(/offline bundle creation and broadcast are blocked/i), + ).toBeVisible(); + await expect( + page.getByRole('button', { name: /export approve bundle/i }), + ).not.toBeVisible(); + await expect(page.getByText(/drop signed \.json/i)).not.toBeVisible(); +}); + +test('offline bundle export rechecks permissions instead of trusting page state', async ({ page }) => { + let unsafeNow = false; + await page.route(`**/api/accounts/${TREASURY}/security`, async (route) => { + if (!unsafeNow) { + await route.continue(); + return; + } + await route.fulfill({ + json: { + accountFound: true, + verificationKeyHash: 'canonical-vk', + verificationKeyMatches: true, + permissionKinds: { send: 'Either' }, + expectedPermissionKinds: { send: 'Proof' }, + permissionMismatches: ['send'], + safe: false, + }, + }); + }); + + await openProposal(page, PROPOSALS.pendingTransfer); + await page.getByRole('button', { name: 'Offline', exact: true }).click(); + await page.getByPlaceholder('B62q...').fill(OWNER_2); + const exportButton = page.getByRole('button', { + name: /export approve bundle/i, + }); + await expect(exportButton).toBeVisible(); + + // The hook admitted the page while the account was safe. Flip only the live + // response and prove the export callback checks again before creating a file. + unsafeNow = true; + await exportButton.click(); + await expect( + page.getByText(/vault permissions have not passed the canonical security check/i), + ).toBeVisible(); +}); diff --git a/ui/app/transactions/[id]/page.tsx b/ui/app/transactions/[id]/page.tsx index 7749dda1..6556a4e6 100644 --- a/ui/app/transactions/[id]/page.tsx +++ b/ui/app/transactions/[id]/page.tsx @@ -16,6 +16,7 @@ import { fetchApprovals, extractTxHash, fetchBalance, + fetchContract, fetchVaultSecurityStatus, isCanonicalVaultSecurity, recordSubmission, @@ -40,6 +41,37 @@ import { useVaultSecurity } from '@/hooks/useVaultSecurity'; import { assertValidMinaAddress, buildOfflineApproveBundle, buildOfflineExecuteBundle } from '@/lib/offline-signing'; import { DownloadCLILink, OfflineSigningFlow, UploadSignedResponse } from '@/components/OfflineSigningFlow'; +/** + * Re-authenticates every account involved in a proposal immediately before an + * online or offline action. The target SubVault is intentionally checked via + * Mina directly: a malicious CREATE_CHILD deployment must never become safe + * merely because the parent proposal itself was indexed. + */ +async function assertProposalVaultSecurity( + parentAddress: string, + childAddress: string | null, +): Promise { + const [freshParent, parentSecurity, childSecurity] = await Promise.all([ + fetchContract(parentAddress), + fetchVaultSecurityStatus(parentAddress), + childAddress ? fetchVaultSecurityStatus(childAddress) : Promise.resolve(null), + ]); + + if ( + !freshParent?.permissionsVerified || + !isCanonicalVaultSecurity(parentSecurity) + ) { + throw new Error( + 'Vault permissions have not passed the canonical security check', + ); + } + if (childAddress && !isCanonicalVaultSecurity(childSecurity)) { + throw new Error( + 'SubVault permissions have not passed the canonical security check', + ); + } +} + /** Proposal detail page with approve/execute actions and lifecycle status. */ export default function TransactionDetailPage() { const params = useParams(); @@ -373,6 +405,10 @@ export default function TransactionDetailPage() { } let success = false; await startOperation('Submitting approval on-chain...', async (onProgress) => { + await assertProposalVaultSecurity( + captured.contractAddress, + captured.proposal.childAccount, + ); const result = await approveProposalOnchain({ contractAddress: captured.contractAddress, approverAddress: captured.approverAddress, @@ -410,6 +446,10 @@ export default function TransactionDetailPage() { } let success = false; await startOperation('Building execute transaction...', async (onProgress) => { + await assertProposalVaultSecurity( + captured.contractAddress, + captured.proposal.childAccount, + ); const isCreateChild = captured.proposal.txType === 'createChild'; const isRemoteLifecycle = captured.proposal.destination === 'remote' && @@ -906,82 +946,105 @@ export default function TransactionDetailPage() {

This must be the public key corresponding to the MINA_PRIVATE_KEY used on the air-gapped machine.

-
- {proposal.approvalCount < owners.length && ( - { - assertValidMinaAddress(offlineFeePayerAddress); - if (!owners.some((o) => o.address === offlineFeePayerAddress)) { - throw new Error('Signer address is not an owner of this multisig'); - } - if (approvalAddresses.includes(offlineFeePayerAddress)) { - throw new Error('This address has already approved this proposal'); - } - if (childConfigCheck === 'mismatch') { - throw new Error( - 'SubVault config mismatch: the displayed owners/threshold do not match the ' + - 'signed proposal data. Do not approve this proposal.', + {permissionsSafe && childPermissionsSafe ? ( +
+ {proposal.approvalCount < owners.length && ( + { + assertValidMinaAddress(offlineFeePayerAddress); + if (!owners.some((o) => o.address === offlineFeePayerAddress)) { + throw new Error('Signer address is not an owner of this multisig'); + } + if (approvalAddresses.includes(offlineFeePayerAddress)) { + throw new Error('This address has already approved this proposal'); + } + if (childConfigCheck === 'mismatch') { + throw new Error( + 'SubVault config mismatch: the displayed owners/threshold do not match the ' + + 'signed proposal data. Do not approve this proposal.', + ); + } + if (addOwnerDataCheck === 'mismatch') { + throw new Error( + 'This Add Owner proposal arranges owners in an order this app cannot reproduce. ' + + 'Do not approve it.', + ); + } + const p = proposal!; + await assertProposalVaultSecurity( + multisig!.address, + p.childAccount, ); - } - if (addOwnerDataCheck === 'mismatch') { - throw new Error( - 'This Add Owner proposal arranges owners in an order this app cannot reproduce. ' + - 'Do not approve it.', + return buildOfflineApproveBundle({ + contractAddress: multisig!.address, + feePayerAddress: offlineFeePayerAddress, + proposal: { ...p, receivers: p.receivers.map((r) => ({ address: r.address, amount: r.amount })) }, + }); + }} + /> + )} + {proposal.approvalCount >= threshold && ( + { + assertValidMinaAddress(offlineFeePayerAddress); + const p = proposal!; + await assertProposalVaultSecurity( + multisig!.address, + p.childAccount, ); - } - const p = proposal!; - return buildOfflineApproveBundle({ - contractAddress: multisig!.address, - feePayerAddress: offlineFeePayerAddress, - proposal: { ...p, receivers: p.receivers.map((r) => ({ address: r.address, amount: r.amount })) }, - }); - }} - /> - )} - {proposal.approvalCount >= threshold && ( - { - assertValidMinaAddress(offlineFeePayerAddress); - const p = proposal!; - return buildOfflineExecuteBundle({ - contractAddress: multisig!.address, - feePayerAddress: offlineFeePayerAddress, - proposal: { ...p, receivers: p.receivers.map((r) => ({ address: r.address, amount: r.amount })) }, - }); - }} - /> - )} -
- = threshold ? ['approve', 'execute'] : ['approve']} - expectedContractAddress={multisig!.address} - expectedProposalHash={proposal!.proposalHash} - onComplete={(response, txHash) => { - const kind = response.action as 'approve' | 'execute'; - void recordSubmission(multisig!.address, proposal!.proposalHash, kind, txHash); - savePendingTx({ - kind, - contractAddress: multisig!.address, - proposalHash: proposal!.proposalHash, - txHash, - signerPubkey: offlineFeePayerAddress, - createdAt: new Date().toISOString(), - }); - if (kind === 'execute') { - router.push(`/accounts/${multisig!.address}`); - } else { - router.push('/transactions'); + return buildOfflineExecuteBundle({ + contractAddress: multisig!.address, + feePayerAddress: offlineFeePayerAddress, + proposal: { ...p, receivers: p.receivers.map((r) => ({ address: r.address, amount: r.amount })) }, + }); + }} + /> + )} +
+ ) : ( +

+ Offline bundle creation and broadcast are blocked until + the Vault and target SubVault pass their live permission checks. +

+ )} + {permissionsSafe && childPermissionsSafe && ( + = threshold ? ['approve', 'execute'] : ['approve']} + expectedContractAddress={multisig!.address} + expectedProposalHash={proposal!.proposalHash} + beforeBroadcast={() => + assertProposalVaultSecurity( + multisig!.address, + proposal!.childAccount, + ) } - }} - /> + onComplete={(response, txHash) => { + const kind = response.action as 'approve' | 'execute'; + void recordSubmission(multisig!.address, proposal!.proposalHash, kind, txHash); + savePendingTx({ + kind, + contractAddress: multisig!.address, + proposalHash: proposal!.proposalHash, + txHash, + signerPubkey: offlineFeePayerAddress, + createdAt: new Date().toISOString(), + }); + if (kind === 'execute') { + router.push(`/accounts/${multisig!.address}`); + } else { + router.push('/transactions'); + } + }} + /> + )} )}
diff --git a/ui/components/OfflineSigningFlow.tsx b/ui/components/OfflineSigningFlow.tsx index 3c802856..6c941b4d 100644 --- a/ui/components/OfflineSigningFlow.tsx +++ b/ui/components/OfflineSigningFlow.tsx @@ -312,10 +312,18 @@ interface UploadSignedResponseProps { expectedContractAddress?: string; /** When set, reject signed responses that target a different proposal hash. */ expectedProposalHash?: string; + /** Live, online policy check performed immediately before broadcasting. */ + beforeBroadcast?: (response: OfflineSignedTxResponse) => Promise; onComplete?: (response: OfflineSignedTxResponse, txHash: string) => void; } -export function UploadSignedResponse({ acceptActions, expectedContractAddress, expectedProposalHash, onComplete }: UploadSignedResponseProps) { +export function UploadSignedResponse({ + acceptActions, + expectedContractAddress, + expectedProposalHash, + beforeBroadcast, + onComplete, +}: UploadSignedResponseProps) { const [broadcasting, setBroadcasting] = useState(false); const [error, setError] = useState(null); const [done, setDone] = useState(false); @@ -372,6 +380,12 @@ export function UploadSignedResponse({ acceptActions, expectedContractAddress, e ); } + // A signed offline response can outlive the page state from which its + // bundle was exported. Re-run the caller's live security policy at the + // final online boundary instead of trusting an old UI check or any + // account snapshot carried inside the untrusted bundle. + await beforeBroadcast?.(response); + setBroadcasting(true); const txJson = typeof response.transaction === 'string' ? response.transaction : JSON.stringify(response.transaction); From 42d972924edcd8d4ce49d6cdeb03c0c509a9f488 Mon Sep 17 00:00:00 2001 From: Jason Park Date: Mon, 14 Sep 2026 08:02:56 +0000 Subject: [PATCH 5/9] fix: seal permissions during vault initialization --- contracts/.vk-hash | 4 +- contracts/src/MinaGuard.ts | 55 +++++++++++------- contracts/src/guard-permissions.ts | 11 ++++ contracts/src/index.ts | 1 + contracts/src/tests/child.test.ts | 64 +++++++++++++++++++++ contracts/src/tests/setup.test.ts | 65 ++++++++++++++++++++- contracts/src/tests/test-helpers.ts | 20 +++---- docs/contracts-audit-guide.md | 41 +++++++++----- docs/security-audit-guide.md | 15 ++++- docs/ui-audit-guide.md | 10 ++-- ui/lib/multisigClient.ts | 23 -------- ui/lib/multisigClient.worker.ts | 88 ----------------------------- 12 files changed, 231 insertions(+), 166 deletions(-) diff --git a/contracts/.vk-hash b/contracts/.vk-hash index c8e91c28..8537b119 100644 --- a/contracts/.vk-hash +++ b/contracts/.vk-hash @@ -12,5 +12,5 @@ # bun run dev-helpers/cli.ts vk-hash compile # MINA_NETWORK_DOMAIN=mainnet bun run dev-helpers/cli.ts vk-hash compile # Then update the two lines below with the printed hashes. -testnet=17954459212229963636719017180527602577658411613195319286124003403812033745424 -mainnet=4889704935690266535842858270757274243877043121058041729760084101432014581919 +testnet=2314409526151640805504309350214287339499092137329507508633408625582908500232 +mainnet=14973252370943668189782431626091346743624422522271991776262338893002628132017 diff --git a/contracts/src/MinaGuard.ts b/contracts/src/MinaGuard.ts index ee9ed596..56abd6aa 100644 --- a/contracts/src/MinaGuard.ts +++ b/contracts/src/MinaGuard.ts @@ -14,7 +14,10 @@ import { UInt32, UInt64, } from 'o1js'; -import { GUARD_PERMISSIONS } from './guard-permissions.js'; +import { + GUARD_DEPLOY_PERMISSIONS, + GUARD_PERMISSIONS, +} from './guard-permissions.js'; import { MAX_OWNERS, @@ -282,30 +285,31 @@ export class MinaGuard extends SmartContract { }; /** - * Configures account permissions and emits a deploy discovery event. + * Configures temporary deployment permissions and emits a discovery event. * * SECURITY, authenticate the deployed account: deploy() is ordinary * transaction-construction code, not part of MinaGuard's proved circuit. A * creator can deploy the canonical verification key while changing this * signature-authorized AccountUpdate's permissions. Never recognize or use - * a MinaGuard account based on its verification key alone. Clients and - * indexers MUST compare every on-chain permission against GUARD_PERMISSIONS - * before displaying, funding, proposing, approving, or executing for it. + * a MinaGuard account based on its verification key alone. The supported + * atomic setup()/reserveForParent() flow overwrites creator-selected values + * with GUARD_PERMISSIONS under proof authorization. Clients and indexers + * MUST still compare every stored permission against GUARD_PERMISSIONS to + * reject deployments constructed outside that flow. * - * SECURITY, initialize atomically: deploy() only publishes the account and its - * proof-authorized permissions; it does NOT set governance. Between deploy() - * and setup()/reserveForParent() the guard is uninitialized, and those init - * methods are authorized by proof alone with no deployer binding, so anyone - * can front-run initialization: call setup() with their own owner set, or - * reserveForParent() to bind the address to an attacker parent (which then - * permanently blocks the legitimate setup()). Callers MUST include deploy() - * and setup()/reserveForParent() in the SAME transaction, so no uninitialized - * on-chain window exists. Never deploy a guard in one transaction and - * initialize it in a later one. + * SECURITY, initialize atomically: deploy() temporarily leaves + * setPermissions proof-authorized. setup()/reserveForParent() installs and + * seals the final canonical vector while also initializing governance. A + * creator who weakens another deployment permission is overwritten; a + * creator who makes setPermissions impossible causes the entire transaction + * to fail. These init methods are also authorized by proof alone with no + * deployer binding, so a separately deployed guard could be front-run. + * Callers MUST include deploy() and setup()/reserveForParent() in the SAME + * transaction. Never broadcast deploy() by itself. */ async deploy() { await super.deploy(); - this.account.permissions.set(GUARD_PERMISSIONS); + this.account.permissions.set(GUARD_DEPLOY_PERMISSIONS); this.emitEvent('deployed', { guardAddress: this.address, @@ -314,6 +318,11 @@ export class MinaGuard extends SmartContract { // -- Shared validation helpers --------------------------------------------- + /** Permanently installs the only permission vector accepted by MinaGuard. */ + private lockPermissions(): void { + this.account.permissions.set(GUARD_PERMISSIONS); + } + private getInitializedOwnersCommitment(): Field { const ownersCommitment = this.ownersCommitment.getAndRequireEquals(); ownersCommitment.assertNotEquals(Field(0), 'Wallet not initialized'); @@ -721,14 +730,16 @@ export class MinaGuard extends SmartContract { * with the owner set — no client-side cross-check is required. * * MUST be called in the SAME transaction as deploy() (see deploy()): this - * method is authorized by proof alone with no deployer binding, so a guard - * left deployed-but-uninitialized can be front-run and set up by anyone. + * method installs and permanently seals GUARD_PERMISSIONS. It is authorized + * by proof alone with no deployer binding, so a guard left + * deployed-but-uninitialized can be front-run and set up by anyone. */ @method async setup( threshold: Field, numOwners: Field, initialOwners: SetupOwnersInput ) { + this.lockPermissions(); this.parent.requireEquals(PublicKey.empty()); this.initializeState( threshold, @@ -751,9 +762,10 @@ export class MinaGuard extends SmartContract { * Can only be called once (parent must be empty). * * SECURITY: this MUST share the deploy() transaction (see deploy()). It is - * authorized by proof alone with no deployer binding, so a child left - * deployed-but-unreserved can be front-run: an attacker reserves it to their - * own parent, permanently bricking the intended child address. + * the child path that installs and permanently seals GUARD_PERMISSIONS. It + * is authorized by proof alone with no deployer binding, so a child left + * deployed-but-unreserved can be front-run: an attacker reserves it to + * their own parent, permanently bricking the intended child address. */ @method async reserveForParent( parentAddress: PublicKey, @@ -762,6 +774,7 @@ export class MinaGuard extends SmartContract { numOwners: Field, initialOwners: SetupOwnersInput, ) { + this.lockPermissions(); this.ownersCommitment.requireEquals(Field(0)); this.parent.requireEquals(PublicKey.empty()); parentAddress.equals(PublicKey.empty()).assertFalse('Parent address must not be empty'); diff --git a/contracts/src/guard-permissions.ts b/contracts/src/guard-permissions.ts index 57ddf646..5a75268f 100644 --- a/contracts/src/guard-permissions.ts +++ b/contracts/src/guard-permissions.ts @@ -25,6 +25,17 @@ export const GUARD_PERMISSIONS = { access: Permissions.none(), }; +/** + * Permission vector used only by the signature-authorized deployment update. + * The proof-authorized setup()/reserveForParent() update replaces this with + * GUARD_PERMISSIONS in the same atomic transaction. The sole temporary + * difference is that a MinaGuard proof may close the permission vector. + */ +export const GUARD_DEPLOY_PERMISSIONS = { + ...GUARD_PERMISSIONS, + setPermissions: Permissions.proof(), +}; + export const GUARD_PERMISSION_NAMES = [ 'editState', 'send', diff --git a/contracts/src/index.ts b/contracts/src/index.ts index fc300cc7..58cbabb0 100644 --- a/contracts/src/index.ts +++ b/contracts/src/index.ts @@ -34,6 +34,7 @@ export { export { OwnerWitness, PublicKeyOption, computeOwnerChain, assertOwnerMembership, addOwnerToCommitment, removeOwnerFromCommitment } from './list-commitment.js'; export { GUARD_PERMISSIONS, + GUARD_DEPLOY_PERMISSIONS, GUARD_PERMISSION_NAMES, GUARD_PERMISSION_KINDS, type GuardPermissionName, diff --git a/contracts/src/tests/child.test.ts b/contracts/src/tests/child.test.ts index df6b8fa3..d331b03f 100644 --- a/contracts/src/tests/child.test.ts +++ b/contracts/src/tests/child.test.ts @@ -4,6 +4,7 @@ import { MerkleMap, MerkleMapWitness, Mina, + Permissions, Poseidon, PrivateKey, PublicKey, @@ -36,6 +37,10 @@ import { type TestContext, } from './test-helpers.js'; import { computeOwnerChain } from '../list-commitment.js'; +import { + GUARD_DEPLOY_PERMISSIONS, + GUARD_PERMISSIONS, +} from '../guard-permissions.js'; import { beforeEach, describe, expect, it } from 'bun:test'; describe('MinaGuard - Child Lifecycle', () => { @@ -91,6 +96,62 @@ describe('MinaGuard - Child Lifecycle', () => { // -- executeSetupChild ------------------------------------------------------ describe('executeSetupChild', () => { + it('overwrites creator-weakened child permissions during reservation', async () => { + const setupOwners = toFixedSetupOwners( + parentCtx.owners.map((owner) => owner.pub), + ); + + const txn = await Mina.transaction(parentCtx.deployerAccount, async () => { + AccountUpdate.fundNewAccount(parentCtx.deployerAccount); + await childZkApp.deploy(); + childZkApp.account.permissions.set({ + ...GUARD_DEPLOY_PERMISSIONS, + send: Permissions.proofOrSignature(), + }); + await childZkApp.reserveForParent( + parentCtx.zkAppAddress, + Field(1234), + Field(2), + Field(3), + new SetupOwnersInput({ owners: setupOwners }), + ); + }); + await txn.prove(); + await txn.sign([parentCtx.deployerKey, childKey]).send(); + + expect(Mina.getAccount(childAddress).permissions).toEqual( + GUARD_PERMISSIONS, + ); + }); + + it('rejects a child creator blocking the reservation permission lock', async () => { + const setupOwners = toFixedSetupOwners( + parentCtx.owners.map((owner) => owner.pub), + ); + + await expect(async () => { + const txn = await Mina.transaction(parentCtx.deployerAccount, async () => { + AccountUpdate.fundNewAccount(parentCtx.deployerAccount); + await childZkApp.deploy(); + childZkApp.account.permissions.set({ + ...GUARD_DEPLOY_PERMISSIONS, + setPermissions: Permissions.impossible(), + }); + await childZkApp.reserveForParent( + parentCtx.zkAppAddress, + Field(1234), + Field(2), + Field(3), + new SetupOwnersInput({ owners: setupOwners }), + ); + }); + await txn.prove(); + await txn.sign([parentCtx.deployerKey, childKey]).send(); + }).toThrow(); + + expect(Mina.hasAccount(childAddress)).toBe(false); + }); + it('initializes a child guard with parent approval', async () => { const { proposalHash } = await setupChildWithParentOwners(); @@ -99,6 +160,9 @@ describe('MinaGuard - Child Lifecycle', () => { expect(childZkApp.childExecutionRoot.get()).toEqual(EMPTY_MERKLE_MAP_ROOT); expect(childZkApp.threshold.get()).toEqual(Field(2)); expect(childZkApp.numOwners.get()).toEqual(Field(3)); + expect(Mina.getAccount(childAddress).permissions).toEqual( + GUARD_PERMISSIONS, + ); expect(proposalHash).toBeDefined(); }); diff --git a/contracts/src/tests/setup.test.ts b/contracts/src/tests/setup.test.ts index 2bb8d825..81c7e32b 100644 --- a/contracts/src/tests/setup.test.ts +++ b/contracts/src/tests/setup.test.ts @@ -1,5 +1,9 @@ -import { Field, Mina, AccountUpdate, UInt64 } from 'o1js'; +import { Field, Mina, AccountUpdate, Permissions, UInt64 } from 'o1js'; import { EMPTY_MERKLE_MAP_ROOT } from '../constants.js'; +import { + GUARD_DEPLOY_PERMISSIONS, + GUARD_PERMISSIONS, +} from '../guard-permissions.js'; import { SetupOwnersInput } from '../MinaGuard.js'; import { setupLocalBlockchain, @@ -29,6 +33,65 @@ describe('MinaGuard - Setup', () => { expect(ctx.zkApp.configNonce.get()).toEqual(Field(0)); expect(ctx.zkApp.approvalRoot.get()).toEqual(EMPTY_MERKLE_MAP_ROOT); expect(ctx.zkApp.voteNullifierRoot.get()).toEqual(EMPTY_MERKLE_MAP_ROOT); + expect(Mina.getAccount(ctx.zkAppAddress).permissions).toEqual( + GUARD_PERMISSIONS, + ); + }); + + it('should overwrite creator-weakened deploy permissions during setup', async () => { + const { zkApp, zkAppKey, deployerKey, deployerAccount, owners } = ctx; + const setupOwners = toFixedSetupOwners(owners.map((owner) => owner.pub)); + + const txn = await Mina.transaction(deployerAccount, async () => { + AccountUpdate.fundNewAccount(deployerAccount); + await zkApp.deploy(); + + // A malicious creator controls the signed deployment update. setup() is + // a separate proof-authorized update and must replace this weakened send. + zkApp.account.permissions.set({ + ...GUARD_DEPLOY_PERMISSIONS, + send: Permissions.proofOrSignature(), + }); + await zkApp.setup( + Field(2), + Field(owners.length), + new SetupOwnersInput({ owners: setupOwners }), + ); + }); + await txn.prove(); + await txn.sign([deployerKey, zkAppKey]).send(); + + expect(Mina.getAccount(ctx.zkAppAddress).permissions).toEqual( + GUARD_PERMISSIONS, + ); + }); + + it('should reject a creator blocking the proof-authorized permission lock', async () => { + const { zkApp, zkAppKey, deployerKey, deployerAccount, owners } = ctx; + const setupOwners = toFixedSetupOwners(owners.map((owner) => owner.pub)); + + await expect(async () => { + const txn = await Mina.transaction(deployerAccount, async () => { + AccountUpdate.fundNewAccount(deployerAccount); + await zkApp.deploy(); + + // This prevents setup() from writing GUARD_PERMISSIONS. Atomicity must + // make the complete deployment fail rather than leave an unsafe vault. + zkApp.account.permissions.set({ + ...GUARD_DEPLOY_PERMISSIONS, + setPermissions: Permissions.impossible(), + }); + await zkApp.setup( + Field(2), + Field(owners.length), + new SetupOwnersInput({ owners: setupOwners }), + ); + }); + await txn.prove(); + await txn.sign([deployerKey, zkAppKey]).send(); + }).toThrow(); + + expect(Mina.hasAccount(ctx.zkAppAddress)).toBe(false); }); it('should emit deploy and setup bootstrap events', async () => { diff --git a/contracts/src/tests/test-helpers.ts b/contracts/src/tests/test-helpers.ts index 2893291c..1d4de17d 100644 --- a/contracts/src/tests/test-helpers.ts +++ b/contracts/src/tests/test-helpers.ts @@ -132,9 +132,18 @@ export async function deployAndSetup( ): Promise { const { zkApp, zkAppKey, zkAppAddress, deployerKey, deployerAccount, owners } = ctx; + const setupOwners = toFixedSetupOwners(owners.map((o) => o.pub)); + + // deploy() deliberately leaves setPermissions proof-authorized only until + // setup() installs and seals the canonical vector. Keep both updates atomic. const deployTxn = await Mina.transaction(deployerAccount, async () => { AccountUpdate.fundNewAccount(deployerAccount); await zkApp.deploy(); + await zkApp.setup( + Field(threshold), + Field(owners.length), + new SetupOwnersInput({ owners: setupOwners }) + ); }); await deployTxn.prove(); await deployTxn.sign([deployerKey, zkAppKey]).send(); @@ -146,17 +155,6 @@ export async function deployAndSetup( await fundTxn.prove(); await fundTxn.sign([deployerKey]).send(); - const setupOwners = toFixedSetupOwners(owners.map((o) => o.pub)); - - const setupTxn = await Mina.transaction(deployerAccount, async () => { - await zkApp.setup( - Field(threshold), - Field(owners.length), - new SetupOwnersInput({ owners: setupOwners }) - ); - }); - await setupTxn.prove(); - await setupTxn.sign([deployerKey, zkAppKey]).send(); } // -- Proposal Helpers -------------------------------------------------------- diff --git a/docs/contracts-audit-guide.md b/docs/contracts-audit-guide.md index 513b072a..a959dc23 100644 --- a/docs/contracts-audit-guide.md +++ b/docs/contracts-audit-guide.md @@ -235,16 +235,21 @@ Defined in `constants.ts`: ### On-chain multi-step flow -**Deploy.** `deploy()` sets account permissions (see [Permissions](#permissions)) and emits -a `DeployEvent` with the contract address for indexer discovery. `deploy()` is transaction-building -code, not a proved method: the deployment signature authenticates the installed values, while the -MinaGuard verification key does not. A vault MUST NOT be recognized from its verification-key hash -alone; online consumers must also compare every stored permission with `GUARD_PERMISSIONS`. +**Deploy.** `deploy()` installs `GUARD_DEPLOY_PERMISSIONS` and emits a `DeployEvent` with the +contract address for indexer discovery. The temporary vector matches the final vector except that +`setPermissions` is `proof()`. `deploy()` is transaction-building code, not a proved method: the +deployment signature authenticates the installed values, while the MinaGuard verification key does +not. The proved `setup()` or `reserveForParent()` update MUST be included in the same transaction; +it overwrites the entire vector with `GUARD_PERMISSIONS` and seals `setPermissions` as +`impossible()`. A vault MUST NOT be recognized from its verification-key hash alone; online +consumers must also compare every stored permission with `GUARD_PERMISSIONS` to reject accounts +created outside the supported flow. **Setup.** `setup(threshold, numOwners, initialOwners)` — one-time root-guard initialization. - Guard: `ownersCommitment == Field(0)` (not yet initialized) +- Installs the complete `GUARD_PERMISSIONS` vector under proof authorization and permanently seals `setPermissions` - Computes `ownersCommitment` **on-chain** from `initialOwners` via `computeSetupOwnersChain`, after `assertCoherentSetupOwners` rejects non-empty padding slots and duplicate active owners - Validates: `threshold > 0`, `numOwners >= threshold`, `numOwners <= MAX_OWNERS` - Initializes the guard state: `nonce = 0`, `parentNonce = 0`, `approvalRoot`, `voteNullifierRoot`, `childExecutionRoot` set to `EMPTY_MERKLE_MAP_ROOT`; `parent = PublicKey.empty()`; `childMultiSigEnabled = Field(1)` (`reservedConfigHash` is untouched — `Field(0)` on a root guard) @@ -375,6 +380,9 @@ on-chain-computed commitment (write-once — a second reserve is blocked by the guard), and emits `CreateChildConfigEvent` + 20 `CreateChildOwnerEvent`s on-chain so the child's intended owner list is publicly available before `executeSetupChild` runs. (The indexer stores these as raw events but does not parse them; the UI and offline CLI fetch and parse them directly.) +It also installs the complete `GUARD_PERMISSIONS` vector under proof authorization and permanently +seals `setPermissions`; this must happen at reservation time, not later in `executeSetupChild`, so +the child is never included on chain with creator-chosen permissions. - **Why this method exists:** `executeSetupChild` requires the child's owner list and threshold as arguments. Without `reserveForParent`, the `ProposalEvent` only contains a `data` hash (`Poseidon([ownersCommitment, threshold, numOwners])`) — the individual owner addresses are not recoverable from the hash. By emitting the full owner list on the child at propose time, any user can retrieve the config from on-chain events and execute `setupChild` without coordinating with the proposer. - **Anti-front-running:** Setting `this.parent` at propose time prevents attackers from calling `setup()` on the uninitialized child between deploy and execute. `setup()` asserts `this.parent == PublicKey.empty()`, which fails once `reserveForParent` has run. `executeSetupChild` verifies `this.parent == proposal.guardAddress`, ensuring only the designated parent can initialize the child. @@ -452,7 +460,10 @@ parent-walk for REMOTE executions) live in ### Permissions -Set in `deploy()`: +The signature-authorized `deploy()` update installs `GUARD_DEPLOY_PERMISSIONS`. It is identical to +the table below except that `setPermissions` is temporarily `proof()`. In the same transaction, the +proof-authorized `setup()` or `reserveForParent()` update overwrites the complete vector with +`GUARD_PERMISSIONS`: | Permission | Value | Rationale | | ---------- | ----- | --------- | @@ -470,10 +481,12 @@ Set in `deploy()`: | `setTiming` | `impossible()` | Not used | | `access` | `none()` | No additional access gate | -The canonical vector is defined once as `GUARD_PERMISSIONS` in -`contracts/src/guard-permissions.ts`. If that exact vector was installed, the one-shot deploy key is -powerless afterward: every state/fund knob requires a proof and every permission knob is -`impossible`. +Both vectors are defined in `contracts/src/guard-permissions.ts`, with the temporary vector derived +from the canonical one by overriding only `setPermissions`. If the atomic initialization succeeds, +the one-shot deploy key is powerless afterward: every state/fund knob requires a proof and every +permission knob is `impossible`. If a creator weakens `send` in the deployment update, the proved +initialization overwrites it. If the creator makes `setPermissions` impossible early, the proved +write cannot execute and the entire atomic creation transaction fails. That statement is conditional on checking the stored vector. `deploy()` is not part of the proved circuit, and its AccountUpdate is authorized by the vault account signature. A creator can use the @@ -531,9 +544,11 @@ initialize the double-bound config (`proposal.data` **and** `reservedConfigHash` sequence of owner/threshold changes can drive threshold above the owner count (permanent lock) or past `MAX_OWNERS` (circuit-size overflow). -**5. Permissions and VK immutability.** `deploy()` sets `setPermissions: impossible()` and -`setVerificationKey: impossibleDuringCurrentVersion()`. Confirm there is no method path that -re-authorizes state/fund movement outside a proof, and that the deployed VK matches the pinned +**5. Permissions and VK immutability.** `deploy()` temporarily sets `setPermissions: proof()`; +proof-authorized `setup()` or `reserveForParent()` overwrites the full vector and sets +`setPermissions: impossible()` in the same transaction. Confirm no production path broadcasts +`deploy()` alone, no other method writes permissions, and no method path re-authorizes state/fund +movement outside a proof. Also confirm that the deployed VK matches the pinned `contracts/.vk-hash` (the `check-vk-hash` CI job enforces this per network). ## Security properties diff --git a/docs/security-audit-guide.md b/docs/security-audit-guide.md index d5e613b6..d4a6a7ad 100644 --- a/docs/security-audit-guide.md +++ b/docs/security-audit-guide.md @@ -38,6 +38,13 @@ the backend and online UI must also verify the complete stored permission vector configured Mina node and compares it with a build-time canonical vector; it does not trust the indexer to report either side of the comparison. +In the supported creation flow, the signed `deploy()` update installs a temporary permission +vector whose `setPermissions` field is `proof()`. In the same atomic transaction, the proved +`setup()` (root) or `reserveForParent()` (child) update overwrites the complete vector with +`GUARD_PERMISSIONS`, including `setPermissions: impossible()`. This prevents the supported client +from producing a vault with creator-retained signature authority. Stored-permission checks remain +mandatory because a creator can bypass the supported flow and deploy a lookalike directly. + ### Trusted computing base The code that must be correct for funds to be safe: @@ -100,8 +107,10 @@ proof alone with no deployer binding, so a guard left deployed-but-uninitialized anyone can call `setup()` with their own owner set, or `reserveForParent()` to bind the address to an attacker parent (permanently blocking the legitimate `setup()`). Callers therefore MUST include `deploy()` and `setup()`/`reserveForParent()` in the SAME transaction so no uninitialized on-chain -window exists — this is a caller obligation, not something the circuit enforces (see the #112 -doc-comments on `deploy()`/`setup()`/`reserveForParent()` in `MinaGuard.ts`). +window exists. Atomicity is also required for permission safety: `deploy()` deliberately leaves +`setPermissions` open to a MinaGuard proof until the initialization proof installs and seals the +final vector. This is a caller obligation, not something the circuit enforces (see the doc-comments +on `deploy()`/`setup()`/`reserveForParent()` in `MinaGuard.ts`). ## Invariants @@ -134,7 +143,7 @@ This table maps each claim to its enforcement point and primary test coverage (a | Parent can always recover child funds | `executeReclaimToParent` / `executeDestroy` deliberately skip the `childMultiSigEnabled` check — disabling a child never strands its balance | `child.test.ts` | | Parent state drift voids REMOTE approvals | child pins parent state via AccountUpdate preconditions | `child.test.ts` | | Governance preserves `0 < threshold ≤ numOwners ≤ MAX_OWNERS` | `setup()`, `executeOwnerChange()`, `executeThresholdChange()` all assert the bounds — the vault can be neither locked (threshold unreachable) nor unbounded | `setup.test.ts`, `governance.test.ts` | -| No hidden signature authority / later permission downgrade | Backend and online UI reject any stored vector other than `GUARD_PERMISSIONS`; once accepted, `setPermissions: impossible()` and `setVerificationKey: impossibleDuringCurrentVersion()` prevent later changes | `vault-security.test.ts`, `routes-subscribe.test.ts`, `indexer-archive-discovery.test.ts`, `indexer-autosubscribe.test.ts` | +| No hidden signature authority / later permission downgrade | In the supported atomic flow, proof-authorized `setup()`/`reserveForParent()` overwrite the creator-controlled deployment vector with `GUARD_PERMISSIONS` and seal `setPermissions: impossible()`; backend and online UI reject externally deployed accounts whose stored vector differs | `setup.test.ts`, `child.test.ts`, `vault-security.test.ts`, `routes-subscribe.test.ts`, `indexer-archive-discovery.test.ts`, `indexer-autosubscribe.test.ts` | Off-chain, one invariant matters for the trust argument above: **clients recompute the hash they sign from the fields they display and verify it equals the selected proposal's identity** — diff --git a/docs/ui-audit-guide.md b/docs/ui-audit-guide.md index 7dee3e10..553b3b64 100644 --- a/docs/ui-audit-guide.md +++ b/docs/ui-audit-guide.md @@ -252,10 +252,12 @@ deliberately ignores `kind='deploy'` (`useContractTxLock.ts:60-79`). **5. Ephemeral zkApp key lifecycle & local storage.** The only private key the UI holds is the in-browser zkApp deploy key (`generateKeypair`), generated for a single tx and not persisted. It is -powerless after deploy: `deploy()` sets proofs-only account permissions in the -same transaction (`MinaGuard.ts:282-295`). The same applies to the child key -inside the CREATE_CHILD propose tx. `lib/storage.ts` holds non-secret prefs + -pending-tx metadata. +powerless after a successful atomic creation: proof-authorized `setup()` (root) +or `reserveForParent()` (child) overwrites the signed deployment update with +the canonical proof-only permission vector and permanently seals it in the +same transaction. The UI must never broadcast `deploy()` alone. The same +applies to the child key inside the CREATE_CHILD propose tx. `lib/storage.ts` +holds non-secret prefs + pending-tx metadata. **6. Test-only escape hatches.** `setTestKey` / `setSkipProofs` enable direct signing and dummy proofs, gated diff --git a/ui/lib/multisigClient.ts b/ui/lib/multisigClient.ts index 86eed0b4..550d6f68 100644 --- a/ui/lib/multisigClient.ts +++ b/ui/lib/multisigClient.ts @@ -179,29 +179,6 @@ export async function generateKeypair(): Promise<{ privateKey: string; publicKey } -/** - * Deploys MinaGuard contract account update and submits via Auro or Ledger. - * The zkApp private key remains in browser memory for this call only. - */ -export async function deployContract(params: { - feePayerAddress: string; - zkAppPrivateKeyBase58: string; -}, onProgress?: OnProgress, signer?: SignerConfig): Promise { - await assertLedgerReady(signer); - return getWorkerApi().deployContract(params, proxiedSendTx(signer), proxiedProgress(onProgress), proxiedSignFeePayer(signer)); -} - -/** Submits setup transaction with fixed-size owner list and threshold bootstrap. */ -export async function setupContract(params: { - zkAppAddress: string; - feePayerAddress: string; - owners: string[]; - threshold: number; -}, onProgress?: OnProgress, signer?: SignerConfig): Promise { - await assertLedgerReady(signer); - return getWorkerApi().setupContract(params, proxiedSendTx(signer), proxiedProgress(onProgress), proxiedSignFeePayer(signer)); -} - /** Deploys and initializes the contract in a single transaction. */ export async function deployAndSetupContract(params: { feePayerAddress: string; diff --git a/ui/lib/multisigClient.worker.ts b/ui/lib/multisigClient.worker.ts index ea1e4951..a9284d64 100644 --- a/ui/lib/multisigClient.worker.ts +++ b/ui/lib/multisigClient.worker.ts @@ -778,43 +778,6 @@ const workerApi = { skipProofs = skip; }, - async deployContract( - params: { feePayerAddress: string; zkAppPrivateKeyBase58: string }, - sendFn: SendTxFn | null, - progressFn: ProgressFn, - signFeePayerFn?: SignFeePayerFn - ): Promise { - progressFn('Compiling contract...'); - const ok = await compileContract(); - if (!ok) return null; - - progressFn('Building transaction...'); - const feePayer = PublicKey.fromBase58(params.feePayerAddress); - const zkAppKey = PrivateKey.fromBase58(params.zkAppPrivateKeyBase58); - const zkAppAddress = zkAppKey.toPublicKey(); - const zkApp = new MinaGuard(zkAppAddress); - - await fetchAccount({ publicKey: feePayer }); - clearStaleTransaction(); - const tx = await Mina.transaction(txSender(feePayer), async () => { - AccountUpdate.fundNewAccount(feePayer); - await zkApp.deploy(); - }); - - console.log('mina transaction constructed'); - - progressFn('Generating proof...'); - await maybeProve(tx); - - console.log('proof done'); - - progressFn(testPrivateKey ? 'Signing and sending transaction...' : 'Submitting transaction...'); - const deployHash = await submitTx(tx, sendFn, signFeePayerFn, [zkAppKey]); - console.log('[MultisigWorker] deploy tx result:', deployHash); - if (!deployHash) return null; - return `Transaction submitted: ${deployHash}`; - }, - async deployAndSetupContract( params: { feePayerAddress: string; @@ -868,57 +831,6 @@ const workerApi = { return `Transaction submitted: ${txHash}`; }, - async setupContract( - params: { - zkAppAddress: string; - feePayerAddress: string; - owners: string[]; - threshold: number; - }, - sendFn: SendTxFn | null, - progressFn: ProgressFn, - signFeePayerFn?: SignFeePayerFn - ): Promise { - progressFn('Compiling contract...'); - const ok = await compileContract(); - if (!ok) return null; - - progressFn('Building transaction...'); - const ownerStore = new OwnerStore(); - const ownerKeys = params.owners.map((address) => PublicKey.fromBase58(address)); - for (const owner of ownerKeys) ownerStore.addSorted(owner); - - const paddedOwners = [...ownerStore.owners]; - while (paddedOwners.length < MAX_OWNERS) { - paddedOwners.push(PublicKey.empty()); - } - - const zkAppAddress = PublicKey.fromBase58(params.zkAppAddress); - const feePayer = PublicKey.fromBase58(params.feePayerAddress); - const zkApp = new MinaGuard(zkAppAddress); - - await fetchAccount({ publicKey: feePayer }); - clearStaleTransaction(); - const tx = await Mina.transaction(txSender(feePayer), async () => { - await zkApp.setup( - Field(params.threshold), - Field(ownerStore.length), - new SetupOwnersInput({ - owners: paddedOwners.slice(0, MAX_OWNERS), - }) - ); - }); - - progressFn('Generating proof...'); - await maybeProve(tx); - - progressFn(testPrivateKey ? 'Signing and sending transaction...' : 'Submitting transaction...'); - const txHash = await submitTx(tx, sendFn, signFeePayerFn); - console.log('[MultisigWorker] setup tx result:', txHash); - if (!txHash) return null; - return `Transaction submitted: ${txHash}`; - }, - /** * Creates an on-chain proposal via zkApp.propose(). Auto-approves the proposer. * Returns both proposalHash and the broadcast tx hash so the UI can route to From afd2fb9d84defe856b042bea929fa4f88fa088cd Mon Sep 17 00:00:00 2001 From: Jason Park Date: Mon, 14 Sep 2026 08:13:57 +0000 Subject: [PATCH 6/9] docs: clarify deployment authorization boundary --- contracts/src/MinaGuard.ts | 15 +++++++++------ contracts/src/guard-permissions.ts | 7 ++++--- docs/contracts-audit-guide.md | 12 +++++++----- docs/ui-audit-guide.md | 6 ++---- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/contracts/src/MinaGuard.ts b/contracts/src/MinaGuard.ts index 56abd6aa..6db2860a 100644 --- a/contracts/src/MinaGuard.ts +++ b/contracts/src/MinaGuard.ts @@ -290,12 +290,15 @@ export class MinaGuard extends SmartContract { * SECURITY, authenticate the deployed account: deploy() is ordinary * transaction-construction code, not part of MinaGuard's proved circuit. A * creator can deploy the canonical verification key while changing this - * signature-authorized AccountUpdate's permissions. Never recognize or use - * a MinaGuard account based on its verification key alone. The supported - * atomic setup()/reserveForParent() flow overwrites creator-selected values - * with GUARD_PERMISSIONS under proof authorization. Clients and indexers - * MUST still compare every stored permission against GUARD_PERMISSIONS to - * reject deployments constructed outside that flow. + * signature-authorized AccountUpdate's permissions. setup() and + * reserveForParent() produce separate, proof-authorized AccountUpdates; + * putting them in one atomic transaction does not make their proof + * authenticate the signed deployment update. Never recognize or use a + * MinaGuard account based on its verification key alone. The supported + * atomic setup()/reserveForParent() flow instead overwrites creator-selected + * values with GUARD_PERMISSIONS under proof authorization. Clients and + * indexers MUST still compare every stored permission against + * GUARD_PERMISSIONS to reject deployments constructed outside that flow. * * SECURITY, initialize atomically: deploy() temporarily leaves * setPermissions proof-authorized. setup()/reserveForParent() installs and diff --git a/contracts/src/guard-permissions.ts b/contracts/src/guard-permissions.ts index 5a75268f..c6461e04 100644 --- a/contracts/src/guard-permissions.ts +++ b/contracts/src/guard-permissions.ts @@ -3,9 +3,10 @@ import { Permissions } from 'o1js'; /** * The only account-permission vector supported by MinaGuard. * - * A verification-key match does not authenticate permissions because both are - * installed by the signature-authorized deployment AccountUpdate. Every - * component that accepts a vault must also compare its on-chain permissions + * A verification-key match does not authenticate permissions: a creator can + * bypass MinaGuard's proof-authorized initialization and install both the key + * and a different vector in a signature-authorized deployment AccountUpdate. + * Every component that accepts a vault must compare its stored permissions * against this complete vector. */ export const GUARD_PERMISSIONS = { diff --git a/docs/contracts-audit-guide.md b/docs/contracts-audit-guide.md index a959dc23..e1cfc0b3 100644 --- a/docs/contracts-audit-guide.md +++ b/docs/contracts-audit-guide.md @@ -239,11 +239,13 @@ Defined in `constants.ts`: contract address for indexer discovery. The temporary vector matches the final vector except that `setPermissions` is `proof()`. `deploy()` is transaction-building code, not a proved method: the deployment signature authenticates the installed values, while the MinaGuard verification key does -not. The proved `setup()` or `reserveForParent()` update MUST be included in the same transaction; -it overwrites the entire vector with `GUARD_PERMISSIONS` and seals `setPermissions` as -`impossible()`. A vault MUST NOT be recognized from its verification-key hash alone; online -consumers must also compare every stored permission with `GUARD_PERMISSIONS` to reject accounts -created outside the supported flow. +not. `setup()` and `reserveForParent()` create separate, proof-authorized AccountUpdates. Atomicity +alone does not make their proof authenticate the signed deployment update, so these methods +explicitly overwrite its permissions. The proved `setup()` or `reserveForParent()` update MUST be +included in the same transaction; it writes the entire vector as `GUARD_PERMISSIONS` and seals +`setPermissions` as `impossible()`. A vault MUST NOT be recognized from its verification-key hash +alone; online consumers must also compare every stored permission with `GUARD_PERMISSIONS` to +reject accounts created outside the supported flow. **Setup.** `setup(threshold, numOwners, initialOwners)` — one-time root-guard initialization. diff --git a/docs/ui-audit-guide.md b/docs/ui-audit-guide.md index 553b3b64..9f011f0a 100644 --- a/docs/ui-audit-guide.md +++ b/docs/ui-audit-guide.md @@ -185,10 +185,8 @@ the signer decides what the user authorizes. The moving parts: A guard that is deployed but not yet configured could be controlled by whoever calls `setup()` first. - Top-level vaults use the atomic `deployAndSetupContract` — one tx doing - `fundNewAccount` + `deploy` + `setup` (`worker.ts:755-806`, tx at - `789-797`; called from `accounts/new/page.tsx:160`). Separate - `deployContract` / `setupContract` methods exist with no UI caller - (`worker.ts:718-753`, `808-857`). + `fundNewAccount` + `deploy` + `setup`; the worker exposes no separate + deploy-only or setup-only API. - CREATE_CHILD spans two transactions by design: the propose tx does `deploy(child)` + `reserveForParent(child)` + `propose(parent)` atomically (`worker.ts:979-1003`); the later `executeSetupChild` is bound on-chain to From 63628e9b0e1f2a11963390706ecdec2478c85afe Mon Sep 17 00:00:00 2001 From: Jason Park Date: Mon, 14 Sep 2026 08:50:35 +0000 Subject: [PATCH 7/9] Harden canonical permission enforcement --- .github/workflows/e2e.yml | 6 +-- backend/src/config.ts | 6 +-- backend/src/mina-client.ts | 11 ++--- backend/src/routes.ts | 10 +++-- backend/src/tests/config.test.ts | 2 +- backend/src/tests/routes-api.test.ts | 47 +++++++++++++++++++++ backend/src/tests/vault-security.test.ts | 13 ++++++ backend/src/vault-security.ts | 16 ++++--- contracts/package.json | 10 +++++ contracts/src/guard-permission-policy.ts | 48 +++++++++++++++++++++ contracts/src/guard-permissions.ts | 53 +++++++----------------- contracts/src/index.ts | 6 ++- contracts/src/tests/child.test.ts | 21 ++++++++++ contracts/src/tests/setup.test.ts | 19 +++++++++ deploy/deploy-trail.sh | 7 ++-- deploy/deploy.sh | 10 +++++ deploy/docker-compose.trail.yml | 4 +- deploy/docker-compose.yml | 8 ++-- desktop/src/backend-embed.ts | 6 +-- docs/backend-audit-guide.md | 6 +-- docs/desktop-audit-guide.md | 6 +-- docs/security-audit-guide.md | 6 ++- e2e/ui/display.test.ts | 25 +++++++++++ preview-env/Dockerfile.backend | 12 +++--- preview-env/Dockerfile.frontend | 5 ++- preview-env/docker-compose.e2e-ci.yml | 5 ++- preview-env/docker-compose.preview.yml | 1 + ui/app/accounts/new/page.tsx | 44 ++++++++++++++++++-- ui/lib/api.ts | 53 +++++++----------------- 29 files changed, 333 insertions(+), 133 deletions(-) create mode 100644 contracts/src/guard-permission-policy.ts diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 96b406e8..41212969 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -58,9 +58,9 @@ jobs: run: cd e2e && npx playwright install chromium --with-deps - name: Start compose stack - run: docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e up -d --build --wait --wait-timeout 600 - env: - MINAGUARD_VK_HASH: "skip" + run: | + export MINAGUARD_VK_HASH="$(contracts/scripts/read-vk-hash.sh testnet)" + docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e up -d --build --wait --wait-timeout 600 - name: Wait for services run: | diff --git a/backend/src/config.ts b/backend/src/config.ts index 277df038..7aff74d3 100644 --- a/backend/src/config.ts +++ b/backend/src/config.ts @@ -101,9 +101,9 @@ export function loadConfig(): BackendConfig { archiveFallbackEndpoint: process.env.ARCHIVE_FALLBACK_ENDPOINT ?? null, indexPollIntervalMs: numericEnv('INDEX_POLL_INTERVAL_MS', 15000), indexStartHeight: numericEnv('INDEX_START_HEIGHT', 0), - // An empty value is the documented E2E/dev sentinel for disabling the VK - // filter. Normalize it to the same representation as an unset variable so - // downstream security checks do not try to match every vault against "". + // Normalize empty to null. Live vault authentication treats null as a + // missing trust anchor and fails closed; only the chainless + // INDEXER_DISABLED UI harness synthesizes fixture security responses. minaguardVkHash: process.env.MINAGUARD_VK_HASH || null, indexerMode, indexerDisabled: process.env.INDEXER_DISABLED === 'true', diff --git a/backend/src/mina-client.ts b/backend/src/mina-client.ts index 6dd2380e..0ad3ea19 100644 --- a/backend/src/mina-client.ts +++ b/backend/src/mina-client.ts @@ -1,8 +1,9 @@ import { Mina, PublicKey, fetchAccount, UInt32 } from 'o1js'; import { GUARD_PERMISSION_KINDS, MinaGuard } from 'contracts'; import type { Pool } from 'pg'; -import type { BackendConfig, } from './config.js'; +import type { BackendConfig } from './config.js'; import { + matchesExpectedVerificationKey, validatePermissionVector, type PermissionKindVector, } from './vault-security.js'; @@ -374,10 +375,10 @@ export async function fetchVaultSecurityStatus( } const verificationKeyHash = account.verificationKey?.hash ?? null; - const verificationKeyMatches = - verificationKeyHash !== null && - (config.minaguardVkHash === null || - verificationKeyHash === config.minaguardVkHash); + const verificationKeyMatches = matchesExpectedVerificationKey( + verificationKeyHash, + config.minaguardVkHash, + ); const { permissionKinds, mismatches } = validatePermissionVector( account.permissions ); diff --git a/backend/src/routes.ts b/backend/src/routes.ts index f67a3008..f04ebd89 100644 --- a/backend/src/routes.ts +++ b/backend/src/routes.ts @@ -810,9 +810,9 @@ function toContractState( async function resolveChildState(address: string): Promise { const child = await prisma.contract.findUnique({ where: { address }, - select: { id: true }, + select: { id: true, ready: true, permissionsVerified: true }, }); - if (!child) return null; + if (!child?.ready || !child.permissionsVerified) return null; return toContractState(await latestContractConfig(child.id)); } @@ -831,7 +831,11 @@ async function buildChildStateMap( if (childAddresses.length === 0) return new Map(); const childContracts = await prisma.contract.findMany({ - where: { address: { in: childAddresses } }, + where: { + address: { in: childAddresses }, + ready: true, + permissionsVerified: true, + }, select: { id: true, address: true }, }); if (childContracts.length === 0) return new Map(); diff --git a/backend/src/tests/config.test.ts b/backend/src/tests/config.test.ts index 3fb056f4..8c52b10b 100644 --- a/backend/src/tests/config.test.ts +++ b/backend/src/tests/config.test.ts @@ -11,7 +11,7 @@ afterEach(() => { }); describe('backend configuration', () => { - it('normalizes an empty verification-key hash to an unset filter', () => { + it('normalizes an empty verification-key hash to a missing trust anchor', () => { process.env.DATABASE_URL = 'postgresql://localhost/minaguard'; process.env.MINA_ENDPOINT = 'http://localhost:8080/graphql'; process.env.ARCHIVE_ENDPOINT = 'http://localhost:8282'; diff --git a/backend/src/tests/routes-api.test.ts b/backend/src/tests/routes-api.test.ts index 0210895c..6cf14f3a 100644 --- a/backend/src/tests/routes-api.test.ts +++ b/backend/src/tests/routes-api.test.ts @@ -18,6 +18,7 @@ const childTwoAddress = PrivateKey.random().toPublicKey().toBase58(); // disturbed. const invalidStateContractAddress = PrivateKey.random().toPublicKey().toBase58(); const invalidStateChildAddress = PrivateKey.random().toPublicKey().toBase58(); +const unverifiedChildAddress = PrivateKey.random().toPublicKey().toBase58(); const ownerA = PrivateKey.random().toPublicKey().toBase58(); const ownerB = PrivateKey.random().toPublicKey().toBase58(); const ownerC = PrivateKey.random().toPublicKey().toBase58(); @@ -39,6 +40,7 @@ const localStaleHash = '602'; const remoteStaleHash = '603'; const createChildHash = '604'; const freshHash = '605'; +const unverifiedChildHash = '606'; function get(path: string) { return fetch(`${baseUrl}${path}`); @@ -71,6 +73,12 @@ async function seedDatabase() { ready: true, permissionsVerified: true, }, + { + address: unverifiedChildAddress, + parent: invalidStateContractAddress, + ready: false, + permissionsVerified: false, + }, ], }); @@ -164,6 +172,9 @@ async function seedDatabase() { const invalidChild = await prisma.contract.findUniqueOrThrow({ where: { address: invalidStateChildAddress }, }); + const unverifiedChild = await prisma.contract.findUniqueOrThrow({ + where: { address: unverifiedChildAddress }, + }); await prisma.contractConfig.createMany({ data: [ @@ -185,6 +196,17 @@ async function seedDatabase() { parentNonce: 4, configNonce: 0, }, + { + // This state must never influence proposal status because the child + // has not passed the canonical vault-security check. + contractId: unverifiedChild.id, + validFromBlock: 100, + networkId: '1', + childMultiSigEnabled: true, + nonce: 0, + parentNonce: 99, + configNonce: 0, + }, ], }); @@ -239,6 +261,16 @@ async function seedDatabase() { nonce: '6', destination: 'local', }, + { + contractId: invalidContract.id, + proposalHash: unverifiedChildHash, + createdAtBlock: 115, + configNonce: '3', + nonce: '6', + destination: 'remote', + childAccount: unverifiedChildAddress, + txType: '7', + }, ], }); @@ -561,6 +593,21 @@ describe('proposal invalidation derivation', () => { expect(fresh?.invalidReason).toBeNull(); }); + test('unverified child state cannot invalidate a REMOTE proposal', async () => { + const fromList = (await getProposalsByContract( + invalidStateContractAddress, + )).find((p) => p.proposalHash === unverifiedChildHash); + expect(fromList?.status).toBe('pending'); + expect(fromList?.invalidReason).toBeNull(); + + const fromDetail = await getProposalByHash( + invalidStateContractAddress, + unverifiedChildHash, + ); + expect(fromDetail.status).toBe('pending'); + expect(fromDetail.invalidReason).toBeNull(); + }); + test('single-proposal endpoint reports invalidated status + reason', async () => { const body = await getProposalByHash(invalidStateContractAddress, configStaleHash); expect(body.status).toBe('invalidated'); diff --git a/backend/src/tests/vault-security.test.ts b/backend/src/tests/vault-security.test.ts index b304fe69..0c339b7f 100644 --- a/backend/src/tests/vault-security.test.ts +++ b/backend/src/tests/vault-security.test.ts @@ -4,14 +4,24 @@ import { GUARD_PERMISSION_KINDS, GUARD_PERMISSION_NAMES, GUARD_PERMISSIONS, + GUARD_SET_VERIFICATION_KEY_TXN_VERSION, } from 'contracts'; import { + matchesExpectedVerificationKey, permissionKindVector, permissionMismatches, validatePermissionVector, } from '../vault-security.js'; describe('canonical MinaGuard permissions', () => { + it('fails closed when the expected verification-key hash is missing', () => { + expect(matchesExpectedVerificationKey('actual-vk', null)).toBe(false); + expect(matchesExpectedVerificationKey(null, 'expected-vk')).toBe(false); + expect(matchesExpectedVerificationKey('expected-vk', 'expected-vk')).toBe( + true, + ); + }); + it('accepts every field of the canonical vector', () => { const result = validatePermissionVector(GUARD_PERMISSIONS); @@ -20,6 +30,9 @@ describe('canonical MinaGuard permissions', () => { expect(Object.keys(result.permissionKinds).sort()).toEqual( [...GUARD_PERMISSION_NAMES].sort() ); + expect(GUARD_SET_VERIFICATION_KEY_TXN_VERSION).toBe( + GUARD_PERMISSIONS.setVerificationKey.txnVersion.toString(), + ); }); it('rejects the hidden send: Either withdrawal permission', () => { diff --git a/backend/src/vault-security.ts b/backend/src/vault-security.ts index 75e77a9e..6c947085 100644 --- a/backend/src/vault-security.ts +++ b/backend/src/vault-security.ts @@ -2,20 +2,24 @@ import { GUARD_PERMISSION_KINDS, GUARD_PERMISSION_NAMES, GUARD_PERMISSIONS, + type GuardPermissionKind, type GuardPermissionName, } from 'contracts'; -export type PermissionKind = - | 'None' - | 'Either' - | 'Proof' - | 'Signature' - | 'Impossible'; +export type PermissionKind = GuardPermissionKind; export type PermissionKindVector = Partial< Record >; +/** A missing trust anchor must never turn an arbitrary zkApp VK into a match. */ +export function matchesExpectedVerificationKey( + actual: string | null, + expected: string | null, +): boolean { + return actual !== null && expected !== null && actual === expected; +} + type BoolLike = boolean | { toBoolean(): boolean }; type PermissionLike = { constant?: BoolLike; diff --git a/contracts/package.json b/contracts/package.json index afab1870..6aa7a8f0 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -13,6 +13,16 @@ "type": "module", "main": "build/src/index.js", "types": "build/src/index.d.ts", + "exports": { + ".": { + "types": "./build/src/index.d.ts", + "default": "./build/src/index.js" + }, + "./guard-permission-policy": { + "types": "./build/src/guard-permission-policy.d.ts", + "default": "./build/src/guard-permission-policy.js" + } + }, "scripts": { "build": "rm -rf build && tsc", "buildw": "tsc --watch", diff --git a/contracts/src/guard-permission-policy.ts b/contracts/src/guard-permission-policy.ts new file mode 100644 index 00000000..5cbb99e9 --- /dev/null +++ b/contracts/src/guard-permission-policy.ts @@ -0,0 +1,48 @@ +/** Pure serialized policy shared with browser code without importing o1js. */ +export const GUARD_PERMISSION_NAMES = [ + 'editState', + 'send', + 'receive', + 'setDelegate', + 'setPermissions', + 'setVerificationKey', + 'setZkappUri', + 'editActionState', + 'setTokenSymbol', + 'incrementNonce', + 'setVotingFor', + 'setTiming', + 'access', +] as const; + +export type GuardPermissionName = (typeof GUARD_PERMISSION_NAMES)[number]; +export type GuardPermissionKind = + | 'None' + | 'Either' + | 'Proof' + | 'Signature' + | 'Impossible'; + +export const GUARD_PERMISSION_KINDS: Record< + GuardPermissionName, + GuardPermissionKind +> = { + editState: 'Proof', + send: 'Proof', + receive: 'None', + setDelegate: 'Proof', + setPermissions: 'Impossible', + setVerificationKey: 'Impossible', + setZkappUri: 'Impossible', + editActionState: 'Proof', + setTokenSymbol: 'Impossible', + incrementNonce: 'Impossible', + setVotingFor: 'Impossible', + setTiming: 'Impossible', + access: 'None', +}; + +// o1js@3.0.0-mesa.final's current transaction version, committed by +// impossibleDuringCurrentVersion(). vault-security.test.ts proves this stays +// equal to the version embedded in GUARD_PERMISSIONS. +export const GUARD_SET_VERIFICATION_KEY_TXN_VERSION = '4'; diff --git a/contracts/src/guard-permissions.ts b/contracts/src/guard-permissions.ts index c6461e04..b75c8bfe 100644 --- a/contracts/src/guard-permissions.ts +++ b/contracts/src/guard-permissions.ts @@ -1,4 +1,19 @@ import { Permissions } from 'o1js'; +import { + GUARD_PERMISSION_KINDS, + GUARD_PERMISSION_NAMES, + GUARD_SET_VERIFICATION_KEY_TXN_VERSION, + type GuardPermissionKind, + type GuardPermissionName, +} from './guard-permission-policy.js'; + +export { + GUARD_PERMISSION_KINDS, + GUARD_PERMISSION_NAMES, + GUARD_SET_VERIFICATION_KEY_TXN_VERSION, + type GuardPermissionKind, + type GuardPermissionName, +}; /** * The only account-permission vector supported by MinaGuard. @@ -36,41 +51,3 @@ export const GUARD_DEPLOY_PERMISSIONS = { ...GUARD_PERMISSIONS, setPermissions: Permissions.proof(), }; - -export const GUARD_PERMISSION_NAMES = [ - 'editState', - 'send', - 'receive', - 'setDelegate', - 'setPermissions', - 'setVerificationKey', - 'setZkappUri', - 'editActionState', - 'setTokenSymbol', - 'incrementNonce', - 'setVotingFor', - 'setTiming', - 'access', -] as const; - -export type GuardPermissionName = (typeof GUARD_PERMISSION_NAMES)[number]; - -/** JSON/GraphQL representation used by online clients for field-by-field checks. */ -export const GUARD_PERMISSION_KINDS: Record< - GuardPermissionName, - 'None' | 'Either' | 'Proof' | 'Signature' | 'Impossible' -> = { - editState: 'Proof', - send: 'Proof', - receive: 'None', - setDelegate: 'Proof', - setPermissions: 'Impossible', - setVerificationKey: 'Impossible', - setZkappUri: 'Impossible', - editActionState: 'Proof', - setTokenSymbol: 'Impossible', - incrementNonce: 'Impossible', - setVotingFor: 'Impossible', - setTiming: 'Impossible', - access: 'None', -}; diff --git a/contracts/src/index.ts b/contracts/src/index.ts index 58cbabb0..f4ec5d27 100644 --- a/contracts/src/index.ts +++ b/contracts/src/index.ts @@ -35,10 +35,14 @@ export { OwnerWitness, PublicKeyOption, computeOwnerChain, assertOwnerMembership export { GUARD_PERMISSIONS, GUARD_DEPLOY_PERMISSIONS, +} from './guard-permissions.js'; +export { GUARD_PERMISSION_NAMES, GUARD_PERMISSION_KINDS, + GUARD_SET_VERIFICATION_KEY_TXN_VERSION, type GuardPermissionName, -} from './guard-permissions.js'; + type GuardPermissionKind, +} from './guard-permission-policy.js'; export { ownerKey } from './utils.js'; diff --git a/contracts/src/tests/child.test.ts b/contracts/src/tests/child.test.ts index d331b03f..84a1a6f4 100644 --- a/contracts/src/tests/child.test.ts +++ b/contracts/src/tests/child.test.ts @@ -116,6 +116,27 @@ describe('MinaGuard - Child Lifecycle', () => { new SetupOwnersInput({ owners: setupOwners }), ); }); + const accountUpdates = ( + JSON.parse(txn.toJSON()) as { accountUpdates: any[] } + ).accountUpdates.filter( + (update) => update.body.publicKey === childAddress.toBase58(), + ); + const signedDeploy = accountUpdates.find( + (update) => update.body.authorizationKind.isSigned === true, + ); + const provedReservation = accountUpdates.find( + (update) => update.body.authorizationKind.isProved === true, + ); + + expect(accountUpdates).toHaveLength(2); + expect(signedDeploy?.body.update.permissions.send).toBe('Either'); + expect(signedDeploy?.body.update.permissions.setPermissions).toBe( + 'Proof', + ); + expect(provedReservation?.body.update.permissions.send).toBe('Proof'); + expect( + provedReservation?.body.update.permissions.setPermissions, + ).toBe('Impossible'); await txn.prove(); await txn.sign([parentCtx.deployerKey, childKey]).send(); diff --git a/contracts/src/tests/setup.test.ts b/contracts/src/tests/setup.test.ts index 81c7e32b..f203f64c 100644 --- a/contracts/src/tests/setup.test.ts +++ b/contracts/src/tests/setup.test.ts @@ -58,6 +58,25 @@ describe('MinaGuard - Setup', () => { new SetupOwnersInput({ owners: setupOwners }), ); }); + const accountUpdates = ( + JSON.parse(txn.toJSON()) as { accountUpdates: any[] } + ).accountUpdates.filter( + (update) => update.body.publicKey === ctx.zkAppAddress.toBase58(), + ); + const signedDeploy = accountUpdates.find( + (update) => update.body.authorizationKind.isSigned === true, + ); + const provedSetup = accountUpdates.find( + (update) => update.body.authorizationKind.isProved === true, + ); + + expect(accountUpdates).toHaveLength(2); + expect(signedDeploy?.body.update.permissions.send).toBe('Either'); + expect(signedDeploy?.body.update.permissions.setPermissions).toBe('Proof'); + expect(provedSetup?.body.update.permissions.send).toBe('Proof'); + expect(provedSetup?.body.update.permissions.setPermissions).toBe( + 'Impossible', + ); await txn.prove(); await txn.sign([deployerKey, zkAppKey]).send(); diff --git a/deploy/deploy-trail.sh b/deploy/deploy-trail.sh index ee3338a1..ef64b8a3 100755 --- a/deploy/deploy-trail.sh +++ b/deploy/deploy-trail.sh @@ -50,11 +50,10 @@ if [ -z "${MINAGUARD_VK_HASH:-}" ] && [ -f contracts/.vk-hash ]; then MINAGUARD_VK_HASH=$(contracts/scripts/read-vk-hash.sh testnet) || exit 1 export MINAGUARD_VK_HASH fi -# A pre-set MINAGUARD_VK_HASH bypasses the helper's validation; still reject -# empty/garbage and allow the explicit "skip" opt-out (indexer accepts all). +# A pre-set MINAGUARD_VK_HASH bypasses the helper's parser, so still reject +# empty or non-decimal values. Production admission has no skip mode. case "${MINAGUARD_VK_HASH:-}" in - skip) ;; - ''|*[!0-9]*) echo "MINAGUARD_VK_HASH must be a decimal hash or 'skip' (got '${MINAGUARD_VK_HASH:-}')" >&2; exit 1 ;; + ''|*[!0-9]*) echo "MINAGUARD_VK_HASH must be a decimal hash (got '${MINAGUARD_VK_HASH:-}')" >&2; exit 1 ;; esac COMMAND="${1:-}" diff --git a/deploy/deploy.sh b/deploy/deploy.sh index 260c7b6b..c6ab44ed 100755 --- a/deploy/deploy.sh +++ b/deploy/deploy.sh @@ -11,6 +11,16 @@ set -euo pipefail +# The local lightnet stack uses the testnet circuit domain. Thread the pinned +# hash through both images so backend and browser enforce the same trust anchor. +if [ -z "${MINAGUARD_VK_HASH:-}" ]; then + MINAGUARD_VK_HASH=$(contracts/scripts/read-vk-hash.sh testnet) || exit 1 + export MINAGUARD_VK_HASH +fi +case "$MINAGUARD_VK_HASH" in + ''|*[!0-9]*) echo "MINAGUARD_VK_HASH must be a decimal hash (got '$MINAGUARD_VK_HASH')" >&2; exit 1 ;; +esac + COMMAND="${1:-}" PORT=10000 CADDY_API="http://localhost:2019" diff --git a/deploy/docker-compose.trail.yml b/deploy/docker-compose.trail.yml index d593f872..d0fbb4e6 100644 --- a/deploy/docker-compose.trail.yml +++ b/deploy/docker-compose.trail.yml @@ -47,7 +47,7 @@ services: # Skip the (memory-heavy) in-image circuit compile — host computes the # vk-hash once and threads it through. See preview-env/preview.sh for # how the localnet/preview path does the same. - - MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:-} + - MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:?MINAGUARD_VK_HASH must be set} restart: unless-stopped environment: - DATABASE_URL=postgresql://minaguard:minaguard@db:5432/minaguard @@ -91,7 +91,7 @@ services: - NEXT_PUBLIC_POLL_INTERVAL_MS=1000 # Same host-computed hash the backend gets above — the worker rejects a # compile whose verification key doesn't match it. - - NEXT_PUBLIC_MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:-} + - NEXT_PUBLIC_MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:?MINAGUARD_VK_HASH must be set} - ENABLE_SOURCE_MAPS=false restart: unless-stopped depends_on: diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 847f0b1a..98d5f6c1 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -45,6 +45,8 @@ services: build: context: .. dockerfile: preview-env/Dockerfile.backend + args: + - MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:?MINAGUARD_VK_HASH must be set} restart: unless-stopped environment: - DATABASE_URL=postgresql://minaguard:minaguard@db:5432/minaguard @@ -69,10 +71,8 @@ services: - NEXT_PUBLIC_ARCHIVE_ENDPOINT=https://mina-nodes.duckdns.org/app/archive - NEXT_PUBLIC_MINA_NETWORK=testnet - NEXT_PUBLIC_BLOCK_EXPLORER_URL=https://mina-nodes.duckdns.org/app/explorer - # Rejects a compile whose verification key doesn't match. deploy.sh does - # not compute MINAGUARD_VK_HASH (this lightnet stack pins no VK on the - # backend either), so this is inert until it is exported. - - NEXT_PUBLIC_MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:-} + # Same pinned hash used by the backend's vault-admission check. + - NEXT_PUBLIC_MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:?MINAGUARD_VK_HASH must be set} - ENABLE_SOURCE_MAPS=false restart: unless-stopped depends_on: diff --git a/desktop/src/backend-embed.ts b/desktop/src/backend-embed.ts index 9d67e774..7737db9d 100644 --- a/desktop/src/backend-embed.ts +++ b/desktop/src/backend-embed.ts @@ -106,9 +106,9 @@ export async function startEmbeddedBackend( // configured network selects which line applies — devnet shares the testnet // circuit (anything except mainnet does, mirroring contracts/src/constants.ts). // Files predating the per-network format (a comment header + one bare - // decimal) parse via the legacy fallback. Left unset if the file is missing - // or yields no usable value — the VK match check then no-ops rather than the - // backend failing to start. + // decimal) parse via the legacy fallback. If the file is missing or has no + // usable value, the backend starts but vault authentication fails closed: + // no account can become permission-verified or visible as ready. if (opts.vkHashPath && existsSync(opts.vkHashPath)) { const lines = readFileSync(opts.vkHashPath, 'utf8') .split('\n') diff --git a/docs/backend-audit-guide.md b/docs/backend-audit-guide.md index 69034274..fbf9c66b 100644 --- a/docs/backend-audit-guide.md +++ b/docs/backend-audit-guide.md @@ -228,8 +228,8 @@ lookups positively succeed (a genuine `pending` from `fetchZkappTxStatus` — an treated as absent — **and** a real mempool set, `null` on network failure). Verify neither lookup failing can misclassify an included tx as dropped, since that flag releases the UI signer lock. -**4. Deployment authentication.** Discovery filters candidates by `MINAGUARD_VK_HASH` (required for -archive, optional for daemon), then validates the live VK and every permission against +**4. Deployment authentication.** Discovery uses `MINAGUARD_VK_HASH` as its trust anchor (also as a +required archive query filter), then validates the live VK and every permission against `GUARD_PERMISSIONS`. Confirm that a canonical VK with even one altered field (especially `send: proofOrSignature`) never becomes `permissionsVerified` or `ready`, and that legacy rows are re-checked rather than grandfathered. @@ -303,7 +303,7 @@ From `backend/`: | `INDEXER_DISABLED` | `false` | Test-harness knob: when `true`, boot the API without starting the polling indexer (UI tests run against a pre-seeded DB with no chain behind it) | | `INDEXER_FIXED_LATEST_SLOT` | empty | Test-harness knob: with the indexer disabled there is no genesis to derive slots from, so `status.latestSlot` (used for read-time expiry) is primed with this fixed value | | `DISCOVERY_BACKEND` | `daemon` | Candidate source for full-mode discovery: `daemon` (bestChain scan, ~290-block reach) or `archive` (direct archive-postgres SQL, unbounded history) | -| `MINAGUARD_VK_HASH` | empty | Verification key hash filter for discovery. Optional for `daemon`; **required** for `archive` (the SQL filters on it). The canonical value is committed at `contracts/.vk-hash` (two labeled entries: `testnet=` and `mainnet=`; use the one matching the target network) | +| `MINAGUARD_VK_HASH` | empty | Verification-key trust anchor. If unset, live vault authentication fails closed and no candidate can become ready. It is also required at startup for `archive` discovery because the SQL uses it as a bounded filter. The canonical value is committed at `contracts/.vk-hash` (two labeled entries: `testnet=` and `mainnet=`; use the one matching the target network) | | `ARCHIVE_DB_HOST` | — | Archive postgres host (required when `DISCOVERY_BACKEND=archive`) | | `ARCHIVE_DB_PORT` | `5432` | Archive postgres port | | `ARCHIVE_DB_USER` | — | Archive postgres user (read-only role; required for `archive`) | diff --git a/docs/desktop-audit-guide.md b/docs/desktop-audit-guide.md index d3f277ec..56cf643d 100644 --- a/docs/desktop-audit-guide.md +++ b/docs/desktop-audit-guide.md @@ -243,9 +243,9 @@ macOS, `%APPDATA%\MinaGuard` on Windows): structurally distinct); the embed picks the line matching the configured network (`backend-embed.ts:112-128`, devnet sharing the testnet circuit) and still accepts the pre-#93 single-bare-number format. When the file is - missing, or a keyed file has no line for the configured network, the check - no-ops rather than blocking startup or comparing against a wrong-network - hash. + missing, or a keyed file has no line for the configured network, the backend + may start but vault authentication fails closed: no account can become + permission-verified or visible as ready. - **DB bootstrap & schema versioning:** when `minaguard.db` is missing — or stale — the bundled `assets/schema.sql` (generated from `schema.sqlite.prisma` via `prisma migrate diff`) is executed diff --git a/docs/security-audit-guide.md b/docs/security-audit-guide.md index d4a6a7ad..2e42b0f3 100644 --- a/docs/security-audit-guide.md +++ b/docs/security-audit-guide.md @@ -35,8 +35,10 @@ MinaGuard deployment, the account permissions (`send: proof()`, `editState: proo Verification-key equality alone does not authenticate those signature-installed permissions: the backend and online UI must also verify the complete stored permission vector against `GUARD_PERMISSIONS` before accepting the vault. The UI fetches this snapshot directly from its -configured Mina node and compares it with a build-time canonical vector; it does not trust the -indexer to report either side of the comparison. +configured Mina node and compares it with the pure serialized policy exported by +`contracts/guard-permission-policy`; it does not trust the indexer to report either side of the +comparison. A backend test compares that browser-safe policy with the actual o1js +`GUARD_PERMISSIONS` value so the two representations cannot drift unnoticed. In the supported creation flow, the signed `deploy()` update installs a temporary permission vector whose `setPermissions` field is `proof()`. In the same atomic transaction, the proved diff --git a/e2e/ui/display.test.ts b/e2e/ui/display.test.ts index 5e35e3d4..53d7192c 100644 --- a/e2e/ui/display.test.ts +++ b/e2e/ui/display.test.ts @@ -190,6 +190,31 @@ test('unsafe CREATE_CHILD target blocks online and offline approval', async ({ p await expect(page.getByText(/drop signed \.json/i)).not.toBeVisible(); }); +test('unsafe parent blocks the dedicated SubVault creation wizard', async ({ page }) => { + await openVault(page, TREASURY); + await page.route(`**/api/accounts/${TREASURY}/security`, (route) => + route.fulfill({ + json: { + accountFound: true, + verificationKeyHash: 'canonical-vk', + verificationKeyMatches: true, + permissionKinds: { send: 'Either' }, + expectedPermissionKinds: { send: 'Proof' }, + permissionMismatches: ['send'], + safe: false, + }, + }), + ); + + await navigateTo(page, `/accounts/new?parent=${TREASURY}`); + await page.getByRole('button', { name: 'Next', exact: true }).click(); + + await expect(page.getByText(/unsafe parent vault/i)).toBeVisible(); + await expect( + page.getByRole('button', { name: 'Propose SubVault' }), + ).toBeDisabled(); +}); + test('offline bundle export rechecks permissions instead of trusting page state', async ({ page }) => { let unsafeNow = false; await page.route(`**/api/accounts/${TREASURY}/security`, async (route) => { diff --git a/preview-env/Dockerfile.backend b/preview-env/Dockerfile.backend index 0da6a37b..cafa6cc7 100644 --- a/preview-env/Dockerfile.backend +++ b/preview-env/Dockerfile.backend @@ -26,23 +26,21 @@ COPY dev-helpers/ dev-helpers/ RUN cd contracts && bun run build # Compile contract circuit and extract verification key hash. -# When MINAGUARD_VK_HASH is supplied as a build arg the compile is skipped — +# When a decimal MINAGUARD_VK_HASH is supplied as a build arg the compile is skipped — # the host runner is expected to have computed it once (full host memory) # and threaded it through. The o1js circuit compile peaks well above the # memory available inside a typical CI Docker build, OOM-killing the step. ARG MINAGUARD_VK_HASH= -RUN if [ "$MINAGUARD_VK_HASH" = "skip" ]; then \ - echo "MINAGUARD_VK_HASH=skip — skipping VK filter (indexer accepts all contracts)"; \ - echo "" > /app/.vk-hash; \ - elif [ -n "$MINAGUARD_VK_HASH" ]; then \ +RUN if [ -n "$MINAGUARD_VK_HASH" ]; then \ + case "$MINAGUARD_VK_HASH" in *[!0-9]*) echo "ERROR: MINAGUARD_VK_HASH must be decimal" >&2; exit 1;; esac; \ echo "Using prebuilt MINAGUARD_VK_HASH: $MINAGUARD_VK_HASH"; \ echo "$MINAGUARD_VK_HASH" > /app/.vk-hash; \ else \ echo "MINAGUARD_VK_HASH build arg not set — compiling circuit (memory-heavy)..."; \ - vk_output="$(bun run dev-helpers/cli.ts vk-hash compile 2>&1)" || { echo "$vk_output" >&2; echo "ERROR: VK hash compile failed — refusing to build with an empty VK filter (the indexer would accept ALL contracts as MinaGuard vaults)" >&2; exit 1; }; \ + vk_output="$(bun run dev-helpers/cli.ts vk-hash compile 2>&1)" || { echo "$vk_output" >&2; echo "ERROR: VK hash compile failed — refusing to build without a MinaGuard verification-key trust anchor" >&2; exit 1; }; \ echo "$vk_output"; \ vk_hash="$(printf '%s\n' "$vk_output" | awk '/^vkHash/{print $NF; exit}')"; \ - case "$vk_hash" in ''|*[!0-9]*) echo "ERROR: no decimal VK hash found in compile output — refusing to build with an empty VK filter" >&2; exit 1;; esac; \ + case "$vk_hash" in ''|*[!0-9]*) echo "ERROR: no decimal VK hash found in compile output — refusing to build without a MinaGuard verification-key trust anchor" >&2; exit 1;; esac; \ printf '%s\n' "$vk_hash" > /app/.vk-hash; \ fi diff --git a/preview-env/Dockerfile.frontend b/preview-env/Dockerfile.frontend index e85fc016..cbb37a5a 100644 --- a/preview-env/Dockerfile.frontend +++ b/preview-env/Dockerfile.frontend @@ -39,8 +39,9 @@ ARG NEXT_PUBLIC_MINA_NETWORK ARG NEXT_PUBLIC_BLOCK_EXPLORER_URL ARG NEXT_PUBLIC_POLL_INTERVAL_MS ARG NEXT_PUBLIC_E2E_TEST="" -# Expected MinaGuard VK hash for this build's network domain. Empty ⇒ the -# worker's post-compile check is skipped, matching the backend's MINAGUARD_VK_HASH. +# Expected MinaGuard VK hash for this build's network domain. An empty value +# disables the worker's compile-cache check for development, but live browser +# vault admission still fails closed until a hash is configured. ARG NEXT_PUBLIC_MINAGUARD_VK_HASH="" ARG ENABLE_SOURCE_MAPS=true diff --git a/preview-env/docker-compose.e2e-ci.yml b/preview-env/docker-compose.e2e-ci.yml index c3d313cd..a0466b49 100644 --- a/preview-env/docker-compose.e2e-ci.yml +++ b/preview-env/docker-compose.e2e-ci.yml @@ -10,7 +10,7 @@ # in .github/workflows/e2e.yml; internal service-to-service URLs are unchanged. # # Usage: -# MINAGUARD_VK_HASH=skip docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e up -d --build --wait +# MINAGUARD_VK_HASH=$(contracts/scripts/read-vk-hash.sh testnet) docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e up -d --build --wait # docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e down -v services: @@ -54,7 +54,7 @@ services: context: .. dockerfile: preview-env/Dockerfile.backend args: - - MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:-skip} + - MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:?MINAGUARD_VK_HASH must be set} restart: "no" environment: - DATABASE_URL=postgresql://postgres:postgres@db:5432/minaguard @@ -82,6 +82,7 @@ services: - NEXT_PUBLIC_E2E_TEST=true - NEXT_PUBLIC_POLL_INTERVAL_MS=3000 - NEXT_PUBLIC_MINA_NETWORK=testnet + - NEXT_PUBLIC_MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:?MINAGUARD_VK_HASH must be set} restart: "no" ports: - "127.0.0.1:13000:3000" diff --git a/preview-env/docker-compose.preview.yml b/preview-env/docker-compose.preview.yml index 13d65984..1030c479 100644 --- a/preview-env/docker-compose.preview.yml +++ b/preview-env/docker-compose.preview.yml @@ -77,6 +77,7 @@ services: - NEXT_PUBLIC_MINA_ENDPOINT=https://mina-nodes.duckdns.org/preview/${PR_NUMBER:-1}/graphql - NEXT_PUBLIC_ARCHIVE_ENDPOINT=https://mina-nodes.duckdns.org/preview/${PR_NUMBER:-1}/archive - NEXT_PUBLIC_MINA_NETWORK=testnet + - NEXT_PUBLIC_MINAGUARD_VK_HASH=${MINAGUARD_VK_HASH:?MINAGUARD_VK_HASH must be set} - NEXT_PUBLIC_BLOCK_EXPLORER_URL=https://mina-nodes.duckdns.org/preview/${PR_NUMBER:-1}/explorer # Idle poll cadence on the preview; adaptive polling drops to a faster # interval automatically while a PendingTx is in flight. diff --git a/ui/app/accounts/new/page.tsx b/ui/app/accounts/new/page.tsx index d8482814..480dd819 100644 --- a/ui/app/accounts/new/page.tsx +++ b/ui/app/accounts/new/page.tsx @@ -13,8 +13,15 @@ import { generateKeypair, } from '@/lib/multisigClient'; import { saveAccountName, savePendingTx } from '@/lib/storage'; -import { extractTxHash, subscribeAddress } from '@/lib/api'; +import { + extractTxHash, + fetchContract, + fetchVaultSecurityStatus, + isCanonicalVaultSecurity, + subscribeAddress, +} from '@/lib/api'; import { resolveIndexerMode } from '@/lib/indexer-mode'; +import { useVaultSecurity } from '@/hooks/useVaultSecurity'; const NETWORKS = [ { label: 'Testnet', value: 'testnet', enabled: true }, @@ -53,6 +60,9 @@ function CreateAccountWizard() { () => (parentAddress ? contracts.find((c) => c.address === parentAddress) ?? null : null), [contracts, parentAddress], ); + const parentLiveSecurity = useVaultSecurity(parentContract?.address ?? null); + const parentPermissionsSafe = + parentContract?.permissionsVerified === true && parentLiveSecurity === 'safe'; const [step, setStep] = useState<1 | 2>(1); @@ -206,6 +216,19 @@ function CreateAccountWizard() { const childAddress = keypair.publicKey; void startOperation('Preparing SubVault proposal…', async (onProgress) => { + const [freshParent, parentSecurity] = await Promise.all([ + fetchContract(parentAddress), + fetchVaultSecurityStatus(parentAddress), + ]); + if ( + !freshParent?.permissionsVerified || + !isCanonicalVaultSecurity(parentSecurity) + ) { + throw new Error( + 'Vault permissions have not passed the canonical security check', + ); + } + onProgress('Computing SubVault config hash…'); const { configHash } = await computeCreateChildConfigHash({ childOwners: parsedOwners, @@ -432,6 +455,13 @@ function CreateAccountWizard() {
+ {isSubaccount && parentContract && !parentPermissionsSafe && ( +

+ {parentLiveSecurity === 'checking' + ? 'Checking the parent Vault permission vector. SubVault creation remains blocked.' + : 'Unsafe parent Vault: its permission vector has not passed the canonical security check.'} +

+ )} {formError &&

{formError}

}
)} @@ -463,10 +493,18 @@ function CreateAccountWizard() { ) : isSubaccount ? ( diff --git a/ui/lib/api.ts b/ui/lib/api.ts index 4c7efca6..764958d1 100644 --- a/ui/lib/api.ts +++ b/ui/lib/api.ts @@ -9,6 +9,12 @@ import { normalizeTxType, } from '@/lib/types'; import { getMinaGuardConfig } from '@/lib/endpoints'; +import { + GUARD_PERMISSION_KINDS, + GUARD_PERMISSION_NAMES, + GUARD_SET_VERIFICATION_KEY_TXN_VERSION, + type GuardPermissionName, +} from 'contracts/guard-permission-policy'; const API_BASE = process.env.NEXT_PUBLIC_API_BASE_URL ?? 'http://localhost:3001'; @@ -40,47 +46,16 @@ export interface VaultSecurityStatus { safe: boolean; } -const PERMISSION_FIELD_NAMES = [ - 'editState', - 'send', - 'receive', - 'setDelegate', - 'setPermissions', - 'setVerificationKey', - 'setZkappUri', - 'editActionState', - 'setTokenSymbol', - 'incrementNonce', - 'setVotingFor', - 'setTiming', - 'access', -] as const; -type PermissionFieldName = (typeof PERMISSION_FIELD_NAMES)[number]; +type PermissionFieldName = GuardPermissionName; /** * Browser-side trust anchor. Keep this serialized form in lockstep with the * o1js GUARD_PERMISSIONS constant; unlike API-supplied expected values, it * cannot be changed by a compromised indexer response. */ -const EXPECTED_PERMISSION_KINDS: Record = { - editState: 'Proof', - send: 'Proof', - receive: 'None', - setDelegate: 'Proof', - setPermissions: 'Impossible', - setVerificationKey: 'Impossible', - setZkappUri: 'Impossible', - editActionState: 'Proof', - setTokenSymbol: 'Impossible', - incrementNonce: 'Impossible', - setVotingFor: 'Impossible', - setTiming: 'Impossible', - access: 'None', -}; - -// o1js@3.0.0-mesa.final's current transaction version, committed by -// impossibleDuringCurrentVersion(). The backend compares the UInt32 directly. -const EXPECTED_SET_VK_TXN_VERSION = '4'; +const EXPECTED_PERMISSION_KINDS: Record = + GUARD_PERMISSION_KINDS; +const PERMISSION_FIELD_NAMES = GUARD_PERMISSION_NAMES; /** * Fetches and validates the account directly from the configured Mina node. @@ -165,17 +140,19 @@ export async function fetchVaultSecurityStatus( ); if ( String(setVerificationKey?.txnVersion ?? '') !== - EXPECTED_SET_VK_TXN_VERSION && + GUARD_SET_VERIFICATION_KEY_TXN_VERSION && !permissionMismatches.includes('setVerificationKey') ) { permissionMismatches.push('setVerificationKey'); } const verificationKeyHash = account.verificationKey?.hash ?? null; - const expectedVkHash = process.env.NEXT_PUBLIC_MINAGUARD_VK_HASH; + const expectedVkHash = + process.env.NEXT_PUBLIC_MINAGUARD_VK_HASH?.trim() || null; const verificationKeyMatches = verificationKeyHash !== null && - (!expectedVkHash || verificationKeyHash === expectedVkHash); + expectedVkHash !== null && + verificationKeyHash === expectedVkHash; return { accountFound: true, verificationKeyHash, From 72b16b7f1d715c29bb3761d8dd9bc8fa87cde3c7 Mon Sep 17 00:00:00 2001 From: Jason Park Date: Mon, 14 Sep 2026 09:20:10 +0000 Subject: [PATCH 8/9] ci: keep proofless e2e within runner limits --- .github/workflows/e2e.yml | 23 +++++++++++++++++------ e2e/network-config.ts | 8 ++++---- e2e/onchain-flow.test.ts | 3 ++- ui/lib/multisigClient.worker.ts | 18 ++++++++++++++++-- 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 41212969..a36d3ade 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -40,9 +40,6 @@ jobs: - name: Verify Docker Compose v2 available run: docker compose version - - name: Clean up previous run - run: docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e down -v --remove-orphans 2>/dev/null || true - - name: Install Bun uses: oven-sh/setup-bun@v2 @@ -54,13 +51,27 @@ jobs: - name: Install dependencies run: bun install + # Lightnet runs with PROOF_LEVEL=none and the browser deliberately uses + # o1js dummy proofs. Use the matching dummy verification key as this + # test stack's explicit trust anchor; the real network-specific MinaGuard + # keys are compiled and checked by the separate check-vk-hash job. + # Persist it through GITHUB_ENV because every later Compose invocation, + # including failure log collection and teardown, must be able to + # interpolate the required build argument. + - name: Configure proofless Lightnet verification key + run: | + e2e_dummy_vk_hash="$(cd contracts && bun -e "import { VerificationKey } from 'o1js'; console.log((await VerificationKey.dummy()).hash.toString())")" + [[ "$e2e_dummy_vk_hash" =~ ^[0-9]+$ ]] + echo "MINAGUARD_VK_HASH=$e2e_dummy_vk_hash" >> "$GITHUB_ENV" + + - name: Clean up previous run + run: docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e down -v --remove-orphans 2>/dev/null || true + - name: Install Playwright browsers run: cd e2e && npx playwright install chromium --with-deps - name: Start compose stack - run: | - export MINAGUARD_VK_HASH="$(contracts/scripts/read-vk-hash.sh testnet)" - docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e up -d --build --wait --wait-timeout 600 + run: docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e up -d --build --wait --wait-timeout 600 - name: Wait for services run: | diff --git a/e2e/network-config.ts b/e2e/network-config.ts index 7220a8ee..d3ee0812 100644 --- a/e2e/network-config.ts +++ b/e2e/network-config.ts @@ -65,10 +65,10 @@ const LIGHTNET_CONFIG: NetworkConfig = { blockTimeMs: 3_000, indexerPollIntervalMs: 5_000, indexerTimeoutMs: 240_000, - // Banner wait covers first-test compile + tx build + broadcast + inclusion. - // Local serial worst-case is ~90-120s on 8 vCPUs, but CI on 4 vCPUs with - // `next dev` on-demand bundling pushes first-test compile close to 180s. - // 5 min covers both environments without masking real hangs. + // Banner wait covers tx construction, broadcast, and inclusion. Proofless + // Lightnet E2E skips the real circuit compile; check-vk-hash and contract + // tests cover compilation separately. Keep enough headroom for a busy local + // daemon and indexer without masking a permanent operation hang. bannerTimeoutMs: 900_000, // Per-test hard cap, strictly greater than bannerTimeoutMs so the banner // wait has time to surface a clear error before the outer cap fires. diff --git a/e2e/onchain-flow.test.ts b/e2e/onchain-flow.test.ts index 78a9c20a..e1836da0 100644 --- a/e2e/onchain-flow.test.ts +++ b/e2e/onchain-flow.test.ts @@ -106,7 +106,8 @@ test.describe.configure({ mode: 'serial' }); /** * Navigate to a path with the mock wallet active for the given account. - * The first call does a full page.goto (which starts contract compilation). + * The first call does a full page.goto (which starts contract compilation on + * proof-enabled networks; proofless Lightnet E2E deliberately skips it). * Subsequent calls use client-side navigation to preserve the Web Worker. */ async function gotoWithWallet( diff --git a/ui/lib/multisigClient.worker.ts b/ui/lib/multisigClient.worker.ts index a9284d64..da807fc8 100644 --- a/ui/lib/multisigClient.worker.ts +++ b/ui/lib/multisigClient.worker.ts @@ -208,6 +208,16 @@ async function assertExpectedVerificationKey(actualHash: string): Promise async function compileContract(): Promise { if (compileSucceeded) return true; + // The Lightnet E2E build runs its daemon with PROOF_LEVEL=none and replaces + // lazy proofs with o1js dummy proofs. Compiling the real circuit here adds no + // proof-verification coverage and can exhaust the small shared runner after + // the circuit grows. Its Compose stack instead pins the matching dummy VK; + // check-vk-hash separately compiles and verifies the real network VKs. + if (skipProofs && process.env.NEXT_PUBLIC_E2E_TEST === 'true') { + console.log('[MultisigWorker] Skipping circuit compile in proofless E2E mode'); + return true; + } + if (!compilePromise) { compilePromise = (async () => { console.log('[MultisigWorker] MinaGuard.compile() starting'); @@ -1422,5 +1432,9 @@ export type WorkerApi = typeof workerApi; console.log('[MultisigWorker] worker module loaded, exposing API'); Comlink.expose(workerApi); -// Eagerly start compilation as soon as the worker loads -compileContract().catch(() => { }); +// Eagerly start compilation as soon as the worker loads in real builds. The +// E2E harness must first enable its explicit proofless mode, otherwise this +// synchronous WASM work prevents the worker from receiving that configuration. +if (process.env.NEXT_PUBLIC_E2E_TEST !== 'true') { + compileContract().catch(() => { }); +} From 2efe9a857915e18f6c12638d361d225abc69f485 Mon Sep 17 00:00:00 2001 From: Jason Park Date: Mon, 14 Sep 2026 12:29:30 +0000 Subject: [PATCH 9/9] docs: correct proofless e2e verification key usage --- preview-env/docker-compose.e2e-ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/preview-env/docker-compose.e2e-ci.yml b/preview-env/docker-compose.e2e-ci.yml index a0466b49..f013b17c 100644 --- a/preview-env/docker-compose.e2e-ci.yml +++ b/preview-env/docker-compose.e2e-ci.yml @@ -10,8 +10,9 @@ # in .github/workflows/e2e.yml; internal service-to-service URLs are unchanged. # # Usage: -# MINAGUARD_VK_HASH=$(contracts/scripts/read-vk-hash.sh testnet) docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e up -d --build --wait -# docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e down -v +# E2E_VK_HASH=$(cd contracts && bun -e "import { VerificationKey } from 'o1js'; console.log((await VerificationKey.dummy()).hash.toString())") +# MINAGUARD_VK_HASH="$E2E_VK_HASH" docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e up -d --build --wait +# MINAGUARD_VK_HASH="$E2E_VK_HASH" docker compose -f preview-env/docker-compose.e2e-ci.yml -p mina-guard-e2e down -v services: lightnet: