diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..689504a3 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,40 @@ +version: 2 +updates: + - package-ecosystem: "npm" + directory: "/" + schedule: + interval: "weekly" + day: "monday" + open-pull-requests-limit: 5 + groups: + mesh-sdk: + patterns: + - "@meshsdk/*" + next: + patterns: + - "next" + - "next-*" + - "@next/*" + prisma: + patterns: + - "prisma" + - "@prisma/*" + trpc: + patterns: + - "@trpc/*" + types: + patterns: + - "@types/*" + update-types: + - "minor" + - "patch" + labels: + - "dependencies" + + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "monthly" + labels: + - "dependencies" + - "ci" diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml new file mode 100644 index 00000000..6f9b268f --- /dev/null +++ b/.github/workflows/pr-checks.yml @@ -0,0 +1,47 @@ +name: PR Checks + +on: + pull_request: + branches: [main] + push: + branches: [main] + +concurrency: + group: pr-checks-${{ github.ref }} + cancel-in-progress: true + +jobs: + checks: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + + - name: Install dependencies + run: npm ci + + - name: Generate Prisma client + run: npx prisma generate + + # Lint stays non-blocking until the rule set is cleaned up; tracked separately. + - name: Lint + run: npm run lint + continue-on-error: true + + # Typecheck, test, and build are gates — failures must fail the PR. + - name: Type check + run: npx tsc --noEmit + + - name: Test + run: npm run test:ci + + - name: Build + run: npm run build + env: + SKIP_ENV_VALIDATION: 'true' diff --git a/prisma/migrations/20260510160404_audit_log_and_indexes/migration.sql b/prisma/migrations/20260510160404_audit_log_and_indexes/migration.sql new file mode 100644 index 00000000..058fae30 --- /dev/null +++ b/prisma/migrations/20260510160404_audit_log_and_indexes/migration.sql @@ -0,0 +1,88 @@ +-- AlterTable +ALTER TABLE "Ballot" ALTER COLUMN "anchorUrls" SET DEFAULT ARRAY[]::TEXT[], +ALTER COLUMN "anchorHashes" SET DEFAULT ARRAY[]::TEXT[]; + +-- CreateTable +CREATE TABLE "AuditLog" ( + "id" TEXT NOT NULL, + "actorAddress" TEXT, + "actorType" TEXT NOT NULL, + "action" TEXT NOT NULL, + "resourceType" TEXT, + "resourceId" TEXT, + "ip" TEXT, + "userAgent" TEXT, + "outcome" TEXT NOT NULL, + "reason" TEXT, + "metadata" JSONB, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "AuditLog_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE INDEX "AuditLog_actorAddress_idx" ON "AuditLog"("actorAddress"); + +-- CreateIndex +CREATE INDEX "AuditLog_action_idx" ON "AuditLog"("action"); + +-- CreateIndex +CREATE INDEX "AuditLog_resourceType_resourceId_idx" ON "AuditLog"("resourceType", "resourceId"); + +-- CreateIndex +CREATE INDEX "AuditLog_createdAt_idx" ON "AuditLog"("createdAt"); + +-- CreateIndex +CREATE INDEX "AuditLog_actorAddress_createdAt_idx" ON "AuditLog"("actorAddress", "createdAt"); + +-- CreateIndex +CREATE INDEX "BalanceSnapshot_walletId_idx" ON "BalanceSnapshot"("walletId"); + +-- CreateIndex +CREATE INDEX "BalanceSnapshot_walletId_snapshotDate_idx" ON "BalanceSnapshot"("walletId", "snapshotDate"); + +-- CreateIndex +CREATE INDEX "Ballot_walletId_idx" ON "Ballot"("walletId"); + +-- CreateIndex +CREATE INDEX "NewWallet_ownerAddress_idx" ON "NewWallet"("ownerAddress"); + +-- CreateIndex +CREATE INDEX "NewWallet_signersAddresses_idx" ON "NewWallet" USING GIN ("signersAddresses" array_ops); + +-- CreateIndex +CREATE INDEX "Proxy_walletId_idx" ON "Proxy"("walletId"); + +-- CreateIndex +CREATE INDEX "Proxy_userId_idx" ON "Proxy"("userId"); + +-- CreateIndex +CREATE INDEX "Proxy_walletId_isActive_idx" ON "Proxy"("walletId", "isActive"); + +-- CreateIndex +CREATE INDEX "Proxy_userId_isActive_idx" ON "Proxy"("userId", "isActive"); + +-- CreateIndex +CREATE INDEX "Signable_walletId_idx" ON "Signable"("walletId"); + +-- CreateIndex +CREATE INDEX "Signable_state_idx" ON "Signable"("state"); + +-- CreateIndex +CREATE INDEX "Signable_walletId_state_idx" ON "Signable"("walletId", "state"); + +-- CreateIndex +CREATE INDEX "Transaction_walletId_idx" ON "Transaction"("walletId"); + +-- CreateIndex +CREATE INDEX "Transaction_state_idx" ON "Transaction"("state"); + +-- CreateIndex +CREATE INDEX "Transaction_walletId_state_idx" ON "Transaction"("walletId", "state"); + +-- CreateIndex +CREATE INDEX "Wallet_ownerAddress_idx" ON "Wallet"("ownerAddress"); + +-- CreateIndex +CREATE INDEX "Wallet_signersAddresses_idx" ON "Wallet" USING GIN ("signersAddresses" array_ops); + diff --git a/prisma/migrations/20260510170000_make_user_nostrkey_optional/migration.sql b/prisma/migrations/20260510170000_make_user_nostrkey_optional/migration.sql new file mode 100644 index 00000000..7d33c422 --- /dev/null +++ b/prisma/migrations/20260510170000_make_user_nostrkey_optional/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "User" ALTER COLUMN "nostrKey" DROP NOT NULL; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 23d45677..30902c9b 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -16,12 +16,12 @@ datasource db { } model User { - id String @id @default(cuid()) - address String @unique - stakeAddress String @unique - drepKeyHash String @default("") - nostrKey String @unique - discordId String @default("") + id String @id @default(cuid()) + address String @unique + stakeAddress String @unique + drepKeyHash String @default("") + nostrKey String? @unique + discordId String @default("") } model Wallet { @@ -257,3 +257,24 @@ model BotClaimToken { @@index([tokenHash]) } + +model AuditLog { + id String @id @default(cuid()) + actorAddress String? // Wallet address that performed the action (null for system/anonymous) + actorType String // "user" | "bot" | "system" + action String // e.g. "wallet.create", "tx.sign", "bot.grant", "auth.login" + resourceType String? // "wallet" | "transaction" | "bot" | "ballot" | etc. + resourceId String? + ip String? + userAgent String? + outcome String // "success" | "denied" | "error" + reason String? // Short reason on denied/error + metadata Json? // Additional context (no secrets, redacted) + createdAt DateTime @default(now()) + + @@index([actorAddress]) + @@index([action]) + @@index([resourceType, resourceId]) + @@index([createdAt]) + @@index([actorAddress, createdAt]) +} diff --git a/scripts/ci/framework/markdown.ts b/scripts/ci/framework/markdown.ts index 449dca31..a13880fb 100644 --- a/scripts/ci/framework/markdown.ts +++ b/scripts/ci/framework/markdown.ts @@ -12,7 +12,12 @@ function fmtMs(ms: number): string { } function escapeCell(s: string): string { - return s.replace(/\\/g, "\\\\").replace(/\|/g, "\\|").replace(/\n/g, " "); + // Escape both `\` and `|` in a single pass with a character class so the + // replacement can't be mis-ordered or partially applied — CodeQL's + // js/incomplete-sanitization rule flagged the previous chained version. + // Backslash is added in the replacement, so we capture and prefix in + // one regex (no second pass that could double-escape). + return s.replace(/[\\|]/g, "\\$&").replace(/[\r\n]+/g, " "); } function renderSteps(steps: StepReport[]): string { diff --git a/src/__tests__/__mocks__/styleMock.cjs b/src/__tests__/__mocks__/styleMock.cjs new file mode 100644 index 00000000..f053ebf7 --- /dev/null +++ b/src/__tests__/__mocks__/styleMock.cjs @@ -0,0 +1 @@ +module.exports = {}; diff --git a/src/__tests__/og.test.ts b/src/__tests__/og.test.ts new file mode 100644 index 00000000..afa0a471 --- /dev/null +++ b/src/__tests__/og.test.ts @@ -0,0 +1,169 @@ +import { describe, expect, it, jest, beforeEach, afterEach } from "@jest/globals"; +import type { NextApiRequest, NextApiResponse } from "next"; + +// SSRF tripwire suite for /api/v1/og +// +// The handler must reject: +// - non-https URLs +// - hosts not on the allowlist +// - hosts that resolve to private / loopback / link-local addresses +// - upstream redirects (no auto-follow) +// +// The most important regression is the IMDS URL case: +// http://169.254.169.254/latest/meta-data/ (AWS instance metadata) +// — historically the canonical SSRF target. If this ever returns 200, an +// attacker who can hit our public OG endpoint can pivot into cloud metadata. + +const dnsLookupMock = jest.fn() as jest.MockedFunction< + (host: string, opts?: unknown) => Promise> +>; + +jest.unstable_mockModule("dns", () => ({ + __esModule: true, + default: { promises: { lookup: dnsLookupMock } }, + promises: { lookup: dnsLookupMock }, +})); + +const envState: { OG_ALLOWED_HOSTS?: string } = {}; +jest.unstable_mockModule("@/env", () => ({ + __esModule: true, + env: new Proxy({}, { + get(_t, key: string) { + if (key === "OG_ALLOWED_HOSTS") return envState.OG_ALLOWED_HOSTS; + return undefined; + }, + }), +})); + +const fetchMock = jest.fn() as jest.MockedFunction; +const realFetch = global.fetch; + +function makeRes() { + const status = jest.fn(); + const json = jest.fn(); + const setHeader = jest.fn(); + const res = { + status: status.mockImplementation(() => res), + json: json.mockImplementation(() => res), + setHeader, + } as unknown as NextApiResponse; + return { res, status, json }; +} + +function makeReq(url: string | undefined): NextApiRequest { + return { + query: url === undefined ? {} : { url }, + method: "GET", + headers: {}, + } as unknown as NextApiRequest; +} + +const handlerPromise = import("../pages/api/v1/og"); + +beforeEach(() => { + dnsLookupMock.mockReset(); + fetchMock.mockReset(); + global.fetch = fetchMock as unknown as typeof fetch; + envState.OG_ALLOWED_HOSTS = undefined; +}); + +afterEach(() => { + global.fetch = realFetch; +}); + +describe("og handler — SSRF defense", () => { + it("rejects missing url with 400", async () => { + const { default: handler } = await handlerPromise; + const { res, status, json } = makeRes(); + await handler(makeReq(undefined), res); + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringMatching(/missing/i) })); + }); + + it("rejects http:// URLs with 400", async () => { + const { default: handler } = await handlerPromise; + const { res, status } = makeRes(); + await handler(makeReq("http://github.com/example"), res); + expect(status).toHaveBeenCalledWith(400); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("rejects IMDS URL (http://169.254.169.254/...) — TRIPWIRE", async () => { + // This test is the one we never let regress. AWS instance metadata URL. + // Even if someone allowlists `*` for OG_ALLOWED_HOSTS, the http:// scheme + // check rejects this immediately. No DNS lookup, no fetch. + const { default: handler } = await handlerPromise; + const { res, status } = makeRes(); + await handler(makeReq("http://169.254.169.254/latest/meta-data/"), res); + expect(status).toHaveBeenCalledWith(400); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("rejects https IMDS-style URL when wildcard hosts but private IP", async () => { + // Even with OG_ALLOWED_HOSTS=*, the DNS / address-class check must reject + // direct private-IP literals, including the link-local 169.254.0.0/16. + envState.OG_ALLOWED_HOSTS = "*"; + const { default: handler } = await handlerPromise; + const { res, status, json } = makeRes(); + await handler(makeReq("https://169.254.169.254/"), res); + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringMatching(/private|loopback/i) })); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("rejects host not on the allowlist with 400", async () => { + envState.OG_ALLOWED_HOSTS = "github.com,x.com"; + const { default: handler } = await handlerPromise; + const { res, status } = makeRes(); + await handler(makeReq("https://evil.example.com/"), res); + expect(status).toHaveBeenCalledWith(400); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("rejects when DNS resolves to an RFC1918 address", async () => { + envState.OG_ALLOWED_HOSTS = "internal.example.com"; + dnsLookupMock.mockResolvedValueOnce([{ address: "10.0.0.5", family: 4 }]); + const { default: handler } = await handlerPromise; + const { res, status, json } = makeRes(); + await handler(makeReq("https://internal.example.com/"), res); + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringMatching(/private|loopback/i) })); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("rejects when upstream returns a redirect (no auto-follow)", async () => { + envState.OG_ALLOWED_HOSTS = "github.com"; + dnsLookupMock.mockResolvedValueOnce([{ address: "140.82.114.4", family: 4 }]); + fetchMock.mockResolvedValueOnce( + new Response(null, { status: 302, headers: { location: "http://169.254.169.254/" } }), + ); + const { default: handler } = await handlerPromise; + const { res, status } = makeRes(); + await handler(makeReq("https://github.com/example"), res); + expect(status).toHaveBeenCalledWith(500); + }); + + it("returns 200 with extracted OG metadata for an allowlisted public host", async () => { + envState.OG_ALLOWED_HOSTS = "example.com"; + dnsLookupMock.mockResolvedValueOnce([{ address: "93.184.216.34", family: 4 }]); + const html = ` + + + + + `; + fetchMock.mockResolvedValueOnce(new Response(html, { status: 200 })); + const { default: handler } = await handlerPromise; + const { res, status, json } = makeRes(); + await handler(makeReq("https://example.com/page"), res); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith( + expect.objectContaining({ + title: "Hello", + description: "World", + image: "https://example.com/img.png", + siteName: "Example", + }), + ); + }); +}); diff --git a/src/__tests__/resolveDRepAnchorFromUrl.test.ts b/src/__tests__/resolveDRepAnchorFromUrl.test.ts index 8f38f629..7baf3ee5 100644 --- a/src/__tests__/resolveDRepAnchorFromUrl.test.ts +++ b/src/__tests__/resolveDRepAnchorFromUrl.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it, jest } from "@jest/globals"; +import { Readable } from "stream"; import { hashDrepAnchor } from "@meshsdk/core"; jest.mock("node:dns/promises", () => ({ @@ -7,22 +8,34 @@ jest.mock("node:dns/promises", () => ({ ), })); +// undici.request is the transport used by resolveDRepAnchorFromUrl — the +// previous test mocked global.fetch, but the implementation now pins the +// resolved IP via undici's buildConnector to close the DNS-rebinding TOCTOU. +const requestMock = jest.fn<(...args: unknown[]) => unknown>(); +jest.mock("undici", () => ({ + request: (...args: unknown[]) => requestMock(...args), + Agent: jest.fn(), + buildConnector: jest.fn(() => () => undefined), +})); + import { resolveDRepAnchorFromUrl } from "@/lib/server/resolveDRepAnchorFromUrl"; -const originalFetch = global.fetch; +function makeResponse(body: string, statusCode = 200) { + return { + statusCode, + headers: { "content-type": "application/json" }, + body: Readable.from(Buffer.from(body, "utf8")), + }; +} afterEach(() => { - global.fetch = originalFetch; jest.clearAllMocks(); }); describe("resolveDRepAnchorFromUrl", () => { it("computes hash from JSON body", async () => { const doc = { "@context": "https://example.com", name: "Test" }; - const body = JSON.stringify(doc); - global.fetch = jest.fn(async () => { - return new Response(body, { status: 200, headers: { "Content-Type": "application/json" } }); - }) as unknown as typeof fetch; + requestMock.mockResolvedValueOnce(makeResponse(JSON.stringify(doc))); const r = await resolveDRepAnchorFromUrl("https://example.test/drep.json"); expect(r.anchorUrl).toBe("https://example.test/drep.json"); @@ -31,9 +44,7 @@ describe("resolveDRepAnchorFromUrl", () => { it("rejects when optional anchorDataHash mismatches", async () => { const doc = { x: 1 }; - global.fetch = jest.fn(async () => { - return new Response(JSON.stringify(doc), { status: 200 }); - }) as unknown as typeof fetch; + requestMock.mockResolvedValueOnce(makeResponse(JSON.stringify(doc))); await expect( resolveDRepAnchorFromUrl("https://example.test/a.json", "deadbeef"), diff --git a/src/__tests__/reviewSignersCardKey.test.ts b/src/__tests__/reviewSignersCardKey.test.ts new file mode 100644 index 00000000..c55c5f9c --- /dev/null +++ b/src/__tests__/reviewSignersCardKey.test.ts @@ -0,0 +1,289 @@ +/** + * Regression test for Finding 1.1 (rocksolid/harden-pr-233): + * React key collision when multiple empty signer rows exist. + * + * The fix introduced a parallel `signerIds: string[]` array in + * useWalletFlowState / useMigrationWalletFlowState that is used as the + * React `key` on each signer row, instead of the (possibly empty, + * possibly duplicated) address string. + * + * This test pins the invariant on two layers: + * 1. The data-shape invariants of the React-key fix — exercised here + * by inline simulation of the hook's add/remove array logic. We + * cannot drive the hook itself in a node-environment jest run + * (the hook depends on next/router, zustand, tRPC, and toast + * providers), so this layer pins the *shape* the hook is + * contracted to maintain: synthetic ids stay distinct across + * empty-row collisions, and removal preserves index alignment + * across all five parallel arrays. + * 2. A source-level tripwire (separate describe block below) that + * catches anyone reverting the JSX back to address-as-key, or + * removing the parallel signerIds array from either hook. The + * React-key behavior itself — that the JSX consumes signerIds + * and that the hook exposes it — is pinned by that tripwire. + * + * Why no `renderHook`: `useWalletFlowState` depends on next/router, + * zustand, tRPC, and toast providers, none of which exist in jest's + * `node` test environment. Adding `@testing-library/react` + jsdom + * would be substantial scaffolding and out of scope for this change. + */ + +import fs from "fs"; +import path from "path"; +import { fileURLToPath } from "url"; +import { makeSignerId } from "@/components/pages/homepage/wallets/new-wallet-flow/shared/signerRows"; + +// ESM equivalent of CJS __dirname. Tests run under +// node --experimental-vm-modules so CJS globals are unavailable. +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +// Mirror the splice-based array logic that addSigner / removeSigner +// implement inside the two flow-state hooks. We cannot import the hooks +// directly because they pull in next/router, zustand, tRPC, and toast +// providers — none of which exist in jest's node test environment. So +// we replicate the data flow at the same level of abstraction the hook +// uses, and pin the invariant: after add/remove cycles, each parallel +// array stays index-aligned with the others. +type SignerRow = { + address: string; + description: string; + stakeKey: string; + drepKey: string; + id: string; +}; + +function applyAdd(rows: SignerRow[], newId: string): SignerRow[] { + // Mirrors addSigner: pushes empty fields with a fresh synthetic id. + return [ + ...rows, + { address: "", description: "", stakeKey: "", drepKey: "", id: newId }, + ]; +} + +function applyRemove(rows: SignerRow[], index: number): SignerRow[] { + // Mirrors removeSigner: splices the same index out of every parallel + // array. With address-as-key, two empty rows would collide on key="" + // and React could splice the wrong one — synthetic ids prevent that. + const next = rows.slice(); + next.splice(index, 1); + return next; +} + +describe("ReviewSignersCard signer-row key invariant", () => { + test("removing the middle of three signers keeps remaining rows aligned", () => { + const initial: SignerRow[] = [ + { + address: "addr1qx_creator", + description: "Alice", + stakeKey: "stake1_alice", + drepKey: "drep_alice", + id: "id-creator", + }, + ]; + + // User clicks "Add Signer" twice. Both new rows have address "" — + // the bug case. Synthetic ids must still differ. + const afterAdd1 = applyAdd(initial, "id-bob"); + const afterAdd2 = applyAdd(afterAdd1, "id-carol"); + + expect(afterAdd2).toHaveLength(3); + // Two empty addresses, but ids are distinct. + expect(afterAdd2[1]!.address).toBe(""); + expect(afterAdd2[2]!.address).toBe(""); + expect(afterAdd2[1]!.id).not.toBe(afterAdd2[2]!.id); + + // Fill them in so we can tell which row is which. + afterAdd2[1] = { ...afterAdd2[1]!, address: "addr1_bob", description: "Bob", stakeKey: "stake1_bob" }; + afterAdd2[2] = { ...afterAdd2[2]!, address: "addr1_carol", description: "Carol", stakeKey: "stake1_carol" }; + + // Remove Bob (index 1). + const afterRemove = applyRemove(afterAdd2, 1); + + expect(afterRemove).toHaveLength(2); + // Alice still at index 0, Carol now at index 1 — descriptions and + // stake keys must follow the same row, NOT slip out of alignment. + expect(afterRemove[0]!.description).toBe("Alice"); + expect(afterRemove[0]!.stakeKey).toBe("stake1_alice"); + expect(afterRemove[1]!.description).toBe("Carol"); + expect(afterRemove[1]!.stakeKey).toBe("stake1_carol"); + expect(afterRemove[1]!.address).toBe("addr1_carol"); + }); + + test("two consecutive Add Signer clicks produce distinct synthetic ids", () => { + // The exact bug case: with key={signer} (address) two empty rows + // would collide. Synthetic ids must differ regardless of address. + let rows: SignerRow[] = []; + rows = applyAdd(rows, "id-1"); + rows = applyAdd(rows, "id-2"); + + const ids = rows.map((r) => r.id); + expect(new Set(ids).size).toBe(rows.length); + }); + + test( + "addSigner-then-removeSigner: surviving row keeps its synthetic id, " + + "data-shape invariant pinned via inline simulation", + () => { + // Mirrors the array-manipulation contract the hook's `addSigner` + // / `removeSigner` setters apply, inline. `makeSignerId` runs + // unmocked so we exercise real id minting (crypto.randomUUID + // under Node >= 14.17, fallback otherwise). + // + // Scenario: start with one creator-seeded row (the first-user + // effect), append two empty rows, capture the id of the third + // row (the survivor), then splice index 1 — a mid-array empty + // remove. Assert that ids[1] is the survivor's *original* id + // (not a re-issued one) and that every parallel array stays + // index-aligned with ids. + + type Arrays = { + addresses: string[]; + descriptions: string[]; + stakeKeys: string[]; + drepKeys: string[]; + ids: string[]; + }; + + // Seed three rows. Index 0 is the creator (Alice). Index 1 is an + // empty row (Bob, never filled in) — preserves the empty-row + // collision scenario this test was originally written for. Index + // 2 (Carol, the survivor) carries distinct, non-empty values in + // every parallel array. After removing index 1, index 1 must + // hold Carol's distinct values; an off-by-one that mis-spliced + // any single non-id array would leave "" in that array's slot + // and fail the assertion. + let state: Arrays = { + addresses: ["addr1_creator", "", "addr1_carol"], + descriptions: ["Alice", "", "Carol"], + stakeKeys: ["stake1_creator", "", "stake1_carol"], + drepKeys: ["", "", "drep1_carol"], + ids: [makeSignerId(), makeSignerId(), makeSignerId()], + }; + + expect(state.ids).toHaveLength(3); + // Three distinct synthetic ids — the bug case (two empty rows + // sharing key="") cannot reproduce when ids are minted per-row. + expect(new Set(state.ids).size).toBe(3); + + // Survivor: the row currently at index 2. Its id must follow the + // row, not the index, after we remove index 1. + const survivorOriginalId = state.ids[2]!; + + // Splice index 1 out of every parallel array, mirroring + // removeSigner. + const spliceOut = (arr: T[], i: number): T[] => { + const next = arr.slice(); + next.splice(i, 1); + return next; + }; + state = { + addresses: spliceOut(state.addresses, 1), + descriptions: spliceOut(state.descriptions, 1), + stakeKeys: spliceOut(state.stakeKeys, 1), + drepKeys: spliceOut(state.drepKeys, 1), + ids: spliceOut(state.ids, 1), + }; + + expect(state.ids).toHaveLength(2); + // The id now at index 1 must be the survivor's original id — + // proving identity follows the row, not the position. If the hook + // re-minted ids on remove (the broken pattern), this would fail. + expect(state.ids[1]).toBe(survivorOriginalId); + // Every parallel array stays index-aligned with ids AND the + // survivor's distinct values land at index 1. An off-by-one that + // spliced index 2 instead of index 1 would leave "" here in any + // single array — making the splice direction directly testable. + expect(state.addresses[1]).toBe("addr1_carol"); + expect(state.descriptions[1]).toBe("Carol"); + expect(state.stakeKeys[1]).toBe("stake1_carol"); + expect(state.drepKeys[1]).toBe("drep1_carol"); + // Creator row at index 0 untouched. + expect(state.ids[0]).not.toBe(survivorOriginalId); + expect(state.addresses[0]).toBe("addr1_creator"); + expect(state.descriptions[0]).toBe("Alice"); + expect(state.stakeKeys[0]).toBe("stake1_creator"); + }, + ); +}); + +describe("ReviewSignersCard tripwire on source", () => { + // Pin the source-level fix: anyone reverting back to address-as-key + // (or removing the parallel signerIds) will see this test fail. + const SOURCE_PATH = path.resolve( + __dirname, + "../components/pages/homepage/wallets/new-wallet-flow/create/ReviewSignersCard.tsx", + ); + const HOOK_PATH = path.resolve( + __dirname, + "../components/pages/homepage/wallets/new-wallet-flow/shared/useWalletFlowState.tsx", + ); + const MIGRATION_HOOK_PATH = path.resolve( + __dirname, + "../components/pages/wallet/info/migration/useMigrationWalletFlowState.tsx", + ); + + test("ReviewSignersCard never uses the raw address as a React key", () => { + const src = fs.readFileSync(SOURCE_PATH, "utf8"); + + // ---- Negative tripwire ---- + // + // Nothing within `key={ ... }` may start with the bare identifier + // `signer` (the per-iteration address). The boundary class + // `(\s|\}|[^a-zA-Z_0-9])` after `signer` rejects the entire + // address-as-key family: + // - `key={signer}` (the original bug) + // - `key={ signer }` (whitespace variant) + // - `key={signer ?? ""}` (nullish-coalescing fallback) + // - `key={signer.address}` (member-access — different revert) + // - `key={String(signer)}` ('(' is non-alphanumeric) + // + // It deliberately does NOT trip on the synthetic forms + // `key={signerIds[index]}` or `key={signerIds[index] ?? "..."}` + // because the `I` in `Ids` is a-zA-Z and falls outside the + // boundary class — `signer` followed by a word char is fine. + expect(src).not.toMatch(/key=\{\s*signer(\s|\}|[^a-zA-Z_0-9])/); + + // ---- Positive tripwire ---- + // + // Within 200 chars of an opening ` + // without breaking this test (the synthetic-id behavior survives). + // It still fails if anyone reverts to the bare address or to a + // bare index-as-key. + expect(src).toMatch( + //g) ?? []; + const mobileCardBlock = divMobileBlocks.find( + (block) => + /rounded-lg border/.test(block) && + /key=\{[\s\S]{0,80}(signerIds|rowKey\()/.test(block), + ); + expect(mobileCardBlock).toBeDefined(); + }); + + test("useWalletFlowState exposes signerIds parallel to signersAddresses", () => { + const src = fs.readFileSync(HOOK_PATH, "utf8"); + expect(src).toMatch(/signerIds/); + expect(src).toMatch(/setSignerIds/); + }); + + test("useMigrationWalletFlowState exposes signerIds parallel to signersAddresses", () => { + const src = fs.readFileSync(MIGRATION_HOOK_PATH, "utf8"); + expect(src).toMatch(/signerIds/); + expect(src).toMatch(/setSignerIds/); + }); +}); diff --git a/src/__tests__/setupEnv.cjs b/src/__tests__/setupEnv.cjs new file mode 100644 index 00000000..1314d88b --- /dev/null +++ b/src/__tests__/setupEnv.cjs @@ -0,0 +1,21 @@ +// @ts-nocheck — env bootstrap; checkJs flags `NODE_ENV` as read-only +// because @types/node narrows it to a literal union, but writing it here +// is intentional and safe (runs before any test module is imported). +// +// Sets dummy env vars so that `src/env.js` (t3-oss validate) does not throw +// when test files import server modules transitively. +// Tests that need real values can override per-test with `process.env.X = ...` +// inside `beforeEach`. + +process.env['NODE_ENV'] = process.env['NODE_ENV'] || 'test'; +process.env.SKIP_ENV_VALIDATION = '1'; + +process.env.DATABASE_URL = process.env.DATABASE_URL || 'postgresql://test:test@localhost:5432/test'; +process.env.JWT_SECRET = process.env.JWT_SECRET || 'a'.repeat(48); +process.env.PINATA_JWT = process.env.PINATA_JWT || 'test-pinata-jwt'; + +process.env.NEXT_PUBLIC_BLOCKFROST_API_KEY_MAINNET = + process.env.NEXT_PUBLIC_BLOCKFROST_API_KEY_MAINNET || 'test-blockfrost-mainnet'; +process.env.NEXT_PUBLIC_BLOCKFROST_API_KEY_PREPROD = + process.env.NEXT_PUBLIC_BLOCKFROST_API_KEY_PREPROD || 'test-blockfrost-preprod'; +process.env.NEXT_PUBLIC_NETWORK_ID = process.env.NEXT_PUBLIC_NETWORK_ID || '0'; diff --git a/src/__tests__/signing.test.ts b/src/__tests__/signing.test.ts new file mode 100644 index 00000000..81d46b3d --- /dev/null +++ b/src/__tests__/signing.test.ts @@ -0,0 +1,107 @@ +import fs from "fs"; +import path from "path"; +import { fileURLToPath } from "url"; +import { describe, expect, it, jest } from "@jest/globals"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +// --------------------------------------------------------------------------- +// Tripwire: the "broken" pattern — `return true ? signature : undefined;` +// must never reappear in `src/utils/signing.ts`. It both throws away the +// `checkSignature` result and obscures the actual signing contract. The +// regression we fixed here was that a failed signature verification still +// returned a (forged-looking) signature to the caller because the ternary +// was always truthy. +// --------------------------------------------------------------------------- +describe("signing.ts source contract", () => { + it("never returns the always-true ternary on the verification result", () => { + const src = fs.readFileSync( + path.resolve(__dirname, "../utils/signing.ts"), + "utf8", + ); + expect(src).not.toMatch(/return\s+true\s*\?/); + // Positive: the verified result must drive an explicit `if (!verified)` + // throw. The exact identifier we use is `verified` — accept either name + // so a future rename doesn't trip the tripwire. + expect(src).toMatch(/if\s*\(\s*!\s*(verified|result)\b/); + }); +}); + +// --------------------------------------------------------------------------- +// Behavioural: import the real `sign` and exercise every role plus the +// failure path. We mock the @meshsdk/core helpers because they pull in +// CSL/serialization which is heavyweight for a unit test. +// --------------------------------------------------------------------------- +const checkSignatureMock = jest.fn< + (nonce: string, signature: { signature: string; key: string }, address?: string) => Promise +>(); +const generateNonceMock = jest.fn<(payload: string) => string>(); + +jest.unstable_mockModule("@meshsdk/core", () => ({ + __esModule: true, + checkSignature: checkSignatureMock, + generateNonce: generateNonceMock, +})); + +const { sign } = await import("../utils/signing"); + +type MockWallet = { + signData: jest.Mock<(payload: string, address?: string) => Promise<{ signature: string; key: string }>>; + getRewardAddresses: jest.Mock<() => Promise>; +}; + +function createWallet(overrides?: Partial): MockWallet { + return { + signData: jest.fn<(payload: string, address?: string) => Promise<{ signature: string; key: string }>>( + async () => ({ signature: "deadbeef", key: "cafe" }), + ), + getRewardAddresses: jest.fn<() => Promise>(async () => ["stake_addr"]), + ...overrides, + } as MockWallet; +} + +describe("sign", () => { + beforeEach(() => { + checkSignatureMock.mockReset(); + generateNonceMock.mockReset(); + generateNonceMock.mockReturnValue("nonce-payload"); + }); + + it("role=0 signs with the user payment address and returns the signature", async () => { + checkSignatureMock.mockResolvedValueOnce(true); + const wallet = createWallet(); + const sig = await sign("payload", wallet as never, 0, "addr_test_user"); + expect(sig).toEqual({ signature: "deadbeef", key: "cafe" }); + expect(wallet.signData).toHaveBeenCalledWith("payload", "addr_test_user"); + }); + + it("role=2 signs with the wallet's reward (stake) address", async () => { + checkSignatureMock.mockResolvedValueOnce(true); + const wallet = createWallet(); + await sign("payload", wallet as never, 2); + expect(wallet.getRewardAddresses).toHaveBeenCalled(); + expect(wallet.signData).toHaveBeenCalledWith("payload", "stake_addr"); + }); + + it("role=3 requires an explicit dRepAddress and uses it", async () => { + checkSignatureMock.mockResolvedValueOnce(true); + const wallet = createWallet(); + await sign("payload", wallet as never, 3, undefined, "drep_xxx"); + expect(wallet.signData).toHaveBeenCalledWith("payload", "drep_xxx"); + }); + + it("throws when the chosen role has no resolved address", async () => { + const wallet = createWallet(); + await expect(sign("payload", wallet as never, 0, undefined)).rejects.toThrow( + /missing address/i, + ); + }); + + it("throws when checkSignature returns false (no silent ternary fallback)", async () => { + checkSignatureMock.mockResolvedValueOnce(false); + const wallet = createWallet(); + await expect(sign("payload", wallet as never, 0, "addr_test_user")).rejects.toThrow( + /Signature failed verification/i, + ); + }); +}); diff --git a/src/components/common/overall-layout/layout.tsx b/src/components/common/overall-layout/layout.tsx index 2f2da2f8..78dc6e36 100644 --- a/src/components/common/overall-layout/layout.tsx +++ b/src/components/common/overall-layout/layout.tsx @@ -1,7 +1,6 @@ import React, { useEffect, Component, ReactNode, useMemo, useCallback, useState, useRef } from "react"; import { useRouter } from "next/router"; import Link from "next/link"; -import { useNostrChat } from "@jinglescode/nostr-chat-plugin"; import { useWallet, useAddress } from "@meshsdk/react"; import { publicRoutes } from "@/data/public-routes"; import { api } from "@/utils/api"; @@ -156,7 +155,6 @@ export default function RootLayout({ const router = useRouter(); const { appWallet } = useAppWallet(); const { multisigWallet } = useMultisigWallet(); - const { generateNsec } = useNostrChat(); const { isEnabled: isUtxosEnabled } = useUTXOS(); const userAddress = useUserStore((state) => state.userAddress); @@ -229,7 +227,6 @@ export default function RootLayout({ address: variables.address, stakeAddress: variables.stakeAddress, drepKeyHash: variables.drepKeyHash ?? "", - nostrKey: variables.nostrKey, } ); } @@ -361,12 +358,10 @@ export default function RootLayout({ } // Create or update user - const nostrKey = generateNsec(); createUser({ address: walletAddress, stakeAddress, drepKeyHash, - nostrKey: JSON.stringify(nostrKey), }); } catch (error) { if (error instanceof Error && error.message.includes("account changed")) { @@ -374,9 +369,9 @@ export default function RootLayout({ } } } - + initializeWallet(); - }, [connected, activeWallet, user, userAddress, address, createUser, generateNsec]); + }, [connected, activeWallet, user, userAddress, address, createUser]); // Check wallet session and show authorization modal for first-time connections // Check session as soon as wallet is connected and address is available (don't wait for user) diff --git a/src/components/common/overall-layout/menus/multisig-wallet.tsx b/src/components/common/overall-layout/menus/multisig-wallet.tsx index 4a29fdbc..24148ff5 100644 --- a/src/components/common/overall-layout/menus/multisig-wallet.tsx +++ b/src/components/common/overall-layout/menus/multisig-wallet.tsx @@ -3,7 +3,6 @@ import { useRouter } from "next/router"; import MenuLink from "./menu-link"; import usePendingTransactions from "@/hooks/usePendingTransactions"; import { Badge } from "@/components/ui/badge"; -import { ChatBubbleIcon } from "@radix-ui/react-icons"; import usePendingSignables from "@/hooks/usePendingSignables"; import useMultisigWallet from "@/hooks/useMultisigWallet"; @@ -101,15 +100,6 @@ export default function MenuWallet({ walletId, stakingEnabled }: MenuWalletProps Assets - - - Chat - = 14.17; fall back to a + * timestamp+random string in environments where it isn't. + */ +export function makeSignerId(): string { + if (typeof crypto !== "undefined" && typeof crypto.randomUUID === "function") { + return crypto.randomUUID(); + } + return `signer-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`; +} diff --git a/src/components/pages/wallet/chat/index.tsx b/src/components/pages/wallet/chat/index.tsx deleted file mode 100644 index 37a084be..00000000 --- a/src/components/pages/wallet/chat/index.tsx +++ /dev/null @@ -1,209 +0,0 @@ -import Button from "@/components/common/button"; -import { Label } from "@/components/ui/label"; -import { Textarea } from "@/components/ui/textarea"; -import useAppWallet from "@/hooks/useAppWallet"; -import useUser from "@/hooks/useUser"; -import { useNostrChat } from "@jinglescode/nostr-chat-plugin"; -import { CornerDownLeft } from "lucide-react"; -import { useEffect, useRef, useState } from "react"; -import { api } from "@/utils/api"; -import { AnimatePresence, motion } from "framer-motion"; - -export default function WalletChat() { - const { appWallet } = useAppWallet(); - const { user } = useUser(); - const messagesContainerRef = useRef(null); - const [pubkey, setPubkey] = useState(undefined); - const [connecting, setConnecting] = useState(false); - const [textareaValue, setTextareaValue] = useState(""); - const [usersPubkeyToName, setUsersPubkeyToName] = useState< - { - pubkey: any; - address: string; - name: string | undefined; - }[] - >([]); - - const { data: nostrUsers } = api.user.getNostrKeysByAddresses.useQuery( - { - addresses: appWallet ? appWallet.signersAddresses : [], - }, - { - enabled: appWallet !== undefined, - }, - ); - - const { subscribeRoom, messages, publishMessage, setUser, roomId } = - useNostrChat(); - - useEffect(() => { - if (messagesContainerRef.current) { - messagesContainerRef.current.scrollTop = - messagesContainerRef.current.scrollHeight; - } - }, [messages]); - - useEffect(() => { - async function load() { - if (appWallet && roomId != appWallet.id && !connecting) { - setConnecting(true); - subscribeRoom(appWallet.id); - } - } - load(); - }, [appWallet]); - - useEffect(() => { - if (user && appWallet && nostrUsers && usersPubkeyToName.length === 0) { - const _nostrUsers = nostrUsers.map((user) => { - return { - pubkey: JSON.parse(user.nostrKey).pubkey, - address: user.address, - name: appWallet.signersDescriptions[ - appWallet.signersAddresses.indexOf(user.address) - ], - }; - }); - setUsersPubkeyToName(_nostrUsers); - - const { nsec, pubkey } = JSON.parse(user.nostrKey); - setPubkey(pubkey); - setUser({ nsec, pubkey }); - } - }, [user, appWallet, nostrUsers]); - - function handleSend() { - publishMessage(textareaValue); - setTextareaValue(""); - } - - const handleKeyPress = (event: React.KeyboardEvent) => { - if (event.key === "Enter" && !event.shiftKey) { - event.preventDefault(); - handleSend(); - } - if (event.key === "Enter" && event.shiftKey) { - event.preventDefault(); - setTextareaValue((prev) => prev + "\n"); - } - }; - - if (appWallet === undefined) return <>; - return ( -
-
-
- - {messages && - messages.length > 0 && - [...new Set(messages)] - .sort((a, b) => a.timestamp - b.timestamp) - .map((msg) => ( - -
- {usersPubkeyToName.find((u) => u.pubkey === msg.pubkey) - ?.name && ( -

- { - usersPubkeyToName.find( - (u) => u.pubkey === msg.pubkey, - )?.name - } -

- )} -
-
{msg.message}
-

- {new Date(msg.timestamp * 1000).toLocaleString("en-US", { - weekday: "short", - year: "numeric", - month: "short", - day: "numeric", - hour: "2-digit", - minute: "2-digit", - })} -

-
- ))} -
-
- -
-
- -