Skip to content

gate-30 public-monitoring: matches any controller with 'health' in the name — its remediation would strip auth from an authenticated, per-object-guarded endpoint #218

Description

@rubenvdlinde

Measured on ConductionNL/launchpad @ development, hydra-gates at main (756fe89), full tree.

[gate-30] public-monitoring: FAIL — 2 monitoring endpoint(s) missing @PublicPage
  lib/Controller/HealthPingController.php:96  method=show     rule=monitoring-endpoint-missing-public-page
  lib/Controller/HealthPingController.php:154 method=validate rule=monitoring-endpoint-missing-public-page

Both are false positives, and applying the remediation would remove authentication from an authenticated, per-object-authorized endpoint. This is the gate-9 shape (#160): the advice introduces the vulnerability.

What the endpoint actually is

HealthPingController is not an ADR-006/ADR-040 monitoring endpoint. It is not /api/health and not /api/metrics — this repo has those separately, on HealthController / MetricsController, wired to OpenRegister's AppHost observability engine, and gate-30 does not flag either of them.

HealthPingController::show() is GET /api/health-ping/{placementId} — the service-health badge for one widget placement. Its call path:

#[NoAdminRequired]
#[NoCSRFRequired]
public function show(int $placementId): JSONResponse
{
    $userId = $this->resolveUserId();
    if ($userId === null) {
        return new JSONResponse(['status'=>'error','error'=>'unauthorized'], Http::STATUS_UNAUTHORIZED);
    }
    // REQ-HPING-003 "Caller authorization" — the auth guard runs
    // BEFORE any resolution/ping is attempted.
    if ($this->permissionService->canViewPlacement(userId: $userId, placementId: $placementId) === false) {
        return new JSONResponse(['status'=>'error','error'=>'forbidden'], Http::STATUS_FORBIDDEN);
    }
    $badge = $this->healthPingService->resolveForPlacement(placementId: $placementId);
    …
}

It is deliberately #[NoAdminRequired] (any authenticated user) and then gated per object by PermissionService::canViewPlacement(). It is correct as written.

Why the remediation is actively harmful

The gate wants #[PublicPage]. Adding it would:

  1. Remove the authentication requirement from an endpoint whose entire design is "401 when anonymous, 403 when you may not view the placement" (REQ-HPING-003). resolveUserId() returning null would stop being an anonymous-caller rejection and start being a normal path.
  2. Turn it into an unauthenticated probe oracle. resolveForPlacement() performs an outbound request to an admin-configured URL. Exposing that to anonymous callers hands out a request-forging primitive and leaks reachability of internal hosts.
  3. Leak per-placement health state — which placements exist, and whether their backing service is up — to anyone on the internet.

The seam

_pm_ok already carries a *metrics* special case, precisely because ADR-006 makes /api/metrics admin-only on purpose and demanding #[PublicPage] there is a security regression. The comment says so. The same reasoning applies here, but the matcher selects candidates on the name containing health, so health-ping — a per-object, per-user feature endpoint that merely has "health" in its name — is swept in alongside the real liveness endpoint.

Suggested direction

Do not widen _pm_ok to accept a comment (that is the gate-64 comment-satisfaction failure, .github#184). Two options that are evidence-based:

  • Require the endpoint to be routed as a monitoring endpoint — e.g. only consider controllers whose route path matches ^/api/(health|metrics|livez|readyz)$ — rather than matching the controller/method name. health-ping/{placementId} is parameterised and per-object; a liveness endpoint never is.
  • Or: skip any method that already performs a per-object authorization call before its work, which is the same signal gate-7 looks for. A monitoring endpoint has nothing to authorize against.

Happy to supply the fixture; the launchpad file is a clean reproduction of the shape.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions