fix(add-ons): use fetch_validated_url to trigger webhooks#18815
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66919b9ee2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR hardens add-on webhook delivery by routing outbound webhook requests through the existing URL validation pipeline, preventing delivery to private-network targets (SSRF mitigation) and documenting the security improvement.
Changes:
- Switch webhook delivery from
fetch_url()tofetch_validated_url()withallow_private_targets=False. - Add a regression test ensuring private webhook targets are blocked and logged as errors.
- Note the security improvement in the 5.17 changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
weblate/addons/webhooks.py |
Uses validated outbound HTTP helper to block private webhook targets; extends exception handling to include validation failures. |
weblate/addons/tests.py |
Adds test covering private-target blocking behavior and expected activity log state. |
docs/changes.rst |
Records the security improvement in release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 678146cbc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87d0655896
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| allow_private_targets=not settings.WEBHOOK_RESTRICT_PRIVATE, | ||
| allowed_domains=settings.WEBHOOK_PRIVATE_ALLOWLIST, |
There was a problem hiding this comment.
Honor webhook allowlist during direct delivery checks
Passing allowed_domains=settings.WEBHOOK_PRIVATE_ALLOWLIST here does not actually allow private webhook targets for normal (non-proxy) delivery: fetch_validated_url only uses allowed_domains in the proxy branch, while direct requests still run runtime IP/peer checks that reject private addresses unconditionally. In practice, with WEBHOOK_RESTRICT_PRIVATE=True and WEBHOOK_PRIVATE_ALLOWLIST configured, saving the add-on can succeed but delivery still fails for allowlisted internal hosts unless an HTTP(S) proxy is used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
0deb80e to
e2f4e88
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2f4e88ef4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| raise MessageNotDeliveredError("; ".join(error.messages)) from error | ||
| except requests.exceptions.ConnectionError as error: | ||
| raise MessageNotDeliveredError from error | ||
| msg = "Unable to deliver webhook: could not connect to the remote server." |
There was a problem hiding this comment.
Make webhook connection error text translatable
send_message introduces a hard-coded English message ("Unable to deliver webhook: could not connect to the remote server.") that is surfaced to users through add-on activity logs when delivery fails, so non-English instances will always see this error in English. The repository guidance in /workspace/weblate/AGENTS.md explicitly requires user-facing strings to use Django i18n helpers, so this should be wrapped in a translatable string before raising MessageNotDeliveredError.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This pulls existing external endpoint validation in.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…#18815) This pulls existing external endpoint validation in.
This pulls existing external endpoint validation in.