Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lucky-jokes-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@exactly/server": patch
---

✨ allow accounts on firewall after kyc
5 changes: 5 additions & 0 deletions .changeset/solid-plums-post.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@exactly/server": patch
---

✨ setup gcp credentials and kms
13 changes: 13 additions & 0 deletions .do/app.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,19 @@ services:
- key: DEBUG
scope: RUN_TIME
value: ${{ env.DEBUG }}
- key: GCP_BASE64_JSON
scope: RUN_TIME
type: SECRET
value: ${{ env.ENCRYPTED_GCP_BASE64_JSON || env.GCP_BASE64_JSON }}
- key: GCP_KMS_KEY_RING
scope: RUN_TIME
value: ${{ env.GCP_KMS_KEY_RING }}
- key: GCP_KMS_KEY_VERSION
scope: RUN_TIME
value: ${{ env.GCP_KMS_KEY_VERSION }}
- key: GCP_PROJECT_ID
scope: RUN_TIME
value: ${{ env.GCP_PROJECT_ID }}
- key: INTERCOM_IDENTITY_KEY
scope: RUN_TIME
type: SECRET
Expand Down
1 change: 1 addition & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@
"valibot",
"valierror",
"valkey",
"valora",
"viem",
"viewability",
"vite",
Expand Down
358 changes: 357 additions & 1 deletion pnpm-lock.yaml

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions server/hooks/persona.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import {
union,
} from "valibot";

import { firewallAbi, firewallAddress } from "@exactly/common/generated/chain";
import { Address } from "@exactly/common/validation";

import database, { credentials } from "../database/index";
import { kms } from "../utils/gcp";
import { createUser } from "../utils/panda";
import { addCapita, deriveAssociateId } from "../utils/pax";
import {
Expand Down Expand Up @@ -300,6 +302,22 @@ export default new Hono().post(
if (risk.level === "very_high") return c.json({ code: "very high risk" }, 200);
}

if (firewallAddress) {
const account = safeParse(Address, credential.account);
if (account.success) {
const address = firewallAddress;
kms("allower")
.then((allower) =>
allower.exaSend(
{ name: "firewall.allow", op: "exa.firewall", attributes: { account: account.output } },
{ address, functionName: "allow", args: [account.output, true], abi: firewallAbi },
Comment on lines +309 to +313
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make firewall allow retryable on transient KMS/RPC failures

This kms("allower") call is fire-and-forget and its failures are only captured, while the handler still continues to createUser and returns HTTP 200. I checked the server code paths for firewall.allow, and this is the only place it is triggered, so a transient KMS/RPC failure here leaves a KYC-approved account permanently unallowed (later flows will keep encountering NotAllowed) with no automatic recovery path. Please make this side effect durable/retryable (or fail the webhook when allow cannot be scheduled).

Useful? React with 👍 / 👎.

{ ignore: [`AlreadyAllowed(${account.output})`] },
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The error handling logic compares a checksummed address (account.output) with a potentially non-checksummed address from a revert reason, which may cause the comparison to fail.
Severity: MEDIUM

Suggested Fix

Ensure both addresses in the comparison are in the same format. Either apply checksumAddress to the address extracted from the error's cause.data.args before comparing it, or convert account.output to a non-checksummed format for the check.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: server/hooks/persona.ts#L314

Potential issue: In `persona.ts`, the error handling logic checks if a revert reason
string includes a user's account address. The address used for this check,
`account.output`, is checksummed. However, the revert reason is constructed by directly
converting error arguments to strings using `cause.data?.args?.map(String)`, which
likely does not apply checksumming. This mismatch between a checksummed address and a
non-checksummed address in the error string will cause the check to fail, preventing
certain errors from being correctly ignored.

Did we get this right? 👍 / 👎 to inform future reviews.

),
)
.catch((error: unknown) => captureException(error, { level: "error" }));
}
}
Comment on lines +305 to +319
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Fire-and-forget firewall allow becomes non-retriable after pandaId is set

The firewall allow call at server/hooks/persona.ts:309-317 is fire-and-forget (errors caught and sent to Sentry). If this call fails but the subsequent createUser at line 322 and DB update at line 332 both succeed, the pandaId guard at line 248 will cause any Persona webhook retry to return "already created" without re-attempting the firewall allow. This means a failed firewall allow is permanently lost and requires manual intervention. The team explicitly tests this behavior ("captures allower init failure without blocking panda creation" and "captures exaSend failure without blocking panda creation"), so this appears intentional. However, if the firewall is a hard requirement for the account to function, this could leave users in a broken state after KYC approval.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


// TODO implement error handling to return 200 if event should not be retried
const { id } = await createUser({
accountPurpose: fields.accountPurpose.value,
Expand Down
2 changes: 2 additions & 0 deletions server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"dependencies": {
"@account-kit/infra": "catalog:",
"@exactly/lib": "^0.1.0",
"@google-cloud/kms": "^5.4.0",
"@hono/node-server": "^1.19.13",
"@hono/sentry": "^1.2.2",
"@hono/valibot-validator": "^0.5.3",
Expand All @@ -44,6 +45,7 @@
"@simplewebauthn/server": "^13.3.0",
"@types/debug": "^4.1.13",
"@valibot/to-json-schema": "^1.6.0",
"@valora/viem-account-hsm-gcp": "^1.2.17",
"async-mutex": "^0.5.0",
"bullmq": "^5.71.1",
"debug": "^4.4.3",
Expand Down
5 changes: 5 additions & 0 deletions server/script/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ process.env.AUTH_SECRET = "auth";
process.env.BRIDGE_API_KEY = "bridge";
process.env.BRIDGE_API_URL = "https://bridge.test";
process.env.EXPO_PUBLIC_ALCHEMY_API_KEY = " ";
process.env.GCP_BASE64_JSON = "base64String==";
process.env.GCP_KMS_KEY_RING = "op-sepolia";
process.env.GCP_KMS_KEY_VERSION = "1";
process.env.GCP_PROJECT_ID = "exa-dev";
process.env.GOOGLE_APPLICATION_CREDENTIALS = "path/to/credentials.json";
process.env.INTERCOM_IDENTITY_KEY = "intercom";
process.env.ISSUER_PRIVATE_KEY = padHex("0x420");
process.env.KEEPER_PRIVATE_KEY = padHex("0x420");
Expand Down
100 changes: 99 additions & 1 deletion server/test/hooks/persona.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import "../mocks/deployments";
import "../mocks/pax";
import "../mocks/persona";
import "../mocks/sentry";
Expand All @@ -13,6 +14,7 @@ import deriveAddress from "@exactly/common/deriveAddress";

import database, { credentials } from "../../database";
import app from "../../hooks/persona";
import { kms } from "../../utils/gcp";
import * as panda from "../../utils/panda";
import * as pax from "../../utils/pax";
import * as persona from "../../utils/persona";
Expand All @@ -21,6 +23,7 @@ import * as sardine from "../../utils/sardine";
const appClient = testClient(app);

vi.mock("@sentry/node", { spy: true });
vi.mock("../../utils/gcp", () => ({ kms: vi.fn(() => Promise.resolve({ exaSend: vi.fn() })) }));

describe("with reference", () => {
const referenceId = "hook-persona";
Expand Down Expand Up @@ -384,7 +387,10 @@ describe("persona hook", () => {
});
});

afterEach(() => vi.resetAllMocks());
afterEach(async () => {
vi.resetAllMocks();
await database.update(credentials).set({ pandaId: null }).where(eq(credentials.id, "persona-ref"));
});

it("creates panda and pax user on valid inquiry", async () => {
vi.spyOn(panda, "createUser").mockResolvedValue({ id: "new-panda-id" });
Expand Down Expand Up @@ -431,6 +437,98 @@ describe("persona hook", () => {
product: "travel insurance",
});
});

it("allows account on firewall after kyc approval", async () => {
const mockExaSend = vi.fn();
vi.mocked(kms).mockResolvedValueOnce({ exaSend: mockExaSend });
vi.spyOn(panda, "createUser").mockResolvedValue({ id: "new-panda-id" });
vi.spyOn(pax, "addCapita").mockResolvedValue({});
vi.spyOn(sardine, "customer").mockResolvedValueOnce({ sessionKey: "test", status: "Success", level: "low" });

const response = await appClient.index.$post({
header: { "persona-signature": "t=1,v1=sha256" },
json: {
...validPayload,
data: {
...validPayload.data,
attributes: {
...validPayload.data.attributes,
payload: {
...validPayload.data.attributes.payload,
included: [...validPayload.data.attributes.payload.included],
},
},
},
},
});

expect(response.status).toBe(200);
expect(kms).toHaveBeenCalledWith("allower");
await vi.waitFor(() => expect(mockExaSend).toHaveBeenCalled());
const call = mockExaSend.mock.calls[0] as unknown[];
expect(call[1]).toHaveProperty("functionName", "allow");
expect(call[1]).toHaveProperty("address", inject("Firewall"));
});

it("captures allower init failure without blocking panda creation", async () => {
vi.mocked(kms).mockRejectedValueOnce(new Error("kms unavailable"));
vi.spyOn(panda, "createUser").mockResolvedValue({ id: "new-panda-id" });
vi.spyOn(pax, "addCapita").mockResolvedValue({});
vi.spyOn(sardine, "customer").mockResolvedValueOnce({ sessionKey: "test", status: "Success", level: "low" });

const response = await appClient.index.$post({
header: { "persona-signature": "t=1,v1=sha256" },
json: {
...validPayload,
data: {
...validPayload.data,
attributes: {
...validPayload.data.attributes,
payload: {
...validPayload.data.attributes.payload,
included: [...validPayload.data.attributes.payload.included],
},
},
},
},
});

expect(response.status).toBe(200);
await expect(response.json()).resolves.toStrictEqual({ id: "new-panda-id" });
expect(panda.createUser).toHaveBeenCalled();
await vi.waitFor(() =>
expect(captureException).toHaveBeenCalledWith(new Error("kms unavailable"), { level: "error" }),
);
});

it("captures exaSend failure without blocking panda creation", async () => {
vi.mocked(kms).mockResolvedValueOnce({ exaSend: vi.fn().mockRejectedValueOnce(new Error("revert")) });
vi.spyOn(panda, "createUser").mockResolvedValue({ id: "new-panda-id" });
vi.spyOn(pax, "addCapita").mockResolvedValue({});
vi.spyOn(sardine, "customer").mockResolvedValueOnce({ sessionKey: "test", status: "Success", level: "low" });

const response = await appClient.index.$post({
header: { "persona-signature": "t=1,v1=sha256" },
json: {
...validPayload,
data: {
...validPayload.data,
attributes: {
...validPayload.data.attributes,
payload: {
...validPayload.data.attributes.payload,
included: [...validPayload.data.attributes.payload.included],
},
},
},
},
});

expect(response.status).toBe(200);
await expect(response.json()).resolves.toStrictEqual({ id: "new-panda-id" });
expect(panda.createUser).toHaveBeenCalled();
await vi.waitFor(() => expect(captureException).toHaveBeenCalledWith(new Error("revert"), { level: "error" }));
});
});

describe("manteca template", () => {
Expand Down
43 changes: 43 additions & 0 deletions server/test/utils/gcp.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { access, writeFile } from "node:fs/promises";
import { beforeEach, describe, expect, it, vi } from "vitest";

vi.mock("node:fs/promises", () => ({
writeFile: vi.fn(),
access: vi.fn(),
}));

vi.mock("@google-cloud/kms", () => ({ KeyManagementServiceClient: vi.fn() }));
vi.mock("@valora/viem-account-hsm-gcp", () => ({ gcpHsmToAccount: vi.fn().mockResolvedValue({}) }));

const mockWriteFile = vi.mocked(writeFile);
const mockAccess = vi.mocked(access);

describe("kms", () => {
beforeEach(() => {
vi.clearAllMocks();
vi.resetModules();
mockAccess.mockRejectedValue(new Error("not found"));
});

it("writes credentials with secure permissions", async () => {
const { kms } = await import("../../utils/gcp");
await kms("allower");

expect(mockWriteFile).toHaveBeenCalledWith("/tmp/gcp-service-account.json", expect.any(String), { mode: 0o600 });
});

it("skips writing when credentials already exist", async () => {
mockAccess.mockResolvedValue();
const { kms } = await import("../../utils/gcp");
await kms("allower");

expect(mockWriteFile).not.toHaveBeenCalled();
});

it("caches credentials across calls", async () => {
const { kms } = await import("../../utils/gcp");
await Promise.all([kms("allower"), kms("allower")]);

expect(mockWriteFile).toHaveBeenCalledTimes(1);
});
});
67 changes: 67 additions & 0 deletions server/utils/gcp.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { KeyManagementServiceClient } from "@google-cloud/kms";
import { gcpHsmToAccount } from "@valora/viem-account-hsm-gcp";
import { access, writeFile } from "node:fs/promises";
import { parse } from "valibot";
import { createWalletClient, http } from "viem";

import alchemyAPIKey from "@exactly/common/alchemyAPIKey";
import chain from "@exactly/common/generated/chain";

import { extender } from "./keeper";
import nonceManager from "./nonceManager";
import { captureRequests, Requests } from "./publicClient";

const DECODE_DEPTH = 3;
const CREDENTIALS_PATH = "/tmp/gcp-service-account.json";

if (!process.env.GCP_BASE64_JSON) throw new Error("missing gcp base64 json");
const encoded = process.env.GCP_BASE64_JSON;

if (!process.env.GCP_PROJECT_ID) throw new Error("missing gcp project id");
if (!process.env.GCP_KMS_KEY_RING) throw new Error("missing gcp kms key ring");
if (!process.env.GCP_KMS_KEY_VERSION) throw new Error("missing gcp kms key version");
const projectId = process.env.GCP_PROJECT_ID;
const keyRing = process.env.GCP_KMS_KEY_RING;
const version = process.env.GCP_KMS_KEY_VERSION;

let pending: null | Promise<string> = null;

function setupCredentials() {
return (pending ??= (async () => {
const exists = await access(CREDENTIALS_PATH)
.then(() => true)
.catch(() => false);
if (!exists) {
let json = encoded;
for (let index = 0; index < DECODE_DEPTH; index++) {
json = Buffer.from(json, "base64").toString("utf8");
}
await writeFile(CREDENTIALS_PATH, json, { mode: 0o600 });
}
return CREDENTIALS_PATH;
})().catch((error: unknown) => {
pending = null;
throw error;
}));
}

// eslint-disable-next-line import/prefer-default-export
export async function kms(key: string) {
const account = await gcpHsmToAccount({
hsmKeyVersion: `projects/${projectId}/locations/us-west2/keyRings/${keyRing}/cryptoKeys/${key}/cryptoKeyVersions/${version}`,
kmsClient: new KeyManagementServiceClient({ keyFilename: await setupCredentials() }),
});
account.nonceManager = nonceManager;
return extender(
createWalletClient({
chain,
transport: http(`${chain.rpcUrls.alchemy.http[0]}/${alchemyAPIKey}`, {
batch: true,
async onFetchRequest(request) {
captureRequests(parse(Requests, await request.json()));
},
}),
account,
}),
);
}
Comment on lines +49 to +67
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 kms() creates a new KMS client and fetches public key on every invocation — no caching

Every call to kms() at server/utils/gcp.ts:49-67 creates a brand new KeyManagementServiceClient (establishing a gRPC connection) and calls gcpHsmToAccount (which makes a KMS API call to retrieve the public key). Since kms("allower") is called on every KYC approval in server/hooks/persona.ts:309, this means each approval triggers:

  • A new gRPC connection to GCP KMS
  • An API call to fetch the same immutable public key
  • A new WalletClient with a new HTTP transport

The public key for a given KMS key version never changes, so the resolved account and client could be cached (e.g., in a Map<string, Promise<...>>). Under load, the current implementation could exhaust GCP KMS API quotas and add unnecessary latency to the firewall allow operation.

Prompt for agents
The kms() function in server/utils/gcp.ts creates a new KeyManagementServiceClient, calls gcpHsmToAccount (an API call to fetch the public key), and creates a new WalletClient on every invocation. Since the public key for a given KMS key version is immutable, the result can be cached.

A simple approach: maintain a module-level Map<string, Promise<ReturnType<typeof extender>>> keyed by the key name (e.g. 'allower'). On first call for a given key, store the promise. On subsequent calls, return the cached promise. Add error handling to clear the cache entry if the promise rejects (similar to the pending pattern already used in setupCredentials()).

This avoids repeated gRPC connection setup and KMS API calls for the same key.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

4 changes: 2 additions & 2 deletions server/utils/keeper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import {
WaitForTransactionReceiptTimeoutError,
withRetry,
type HttpTransport,
type LocalAccount,
type MaybePromise,
type Prettify,
type PrivateKeyAccount,
type TransactionReceipt,
type WalletClient,
type WriteContractParameters,
Expand Down Expand Up @@ -50,7 +50,7 @@ export default createWalletClient({
),
}).extend(extender);

export function extender(keeper: WalletClient<HttpTransport, typeof chain, PrivateKeyAccount>) {
export function extender(keeper: WalletClient<HttpTransport, typeof chain, LocalAccount>) {
return {
exaSend: async (
spanOptions: Prettify<Omit<Parameters<typeof startSpan>[0], "name" | "op"> & { name: string; op: string }>,
Expand Down
4 changes: 4 additions & 0 deletions server/vitest.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ export default defineConfig({
BRIDGE_API_KEY: "bridge",
BRIDGE_API_URL: "https://bridge.test",
EXPO_PUBLIC_ALCHEMY_API_KEY: " ",
GCP_BASE64_JSON: "WlhsS01HVllRbXhKYW05blNXNU9iR051V25CWk1sWm1XVmRPYW1JelZuVmtRMG81UTJjOVBRbz0K",
GCP_KMS_KEY_RING: "op-sepolia",
GCP_KMS_KEY_VERSION: "1",
GCP_PROJECT_ID: "exa-dev",
INTERCOM_IDENTITY_KEY: "a9cBeTfEtGPSQ58REZP35Bx00ofajvStEc8TTuBtSmk",
ISSUER_PRIVATE_KEY: padHex("0x420"),
MANTECA_API_URL: "https://manteca.test",
Expand Down