monitoring: no probe covers /user/login — a 500 on the sign-in route is invisible #384

Stängd
öppnade 2026-08-05 18:14:29 +00:00 av supernaut · 2 kommentarer
Ägare

The gap

monitoring_probe_targets probes https://git.bitborg.se — the root, which serves 200
regardless of whether anyone can actually sign in. Nothing probes the login entry point.

This is not hypothetical. During the Gitborg Auth → Bitborg Auth auth-source rename (#383),
production spent a window in this state:

GET https://git.bitborg.se/user/login  → 302 → /user/oauth2/Gitborg%20Auth → 500

The canonical sign-in URL was returning a 500 to every visitor. All six probes reported 1 the
entire time
and the only firing alert was Watchdog, because the root was healthy and no probe
looked at /user/login. It was found by hand, not by monitoring.

The same blind spot applies to Grafana (/api/health is probed, the OAuth button is not) and to
anything else where "the service is up" and "a user can get in" are different questions.

Why the login route is especially exposed

/user/login is not served by Forgejo in the normal case — Caddy rewrites it:

redir @forgejo_login_page /user/oauth2/{{ forgejo_oidc_source_name | urlencode }}?{query}

So the route depends on forgejo_oidc_source_name agreeing with the live Forgejo auth source name.
Those live in two different systems, which means any future rename, restore, or partial apply can
desynchronise them — and the failure is a 500 on the primary way in, with every existing session
still working fine, so nobody notices until a logged-out user complains.

Suggested shape

Add a probe for the login route that asserts the redirect chain, not just reachability:

  • https://git.bitborg.se/user/login should 302 to /user/oauth2/<source name>, which should
    307 to https://auth.bitborg.se/ui/oauth2?....
  • A 2xx/3xx-tolerant module is not enough on its own — the broken state was a 302. The
    failing hop was the second one. Either probe /user/oauth2/<source name> directly (expect 307,
    and specifically not 5xx), or use a blackbox module with fail_if_body_matches_regexp /
    an explicit valid_status_codes on the final hop with follow_redirects: true.

Worth deciding whether this is a probe_success target or a dedicated alert, since a login-route
failure is arguably higher severity than a generic EndpointDown.

Acceptance

  • A probe exists that would have gone red during the window described above.
  • Prove it can fail before trusting its pass: point it at a deliberately wrong source name, confirm
    it goes red, then point it back. A probe that has never failed is not yet evidence of anything.
## The gap `monitoring_probe_targets` probes `https://git.bitborg.se` — the **root**, which serves `200` regardless of whether anyone can actually sign in. Nothing probes the login entry point. This is not hypothetical. During the `Gitborg Auth` → `Bitborg Auth` auth-source rename (#383), production spent a window in this state: ``` GET https://git.bitborg.se/user/login → 302 → /user/oauth2/Gitborg%20Auth → 500 ``` The canonical sign-in URL was returning a 500 to every visitor. **All six probes reported `1` the entire time** and the only firing alert was `Watchdog`, because the root was healthy and no probe looked at `/user/login`. It was found by hand, not by monitoring. The same blind spot applies to Grafana (`/api/health` is probed, the OAuth button is not) and to anything else where "the service is up" and "a user can get in" are different questions. ## Why the login route is especially exposed `/user/login` is not served by Forgejo in the normal case — Caddy rewrites it: ``` redir @forgejo_login_page /user/oauth2/{{ forgejo_oidc_source_name | urlencode }}?{query} ``` So the route depends on `forgejo_oidc_source_name` agreeing with the live Forgejo auth source name. Those live in two different systems, which means any future rename, restore, or partial apply can desynchronise them — and the failure is a 500 on the primary way in, with every existing session still working fine, so nobody notices until a logged-out user complains. ## Suggested shape Add a probe for the login route that asserts the **redirect chain**, not just reachability: - `https://git.bitborg.se/user/login` should `302` to `/user/oauth2/<source name>`, which should `307` to `https://auth.bitborg.se/ui/oauth2?...`. - A `2xx`/`3xx`-tolerant module is not enough on its own — the broken state *was* a `302`. The failing hop was the second one. Either probe `/user/oauth2/<source name>` directly (expect `307`, and specifically **not** `5xx`), or use a blackbox module with `fail_if_body_matches_regexp` / an explicit `valid_status_codes` on the final hop with `follow_redirects: true`. Worth deciding whether this is a `probe_success` target or a dedicated alert, since a login-route failure is arguably higher severity than a generic `EndpointDown`. ## Acceptance - A probe exists that would have gone red during the window described above. - Prove it can fail before trusting its pass: point it at a deliberately wrong source name, confirm it goes red, then point it back. A probe that has never failed is not yet evidence of anything.
Upphovsperson
Ägare

Implemented in #392 — open, not applied. The acceptance criterion here requires driving a probe
red, and alerting goes to email and ntfy, so the deliberate-failure step is left for a waking
operator rather than run overnight.

Most of "prove it can fail" is already satisfied read-only, though, against production:

GET /user/login                        -> 302  Location: .../user/oauth2/Bitborg%20Auth
GET /user/oauth2/Bitborg%20Auth        -> 307  -> auth.bitborg.se/ui/oauth2?...
GET /user/oauth2/Nonexistent%20Auth    -> 500     <-- the signal, reproducible on demand

The 500 that hid during the rename window discriminates cleanly, and the probes pin exact status
codes ([302] / [307]), so neither can pass on it. What still needs an apply is that the probe
goes red and that SignInRouteBroken routes.

Three decisions worth recording against this issue:

  • Two single-hop probes, redirects off — not one follow_redirects probe of /user/login.
    Following the chain terminates inside Kanidm's /ui/oauth2, and an anonymous request there logs an
    Invalid identity: NotAuthenticated ERROR span; that is the documented reason kanidm is probed at
    /status rather than root, and following would reintroduce it.
  • Hop 1 asserts the Location header, which this issue's suggested shape does not. A 302 to
    the wrong slug is still a 302 — and Caddy disagreeing with the live source name is exactly the
    failure described above, so a status-only check would miss it.
  • Dedicated SignInRouteBroken alert, answering the "probe_success target or dedicated alert?"
    question here: dedicated. The generic wording would actively mislead, because everything else is
    green during this failure.

One open question I could not settle: /user/oauth2/<source> begins an OAuth flow, so Forgejo mints
session state per probe (the 307 carries a fresh state and code_challenge). Interval set to 60s
to halve the churn, but the session store's actual growth is unmeasured — worth confirming it is
bounded before trusting hop 2 long-term.

Implemented in #392 — **open, not applied.** The acceptance criterion here requires driving a probe red, and alerting goes to email and ntfy, so the deliberate-failure step is left for a waking operator rather than run overnight. Most of "prove it can fail" is already satisfied read-only, though, against production: ```text GET /user/login -> 302 Location: .../user/oauth2/Bitborg%20Auth GET /user/oauth2/Bitborg%20Auth -> 307 -> auth.bitborg.se/ui/oauth2?... GET /user/oauth2/Nonexistent%20Auth -> 500 <-- the signal, reproducible on demand ``` The 500 that hid during the rename window discriminates cleanly, and the probes pin exact status codes (`[302]` / `[307]`), so neither can pass on it. What still needs an apply is that the *probe* goes red and that `SignInRouteBroken` routes. Three decisions worth recording against this issue: - **Two single-hop probes, redirects off** — not one `follow_redirects` probe of `/user/login`. Following the chain terminates inside Kanidm's `/ui/oauth2`, and an anonymous request there logs an `Invalid identity: NotAuthenticated` ERROR span; that is the documented reason kanidm is probed at `/status` rather than root, and following would reintroduce it. - **Hop 1 asserts the `Location` header**, which this issue's suggested shape does not. A `302` to the *wrong* slug is still a `302` — and Caddy disagreeing with the live source name is exactly the failure described above, so a status-only check would miss it. - **Dedicated `SignInRouteBroken` alert**, answering the "probe_success target or dedicated alert?" question here: dedicated. The generic wording would actively mislead, because everything else is green during this failure. One open question I could not settle: `/user/oauth2/<source>` begins an OAuth flow, so Forgejo mints session state per probe (the 307 carries a fresh `state` and `code_challenge`). Interval set to 60s to halve the churn, but the session store's actual growth is **unmeasured** — worth confirming it is bounded before trusting hop 2 long-term.
Upphovsperson
Ägare

Done — applied and verified

PR #392 merged and applied 2026-08-06. Two probes live on the monitoring host:

blackbox-signin-redirect   https://git.bitborg.se/user/login              302 + Location assertion
blackbox-signin-handoff    https://git.bitborg.se/user/oauth2/Bitborg%20Auth   307

Apply made exactly the 6 predicted changed tasks (3 renders + 3 restarts), monitoring host only;
gitborg-prod stayed changed=0, confirming the forgejo_oidc_source_name move to group_vars/all
is inert on the services host. Both targets register health: up, both report probe_success=1 in
VictoriaMetrics, SignInRouteBroken is loaded in vmalert with a clean log, and only Watchdog fires.

"Prove it can fail" — done at the probe level, with matched controls

Each failure case is paired with its own positive control, run directly against the blackbox modules
so no config was touched and no alert was fired:

case status probe_success
hop 2, wrong source name — the exact incident condition 500 0
hop 2, correct source name 307 1
hop 1, wrong redirect target 301 0
hop 1, correct target 302 1
hop 1, with a query (?redirect_to=/explore) 302 1

The last row matters as much as the failures. Caddy appends ?{query}, so a signed-out user arriving
at /user/login?redirect_to=… produces Location: …/user/oauth2/Bitborg%20Auth?redirect_to=%2Fexplore.
The pattern carries an optional query group specifically for this; without it, every such visitor
would have made the probe red — a permanent false outage, and precisely the kind of probe-that-cries-
wolf this issue warns against.

What was NOT tested, and why — read this before trusting it further

Two honest gaps:

  1. The Location assertion has no isolated negative. The hop-1 failure above was rejected on
    status (301 ≠ 302), not by the header regex — probe_failed_due_to_regex stayed 0. Proving
    the regex specifically needs a target returning exactly 302 with a wrong Location, which does
    not exist on this estate without a config change. The assertion is known to be evaluated (it
    reports probe_failed_due_to_regex 0 rather than absent) and known to pass on both real shapes,
    but a live differential against a mismatching Location was not obtained.

  2. The end-to-end alert path was deliberately not exercised. Decided rather than skipped: firing
    it means a real critical page to admin@bitborg.se and ntfy, and what it would add is
    probe_success=0 → vmalert → Alertmanager → email/ntfy, which EndpointDown already exercises on
    the same wiring, plus a rule whose expression is EndpointDown's with a job matcher swapped in.
    The substantive risk this issue names — a probe that has never failed and is therefore not
    evidence — is disproven above.

If either gap ever matters, the recipe is: set forgejo_oidc_source_name to a wrong value and apply
--limit gitborg-monitoring — that moves only the probe target, leaving Caddy and Forgejo alone
so real sign-in keeps working. Forgetting the --limit breaks actual logins.

Also worth knowing

  • forgejo_oidc_source_name moved from roles/forgejo/defaults/ to group_vars/all/, because
    the probe runs on the monitoring host where the forgejo role never runs, so a role default is
    undefined there. Caught by an ansible -m debug differential before anything depended on it. It is
    deliberately not mirrored back as a fallback — same reasoning as alert_email_from.
  • The sign-in jobs are excluded from EndpointDown (no double page) and from CertificateExpiringSoon
    (they share a cert with the root target and would otherwise fire three warnings for one cert).

⚠️ Open, not resolved here

GET /user/oauth2/<source> begins an OAuth flow — the 307 carries a fresh state and
code_challenge, so Forgejo mints session state on every probe and blackbox keeps no cookies.
Interval is 60s rather than the global 30s to halve that churn, but the session store's actual
growth is unmeasured
. Worth confirming it is bounded; if it is not, hop 2 needs rethinking.

## Done — applied and verified PR #392 merged and applied 2026-08-06. Two probes live on the monitoring host: ```text blackbox-signin-redirect https://git.bitborg.se/user/login 302 + Location assertion blackbox-signin-handoff https://git.bitborg.se/user/oauth2/Bitborg%20Auth 307 ``` Apply made exactly the 6 predicted changed tasks (3 renders + 3 restarts), monitoring host only; `gitborg-prod` stayed `changed=0`, confirming the `forgejo_oidc_source_name` move to `group_vars/all` is inert on the services host. Both targets register `health: up`, both report `probe_success=1` in VictoriaMetrics, `SignInRouteBroken` is loaded in vmalert with a clean log, and only `Watchdog` fires. ## "Prove it can fail" — done at the probe level, with matched controls Each failure case is paired with its own positive control, run directly against the blackbox modules so **no config was touched and no alert was fired**: | case | status | `probe_success` | | --- | --- | --- | | hop 2, **wrong** source name — the exact incident condition | `500` | **0** | | hop 2, correct source name | `307` | 1 | | hop 1, wrong redirect target | `301` | **0** | | hop 1, correct target | `302` | 1 | | hop 1, **with a query** (`?redirect_to=/explore`) | `302` | 1 | The last row matters as much as the failures. Caddy appends `?{query}`, so a signed-out user arriving at `/user/login?redirect_to=…` produces `Location: …/user/oauth2/Bitborg%20Auth?redirect_to=%2Fexplore`. The pattern carries an optional query group specifically for this; without it, every such visitor would have made the probe red — a permanent false outage, and precisely the kind of probe-that-cries- wolf this issue warns against. ## What was NOT tested, and why — read this before trusting it further Two honest gaps: 1. **The `Location` assertion has no isolated negative.** The hop-1 failure above was rejected on *status* (`301` ≠ `302`), not by the header regex — `probe_failed_due_to_regex` stayed `0`. Proving the regex specifically needs a target returning exactly `302` with a *wrong* `Location`, which does not exist on this estate without a config change. The assertion is known to be *evaluated* (it reports `probe_failed_due_to_regex 0` rather than absent) and known to pass on both real shapes, but a live differential against a mismatching Location was not obtained. 2. **The end-to-end alert path was deliberately not exercised.** Decided rather than skipped: firing it means a real critical page to `admin@bitborg.se` and ntfy, and what it would add is `probe_success=0 → vmalert → Alertmanager → email/ntfy`, which `EndpointDown` already exercises on the same wiring, plus a rule whose expression is `EndpointDown`'s with a job matcher swapped in. The substantive risk this issue names — a probe that has never failed and is therefore not evidence — is disproven above. If either gap ever matters, the recipe is: set `forgejo_oidc_source_name` to a wrong value and apply **`--limit gitborg-monitoring`** — that moves only the probe target, leaving Caddy and Forgejo alone so real sign-in keeps working. Forgetting the `--limit` breaks actual logins. ## Also worth knowing - `forgejo_oidc_source_name` **moved from `roles/forgejo/defaults/` to `group_vars/all/`**, because the probe runs on the monitoring host where the forgejo role never runs, so a role default is undefined there. Caught by an `ansible -m debug` differential before anything depended on it. It is deliberately *not* mirrored back as a fallback — same reasoning as `alert_email_from`. - The sign-in jobs are excluded from `EndpointDown` (no double page) and from `CertificateExpiringSoon` (they share a cert with the root target and would otherwise fire three warnings for one cert). ## ⚠️ Open, not resolved here `GET /user/oauth2/<source>` **begins an OAuth flow** — the 307 carries a fresh `state` and `code_challenge`, so Forgejo mints session state on every probe and blackbox keeps no cookies. Interval is 60s rather than the global 30s to halve that churn, but **the session store's actual growth is unmeasured**. Worth confirming it is bounded; if it is not, hop 2 needs rethinking.
Logga in för att delta i denna konversation.
Ingen milstolpe
Inga tilldelade
1 deltagare
Notiser
Förfallodatum
Förfallodatumet är ogiltigt eller utanför gränserna. Använd formatet "åååå-mm-dd".

Inget förfallodatum satt.

Beroenden

Inga beroenden satta

Referens
bitborg/bitborg-infra#384
Ingen beskrivning angiven.