Skip to content

Commit 42c2432

Browse files
yosriadyclaude
andauthored
clean up signature events + harden release workflow (#28)
* 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) <noreply@anthropic.com> * fix(security): stop exfiltrating raw wallet signatures (C1, CRITICAL) 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d8c172d commit 42c2432

9 files changed

Lines changed: 113 additions & 24 deletions

File tree

.github/workflows/release.yml

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,16 @@ jobs:
1919
fetch-depth: 0 # Fetch all history for changelog generation
2020

2121
- name: Verify tag is on main branch
22+
env:
23+
COMMIT_SHA: ${{ github.sha }}
24+
REF_NAME: ${{ github.ref_name }}
2225
run: |
2326
# Check if the tag is reachable from main branch
2427
git fetch origin main
25-
if ! git merge-base --is-ancestor ${{ github.sha }} origin/main; then
28+
if ! git merge-base --is-ancestor "$COMMIT_SHA" origin/main; then
2629
echo "Error: This tag is not on the main branch"
2730
echo "Tags must be created on the main branch to trigger a release"
28-
echo "Current tag: ${{ github.ref_name }}"
31+
echo "Current tag: $REF_NAME"
2932
exit 1
3033
fi
3134
echo "Tag is on main branch, proceeding with release"
@@ -48,14 +51,24 @@ jobs:
4851

4952
- name: Extract version from tag
5053
id: version
54+
env:
55+
REF_NAME: ${{ github.ref_name }}
5156
run: |
52-
VERSION=${GITHUB_REF#refs/tags/v}
53-
echo "version=$VERSION" >> $GITHUB_OUTPUT
57+
# Git ref names can legitimately contain shell metacharacters, and
58+
# this value flows into later steps. Reject anything that is not a
59+
# clean semver release tag before using it anywhere.
60+
if [[ ! "$REF_NAME" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]]; then
61+
echo "Error: Tag '$REF_NAME' is not a valid release tag (expected vX.Y.Z)"
62+
exit 1
63+
fi
64+
VERSION="${REF_NAME#v}"
65+
echo "version=$VERSION" >> "$GITHUB_OUTPUT"
5466
echo "Publishing version: $VERSION"
5567
5668
- name: Verify tag matches package.json version
69+
env:
70+
TAG_VERSION: ${{ steps.version.outputs.version }}
5771
run: |
58-
TAG_VERSION=${{ steps.version.outputs.version }}
5972
PKG_VERSION=$(node -p "require('./package.json').version")
6073
6174
if [ "$TAG_VERSION" != "$PKG_VERSION" ]; then
@@ -74,10 +87,13 @@ jobs:
7487

7588
- name: Generate release notes
7689
id: release_notes
90+
env:
91+
CURRENT_TAG: ${{ github.ref_name }}
92+
VERSION: ${{ steps.version.outputs.version }}
93+
REPO: ${{ github.repository }}
7794
run: |
7895
# Get the previous version tag (second most recent v* tag by version order)
7996
# This is more reliable than using git describe which follows commit ancestry
80-
CURRENT_TAG=${{ github.ref_name }}
8197
PREV_TAG=$(git tag -l 'v*' --sort=-version:refname | grep -v "^${CURRENT_TAG}$" | head -1)
8298
8399
if [ -z "$PREV_TAG" ]; then
@@ -88,7 +104,7 @@ jobs:
88104
89105
# Get current date
90106
RELEASE_DATE=$(date +%Y-%m-%d)
91-
VERSION=${{ steps.version.outputs.version }}
107+
# VERSION is provided via the validated env var declared above
92108
93109
# Generate changelog with PR links
94110
if [ -n "$PREV_TAG" ]; then
@@ -115,11 +131,11 @@ jobs:
115131
PR_NUM="${BASH_REMATCH[1]}"
116132
# Remove the (#PR_NUM) from message to avoid duplication (handles with or without space)
117133
CLEAN_MESSAGE=$(echo "$message" | sed -E 's/ ?\(#[0-9]+\)//')
118-
PR_LINK="[#$PR_NUM](https://github.com/${{ github.repository }}/pull/$PR_NUM)"
119-
COMMIT_LINK="[$hash](https://github.com/${{ github.repository }}/commit/$hash)"
134+
PR_LINK="[#$PR_NUM](https://github.com/$REPO/pull/$PR_NUM)"
135+
COMMIT_LINK="[$hash](https://github.com/$REPO/commit/$hash)"
120136
ITEM="$CLEAN_MESSAGE ($PR_LINK) ($COMMIT_LINK)"
121137
else
122-
COMMIT_LINK="[$hash](https://github.com/${{ github.repository }}/commit/$hash)"
138+
COMMIT_LINK="[$hash](https://github.com/$REPO/commit/$hash)"
123139
ITEM="$message ($COMMIT_LINK)"
124140
fi
125141

src/FormoAnalytics.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -401,13 +401,11 @@ export class FormoAnalytics implements IFormoAnalytics {
401401
chainId,
402402
address,
403403
message,
404-
signatureHash,
405404
}: {
406405
status: SignatureStatus;
407406
chainId?: ChainID;
408407
address: Address;
409408
message: string;
410-
signatureHash?: string;
411409
},
412410
properties?: IFormoEventProperties,
413411
context?: IFormoEventContext,
@@ -424,7 +422,6 @@ export class FormoAnalytics implements IFormoAnalytics {
424422
...(chainId !== undefined && chainId !== null && { chainId }),
425423
address,
426424
message,
427-
...(signatureHash && { signatureHash }),
428425
},
429426
properties,
430427
context,

src/__tests__/FormoAnalytics.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,27 @@ describe('FormoAnalytics', () => {
314314
chainId: 1,
315315
address: '0x742d35cc6634c0532925a3b844bc9e7595f3f6d2',
316316
message: 'test message',
317-
signatureHash: '0xabc123',
318317
});
319318

320319
expect(mockEventManager.addEvent).toHaveBeenCalled();
321320
});
321+
322+
it('never forwards a signatureHash to the event pipeline', async () => {
323+
await analytics.signature({
324+
status: SignatureStatus.CONFIRMED,
325+
chainId: 1,
326+
address: '0x742d35cc6634c0532925a3b844bc9e7595f3f6d2',
327+
message: 'test message',
328+
// signatureHash was removed from the API; a caller forcing it in
329+
// must never reach the emitted event.
330+
signatureHash: '0x' + 'a'.repeat(130),
331+
} as any);
332+
333+
expect(mockEventManager.addEvent).toHaveBeenCalled();
334+
const emitted = mockEventManager.addEvent.mock.calls[0][0];
335+
expect(emitted).not.toHaveProperty('signatureHash');
336+
expect(JSON.stringify(emitted)).not.toContain('a'.repeat(130));
337+
});
322338
});
323339

324340
describe('transaction()', () => {

src/__tests__/signature.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { WagmiEventHandler } from "../lib/wagmi/WagmiEventHandler";
2+
3+
// A realistic 65-byte ECDSA signature (the replayable credential we must not leak).
4+
const RAW_SIGNATURE = "0x" + "a".repeat(130);
5+
6+
// Build a WagmiEventHandler with the formo boundary mocked. The wagmi/query
7+
// plumbing is external integration surface (unavoidable mock); the code under
8+
// test — handleSignatureMutation — is exercised for real.
9+
function makeHandler() {
10+
const signature = jest.fn().mockResolvedValue(undefined);
11+
const formo = {
12+
connect: jest.fn(),
13+
disconnect: jest.fn(),
14+
chain: jest.fn(),
15+
signature,
16+
transaction: jest.fn(),
17+
isAutocaptureEnabled: jest.fn(() => true),
18+
};
19+
const wagmiConfig = { subscribe: () => () => {} };
20+
const handler = new WagmiEventHandler(formo as any, wagmiConfig as any);
21+
// Connection state is established elsewhere; inject it directly so we can
22+
// exercise the signature path in isolation.
23+
(handler as any).trackingState = {
24+
isProcessing: false,
25+
lastChainId: 1,
26+
lastAddress: "0x742d35cc6634c0532925a3b844bc9e7595f3f6d2",
27+
};
28+
const fire = (mutationType: "signMessage" | "signTypedData", state: any) =>
29+
(handler as any).handleSignatureMutation(mutationType, { state });
30+
return { signature, fire };
31+
}
32+
33+
const flat = (obj: unknown) => JSON.stringify(obj);
34+
35+
describe("signature autocapture must never emit the raw signature", () => {
36+
it("signMessage: no signatureHash, no raw signature value", () => {
37+
const { signature, fire } = makeHandler();
38+
39+
fire("signMessage", {
40+
status: "success",
41+
data: RAW_SIGNATURE,
42+
variables: { message: "hello" },
43+
});
44+
45+
expect(signature).toHaveBeenCalledTimes(1);
46+
const arg = signature.mock.calls[0][0];
47+
expect(arg).not.toHaveProperty("signatureHash");
48+
expect(flat(arg)).not.toContain(RAW_SIGNATURE);
49+
});
50+
51+
it("signTypedData: no signatureHash, no raw signature value", () => {
52+
const { signature, fire } = makeHandler();
53+
const permit = {
54+
domain: { name: "USD Coin", chainId: 1 },
55+
primaryType: "Permit",
56+
types: { Permit: [{ name: "owner", type: "address" }] },
57+
message: { owner: "0xVictim", spender: "0xRouter", value: "1", deadline: 9 },
58+
};
59+
60+
fire("signTypedData", {
61+
status: "success",
62+
data: RAW_SIGNATURE,
63+
variables: permit,
64+
});
65+
66+
const arg = signature.mock.calls[0][0];
67+
expect(arg).not.toHaveProperty("signatureHash");
68+
expect(flat(arg)).not.toContain(RAW_SIGNATURE);
69+
});
70+
});

src/lib/event/EventFactory.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,6 @@ class EventFactory implements IEventFactory {
499499
chainId: ChainID | undefined,
500500
address: Address,
501501
message: string,
502-
signatureHash?: string,
503502
properties?: IFormoEventProperties,
504503
context?: IFormoEventContext
505504
): Promise<IFormoEvent> {
@@ -508,7 +507,6 @@ class EventFactory implements IEventFactory {
508507
status,
509508
...(chainId !== undefined && chainId !== null && { chainId }),
510509
message,
511-
...(signatureHash && { signatureHash }),
512510
...properties,
513511
},
514512
address,
@@ -646,7 +644,6 @@ class EventFactory implements IEventFactory {
646644
event.chainId,
647645
event.address,
648646
event.message,
649-
event.signatureHash,
650647
event.properties,
651648
event.context
652649
);

src/lib/event/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ export interface IEventFactory {
6666
chainId: ChainID | undefined,
6767
address: Address,
6868
message: string,
69-
signatureHash?: string,
7069
properties?: IFormoEventProperties,
7170
context?: IFormoEventContext
7271
): Promise<IFormoEvent>;

src/lib/wagmi/WagmiEventHandler.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ interface IFormoAnalyticsInstance {
3333
chainId: number;
3434
address: string;
3535
message: string;
36-
signatureHash?: string;
3736
}): Promise<void>;
3837
transaction(params: {
3938
status: TransactionStatus;
@@ -362,13 +361,11 @@ export class WagmiEventHandler {
362361

363362
try {
364363
let status: SignatureStatus;
365-
let signatureHash: string | undefined;
366364

367365
if (state.status === "pending") {
368366
status = SignatureStatus.REQUESTED;
369367
} else if (state.status === "success") {
370368
status = SignatureStatus.CONFIRMED;
371-
signatureHash = state.data as string;
372369
} else if (state.status === "error") {
373370
status = SignatureStatus.REJECTED;
374371
} else {
@@ -394,7 +391,6 @@ export class WagmiEventHandler {
394391
chainId,
395392
address,
396393
message,
397-
...(signatureHash && { signatureHash }),
398394
}).catch((error) => {
399395
logger.error("WagmiEventHandler: Error tracking signature:", error);
400396
});

src/types/base.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ export interface IFormoAnalytics {
5656
chainId?: ChainID;
5757
address: Address;
5858
message: string;
59-
signatureHash?: string;
6059
},
6160
properties?: IFormoEventProperties,
6261
context?: IFormoEventContext,

src/types/events.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ export interface SignatureAPIEvent {
8585
chainId?: ChainID;
8686
address: Address;
8787
message: string;
88-
signatureHash?: string;
8988
}
9089

9190
export interface ConnectAPIEvent {

0 commit comments

Comments
 (0)