chore: move to chat sdk#2115
Open
thibaultleouay wants to merge 1 commit intomainfrom
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
thibaultleouay
commented
Apr 23, 2026
| return c.json({ ok: true }); | ||
| } | ||
|
|
||
| async function processEvent(body: SlackEvent) { |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 28 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/server/src/routes/slack/actions.ts">
<violation number="1" location="apps/server/src/routes/slack/actions.ts:362">
P2: The cancel action no longer verifies the requesting user matches the pending action owner. That allows any user to cancel another user’s pending action.</violation>
</file>
<file name="apps/server/src/routes/slack/confirmation-store.ts">
<violation number="1" location="apps/server/src/routes/slack/confirmation-store.ts:149">
P2: Refresh the thread mapping TTL when updating the action to keep `findByThread` working for the full TTL window.</violation>
</file>
<file name="apps/server/package.json">
<violation number="1" location="apps/server/package.json:18">
P2: Pin this dependency to an exact version instead of a caret range to align with repo versioning conventions.
(Based on your team's feedback about pinning dependency versions instead of using ranges.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/server/src/routes/slack/index.ts">
<violation number="1" location="apps/server/src/routes/slack/index.ts:129">
P0: The `/webhooks` handler processes `app_uninstalled` and `tokens_revoked` events and deletes database records without verifying the Slack request signature. The old code had `verifySlackSignature` middleware; the new code removes it entirely. An attacker can forge a webhook to delete any workspace's Slack integration.
Verify the request signature (using `SLACK_SIGNING_SECRET` and the `X-Slack-Signature` / `X-Slack-Request-Timestamp` headers) before acting on the parsed body, or move this cleanup logic after the SDK's own verification step.</violation>
<violation number="2" location="apps/server/src/routes/slack/index.ts:161">
P1: The `slackAdapter` is created without `signingSecret` — only `clientId` and `clientSecret` are passed. Without the signing secret, `bot.webhooks.slack()` cannot verify Slack's request signatures. Pass `signingSecret: env.SLACK_SIGNING_SECRET` to `createSlackAdapter` so the SDK can authenticate incoming webhooks.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| }); | ||
|
|
||
| slack.post("/webhooks", async (c) => { |
Contributor
There was a problem hiding this comment.
P0: The /webhooks handler processes app_uninstalled and tokens_revoked events and deletes database records without verifying the Slack request signature. The old code had verifySlackSignature middleware; the new code removes it entirely. An attacker can forge a webhook to delete any workspace's Slack integration.
Verify the request signature (using SLACK_SIGNING_SECRET and the X-Slack-Signature / X-Slack-Request-Timestamp headers) before acting on the parsed body, or move this cleanup logic after the SDK's own verification step.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/src/routes/slack/index.ts, line 129:
<comment>The `/webhooks` handler processes `app_uninstalled` and `tokens_revoked` events and deletes database records without verifying the Slack request signature. The old code had `verifySlackSignature` middleware; the new code removes it entirely. An attacker can forge a webhook to delete any workspace's Slack integration.
Verify the request signature (using `SLACK_SIGNING_SECRET` and the `X-Slack-Signature` / `X-Slack-Request-Timestamp` headers) before acting on the parsed body, or move this cleanup logic after the SDK's own verification step.</comment>
<file context>
@@ -1,30 +1,165 @@
+ }
+});
+
+slack.post("/webhooks", async (c) => {
+ const cloned = c.req.raw.clone();
+ try {
</file context>
| // body parse failed — let the SDK handle it | ||
| } | ||
|
|
||
| const response = await bot.webhooks.slack(c.req.raw); |
Contributor
There was a problem hiding this comment.
P1: The slackAdapter is created without signingSecret — only clientId and clientSecret are passed. Without the signing secret, bot.webhooks.slack() cannot verify Slack's request signatures. Pass signingSecret: env.SLACK_SIGNING_SECRET to createSlackAdapter so the SDK can authenticate incoming webhooks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/src/routes/slack/index.ts, line 161:
<comment>The `slackAdapter` is created without `signingSecret` — only `clientId` and `clientSecret` are passed. Without the signing secret, `bot.webhooks.slack()` cannot verify Slack's request signatures. Pass `signingSecret: env.SLACK_SIGNING_SECRET` to `createSlackAdapter` so the SDK can authenticate incoming webhooks.</comment>
<file context>
@@ -1,30 +1,165 @@
+ // body parse failed — let the SDK handle it
+ }
+
+ const response = await bot.webhooks.slack(c.req.raw);
+ return response;
+});
</file context>
Comment on lines
+362
to
+368
| bot.onAction("cancel", async (event) => { | ||
| const actionId = event.value; | ||
| if (!actionId) return; | ||
| const consumed = await consume(actionId); | ||
| if (!consumed) return; | ||
| await editMessage(event.adapter, event.threadId, event.messageId, ":no_entry_sign: Cancelled."); | ||
| }); |
Contributor
There was a problem hiding this comment.
P2: The cancel action no longer verifies the requesting user matches the pending action owner. That allows any user to cancel another user’s pending action.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/src/routes/slack/actions.ts, line 362:
<comment>The cancel action no longer verifies the requesting user matches the pending action owner. That allows any user to cancel another user’s pending action.</comment>
<file context>
@@ -485,13 +351,19 @@ async function executeAction(
+export function registerActionHandlers() {
+ bot.onAction("approve", (event) => handleApproval(event, false));
+ bot.onAction("approve_notify", (event) => handleApproval(event, true));
+ bot.onAction("cancel", async (event) => {
+ const actionId = event.value;
+ if (!actionId) return;
</file context>
Suggested change
| bot.onAction("cancel", async (event) => { | |
| const actionId = event.value; | |
| if (!actionId) return; | |
| const consumed = await consume(actionId); | |
| if (!consumed) return; | |
| await editMessage(event.adapter, event.threadId, event.messageId, ":no_entry_sign: Cancelled."); | |
| }); | |
| bot.onAction("cancel", async (event) => { | |
| const actionId = event.value; | |
| if (!actionId) return; | |
| const pending = await get(actionId); | |
| if (!pending) return; | |
| if (pending.userId !== event.user.userId) { | |
| if (event.thread) { | |
| await event.thread.postEphemeral( | |
| event.user, | |
| "Only the person who initiated this action can approve or cancel it.", | |
| { fallbackToDM: false }, | |
| ); | |
| } | |
| return; | |
| } | |
| const consumed = await consume(actionId); | |
| if (!consumed) return; | |
| await editMessage(event.adapter, event.threadId, event.messageId, ":no_entry_sign: Cancelled."); | |
| }); |
Comment on lines
+149
to
+151
| await redis.set(`${ACTION_PREFIX}${actionId}`, JSON.stringify(existing), { | ||
| ex: TTL_SECONDS, | ||
| }); |
Contributor
There was a problem hiding this comment.
P2: Refresh the thread mapping TTL when updating the action to keep findByThread working for the full TTL window.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/src/routes/slack/confirmation-store.ts, line 149:
<comment>Refresh the thread mapping TTL when updating the action to keep `findByThread` working for the full TTL window.</comment>
<file context>
@@ -114,6 +118,39 @@ export async function store(
+
+ existing.messageTs = messageTs;
+
+ await redis.set(`${ACTION_PREFIX}${actionId}`, JSON.stringify(existing), {
+ ex: TTL_SECONDS,
+ });
</file context>
Suggested change
| await redis.set(`${ACTION_PREFIX}${actionId}`, JSON.stringify(existing), { | |
| ex: TTL_SECONDS, | |
| }); | |
| await Promise.all([ | |
| redis.set(`${ACTION_PREFIX}${actionId}`, JSON.stringify(existing), { | |
| ex: TTL_SECONDS, | |
| }), | |
| redis.expire(`${THREAD_PREFIX}${existing.threadTs}`, TTL_SECONDS), | |
| ]); |
| "@ai-sdk/anthropic": "1.2.0", | ||
| "@bufbuild/protobuf": "2.10.2", | ||
| "@bufbuild/protovalidate": "^1.1.1", | ||
| "@chat-adapter/slack": "^4.26.0", |
Contributor
There was a problem hiding this comment.
P2: Pin this dependency to an exact version instead of a caret range to align with repo versioning conventions.
(Based on your team's feedback about pinning dependency versions instead of using ranges.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/package.json, line 18:
<comment>Pin this dependency to an exact version instead of a caret range to align with repo versioning conventions.
(Based on your team's feedback about pinning dependency versions instead of using ranges.) </comment>
<file context>
@@ -12,8 +12,11 @@
+ "@ai-sdk/anthropic": "1.2.0",
"@bufbuild/protobuf": "2.10.2",
"@bufbuild/protovalidate": "^1.1.1",
+ "@chat-adapter/slack": "^4.26.0",
+ "@chat-adapter/state-ioredis": "^4.26.0",
"@connectrpc/connect": "2.1.1",
</file context>
Suggested change
| "@chat-adapter/slack": "^4.26.0", | |
| "@chat-adapter/slack": "4.26.0", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.