From 12bfd7b20a072cec925c02de7dd9b326b350a814 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Tue, 12 May 2026 12:29:32 -0400 Subject: [PATCH] Fix review findings from PR #28 - Add length check before timingSafeEqual to prevent RangeError on malformed signatures (returns 401 instead of 500) - Check event.type === reaction_added to prevent reaction_removed from advancing expenses - Move getPermalink call behind isOrigin check to skip unnecessary API call on non-origin stage transitions --- src/expenseProcessor.js | 22 +++++++++++----------- src/expenseReceiver.js | 8 ++++---- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/expenseProcessor.js b/src/expenseProcessor.js index 0c852d8..6fa2f08 100644 --- a/src/expenseProcessor.js +++ b/src/expenseProcessor.js @@ -51,8 +51,8 @@ async function slackPost(method, token, body) { } export const handler = async (event) => { - if (event.reaction !== "white_check_mark") { - console.log("Not white_check_mark reaction, skipping"); + if (event.type !== "reaction_added" || event.reaction !== "white_check_mark") { + console.log(`Skipping event type=${event.type} reaction=${event.reaction}`); return; } @@ -78,21 +78,21 @@ export const handler = async (event) => { }); const originalText = history.messages[0].text; - const permalinkResp = await slackGet("chat.getPermalink", token, { - channel: fromChannel, - message_ts: messageTs, - }); - const permalink = permalinkResp.permalink; - const text = originalText.replace(REACT_HINT_RE, "").trim(); const nextStage = STAGES[toChannel]; const nextLabel = nextStage ? nextStage.label : null; const reactLine = nextLabel ? `\n\n_React_ :white_check_mark: _to advance to ${nextLabel}_` : ""; - const permalinkLine = isOrigin - ? `\n\n:paperclip: *Original Submission:* <${permalink}|View Original Message>` - : ""; + + let permalinkLine = ""; + if (isOrigin) { + const permalinkResp = await slackGet("chat.getPermalink", token, { + channel: fromChannel, + message_ts: messageTs, + }); + permalinkLine = `\n\n:paperclip: *Original Submission:* <${permalinkResp.permalink}|View Original Message>`; + } const fullText = `${text}${permalinkLine}${reactLine}`; await slackPost("chat.postMessage", token, { diff --git a/src/expenseReceiver.js b/src/expenseReceiver.js index e81cf59..59450fe 100644 --- a/src/expenseReceiver.js +++ b/src/expenseReceiver.js @@ -31,10 +31,10 @@ function verifySignature(body, timestamp, signature, secret) { const expected = "v0=" + crypto.createHmac("sha256", secret).update(base).digest("hex"); - return crypto.timingSafeEqual( - Buffer.from(expected), - Buffer.from(signature) - ); + const expectedBuf = Buffer.from(expected); + const signatureBuf = Buffer.from(signature); + if (expectedBuf.length !== signatureBuf.length) return false; + return crypto.timingSafeEqual(expectedBuf, signatureBuf); } export const handler = async (event) => {