monitoring: GET-only Loki read gateway for the paperclip read SA (REN-621) #1119

Merged
Ghost merged 1 commits from atlas/ren-621-loki-read-gateway into master 2026-10-04 12:22:00 +00:00

What changed

  • monitoring/loki-read-gateway.{yaml,conf}: a new nginx-unprivileged Deployment and Service loki-read-gateway:8080. It is a GET-only allow-list in front of loki:3100 that passes /loki/api/v1/{query,query_range,labels,series} and /loki/api/v1/label/<name>/values. Every other path returns 403. Each location rebuilds the upstream URI itself, so the client's raw URI never reaches Loki.
  • monitoring/kustomization.yaml: adds the resource and a configMapGenerator with a hash suffix, so a config edit rolls the gateway pod.
  • paperclip/prometheus-read-rbac.yaml: adds loki-read-gateway:8080 to the existing services/proxy get Role. It does not add loki:3100.

Why

REN-621 and REN-613 need edge-1 zwave-js logs, which exist only in Loki. Loki runs as a SingleBinary with auth_enabled: false and answers GET /ingester/shutdown and /flush, so a direct get on loki:3100 would let callers shut down log ingest. The owner chose this design on REN-622 (option A2).

How verified (lint and local only; not yet in the cluster)

  • kustomize build passes for monitoring and paperclip. kubeconform reports 0 invalid. The Deployment references the hashed ConfigMap.
  • I ran the real nginxinc/nginx-unprivileged:1.27-alpine binary with this config in front of a fake Loki that echoes the method and path. nginx -t passes. These requests pass through unchanged: GET query_range, labels, label/host/values and series. These get 403 and never reach upstream: POST and DELETE, /ingester/shutdown, /flush, /push, /tail, /config, ../ traversal, %2e%2e%2f and %2F encoded traversal.

Safety

New stateless pod. Alloy still pushes directly to loki, so the ingest path is untouched. Loki, Prometheus and Home Assistant do not restart.

Revert

Revert this commit. Flux prunes the Deployment, Service and ConfigMap, and the Role goes back to two names.

🤖 Generated with Claude Code

## What changed - `monitoring/loki-read-gateway.{yaml,conf}`: a new nginx-unprivileged Deployment and Service `loki-read-gateway:8080`. It is a GET-only allow-list in front of `loki:3100` that passes `/loki/api/v1/{query,query_range,labels,series}` and `/loki/api/v1/label/<name>/values`. Every other path returns 403. Each location rebuilds the upstream URI itself, so the client's raw URI never reaches Loki. - `monitoring/kustomization.yaml`: adds the resource and a `configMapGenerator` with a hash suffix, so a config edit rolls the gateway pod. - `paperclip/prometheus-read-rbac.yaml`: adds `loki-read-gateway:8080` to the existing `services/proxy` `get` Role. It does not add `loki:3100`. ## Why REN-621 and REN-613 need edge-1 zwave-js logs, which exist only in Loki. Loki runs as a SingleBinary with `auth_enabled: false` and answers **GET** `/ingester/shutdown` and `/flush`, so a direct `get` on `loki:3100` would let callers shut down log ingest. The owner chose this design on REN-622 (option A2). ## How verified (lint and local only; not yet in the cluster) - `kustomize build` passes for `monitoring` and `paperclip`. `kubeconform` reports 0 invalid. The Deployment references the hashed ConfigMap. - I ran the real `nginxinc/nginx-unprivileged:1.27-alpine` binary with this config in front of a fake Loki that echoes the method and path. `nginx -t` passes. These requests pass through unchanged: GET query_range, labels, label/host/values and series. These get 403 and never reach upstream: POST and DELETE, `/ingester/shutdown`, `/flush`, `/push`, `/tail`, `/config`, `../` traversal, `%2e%2e%2f` and `%2F` encoded traversal. ## Safety New stateless pod. Alloy still pushes directly to `loki`, so the ingest path is untouched. Loki, Prometheus and Home Assistant do not restart. ## Revert Revert this commit. Flux prunes the Deployment, Service and ConfigMap, and the Role goes back to two names. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ghost added 1 commit 2026-10-04 12:20:38 +00:00
monitoring: GET-only Loki read gateway for the paperclip read SA (REN-621)
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
fe2e163b6a
Loki runs SingleBinary with auth disabled and answers GET on
/ingester/shutdown and /flush, so an RBAC `get` on services/proxy for
loki:3100 would not be read-only. Put an nginx allow-list in front of it
(query, query_range, labels, label/<name>/values, series; GET only;
upstream URI rebuilt per location) and grant the paperclip read SA
`get` on that Service only. Owner decision: REN-622 (option A2).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ghost approved these changes 2026-10-04 12:22:00 +00:00
Ghost left a comment

Approved on fe2e163b6a (Argus). Checked: allow-list uses exact/anchored locations matched on nginx-normalized $uri, upstream URI rebuilt per location, only $args forwarded (after ?), label name restricted to [A-Za-z_][A-Za-z0-9_]*; catch-all 403; limit_except GET. RBAC: Role monitoring/paperclip-prometheus-read gains only resourceName loki-read-gateway:8080, verb get, services/proxy; loki:3100 not granted. Cluster: Service monitoring/loki port 3100 exists; no NetworkPolicy/CNP in monitoring; loki-read-gateway not yet present. Pod: non-root, RO rootfs, caps dropped, /tmp emptyDir. Not touching Loki, Alloy or HA. Revert = revert commit.

Approved on fe2e163b6a5895ba5a6b921130d7b0726bc003c9 (Argus). Checked: allow-list uses exact/anchored locations matched on nginx-normalized $uri, upstream URI rebuilt per location, only $args forwarded (after ?), label name restricted to [A-Za-z_][A-Za-z0-9_]*; catch-all 403; limit_except GET. RBAC: Role monitoring/paperclip-prometheus-read gains only resourceName loki-read-gateway:8080, verb get, services/proxy; loki:3100 not granted. Cluster: Service monitoring/loki port 3100 exists; no NetworkPolicy/CNP in monitoring; loki-read-gateway not yet present. Pod: non-root, RO rootfs, caps dropped, /tmp emptyDir. Not touching Loki, Alloy or HA. Revert = revert commit.
Ghost merged commit 573bd670d5 into master 2026-10-04 12:22:00 +00:00
review-bot left a comment
Member

🤖 Automated review — model CLI default (complexity: complex), commit fe2e163b6a5895ba5a6b921130d7b0726bc003c9.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.


Summary

Adds a GET-only nginx allow-list (loki-read-gateway) in front of Loki and grants the paperclip read ServiceAccount get on it via the apiserver proxy, instead of granting get on loki:3100 directly (which would expose GET /ingester/shutdown and /flush). I checked the nginx config, the Deployment/Service/kustomization wiring, and the RBAC change against the rest of the repo's conventions (cluster-view-rbac.yaml, the existing Prometheus/Alertmanager grants). No blocking issues found.

  • The allow-list uses exact-match location = blocks for the four static paths and a regex location for label/<name>/values that restricts label_name to [A-Za-z_][A-Za-z0-9_]*, so the captured value can't be used to break out of the intended upstream path (no ../, no encoded slashes reaching the proxy_pass URI). Exact-match locations take precedence over the regex and the prefix catch-all regardless of declaration order, so there's no way for one of the allowed paths to be shadowed.
  • Every allowed location sets its own proxy_pass URI (http://loki/loki/api/v1/...$is_args$args) rather than passing through the client's raw URI, confirming the PR description's claim that path traversal/encoding tricks can't reach another Loki endpoint.
  • limit_except GET { deny all; } on each proxied location blocks POST/PUT/DELETE etc.; the catch-all location / returns 403 for anything not explicitly listed (/ingester/shutdown, /flush, /push, /tail, /config, ...).
  • RBAC: I confirmed the built-in view ClusterRole (bound cluster-wide to this same ServiceAccount via cluster-view-rbac.yaml) does not include services/proxy, so the dedicated get on services/proxy/loki-read-gateway:8080 in prometheus-read-rbac.yaml:88 is in fact the only path that reaches this Service — the design isn't undermined by the broader view grant.
  • Pod hardening (runAsNonRoot, runAsUser/Group: 101 matching the nginx-unprivileged image, readOnlyRootFilesystem, capabilities: drop: ["ALL"], automountServiceAccountToken: false) and the kustomize configMapGenerator hash-suffix wiring (so a .conf edit rolls the stateless pod) both look correct.

🟡 Suggestions

  • kubernetes/apps/monitoring/loki-read-gateway.conf:62-66 — location = /healthz has no limit_except, so it answers any HTTP method (POST, DELETE, ...) with 200. Harmless since it never proxies anywhere, but adding limit_except GET { deny all; } would make the file's "GET-only" framing uniform across every location.
  • No proxy_set_header Connection "" is set, so with proxy_http_version 1.1 nginx won't reuse upstream connections efficiently without an upstream { keepalive ... } block either. Irrelevant at this traffic volume, just noting it's not a keep-alive-optimized config.

Tests

No unit tests apply to this kind of change — the repo's existing pattern for Kustomize/K8s manifests is kustomize build + kubeconform in CI (.woodpecker/ci-manifests-test.yml), which the PR description says it ran locally with 0 invalid. The author also reports running the real nginxinc/nginx-unprivileged:1.27-alpine binary against a fake Loki backend and exercising both the allow-listed GET paths and the deny cases (non-GET methods, /ingester/shutdown, /flush, /push, /tail, /config, plain and encoded traversal). That's a reasonable verification approach for a change that can't be unit-tested, and the specific deny cases chosen line up with the actual risk (Loki's dangerous GET endpoints). The one gap is that this hasn't run in the cluster yet (stated explicitly in the PR body), so the real apiserver-proxy path (/api/v1/namespaces/monitoring/services/loki-read-gateway:8080/proxy/...) and the RBAC grant haven't been exercised end-to-end — not a reason to block, just worth confirming once deployed.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `fe2e163b6a5895ba5a6b921130d7b0726bc003c9`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Adds a GET-only nginx allow-list (`loki-read-gateway`) in front of Loki and grants the paperclip read ServiceAccount `get` on it via the apiserver proxy, instead of granting `get` on `loki:3100` directly (which would expose `GET /ingester/shutdown` and `/flush`). I checked the nginx config, the Deployment/Service/kustomization wiring, and the RBAC change against the rest of the repo's conventions (`cluster-view-rbac.yaml`, the existing Prometheus/Alertmanager grants). No blocking issues found. - The allow-list uses exact-match `location =` blocks for the four static paths and a regex location for `label/<name>/values` that restricts `label_name` to `[A-Za-z_][A-Za-z0-9_]*`, so the captured value can't be used to break out of the intended upstream path (no `../`, no encoded slashes reaching the `proxy_pass` URI). Exact-match locations take precedence over the regex and the prefix catch-all regardless of declaration order, so there's no way for one of the allowed paths to be shadowed. - Every allowed location sets its own `proxy_pass` URI (`http://loki/loki/api/v1/...$is_args$args`) rather than passing through the client's raw URI, confirming the PR description's claim that path traversal/encoding tricks can't reach another Loki endpoint. - `limit_except GET { deny all; }` on each proxied location blocks POST/PUT/DELETE etc.; the catch-all `location /` returns 403 for anything not explicitly listed (`/ingester/shutdown`, `/flush`, `/push`, `/tail`, `/config`, ...). - RBAC: I confirmed the built-in `view` ClusterRole (bound cluster-wide to this same ServiceAccount via `cluster-view-rbac.yaml`) does not include `services/proxy`, so the dedicated `get` on `services/proxy`/`loki-read-gateway:8080` in `prometheus-read-rbac.yaml:88` is in fact the only path that reaches this Service — the design isn't undermined by the broader `view` grant. - Pod hardening (`runAsNonRoot`, `runAsUser/Group: 101` matching the `nginx-unprivileged` image, `readOnlyRootFilesystem`, `capabilities: drop: ["ALL"]`, `automountServiceAccountToken: false`) and the kustomize `configMapGenerator` hash-suffix wiring (so a `.conf` edit rolls the stateless pod) both look correct. ## 🟡 Suggestions - `kubernetes/apps/monitoring/loki-read-gateway.conf:62-66` — `location = /healthz` has no `limit_except`, so it answers any HTTP method (POST, DELETE, ...) with 200. Harmless since it never proxies anywhere, but adding `limit_except GET { deny all; }` would make the file's "GET-only" framing uniform across every location. - No `proxy_set_header Connection ""` is set, so with `proxy_http_version 1.1` nginx won't reuse upstream connections efficiently without an `upstream { keepalive ... }` block either. Irrelevant at this traffic volume, just noting it's not a keep-alive-optimized config. ## Tests No unit tests apply to this kind of change — the repo's existing pattern for Kustomize/K8s manifests is `kustomize build` + `kubeconform` in CI (`.woodpecker/ci-manifests-test.yml`), which the PR description says it ran locally with 0 invalid. The author also reports running the real `nginxinc/nginx-unprivileged:1.27-alpine` binary against a fake Loki backend and exercising both the allow-listed GET paths and the deny cases (non-GET methods, `/ingester/shutdown`, `/flush`, `/push`, `/tail`, `/config`, plain and encoded traversal). That's a reasonable verification approach for a change that can't be unit-tested, and the specific deny cases chosen line up with the actual risk (Loki's dangerous GET endpoints). The one gap is that this hasn't run in the cluster yet (stated explicitly in the PR body), so the real apiserver-proxy path (`/api/v1/namespaces/monitoring/services/loki-read-gateway:8080/proxy/...`) and the RBAC grant haven't been exercised end-to-end — not a reason to block, just worth confirming once deployed. ## Recommendation Looks reasonable — no blocking issues; comments above are optional. <!-- pr-review:sha=fe2e163b6a5895ba5a6b921130d7b0726bc003c9 -->
Sign in to join this conversation.