-
Notifications
You must be signed in to change notification settings - Fork 990
fix(slack): add missing QStash endpoint for DM messages #1411
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,61 @@ | ||||||||||||||||||||||
| import { Receiver } from "@upstash/qstash"; | ||||||||||||||||||||||
| import { z } from "zod"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| import { env } from "@/env"; | ||||||||||||||||||||||
| import { processAssistantMessage } from "../../events/process-assistant-message"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const receiver = new Receiver({ | ||||||||||||||||||||||
| currentSigningKey: env.QSTASH_CURRENT_SIGNING_KEY, | ||||||||||||||||||||||
| nextSigningKey: env.QSTASH_NEXT_SIGNING_KEY, | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const payloadSchema = z.object({ | ||||||||||||||||||||||
| event: z.object({ | ||||||||||||||||||||||
| type: z.literal("message"), | ||||||||||||||||||||||
| user: z.string(), | ||||||||||||||||||||||
| text: z.string().optional(), | ||||||||||||||||||||||
| ts: z.string(), | ||||||||||||||||||||||
| channel: z.string(), | ||||||||||||||||||||||
| channel_type: z.literal("im"), | ||||||||||||||||||||||
| event_ts: z.string(), | ||||||||||||||||||||||
| thread_ts: z.string().optional(), | ||||||||||||||||||||||
| }), | ||||||||||||||||||||||
| teamId: z.string(), | ||||||||||||||||||||||
| eventId: z.string(), | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export async function POST(request: Request) { | ||||||||||||||||||||||
| const body = await request.text(); | ||||||||||||||||||||||
| const signature = request.headers.get("upstash-signature"); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (!signature) { | ||||||||||||||||||||||
| return Response.json({ error: "Missing signature" }, { status: 401 }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const isValid = await receiver.verify({ | ||||||||||||||||||||||
| body, | ||||||||||||||||||||||
| signature, | ||||||||||||||||||||||
| url: `${env.NEXT_PUBLIC_API_URL}/api/integrations/slack/jobs/process-assistant-message`, | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (!isValid) { | ||||||||||||||||||||||
| return Response.json({ error: "Invalid signature" }, { status: 401 }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const parsed = payloadSchema.safeParse(JSON.parse(body)); | ||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If QStash delivers a corrupted or non-JSON body that passes signature verification, 🛡️ Proposed fix — combine with the Zod parse in a single try-catch- const parsed = payloadSchema.safeParse(JSON.parse(body));
- if (!parsed.success) {
+ let jsonBody: unknown;
+ try {
+ jsonBody = JSON.parse(body);
+ } catch {
+ return Response.json({ error: "Invalid JSON" }, { status: 400 });
+ }
+
+ const parsed = payloadSchema.safeParse(jsonBody);
+ if (!parsed.success) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| if (!parsed.success) { | ||||||||||||||||||||||
| console.error( | ||||||||||||||||||||||
| "[slack/process-assistant-message] Invalid payload:", | ||||||||||||||||||||||
| parsed.error, | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| return Response.json({ error: "Invalid payload" }, { status: 400 }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| await processAssistantMessage({ | ||||||||||||||||||||||
| event: { ...parsed.data.event, subtype: undefined }, | ||||||||||||||||||||||
| teamId: parsed.data.teamId, | ||||||||||||||||||||||
| eventId: parsed.data.eventId, | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return Response.json({ success: true }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+27
to
+61
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Find all route.ts files under jobs directories to identify QStash job handlers
echo "=== QStash Job Routes ==="
find . -path "*/jobs/*/route.ts" -type f | head -20
echo -e "\n=== Content of similar job routes ==="
find . -path "*/jobs/*/route.ts" -type f -exec sh -c 'echo "File: $1"; head -80 "$1"; echo "---"' _ {} \;Repository: superset-sh/superset Length of output: 11813 🏁 Script executed: #!/bin/bash
# Search for receiver.verify patterns to see how signature verification is done elsewhere
echo "=== Receiver.verify usages ==="
rg -n 'receiver\.verify' --type ts --type tsx -B2 -A5Repository: superset-sh/superset Length of output: 139 🏁 Script executed: #!/bin/bash
# Also search for other patterns of Upstash/QStash integration patterns
echo "=== QStash/Upstash patterns ==="
rg -n 'upstash|QStash|Receiver' --type ts --type tsx -l | head -20Repository: superset-sh/superset Length of output: 140 Add development environment check to skip signature verification in development. This route lacks the development environment check present in other QStash job handlers ( const isDev = env.NODE_ENV === "development";
if (!isDev) {
if (!signature) {
return Response.json({ error: "Missing signature" }, { status: 401 });
}
const isValid = await receiver.verify({
body,
signature,
url: `${env.NEXT_PUBLIC_API_URL}/api/integrations/slack/jobs/process-assistant-message`,
});
if (!isValid) {
return Response.json({ error: "Invalid signature" }, { status: 401 });
}
}🤖 Prompt for AI Agents |
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
@upstash/qstash Receiver verify method throws invalid signature documentation💡 Result:
The Upstash QStash docs indicate
Receiver.verify(...)will fail (often surfacing as “Invalid signature”) when any input used for verification doesn’t exactly match what QStash signed, especially:JSON.stringify(req.body)) can change spacing/key order and cause verification to fail. The docs explicitly warn to use the raw body string “as is.” [1]urlyou pass doesn’t exactly match the JWTsubclaim (scheme/host/path). The manual verification example shows the verifier checksp.sub !== urland throws (subject mismatch). So passing only a host, missinghttps://, missing path, or using a different public URL than QStash is calling will fail verification. [2]Helpful cross-check: QStash delivers the body unchanged (“passed as is”), so your handler must read it in that same raw form for hashing/verification. [3]
Sources: Upstash “Verify Signatures” doc (raw body + Receiver usage) [1], Upstash AWS Lambda quickstart (shows
sub/urland body-hash checks) [2], Upstash “Receiving Messages” (body passed as-is) [3].🌐 Web query:
@upstash/qstash Receiver.verify() return type throws error documentation examples💡 Result:
In
@upstash/qstash(TypeScript),Receiver.verify(...)is async and effectively works like:Promise<boolean>(it resolves totruewhen verification succeeds).SignatureError(it does not returnfalsefor invalid signatures). The SDK throws on JWT verification errors, subject (url) mismatch, or body-hash mismatch. [1]So usage patterns in docs/examples differ, but both are valid:
Pattern A: try/catch (recommended, since failures throw)
Example shown in Upstash’s AWS Lambda quickstart. [2]
Pattern B: assign to
isValid(works, butfalseis not expected)Shown in Upstash docs and quickstarts. [3]
Docs/examples: Receiver verify examples are in the “Verify Signatures” doc and the Receiver SDK example page. [3]
Sources:
[1] UNPKG compiled source for
@upstash/qstashshowingSignatureErrorandverify()implementation/behavior.[2] Upstash AWS Lambda (Node) quickstart demonstrating
try/catcharoundverify().[3] Upstash docs pages showing
const isValid = await receiver.verify(...)usage.receiver.verify()throws on invalid signatures — theisValidcheck is unreachable.The QStash
Receiver.verify()method throws aSignatureErrorwhen the signature is invalid rather than returningfalse. This means theif (!isValid)block is dead code, and an invalid signature will produce an unhandled exception (500 error) instead of the intended 401 response. Wrap theverifycall in a try-catch to properly handle verification failures.Proposed fix
🤖 Prompt for AI Agents