fix(prometheus): enforce namespace lockdown on PromQL vector selectors - #533
Conversation
prometheus_query silently ignored heimdall.config.yaml's namespace.locked
setting: kubectl, list_namespaces, helm_release, loki_query, and
kubecost_query all code-enforce the lockdown, but prometheus_query had no
lockedNamespace field at all, letting a model query any namespace's
metrics even on an instance restricted to a single namespace.
Extracts Loki's hardened selector/matcher parser (LogQL and PromQL share
the same `{label="value"}` grammar) into selector-lockdown.ts and reuses
it for prometheus_query the same way loki_query already does — same
fail-closed behavior for missing/wildcard/mismatched namespace matchers.
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR adds shared namespace-selector validation, applies it to Prometheus queries, delegates Loki validation to it, and wires locked namespaces through Prometheus tool configuration and descriptions with corresponding tests. ChangesNamespace Lockdown
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant prometheus_query
participant makePrometheusQuery
participant runPrometheusQuery
participant validateNamespaceLockdown
participant fetch
prometheus_query->>makePrometheusQuery: submit query
makePrometheusQuery->>runPrometheusQuery: pass lockedNamespace
runPrometheusQuery->>validateNamespaceLockdown: validate selectors
validateNamespaceLockdown-->>runPrometheusQuery: allow or block
runPrometheusQuery->>fetch: execute allowed query
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request consolidates the namespace-lockdown enforcement logic for Prometheus and Loki queries into a shared utility in selector-lockdown.ts. The review feedback identifies a critical security bypass vulnerability where bare metrics or range vectors without selectors can evade the lockdown checks. Additionally, the reviewer pointed out that the parser lacks support for single-quoted string literals (which are valid in PromQL) and provided actionable suggestions to update the comment stripper, brace matcher, selector extractor, and matcher parser to handle single quotes, along with adding corresponding test coverage.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a716030f84
ℹ️ 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".
… lockdown
Two independent review bots (Gemini, Codex) confirmed a critical bypass in
the previous commit: validateNamespaceSelectorLockdown only validates the
{...} selectors it can find, but PromQL allows a bare metric name (no
braces at all) as a valid, unscoped vector selector. A query combining one
namespace-scoped selector with a bare reference via a binary/set operator
(e.g. `up{namespace="prod"} + up`, `container_memory_usage_bytes or
kube_pod_info{namespace="prod"}`) passed validation while the bare term
queried every namespace at Prometheus.
Adds a PromQL-aware scan (lib/prometheus.ts) that walks the query and
rejects it if any metric-name-shaped identifier lacks its own selector,
recognizing PromQL's aggregation operators/functions/modifiers (including
`by (...)`/`without (...)` label-list clauses, e.g. histogram_quantile's
`by (le)`) so legitimate queries aren't broken.
Also fixes the shared selector parser (selector-lockdown.ts) to treat
single-quoted strings as opaque spans everywhere double-quoted and
backtick-quoted strings already are — PromQL and LogQL both support
single-quoted string literals, and the parser previously had no way to
recognize their boundaries, the same decoy-spoofing class of bug fixed
for backticks in prior Loki hardening.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0aac41f7b
ℹ️ 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".
| i = end + 1; | ||
| continue; | ||
| } | ||
| if (PROMQL_SAFE_IDENTIFIERS.has(identifier)) continue; |
There was a problem hiding this comment.
When lockdown is active, this exemption lets a bare metric whose name collides with a PromQL function/aggregator pass as soon as the expression also contains one valid namespaced selector, e.g. up{namespace="prod"} + time currently validates even though time is an unscoped vector selector if that metric exists. Prometheus documents that a lone metric name selects all series with that name and only reserves bool, on, ignoring, group_left, and group_right as metric names (https://prometheus.io/docs/prometheus/latest/querying/basics/#instant-vector-selectors), so function names should only be skipped when they are actually used as calls/operators rather than as bare identifiers.
Useful? React with 👍 / 👎.
2 participants
Summary
The Linear
Heimdallproject's backlog and Todo/In Progress states are all empty, and there is no open-PR refactor queue, so per the fallback workflow I searched the codebase for a genuine, real-payoff gap rather than inventing a cosmetic refactor.heimdall.config.yaml'snamespace.lockedsetting is meant to be a code-enforced security boundary:kubectl,list_namespaces,helm_release,loki_query, andkubecost_queryall enforce it.prometheus_querydid not —PrometheusConfighad nolockedNamespacefield andrunPrometheusQuerynever inspected the PromQL query for a namespace matcher before executing it. An operator who locks Heimdall to namespaceprodfor compliance reasons was silently unprotected onprometheus_query: the model could querycontainer_memory_usage_bytes{namespace="other-team-secrets"}even with lockdown configured.Changes
src/lib/loki.ts) into a new shared modulesrc/lib/selector-lockdown.ts. PromQL and LogQL selectors use the same{label="value", label2=~"value2"}grammar, so the parser — already hardened through several prior follow-up fixes for comment/backtick/multi-selector spoofing — applies unchanged to PromQL.loki.tsnow re-exportsvalidateNamespaceLockdownfrom the shared module (public contract unchanged; existing Loki tests pass unmodified).src/lib/prometheus.ts: addedlockedNamespacetoPrometheusConfigand enforce it inrunPrometheusQuery, mirroring Loki's fail-closed behavior (a query with no selector, a wildcard selector, or a mismatched namespace is blocked with aBLOCKED:message).src/tools/prometheus.ts:makePrometheusQuerynow acceptslockedNamespace, threads it into the tool config, and appends aNAMESPACE LOCKDOWN ACTIVEnote to the tool description (same pattern asloki_query).prometheusPlugin.factorynow passesconfig.namespace?.lockedthrough.jaeger_query,newrelic_query, anddatadog_queryhave the same gap but use different query languages (Jaeger tag filters, NRQL, Datadog query syntax) that don't share PromQL/LogQL's selector grammar, so they're out of scope for this change — each would need its own query-language-specific enforcement.Validation
npm run typecheck— passesnpm test— full suite: 108 files / 3073 tests passingnpm run build—flue build --target nodesucceedsNot auto-verified
runPrometheusQuery/validateNamespaceLockdownfunctions and mockedfetch, consistent with this repo's existing test strategy (no real HTTP calls in tests).Generated by Claude Code
Summary by CodeRabbit
New Features
Tests