From dfc775ff6bc2de95760b7aff0f0c6b06658caaf0 Mon Sep 17 00:00:00 2001 From: Yos Riady Date: Sun, 17 May 2026 07:59:24 +0700 Subject: [PATCH 1/3] fix(security): prevent tag-name command injection in release workflow The publish job runs with id-token:write (npm OIDC trusted publishing) and contents:write, and interpolated the attacker-influenceable tag name (github.ref_name and tag-derived step outputs) directly into run: scripts. Git ref names permit shell metacharacters, so a tag like v1.0.0$(...) could execute arbitrary commands with publish privileges. - Move all tag-derived values to env: blocks, referenced as quoted shell variables instead of ${{ }} interpolation in run: bodies - Add strict semver tag validation that fails the workflow before any untrusted value is used Actions are already pinned to commit SHAs; npm pin intentionally omitted (npm@latest retained by decision). Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/release.yml | 36 +++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 21f5986..643eb47 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -19,13 +19,16 @@ jobs: fetch-depth: 0 # Fetch all history for changelog generation - name: Verify tag is on main branch + env: + COMMIT_SHA: ${{ github.sha }} + REF_NAME: ${{ github.ref_name }} run: | # Check if the tag is reachable from main branch git fetch origin main - if ! git merge-base --is-ancestor ${{ github.sha }} origin/main; then + if ! git merge-base --is-ancestor "$COMMIT_SHA" origin/main; then echo "Error: This tag is not on the main branch" echo "Tags must be created on the main branch to trigger a release" - echo "Current tag: ${{ github.ref_name }}" + echo "Current tag: $REF_NAME" exit 1 fi echo "Tag is on main branch, proceeding with release" @@ -48,14 +51,24 @@ jobs: - name: Extract version from tag id: version + env: + REF_NAME: ${{ github.ref_name }} run: | - VERSION=${GITHUB_REF#refs/tags/v} - echo "version=$VERSION" >> $GITHUB_OUTPUT + # Git ref names can legitimately contain shell metacharacters, and + # this value flows into later steps. Reject anything that is not a + # clean semver release tag before using it anywhere. + if [[ ! "$REF_NAME" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]]; then + echo "Error: Tag '$REF_NAME' is not a valid release tag (expected vX.Y.Z)" + exit 1 + fi + VERSION="${REF_NAME#v}" + echo "version=$VERSION" >> "$GITHUB_OUTPUT" echo "Publishing version: $VERSION" - name: Verify tag matches package.json version + env: + TAG_VERSION: ${{ steps.version.outputs.version }} run: | - TAG_VERSION=${{ steps.version.outputs.version }} PKG_VERSION=$(node -p "require('./package.json').version") if [ "$TAG_VERSION" != "$PKG_VERSION" ]; then @@ -74,10 +87,13 @@ jobs: - name: Generate release notes id: release_notes + env: + CURRENT_TAG: ${{ github.ref_name }} + VERSION: ${{ steps.version.outputs.version }} + REPO: ${{ github.repository }} run: | # Get the previous version tag (second most recent v* tag by version order) # This is more reliable than using git describe which follows commit ancestry - CURRENT_TAG=${{ github.ref_name }} PREV_TAG=$(git tag -l 'v*' --sort=-version:refname | grep -v "^${CURRENT_TAG}$" | head -1) if [ -z "$PREV_TAG" ]; then @@ -88,7 +104,7 @@ jobs: # Get current date RELEASE_DATE=$(date +%Y-%m-%d) - VERSION=${{ steps.version.outputs.version }} + # VERSION is provided via the validated env var declared above # Generate changelog with PR links if [ -n "$PREV_TAG" ]; then @@ -115,11 +131,11 @@ jobs: PR_NUM="${BASH_REMATCH[1]}" # Remove the (#PR_NUM) from message to avoid duplication (handles with or without space) CLEAN_MESSAGE=$(echo "$message" | sed -E 's/ ?\(#[0-9]+\)//') - PR_LINK="[#$PR_NUM](https://github.com/${{ github.repository }}/pull/$PR_NUM)" - COMMIT_LINK="[$hash](https://github.com/${{ github.repository }}/commit/$hash)" + PR_LINK="[#$PR_NUM](https://github.com/$REPO/pull/$PR_NUM)" + COMMIT_LINK="[$hash](https://github.com/$REPO/commit/$hash)" ITEM="$CLEAN_MESSAGE ($PR_LINK) ($COMMIT_LINK)" else - COMMIT_LINK="[$hash](https://github.com/${{ github.repository }}/commit/$hash)" + COMMIT_LINK="[$hash](https://github.com/$REPO/commit/$hash)" ITEM="$message ($COMMIT_LINK)" fi From 631426822202915387665e91226d005d79372125 Mon Sep 17 00:00:00 2001 From: Yos Riady Date: Sun, 17 May 2026 10:18:09 +0700 Subject: [PATCH 2/3] fix(security): stop exfiltrating raw wallet signatures (C1, CRITICAL) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Autocaptured signature events shipped the produced signature (state.data) as `signatureHash` to events.formo.so — a replayable permit/Permit2/SIWE bearer credential, on by default. Scope: remove `signatureHash` only. The signed message (plaintext or EIP-712 struct) is still captured as before; no behavior change there and no new configuration option. - Remove `signatureHash` end-to-end: SignatureAPIEvent, IFormoAnalytics /IFormoAnalyticsInstance signature(), FormoAnalytics.signature(), EventFactory.generateSignatureEvent (+ lib/event/types.ts), and the WagmiEventHandler mutation handler (no longer reads state.data). - Add src/__tests__/signature.test.ts: asserts no signatureHash / no raw-signature value is emitted for signMessage or signTypedData. deepsec revalidate: CRITICAL other-signature-exfiltration -> fixed. typecheck/lint clean, 212/212 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/FormoAnalytics.ts | 3 -- src/__tests__/FormoAnalytics.test.ts | 18 ++++++- src/__tests__/signature.test.ts | 70 ++++++++++++++++++++++++++++ src/lib/event/EventFactory.ts | 3 -- src/lib/event/types.ts | 1 - src/lib/wagmi/WagmiEventHandler.ts | 8 ++-- src/types/base.ts | 1 - src/types/events.ts | 3 +- 8 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 src/__tests__/signature.test.ts diff --git a/src/FormoAnalytics.ts b/src/FormoAnalytics.ts index 6af1ea5..9e7d280 100644 --- a/src/FormoAnalytics.ts +++ b/src/FormoAnalytics.ts @@ -401,13 +401,11 @@ export class FormoAnalytics implements IFormoAnalytics { chainId, address, message, - signatureHash, }: { status: SignatureStatus; chainId?: ChainID; address: Address; message: string; - signatureHash?: string; }, properties?: IFormoEventProperties, context?: IFormoEventContext, @@ -424,7 +422,6 @@ export class FormoAnalytics implements IFormoAnalytics { ...(chainId !== undefined && chainId !== null && { chainId }), address, message, - ...(signatureHash && { signatureHash }), }, properties, context, diff --git a/src/__tests__/FormoAnalytics.test.ts b/src/__tests__/FormoAnalytics.test.ts index e75e06e..c634e7d 100644 --- a/src/__tests__/FormoAnalytics.test.ts +++ b/src/__tests__/FormoAnalytics.test.ts @@ -314,11 +314,27 @@ describe('FormoAnalytics', () => { chainId: 1, address: '0x742d35cc6634c0532925a3b844bc9e7595f3f6d2', message: 'test message', - signatureHash: '0xabc123', }); expect(mockEventManager.addEvent).toHaveBeenCalled(); }); + + it('never forwards a signatureHash to the event pipeline (C1)', async () => { + await analytics.signature({ + status: SignatureStatus.CONFIRMED, + chainId: 1, + address: '0x742d35cc6634c0532925a3b844bc9e7595f3f6d2', + message: 'test message', + // signatureHash was removed from the API; a caller forcing it in + // must never reach the emitted event. + signatureHash: '0x' + 'a'.repeat(130), + } as any); + + expect(mockEventManager.addEvent).toHaveBeenCalled(); + const emitted = mockEventManager.addEvent.mock.calls[0][0]; + expect(emitted).not.toHaveProperty('signatureHash'); + expect(JSON.stringify(emitted)).not.toContain('a'.repeat(130)); + }); }); describe('transaction()', () => { diff --git a/src/__tests__/signature.test.ts b/src/__tests__/signature.test.ts new file mode 100644 index 0000000..9a5c069 --- /dev/null +++ b/src/__tests__/signature.test.ts @@ -0,0 +1,70 @@ +import { WagmiEventHandler } from "../lib/wagmi/WagmiEventHandler"; + +// A realistic 65-byte ECDSA signature (the replayable credential C1 leaked). +const RAW_SIGNATURE = "0x" + "a".repeat(130); + +// Build a WagmiEventHandler with the formo boundary mocked. The wagmi/query +// plumbing is external integration surface (unavoidable mock); the code under +// test — handleSignatureMutation — is exercised for real. +function makeHandler() { + const signature = jest.fn().mockResolvedValue(undefined); + const formo = { + connect: jest.fn(), + disconnect: jest.fn(), + chain: jest.fn(), + signature, + transaction: jest.fn(), + isAutocaptureEnabled: jest.fn(() => true), + }; + const wagmiConfig = { subscribe: () => () => {} }; + const handler = new WagmiEventHandler(formo as any, wagmiConfig as any); + // Connection state is established elsewhere; inject it directly so we can + // exercise the signature path in isolation. + (handler as any).trackingState = { + isProcessing: false, + lastChainId: 1, + lastAddress: "0x742d35cc6634c0532925a3b844bc9e7595f3f6d2", + }; + const fire = (mutationType: "signMessage" | "signTypedData", state: any) => + (handler as any).handleSignatureMutation(mutationType, { state }); + return { signature, fire }; +} + +const flat = (obj: unknown) => JSON.stringify(obj); + +describe("C1: signature autocapture must never emit the raw signature", () => { + it("signMessage: no signatureHash, no raw signature value", () => { + const { signature, fire } = makeHandler(); + + fire("signMessage", { + status: "success", + data: RAW_SIGNATURE, + variables: { message: "hello" }, + }); + + expect(signature).toHaveBeenCalledTimes(1); + const arg = signature.mock.calls[0][0]; + expect(arg).not.toHaveProperty("signatureHash"); + expect(flat(arg)).not.toContain(RAW_SIGNATURE); + }); + + it("signTypedData: no signatureHash, no raw signature value", () => { + const { signature, fire } = makeHandler(); + const permit = { + domain: { name: "USD Coin", chainId: 1 }, + primaryType: "Permit", + types: { Permit: [{ name: "owner", type: "address" }] }, + message: { owner: "0xVictim", spender: "0xRouter", value: "1", deadline: 9 }, + }; + + fire("signTypedData", { + status: "success", + data: RAW_SIGNATURE, + variables: permit, + }); + + const arg = signature.mock.calls[0][0]; + expect(arg).not.toHaveProperty("signatureHash"); + expect(flat(arg)).not.toContain(RAW_SIGNATURE); + }); +}); diff --git a/src/lib/event/EventFactory.ts b/src/lib/event/EventFactory.ts index 5109c5b..142c4a2 100644 --- a/src/lib/event/EventFactory.ts +++ b/src/lib/event/EventFactory.ts @@ -499,7 +499,6 @@ class EventFactory implements IEventFactory { chainId: ChainID | undefined, address: Address, message: string, - signatureHash?: string, properties?: IFormoEventProperties, context?: IFormoEventContext ): Promise { @@ -508,7 +507,6 @@ class EventFactory implements IEventFactory { status, ...(chainId !== undefined && chainId !== null && { chainId }), message, - ...(signatureHash && { signatureHash }), ...properties, }, address, @@ -646,7 +644,6 @@ class EventFactory implements IEventFactory { event.chainId, event.address, event.message, - event.signatureHash, event.properties, event.context ); diff --git a/src/lib/event/types.ts b/src/lib/event/types.ts index c4c07bf..b37c794 100644 --- a/src/lib/event/types.ts +++ b/src/lib/event/types.ts @@ -66,7 +66,6 @@ export interface IEventFactory { chainId: ChainID | undefined, address: Address, message: string, - signatureHash?: string, properties?: IFormoEventProperties, context?: IFormoEventContext ): Promise; diff --git a/src/lib/wagmi/WagmiEventHandler.ts b/src/lib/wagmi/WagmiEventHandler.ts index 5ce8f41..53dbc80 100644 --- a/src/lib/wagmi/WagmiEventHandler.ts +++ b/src/lib/wagmi/WagmiEventHandler.ts @@ -33,7 +33,6 @@ interface IFormoAnalyticsInstance { chainId: number; address: string; message: string; - signatureHash?: string; }): Promise; transaction(params: { status: TransactionStatus; @@ -362,19 +361,21 @@ export class WagmiEventHandler { try { let status: SignatureStatus; - let signatureHash: string | undefined; if (state.status === "pending") { status = SignatureStatus.REQUESTED; } else if (state.status === "success") { status = SignatureStatus.CONFIRMED; - signatureHash = state.data as string; } else if (state.status === "error") { status = SignatureStatus.REJECTED; } else { return; } + // C1: never capture the produced signature (`state.data`) — it is a + // replayable permit/Permit2/SIWE bearer credential. The signed + // message itself is still captured (status alone is insufficient + // for the analytics use case). let message: string; if (mutationType === "signMessage") { message = (variables.message as string) || ""; @@ -394,7 +395,6 @@ export class WagmiEventHandler { chainId, address, message, - ...(signatureHash && { signatureHash }), }).catch((error) => { logger.error("WagmiEventHandler: Error tracking signature:", error); }); diff --git a/src/types/base.ts b/src/types/base.ts index 1d76d8f..dcea78c 100644 --- a/src/types/base.ts +++ b/src/types/base.ts @@ -56,7 +56,6 @@ export interface IFormoAnalytics { chainId?: ChainID; address: Address; message: string; - signatureHash?: string; }, properties?: IFormoEventProperties, context?: IFormoEventContext, diff --git a/src/types/events.ts b/src/types/events.ts index ccbd70c..b53e859 100644 --- a/src/types/events.ts +++ b/src/types/events.ts @@ -84,8 +84,9 @@ export interface SignatureAPIEvent { status: SignatureStatus; chainId?: ChainID; address: Address; + // The signed message (plaintext for signMessage, JSON EIP-712 struct + // for signTypedData). The produced signature is never captured (C1). message: string; - signatureHash?: string; } export interface ConnectAPIEvent { From 1e667af5d5189eb6fe51e2d79fae93bbbe456ef5 Mon Sep 17 00:00:00 2001 From: Yos Riady Date: Sun, 17 May 2026 12:51:32 +0700 Subject: [PATCH 3/3] chore: drop internal C1 audit-id references from comments and tests Comment/label-only cleanup, no behavior change. Gates green (212/212). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/__tests__/FormoAnalytics.test.ts | 2 +- src/__tests__/signature.test.ts | 4 ++-- src/lib/wagmi/WagmiEventHandler.ts | 4 ---- src/types/events.ts | 2 -- 4 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/__tests__/FormoAnalytics.test.ts b/src/__tests__/FormoAnalytics.test.ts index c634e7d..707aa26 100644 --- a/src/__tests__/FormoAnalytics.test.ts +++ b/src/__tests__/FormoAnalytics.test.ts @@ -319,7 +319,7 @@ describe('FormoAnalytics', () => { expect(mockEventManager.addEvent).toHaveBeenCalled(); }); - it('never forwards a signatureHash to the event pipeline (C1)', async () => { + it('never forwards a signatureHash to the event pipeline', async () => { await analytics.signature({ status: SignatureStatus.CONFIRMED, chainId: 1, diff --git a/src/__tests__/signature.test.ts b/src/__tests__/signature.test.ts index 9a5c069..bf0ce4c 100644 --- a/src/__tests__/signature.test.ts +++ b/src/__tests__/signature.test.ts @@ -1,6 +1,6 @@ import { WagmiEventHandler } from "../lib/wagmi/WagmiEventHandler"; -// A realistic 65-byte ECDSA signature (the replayable credential C1 leaked). +// A realistic 65-byte ECDSA signature (the replayable credential we must not leak). const RAW_SIGNATURE = "0x" + "a".repeat(130); // Build a WagmiEventHandler with the formo boundary mocked. The wagmi/query @@ -32,7 +32,7 @@ function makeHandler() { const flat = (obj: unknown) => JSON.stringify(obj); -describe("C1: signature autocapture must never emit the raw signature", () => { +describe("signature autocapture must never emit the raw signature", () => { it("signMessage: no signatureHash, no raw signature value", () => { const { signature, fire } = makeHandler(); diff --git a/src/lib/wagmi/WagmiEventHandler.ts b/src/lib/wagmi/WagmiEventHandler.ts index 53dbc80..8f6ebbc 100644 --- a/src/lib/wagmi/WagmiEventHandler.ts +++ b/src/lib/wagmi/WagmiEventHandler.ts @@ -372,10 +372,6 @@ export class WagmiEventHandler { return; } - // C1: never capture the produced signature (`state.data`) — it is a - // replayable permit/Permit2/SIWE bearer credential. The signed - // message itself is still captured (status alone is insufficient - // for the analytics use case). let message: string; if (mutationType === "signMessage") { message = (variables.message as string) || ""; diff --git a/src/types/events.ts b/src/types/events.ts index b53e859..c11ed1f 100644 --- a/src/types/events.ts +++ b/src/types/events.ts @@ -84,8 +84,6 @@ export interface SignatureAPIEvent { status: SignatureStatus; chainId?: ChainID; address: Address; - // The signed message (plaintext for signMessage, JSON EIP-712 struct - // for signTypedData). The produced signature is never captured (C1). message: string; }