Skip to content

Add webhook message routes#1642

Closed
samuelscheit wants to merge 3 commits intospacebarchat:masterfrom
samuelscheit2:fix/131-webhook-message-routes
Closed

Add webhook message routes#1642
samuelscheit wants to merge 3 commits intospacebarchat:masterfrom
samuelscheit2:fix/131-webhook-message-routes

Conversation

@samuelscheit
Copy link
Copy Markdown
Member

Summary

  • add GET/PATCH/DELETE webhook message routes for messages
  • harden webhook token/message helpers, nullable edit semantics, attachment handling, and edit/delete side effects
  • add focused tests for helpers, auth bypass tokens, and route registration/dispatch

Closes #131

Tests

  • npm run build
  • npm run node:tests
  • npm run lint
  • npm run build:src
  • git diff --check

@samuelscheit
Copy link
Copy Markdown
Member Author

Code review found one blocking issue before approval:

  • editWebhookMessage currently forwards message.author_id into handleMessage (src/api/util/handlers/WebhookMessage.ts:167-177). handleMessage runs the normal user SEND_MESSAGES rights check whenever opts.author_id is present before it reaches the webhook branch (src/api/util/handlers/Message.ts:343-349). Existing webhook pseudo-users are created with rights: "0", so PATCHing a webhook message can fail with a 403 instead of authenticating solely with the webhook token.
  • The PR Style check is also failing on Prettier formatting for .lintstagedrc.mjs, eslint.config.mjs, and src/cdn/util/Storage.ts.

Needs another pass before I can approve/mark ready.

@samuelscheit
Copy link
Copy Markdown
Member Author

Code review follow-up: I re-reviewed the updated diff after 9c0236c4.

The previous webhook edit auth blocker is addressed by keeping webhook-authenticated operations out of the normal user author/rights path, with regression coverage in WebhookMessage.test.ts. The Style check is also green after the formatting fixes.

I found no remaining blocking issues in the changed code. Approved from code review.

@samuelscheit samuelscheit marked this pull request as ready for review May 6, 2026 05:43
Copilot AI review requested due to automatic review settings May 6, 2026 05:43
@samuelscheit
Copy link
Copy Markdown
Member Author

Final PR report for the webhook message routes work.

Note: the assigned PR #153 could not be resolved on GitHub, so I created and updated PR #1642 for branch fix/131-webhook-message-routes.

Changed files:

  • .lintstagedrc.mjs
  • eslint.config.mjs
  • assets/openapi.json
  • assets/schemas.json
  • src/api/middlewares/Authentication.ts
  • src/api/middlewares/Authentication.test.ts
  • src/api/routes/channels/#channel_id/messages/#message_id/index.ts
  • src/api/routes/webhooks/#webhook_id/#token/messages/#message_id/index.ts
  • src/api/util/handlers/Message.ts
  • src/api/util/handlers/Webhook.ts
  • src/api/util/handlers/WebhookMessage.ts
  • src/api/util/handlers/WebhookMessage.test.ts
  • src/api/util/handlers/WebhookMessageRoute.test.ts
  • src/cdn/util/Storage.ts
  • src/schemas/uncategorised/WebhookMessageEditSchema.ts
  • src/schemas/uncategorised/index.ts

Tests and checks run locally:

  • node -r dotenv/config -r module-alias/register --enable-source-maps --test dist/api/util/handlers/WebhookMessage.test.js dist/api/middlewares/Authentication.test.js dist/api/util/handlers/WebhookMessageRoute.test.js — passed, 19 tests.
  • npm run node:tests — passed, 72 tests / 8 suites / 0 failures. Existing JsonSerializer MaxListeners warnings appeared, non-fatal.
  • npm run lint — passed with 0 errors and 2 pre-existing deprecation warnings in src/util/util/Token.ts.
  • npx prettier --check . — passed.
  • npm run build — passed and regenerated schema/OpenAPI assets; existing route/schema warnings were reported by the generator.
  • git diff --check master...HEAD — passed.

GitHub checks:

  • Style / build (24.x) — success.
  • Nix build / build-nix — success.

Code-review outcome:

  • Initial review found a webhook edit auth blocker and a Style formatting failure.
  • Follow-up review after 9c0236c4 found the auth blocker fixed with regression coverage and no remaining blocking issues.
  • PR Add webhook message routes #1642 has been marked ready for review.

Decision: approved from code review; no another pass is needed unless new reviewer feedback appears.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Route: /webhooks/:id/:token/messages/:id

2 participants