Skip to content

fix(slack): reject URLs that are not Incoming Webhooks - #6

Open
crstian19 wants to merge 1 commit into
mainfrom
fix/slack-webhook-validation
Open

crstian19 wants to merge 1 commit into
mainfrom
fix/slack-webhook-validation

Conversation

@crstian19

Copy link
Copy Markdown
Contributor

El fallo

Una URL de cliente de Slack (https://app.slack.com/client/T…/D…) responde a un POST con la web de Slack y HTTP 200. post() sólo miraba status_code < 400, así que lo interpretaba como entrega correcta: el bot logueaba Notified Slack about thread … sin haber publicado nada.

Salió en producción — el .env desplegado tenía un enlace a un DM en vez de un webhook, y el bot reportaba éxito en cada hilo. El falso positivo hizo que buscáramos el problema en Discord durante un buen rato.

El arreglo

Dos comprobaciones, porque cada una por separado deja un hueco:

  1. La URL debe empezar por https://hooks.slack.com/services/. Se valida una sola vez en __init__, así una configuración errónea es un error claro al arrancar en lugar de una línea por notificación.
  2. Un 2xx no basta. Un Incoming Webhook responde con el cuerpo ok; cualquier otra cosa significa que el endpoint no es un webhook, por muy sano que parezca el status.

La primera sola no bastaría: un webhook borrado en el dominio correcto puede responder cosas distintas de ok. La segunda sola tampoco: sin ella, cualquier host que devuelva ok pasaría por bueno.

Verificación

  • 67 tests (6 nuevos): cuatro formas de URL que no son webhook —incluida la de DM real que provocó esto—, un 200 con HTML, y que OK\n sí cuenta como entrega.
  • Comprobado contra la URL exacta del .env de producción: el notifier se deshabilita y deja una línea de error.
  • ruff check y ruff format --check limpios.

🤖 Generated with Claude Code

A Slack client URL (https://app.slack.com/client/T…/D…) answers a POST with
the web app and HTTP 200. post() only checked `status_code < 400`, so it
read that as a delivered message: the bot logged "Notified Slack about
thread …" while nothing was ever posted.

Two checks, because either one alone still leaves a hole:

- The URL must start with https://hooks.slack.com/services/. Validated once
  in __init__ so a misconfigured value is one loud error at startup instead
  of one line per notification.
- A 2xx is not enough. An Incoming Webhook answers with the body "ok";
  anything else means the endpoint is not a webhook, however healthy the
  status code looks.

Found in production: the deployed .env held a DM link instead of a webhook,
and the bot reported success on every thread.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants