Skip to content

fix(notifications): validate webhook URL scheme before posting#3031

Merged
enoch85 merged 1 commit into
Maintainerr:developmentfrom
Arvuno:fix/webhook-url-scheme-guard
Jun 5, 2026
Merged

fix(notifications): validate webhook URL scheme before posting#3031
enoch85 merged 1 commit into
Maintainerr:developmentfrom
Arvuno:fix/webhook-url-scheme-guard

Conversation

@Arvuno

@Arvuno Arvuno commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

What

The webhook agent passed settings.options.webhookUrl straight to axios.post. A misconfigured URL pointing at file://, gopher://, or any other non-http(s) scheme would be turned into an outbound request by the agent, with axios's behaviour for those schemes varying across Node versions.

Why

This is a small but real SSRF-shaped guardrail: it keeps a webhook agent pointed at an internal file path or an unexpected scheme from being the way the server leaves the network. The user can still disable the agent entirely; this only narrows what is allowed when it is enabled.

How

  • Parse the configured URL with the platform URL constructor; on failure log and return Failure: invalid webhook URL.
  • Reject anything whose protocol is not http: or https:; log and return Failure: unsupported webhook URL scheme.
  • Pass the parsed (and therefore normalised) URL string to axios.post instead of the raw user input.

Notes

  • Pre-existing TypeScript strict-mode warnings in the logging/notifications modules are untouched — this change introduces no new ones.

@Arvuno Arvuno requested a review from enoch85 as a code owner June 3, 2026 21:11
@enoch85 enoch85 force-pushed the fix/webhook-url-scheme-guard branch from 80a388f to 64a3d8e Compare June 5, 2026 11:31
User-configured notification URLs were passed straight to axios.post, so a
misconfigured value pointing at file://, gopher://, or any other non-http(s)
scheme would be turned into an outbound request. These URLs have no validation
at the settings layer, so this is the only guard.

Add a shared validateWebhookUrl helper and apply it in every agent that posts
to an operator-supplied URL: the webhookUrl agents (webhook, slack, lunasea,
discord) and ntfy's server url. Unparseable URLs and non-http(s) schemes are
rejected before sending and the normalised URL is posted; the agent returns a
clear Failure result.

Also replace ntfy's regex slash-trimming with the codebase's char-based idiom,
per the no-regex convention.

Co-Authored-By: Arvuno <hi@arvuno.xyz>
@enoch85 enoch85 force-pushed the fix/webhook-url-scheme-guard branch from 64a3d8e to a39ab2d Compare June 5, 2026 11:40
@enoch85

enoch85 commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Took this over and pushed an updated version. Summary of what changed:

  • Extracted a shared validateWebhookUrl helper instead of guarding only the generic webhook agent. The original change left the other agents posting an unvalidated URL.
  • Applied the guard in every agent that posts to an operator-supplied URL: the webhookUrl agents (webhook, slack, lunasea, discord) and ntfy's server url. Unparseable URLs and non-http(s) schemes are rejected before any request, and the normalised URL is posted.
  • Replaced ntfy's regex slash-trimming with the codebase's char-based idiom, per the no-regex convention.
  • Added tests - branch coverage for the helper, plus a wiring test per agent confirming a bad scheme returns a Failure and never calls axios.

Verified end to end via POST /api/notifications/test: across all five agents, bad schemes (file://, gopher://) and invalid URLs are rejected with no outbound request, while a valid http URL posts as expected. For ntfy, a trailing-slash URL and leading-slash topic join correctly to a single path.

The branch is now notifications-only - the health and logging commits were dropped since they already landed via #3029 and #3030. Full suite passes. Thanks for the original work, kept you credited on the commit.

@enoch85 enoch85 merged commit 195a912 into Maintainerr:development Jun 5, 2026
13 checks passed
maintainerr-automation Bot added a commit that referenced this pull request Jun 5, 2026
* fix(rules): save rule groups from incomplete payloads and clarify YAML import errors (#3045)

- Default keepLogsForMonths to 6 when omitted instead of binding NaN (create and update)
- Return a structured {code:0} error instead of a silent empty 201 when a save fails
- Guard updateRules against a missing collection block (no throw, no spurious media wipe)
- Clearer message for invalid YAML; log real server-side faults instead of mislabeling them as syntax errors

Refs #3044

* feat(server): add /api/health endpoints with DB liveness check (#3029)

* feat(server): add /api/health endpoints with DB liveness check

The application has no /health endpoint today, which makes it
awkward to wire up Kubernetes livenessProbe / readinessProbe or
Docker HEALTHCHECK directives. /api/app/status is not a substitute
because it also exercises the GitHub releases API.

Add three endpoints under /api/health:

  - GET /api/health       — combined: 200 with database=ok, or 503
                            with database=unreachable.
  - GET /api/health/live  — liveness: 200 as long as the process is
                            up, no DB call. Use for Kubernetes
                            livenessProbe (no restarts on transient
                            DB blips).
  - GET /api/health/ready — readiness: SELECT 1 against the
                            configured TypeORM datasource; returns
                            503 if the query fails. Use for
                            Kubernetes readinessProbe (stop
                            sending traffic until the DB is back).

The DB check uses the same TypeORM datasource the rest of the app
uses, so a misconfigured connection string or a full disk surfaces
as a clear 503 rather than a silent 200.

* refactor(server): move health DB check to a service; add tests and Docker HEALTHCHECK

- Extract the SELECT 1 ping into HealthService so the controller only shapes the
  response, matching the controllers-delegate-to-services pattern.
- Log the caught DB error via MaintainerrLogger (warn + debug) instead of
  swallowing it.
- Use process.uptime(); drop the instantiation-time uptime helper.
- Move the response shapes to @maintainerr/contracts (LivenessResponse,
  HealthResponse); live() returns the shared envelope; root route uses @get().
- Add HealthService and HealthController specs.
- Wire the Dockerfile HEALTHCHECK to /api/health/ready via a BASE_PATH-aware
  probe script.

---------

Co-authored-by: enoch85 <mailto@danielhansson.nu>

* docs: clarify yarn command-not-found means a stale node_modules

* fix(rules): preserve collection link and visibility on partial updates (#3046)

When the collection block is omitted from PUT /api/rules, fall back to the
saved values for manualCollection, manualCollectionName, visibleOnHome and
visibleOnRecommended instead of forwarding undefined. This stops
updateCollection from silently unlinking a manual collection (mediaServerId
cleared) or switching off Plex Home/Recommended visibility.

Follow-up to #3045.

* feat(logging): honour LOG_LEVEL env var on startup (#3030)

Operators can override the persisted log level for a single container by
setting LOG_LEVEL=debug|verbose|info|warn|error|fatal in the environment.
The persisted setting (the value the UI shows) is left untouched, so the
override is never baked into the database. An unrecognised value logs a
single warning at startup and falls back to the persisted level; with the
env var unset, behaviour is unchanged.

The winston factory now also tolerates a missing log_settings row during
first boot, falling back to the shared DEFAULT_LOG_* constants for level,
max size, and max files instead of dereferencing a null row.

Co-authored-by: enoch85 <mailto@danielhansson.nu>

* fix(notifications): validate webhook URL scheme before posting (#3031)

User-configured notification URLs were passed straight to axios.post, so a
misconfigured value pointing at file://, gopher://, or any other non-http(s)
scheme would be turned into an outbound request. These URLs have no validation
at the settings layer, so this is the only guard.

Add a shared validateWebhookUrl helper and apply it in every agent that posts
to an operator-supplied URL: the webhookUrl agents (webhook, slack, lunasea,
discord) and ntfy's server url. Unparseable URLs and non-http(s) schemes are
rejected before sending and the normalised URL is posted; the agent returns a
clear Failure result.

Also replace ntfy's regex slash-trimming with the codebase's char-based idiom,
per the no-regex convention.

Co-authored-by: enoch85 <mailto@danielhansson.nu>

* docs: refresh README - features, health endpoints, deploy examples, credits (#3048)

---------

Co-authored-by: enoch85 <mailto@danielhansson.nu>
Co-authored-by: Arvuno <hi@arvuno.xyz>
@maintainerr-automation

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.14.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

doonga pushed a commit to greyrock-labs/home-ops that referenced this pull request Jun 6, 2026
… ➔ 3.14.0) (#207)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [ghcr.io/maintainerr/maintainerr](https://github.com/Maintainerr/Maintainerr) | minor | `3.13.0` → `3.14.0` |

---

### Release Notes

<details>
<summary>Maintainerr/Maintainerr (ghcr.io/maintainerr/maintainerr)</summary>

### [`v3.14.0`](https://github.com/Maintainerr/Maintainerr/blob/HEAD/CHANGELOG.md#3140-2026-06-05)

[Compare Source](Maintainerr/Maintainerr@v3.13.0...v3.14.0)

#### Highlights

- Added `/api/health` endpoints with liveness and readiness checks for monitoring and integration with tools like Kubernetes and Docker Compose ([#&#8203;3029](Maintainerr/Maintainerr#3029)).
- Collection handler now skips media currently being streamed to avoid disrupting active viewers ([#&#8203;3027](Maintainerr/Maintainerr#3027)).
- Fixed issue where saving log settings would overwrite an active `LOG_LEVEL` environment variable override ([#&#8203;3053](Maintainerr/Maintainerr#3053)).

#### Features

- Added `/api/health` endpoints with liveness and readiness checks ([#&#8203;3029](Maintainerr/Maintainerr#3029)).
- Collection handler now skips media currently being streamed ([#&#8203;3027](Maintainerr/Maintainerr#3027)).
- Logging system now honors the `LOG_LEVEL` environment variable on startup ([#&#8203;3030](Maintainerr/Maintainerr#3030)).

#### Fixes

- Fixed issue where saving log settings would overwrite an active `LOG_LEVEL` environment variable override ([#&#8203;3053](Maintainerr/Maintainerr#3053)).
- Validated webhook URL schemes to prevent invalid or potentially harmful requests ([#&#8203;3031](Maintainerr/Maintainerr#3031)).
- Fixed issue where rule groups lost collection links and visibility on partial updates ([#&#8203;3045](Maintainerr/Maintainerr#3045), [#&#8203;3046](Maintainerr/Maintainerr#3046)).
- Fixed issue with manual collections not being found across libraries on Jellyfin/Emby ([#&#8203;3026](Maintainerr/Maintainerr#3026), [#&#8203;3042](Maintainerr/Maintainerr#3042)).
- Resolved issue where deleted media remained stuck in Jellyfin/Emby collections and caused repeated processing errors ([#&#8203;3023](Maintainerr/Maintainerr#3023), [#&#8203;3024](Maintainerr/Maintainerr#3024), [#&#8203;3040](Maintainerr/Maintainerr#3040)).
- Fixed issue where Seerr requests for episode rules incorrectly deleted entire season requests ([#&#8203;3015](Maintainerr/Maintainerr#3015)).
- Improved error notifications for collection handling failures to include the name of the failing collection ([#&#8203;3013](Maintainerr/Maintainerr#3013)).
- Used Radarr bulk exclusions endpoint to avoid duplicate 400 errors when adding exclusions ([#&#8203;3012](Maintainerr/Maintainerr#3012)).

#### Performance

- Pruned media that no longer exists on the media server to improve collection handling efficiency ([#&#8203;3023](Maintainerr/Maintainerr#3023), [#&#8203;3040](Maintainerr/Maintainerr#3040)).

#### Internal

- Refreshed README with updated features, deployment examples, and credits ([#&#8203;3048](Maintainerr/Maintainerr#3048)).
- Clarified that a missing `yarn` command indicates a stale `node_modules` directory.

#### Dependencies

- Updated 20 dependencies, including `@typescript-eslint/parser`, `react-router-dom`, `axios`, and `vite`.

#### New Contributors

- [@&#8203;Arvuno](https://github.com/Arvuno) made their first contribution in [#&#8203;3029](Maintainerr/Maintainerr#3029)

</details>

---

### Configuration

📅 **Schedule**: (in timezone America/New_York)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yMDYuMCIsInVwZGF0ZWRJblZlciI6IjQzLjIwNi4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9jb250YWluZXIiLCJ0eXBlL21pbm9yIl19-->

Reviewed-on: https://git.greyrock.io/greyrock-labs/home-ops/pulls/207
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants