Skip to content

fix(zohomail): validate handshake signature before persisting secret #616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rudra2637 wants to merge 2 commits into
base: main
Choose a base branch
from
+191 −6
Open

fix(zohomail): validate handshake signature before persisting secret #616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d804426
fix(zohomail): validate handshake signature before persisting secret0
Rudra2637 Aug 6, 2026
01d666d
fix(zohomail): treat repeat handshake with identical secret as no-op ACK
Rudra2637 Aug 9, 2026
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
139 changes: 138 additions & 1 deletion packages/zohomail/webhooks.integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ describe('Zoho Mail webhook — full bound pipeline', () => {
testDb.cleanup();
});

it('rejects handshake when signature does not match the secret', async () => {
it('rejects handshake when signature does not match the secret and does not persist', async () => {
const { corsair, testDb } = await buildCorsair({
webhookSecret: undefined,
});
Expand All @@ -188,6 +188,143 @@ describe('Zoho Mail webhook — full bound pipeline', () => {
rawBody,
});
expect(response.success).toBe(false);
expect(await corsair.zohomail.keys.get_webhook_signature()).toBeNull();

testDb.cleanup();
});

it('cannot overwrite existing secret with a bare x-hook-secret POST (missing signature)', async () => {
const { corsair, testDb } = await buildCorsair({
webhookSecret: undefined,
});
const handshake = corsair.zohomail.webhooks.challenge.handshake;

// 1. Establish first-time secret
const initialSecret = 'initial-secret';
await handshake.handler({
payload: {},
headers: { 'x-hook-secret': initialSecret },
});
expect(await corsair.zohomail.keys.get_webhook_signature()).toBe(
initialSecret,
);

// 2. Attempt to overwrite with bare x-hook-secret request (no signature)
const newSecret = 'new-secret-attempt';
const response = await handshake.handler({
payload: {},
headers: { 'x-hook-secret': newSecret },
});

expect(response.success).toBe(false);
expect(response.error).toMatch(/Cannot overwrite existing secret/i);

// 3. Verify the secret remains the initial one
expect(await corsair.zohomail.keys.get_webhook_signature()).toBe(
initialSecret,
);

testDb.cleanup();
});

it('cannot overwrite existing secret with an invalid signature', async () => {
const { corsair, testDb } = await buildCorsair({
webhookSecret: undefined,
});
const handshake = corsair.zohomail.webhooks.challenge.handshake;

// 1. Establish first-time secret
const initialSecret = 'initial-secret';
await handshake.handler({
payload: {},
headers: { 'x-hook-secret': initialSecret },
});
expect(await corsair.zohomail.keys.get_webhook_signature()).toBe(
initialSecret,
);

// 2. Attempt overwrite with new secret but signed with wrong/new secret instead of proving the old one
const newSecret = 'new-secret-attempt';
const rawBody = eventBody();
const response = await handshake.handler({
payload: JSON.parse(rawBody),
headers: {
'x-hook-secret': newSecret,
'x-hook-signature': sign(rawBody, 'wrong-secret'),
},
rawBody,
});

expect(response.success).toBe(false);
expect(await corsair.zohomail.keys.get_webhook_signature()).toBe(
initialSecret,
);

testDb.cleanup();
});

it('can overwrite/rotate existing secret with a valid signature matched against the old secret', async () => {
const { corsair, testDb } = await buildCorsair({
webhookSecret: undefined,
});
const handshake = corsair.zohomail.webhooks.challenge.handshake;

// 1. Establish first-time secret
const initialSecret = 'initial-secret';
await handshake.handler({
payload: {},
headers: { 'x-hook-secret': initialSecret },
});
expect(await corsair.zohomail.keys.get_webhook_signature()).toBe(
initialSecret,
);

// 2. Legitimate rotation: sign request with the old/existing secret
const newSecret = 'rotated-secret';
const rawBody = eventBody();
const response = await handshake.handler({
payload: JSON.parse(rawBody),
headers: {
'x-hook-secret': newSecret,
'x-hook-signature': sign(rawBody, initialSecret), // signs with old secret
},
rawBody,
});

expect(response.success).toBe(true);
expect(await corsair.zohomail.keys.get_webhook_signature()).toBe(newSecret);

testDb.cleanup();
});

it('treats repeat handshake with the same secret as a no-op ACK', async () => {
const { corsair, testDb } = await buildCorsair({
webhookSecret: undefined,
});
const handshake = corsair.zohomail.webhooks.challenge.handshake;

// 1. Establish first-time secret
const initialSecret = 'initial-secret';
const firstResponse = await handshake.handler({
payload: {},
headers: { 'x-hook-secret': initialSecret },
});
expect(firstResponse.success).toBe(true);
expect(await corsair.zohomail.keys.get_webhook_signature()).toBe(
initialSecret,
);

// 2. Retry handshake with the EXACT same secret (no signature)
const retryResponse = await handshake.handler({
payload: {},
headers: { 'x-hook-secret': initialSecret },
});

expect(retryResponse.success).toBe(true);
expect(retryResponse.data?.hookSecret).toBe(initialSecret);
expect(await corsair.zohomail.keys.get_webhook_signature()).toBe(
initialSecret,
);
Comment on lines +317 to +327

@coderabbitai coderabbitai Bot Aug 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C 4 'jest\.spyOn|set_webhook_signature' packages/zohomail --glob '*.{test,spec}.ts'

Repository: corsairdev/corsair

Length of output: 156

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- matching files ---'
git ls-files 'packages/zohomail/*' | sed -n '1,120p'

printf '%s\n' '--- relevant symbols ---'
rg -n -C 8 'set_webhook_signature|get_webhook_signature|handshake' packages/zohomail --glob '*.{ts,tsx,js,jsx}' || true

printf '%s\n' '--- test context ---'
sed -n '260,350p' packages/zohomail/webhooks.integration.test.ts

Repository: corsairdev/corsair

Length of output: 38802

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- handshake implementation ---'
sed -n '1,125p' packages/zohomail/webhooks/challenge.ts

printf '%s\n' '--- key method definitions and usages ---'
rg -n -C 10 'set_webhook_signature' packages --glob '*.{ts,tsx,js,jsx}' || true

printf '%s\n' '--- test setup and Jest configuration ---'
sed -n '1,120p' packages/zohomail/webhooks.integration.test.ts
cat packages/zohomail/jest.config.cjs

Repository: corsairdev/corsair

Length of output: 31373

Assert that a repeat handshake does not persist the secret.

Spy on corsair.zohomail.keys.set_webhook_signature after the first handshake and before the retry. Assert that the retry makes no calls.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/zohomail/webhooks.integration.test.ts` around lines 317 - 327,
Update the retry-handshake test around handshake.handler to spy on
corsair.zohomail.keys.set_webhook_signature after the initial handshake and
before the retry, then assert it receives no calls after the repeat handshake
while preserving the existing response and secret assertions.


testDb.cleanup();
});
Expand Down
58 changes: 53 additions & 5 deletions packages/zohomail/webhooks/challenge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,38 @@ export const handshake: ZohoMailWebhooks['handshake'] = {
};
}

let existingSecret: string | undefined;
try {
await ctx.keys.set_webhook_signature(hookSecret);
existingSecret = (await ctx.keys.get_webhook_signature()) ?? undefined;
Comment on lines +30 to +32

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the key-store contract and look for conditional-write support.
ast-grep outline packages/zohomail/webhooks/challenge.ts --items all
rg -n -C 12 '\b(get_webhook_signature|set_webhook_signature)\b' packages
rg -n -C 8 '\b(compareAndSet|compare_and_set|createIfAbsent|create_if_absent|transaction|serializ|mutex|lock|version)\b' packages

Repository: corsairdev/corsair

Length of output: 50375

Authorization Bypass (CWE-367): Time-of-check Time-of-use (TOCTOU) Race Condition

Reachability: External · Exploitability: Moderate

Make first-time webhook-secret setup atomic.

When two requests read an empty secret before either write, an unsigned request can overwrite the legitimate secret. Use a create-if-absent or compare-and-set write and add concurrent setup coverage.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/zohomail/webhooks/challenge.ts` around lines 30 - 32, Update the
webhook-secret initialization flow around ctx.keys.get_webhook_signature so
first-time setup uses an atomic create-if-absent or compare-and-set write,
preventing concurrent unsigned requests from overwriting an established secret.
Preserve normal reuse of an existing secret and add coverage for concurrent
initialization attempts.

@greptile-apps greptile-apps Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 security Non-atomic secret ownership check

If two first-time handshakes for the same tenant overlap, both can observe that no secret exists and proceed without ownership verification, allowing a later unsigned write to replace the legitimate secret and break or take control of subsequent webhook authentication.

How this was verified: The handler performs independent read and write operations, while the key manager serializes only writes and the webhook path provides no per-tenant lock around the ownership decision.

} catch (error) {
console.warn(
'[corsair:zohomail] Failed to persist webhook secret:',
'[corsair:zohomail] Failed to retrieve existing webhook secret:',
error,
);
return {
success: false,
statusCode: 500,
error: 'Failed to persist webhook secret',
error: 'Failed to retrieve existing webhook secret',
};
}

if (existingSecret === hookSecret) {
return {
success: true,
data: { hookSecret },
};
}

const signature = getZohoWebhookSignature(headers);
if (signature) {

if (existingSecret) {
if (!signature) {
return {
success: false,
statusCode: 401,
error: 'Cannot overwrite existing secret without a valid signature',
};
}
const rawBody = request.rawBody;
if (!rawBody) {
return {
Expand All @@ -51,13 +67,45 @@ export const handshake: ZohoMailWebhooks['handshake'] = {
error: 'Missing raw body for signature verification',
};
}
if (!verifyZohoWebhookSignature(rawBody, hookSecret, signature)) {
if (!verifyZohoWebhookSignature(rawBody, existingSecret, signature)) {
return {
success: false,
statusCode: 401,
error: 'Invalid signature',
};
}
} else {
if (signature) {
const rawBody = request.rawBody;
if (!rawBody) {
return {
success: false,
statusCode: 401,
error: 'Missing raw body for signature verification',
};
}
if (!verifyZohoWebhookSignature(rawBody, hookSecret, signature)) {
return {
success: false,
statusCode: 401,
error: 'Invalid signature',
};
}
}
}

try {
await ctx.keys.set_webhook_signature(hookSecret);
} catch (error) {
console.warn(
'[corsair:zohomail] Failed to persist webhook secret:',
error,
);
return {
success: false,
statusCode: 500,
error: 'Failed to persist webhook secret',
};
}

return {
Expand Down
Loading