Add logging for Slack webhook middleware #2366
Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe changes implement Slack request signature verification in the middleware layer using HMAC-SHA256, including header validation, request age checks, and context storage. Additional debug logging is added to webhook routes for request tracking. Changes
Sequence DiagramsequenceDiagram
actor Client as Slack Client
participant Middleware as Slack Middleware
participant Crypto as Web Crypto API
participant Sentry as Sentry
participant Context as Request Context
participant Route as Webhook Route
Client->>Middleware: POST /webhook<br/>(body, headers)
Middleware->>Middleware: Log: request body received
alt Missing SLACK_SIGNING_SECRET
Middleware->>Sentry: Report error
Middleware->>Client: 401 Unauthorized
else Missing signature/timestamp headers
Middleware->>Sentry: Report error
Middleware->>Client: 401 Unauthorized
else Request too old (>5 min)
Middleware->>Sentry: Report error
Middleware->>Client: 401 Unauthorized
else Verification needed
Middleware->>Middleware: Build sigBaseString<br/>(timestamp + body)
Middleware->>Crypto: Compute HMAC-SHA256<br/>(sigBaseString, secret)
Crypto-->>Middleware: computedSignature
alt Signature mismatch
Middleware->>Sentry: Report error
Middleware->>Client: 401 Unauthorized
else Signature valid
Middleware->>Context: Store slackRawBody<br/>Store slackTimestamp
Middleware->>Route: next()
Route->>Route: Log: payload type
Route->>Client: 200 OK
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/api/src/routes/webhook.ts (1)
154-163: Consider using structured logging with log levels.These debug logs will output in production. The Stripe webhook handler uses Sentry for observability rather than console.log. For consistency and production-readiness, consider:
- Using a logger with configurable levels (debug/info/warn/error)
- Gating verbose logs behind
env.NODE_ENV !== "production"- } catch (e) { - console.log("[slack/events] Failed to parse payload:", e); + } catch (error) { + if (env.NODE_ENV !== "production") { + console.log("[slack/events] Failed to parse payload:", error); + } return c.json({ error: "invalid_payload" }, 400); } - console.log("[slack/events] Parsed payload type:", payload.type); - if (payload.type === "url_verification" && payload.challenge) { - console.log("[slack/events] URL verification, returning challenge"); return c.json({ challenge: payload.challenge }, 200); }apps/api/src/middleware/slack.ts (1)
12-13: Gate debug logs behind environment check or use a leveled logger.These operational logs are useful for debugging but will be noisy in production. Consider wrapping them in an environment check or using a logger with configurable levels.
+const isDebug = env.NODE_ENV !== "production"; + export const verifySlackWebhook = createMiddleware<{ Variables: { slackRawBody: string; slackTimestamp: string; }; }>(async (c, next) => { - console.log("[slack middleware] Starting verification"); + if (isDebug) console.log("[slack middleware] Starting verification");Apply similar pattern to other debug logs (lines 15, 26, 32, 62, 66, 71).
Also applies to: 15-16, 26-27, 32-33, 62-63, 66-67, 71-71
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/src/middleware/slack.ts(2 hunks)apps/api/src/routes/webhook.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/api/src/routes/webhook.tsapps/api/src/middleware/slack.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Format using
dprint fmtfrom the root. Do not usecargo fmt.
Files:
apps/api/src/routes/webhook.tsapps/api/src/middleware/slack.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props. Just inline them.
Never do manual state management for form/mutation. UseuseFormfrom tanstack-form anduseQuery/useMutationfrom tanstack-query for 99% cases.
Files:
apps/api/src/routes/webhook.tsapps/api/src/middleware/slack.ts
**/*.{ts,tsx,rs,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
By default, avoid writing comments at all. If you write one, it should be about 'Why', not 'What'.
Files:
apps/api/src/routes/webhook.tsapps/api/src/middleware/slack.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: If there are many classNames with conditional logic, usecn(import from@hypr/utils). Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/api/src/routes/webhook.tsapps/api/src/middleware/slack.ts
🧬 Code graph analysis (1)
apps/api/src/middleware/slack.ts (1)
apps/api/src/env.ts (1)
env(4-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
| console.log("[slack middleware] signature:", signature); | ||
| console.log("[slack middleware] timestamp:", timestamp); | ||
|
|
There was a problem hiding this comment.
Do not log signature values.
Logging the X-Slack-Signature header exposes cryptographic verification data. This could aid attackers analyzing your logs in understanding signature patterns.
- console.log("[slack middleware] signature:", signature);
- console.log("[slack middleware] timestamp:", timestamp);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log("[slack middleware] signature:", signature); | |
| console.log("[slack middleware] timestamp:", timestamp); |
🤖 Prompt for AI Agents
In apps/api/src/middleware/slack.ts around lines 22-24, the code currently logs
the raw Slack signature and timestamp; the signature must not be logged. Remove
the console.log that prints the full signature and either (a) log only a
non-sensitive indicator (e.g., "signature present" or the timestamp alone) or
(b) if you must record part of it for debugging, log a safely masked version
(e.g., replace all but the first 4–6 and last 2 characters with asterisks) using
the application logger instead of console.log; ensure no full cryptographic
headers or secrets are written to logs.
| const body = await c.req.text(); | ||
| console.log("[slack middleware] Request body:", body); |
There was a problem hiding this comment.
Avoid logging request body containing sensitive webhook data.
The request body contains full Slack event payloads which may include user data, message content, and other sensitive information. This should not be logged in production.
const body = await c.req.text();
- console.log("[slack middleware] Request body:", body);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const body = await c.req.text(); | |
| console.log("[slack middleware] Request body:", body); | |
| const body = await c.req.text(); |
🤖 Prompt for AI Agents
In apps/api/src/middleware/slack.ts around lines 36-37, the code logs the full
Slack request body which may contain sensitive webhook data; remove the
unconditional console.log and either (a) restrict logging to non-production
environments by checking NODE_ENV and only log in development, or (b) parse the
body and redact sensitive fields (user IDs, message text, tokens) before
logging; use the app's logger at an appropriate log level instead of console.log
and ensure production never outputs raw request payloads.
| console.log("[slack middleware] computedSignature:", computedSignature); | ||
| console.log("[slack middleware] receivedSignature:", signature); |
There was a problem hiding this comment.
Remove signature logging.
Logging both computed and received signatures is a security risk—it exposes your HMAC computation output in logs.
- console.log("[slack middleware] computedSignature:", computedSignature);
- console.log("[slack middleware] receivedSignature:", signature);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log("[slack middleware] computedSignature:", computedSignature); | |
| console.log("[slack middleware] receivedSignature:", signature); |
🤖 Prompt for AI Agents
In apps/api/src/middleware/slack.ts around lines 58-59, remove the two
console.log statements that print computedSignature and received signature;
instead, do not log sensitive HMAC values (you can leave a non-sensitive
debug/info message or conditional debug logging behind a secure flag if needed)
and ensure no other code writes these signature values to logs or external
telemetry.
| console.log("[slack/events] Received request, rawBody:", rawBody); | ||
|
|
There was a problem hiding this comment.
Avoid logging raw webhook payload in production.
Logging the entire rawBody can expose sensitive webhook data (user info, event details) in production logs. Consider either:
- Removing this log, or
- Using a debug-level logger that's disabled in production, or
- Logging only a request identifier/event type instead.
- console.log("[slack/events] Received request, rawBody:", rawBody);
+ // Only log event receipt without sensitive payload data
+ console.log("[slack/events] Received request");🤖 Prompt for AI Agents
In apps/api/src/routes/webhook.ts around lines 149-150, the code logs the entire
rawBody which may expose sensitive webhook data; replace that console.log with a
safer approach—either remove the log, or switch to a debug-level logger that is
disabled in production (e.g., logger.debug(...) behind an environment or
logger.isDebugEnabled check), or log only non-sensitive metadata such as a
request id and event type (extract event.type and a generated/request id) so no
full payload is written to production logs.
• Added logging functionality for the Slack webhook middleware
• Improves observability and debugging capabilities for Slack integration