From 2ea5a4865c51a8b218b9bf5b1130f3549ae691c9 Mon Sep 17 00:00:00 2001 From: Vaibhav Zope Date: Thu, 27 Aug 2026 13:38:24 +0530 Subject: [PATCH 1/4] Fence the culler, and refuse a release that leaves a pod unfenced --- charts/openbot/templates/networkpolicy.yaml | 84 +++++++++++++++++++++ charts/openbot/values.yaml | 5 ++ scripts/check-rendered-chart.ts | 44 +++++++++++ 3 files changed, 133 insertions(+) diff --git a/charts/openbot/templates/networkpolicy.yaml b/charts/openbot/templates/networkpolicy.yaml index d188a428..fff53113 100644 --- a/charts/openbot/templates/networkpolicy.yaml +++ b/charts/openbot/templates/networkpolicy.yaml @@ -180,3 +180,87 @@ spec: {{ toYaml . | indent 4 }} {{- end }} {{- end }} + +{{- if and .Values.networkPolicy.enabled (eq .Values.computers.mode "sandbox") .Values.computers.sandbox.culler.enabled }} +--- +{{- $culler := "culler" -}} +{{/* +The culler, which is the third pod this release runs and was the only one no policy selected. + +A NetworkPolicy applies to the pods its selector matches, and a pod nothing selects is not covered +by the release's policy at all — it keeps the cluster default, which on a cluster with no +default-deny is unrestricted egress. So turning `networkPolicy.enabled` on fenced the API and the +computers and left this one open, which is the wrong way round: it mounts a service account token +bound to create, patch and delete on Sandboxes, and it carries the API's whole environment, +including `KEY_ENCRYPTION_KEY` and `BETTER_AUTH_SECRET`, because it runs the same image with the same +config. It also wakes every five minutes by default. + +Narrower than the API's policy because it does less. It reads the database and it asks the +Kubernetes API server which computers to suspend. It never calls Intelligence, an identity provider +or a Bot, so it gets no rule to the internet — the exception-list egress the API needs is exactly +what this pod should not have. + +No ingress at all: nothing connects to a CronJob. +*/}} +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: {{ include "openbot.componentName" (dict "root" . "component" $culler) }} + labels: +{{ include "openbot.componentLabels" (dict "root" . "component" $culler) | indent 4 }} +spec: + podSelector: + matchLabels: +{{ include "openbot.componentSelectorLabels" (dict "root" . "component" $culler) | indent 6 }} + policyTypes: + - Ingress + - Egress + # No ingress rules, which under an Ingress policyType denies all of it. + ingress: [] + egress: + # DNS, or the database hostname does not resolve and the sweep reads as the database being down. + - ports: + - port: 53 + protocol: UDP + - port: 53 + protocol: TCP + {{- if .Values.postgresql.enabled }} + - to: + - podSelector: + matchLabels: + app.kubernetes.io/name: postgresql + ports: + - port: 5432 + protocol: TCP + {{- end }} + {{- /* + The Kubernetes API server, which is the whole reason this pod holds a token. + + Unscoped unless a deployment says otherwise, for the same reason the API's rule is: the API + server answers on a ClusterIP from a service range no chart can know at template time. Name it + in `networkPolicy.kubernetesApiCidr` and this narrows with it. + */}} + - {{- with .Values.networkPolicy.kubernetesApiCidr }} + to: + - ipBlock: + cidr: {{ . }} + {{- end }} + ports: + - port: 443 + protocol: TCP + - port: 6443 + protocol: TCP + {{- /* + A database that is not the bundled one. + + `postgresql.enabled` off means the rule above did not render, and this pod has no rule to the + internet, so without something here it could not reach its own database and every sweep would + fail on connect. The chart already refuses `networkPolicy.enabled` with an external database + unless `networkPolicy.extraEgress` names it, so that list is where the address already is; + reusing it keeps an operator from having to write the same CIDR twice and keeps an upgrade from + silently breaking the sweep. Set `cullerExtraEgress` to give this pod a narrower list instead. + */}} + {{- with (default .Values.networkPolicy.extraEgress .Values.networkPolicy.cullerExtraEgress) }} +{{ toYaml . | indent 4 }} + {{- end }} +{{- end }} diff --git a/charts/openbot/values.yaml b/charts/openbot/values.yaml index 717bbb46..813bd87f 100644 --- a/charts/openbot/values.yaml +++ b/charts/openbot/values.yaml @@ -421,6 +421,11 @@ networkPolicy: # Where a Bot's computer may reach beyond the public internet. A deployment whose Bots must reach # an internal site adds it here, one address at a time, rather than reopening the private ranges. computerExtraEgress: [] + # Where the culler may reach beyond the bundled database and the Kubernetes API. It needs neither + # Intelligence nor an identity provider nor a Bot, so it has no rule to the internet at all. + # Left empty it falls back to `extraEgress`, which is where an external database already has to be + # named; set it to give the culler a narrower list than the API's. + cullerExtraEgress: [] podSecurityContext: runAsNonRoot: false diff --git a/scripts/check-rendered-chart.ts b/scripts/check-rendered-chart.ts index dc5de112..2885e114 100644 --- a/scripts/check-rendered-chart.ts +++ b/scripts/check-rendered-chart.ts @@ -154,6 +154,50 @@ if (serverPolicy) { } } +/** + * Every pod this release runs is covered by a policy, once any policy exists. + * + * A NetworkPolicy applies only to the pods its selector matches, and a pod nothing selects keeps the + * cluster default rather than being denied — so on a release that has policies at all, a workload + * with none is the one workload that is not fenced. That is invisible in a rendered chart and in + * `kubectl get networkpolicy`, because the policies that do exist look right. + * + * Asked of the rendered objects rather than of the templates, because the question is which pods + * came out, not which conditionals were written. + */ +const policyComponents = new Set( + documents + .filter((document) => /^kind:\s*NetworkPolicy\s*$/m.test(document)) + .flatMap((document) => { + const selector = document.split(/^\s{2}podSelector:\s*$/m)[1] ?? ""; + const found = selector.match( + /app\.kubernetes\.io\/component:\s*([\w-]+)/, + ); + return found?.[1] ? [found[1]] : []; + }), +); +if (policyComponents.size > 0) { + const workloads = documents.filter((document) => + /^kind:\s*(Deployment|StatefulSet|CronJob|Job|DaemonSet)\s*$/m.test( + document, + ), + ); + for (const workload of workloads) { + const component = workload.match( + /app\.kubernetes\.io\/component:\s*([\w-]+)/, + )?.[1]; + const name = workload.match(/^\s{2}name:\s*(\S+)/m)?.[1] ?? "a workload"; + if (!component) continue; + // The migrations Job runs once at install and is torn down; it is not a standing surface. + if (component === "migrations") continue; + if (!policyComponents.has(component)) { + problems.push( + `This release has NetworkPolicies but none selects ${name} (component: ${component}), so it is the one pod left unfenced while everything around it is restricted. Give it a policy or say in the chart why it needs none.`, + ); + } + } +} + if (problems.length > 0) { for (const problem of problems) console.error(`::error::${problem}`); process.exit(1); From d5f716b9e8650400987f5075e90d0835afc579b4 Mon Sep 17 00:00:00 2001 From: Vaibhav Zope Date: Fri, 28 Aug 2026 09:32:05 +0530 Subject: [PATCH 2/4] Fence the routines sweep the same way --- charts/openbot/templates/networkpolicy.yaml | 67 +++++++++++++++++++++ charts/openbot/values.yaml | 2 + 2 files changed, 69 insertions(+) diff --git a/charts/openbot/templates/networkpolicy.yaml b/charts/openbot/templates/networkpolicy.yaml index fff53113..0fd0370e 100644 --- a/charts/openbot/templates/networkpolicy.yaml +++ b/charts/openbot/templates/networkpolicy.yaml @@ -264,3 +264,70 @@ spec: {{ toYaml . | indent 4 }} {{- end }} {{- end }} + +{{- if and .Values.networkPolicy.enabled .Values.routines.enabled }} +--- +{{- $routines := "routines" -}} +{{/* +The routines sweep, which is the other pod a policy has to name. + +Same rule as the culler above: a NetworkPolicy applies to the pods its selector matches, so a +workload nothing selects keeps the cluster default instead of being fenced. This one arrived after +the culler's policy was written, which is the case the coverage check in +`scripts/check-rendered-chart.ts` exists to catch — it fails the render rather than letting a new +component appear unfenced. + +Narrower than the culler's, because this pod holds less. It reads the database to find what is due +and hands each run to the API server over `/internal/routines/run`; it never touches the Kubernetes +API, and `automountServiceAccountToken` is already false on it, so there is no token here to protect +and no rule for the API server to write. It calls nothing outside the cluster. + +No ingress: nothing connects to a CronJob. +*/}} +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: {{ include "openbot.componentName" (dict "root" . "component" $routines) }} + labels: +{{ include "openbot.componentLabels" (dict "root" . "component" $routines) | indent 4 }} +spec: + podSelector: + matchLabels: +{{ include "openbot.componentSelectorLabels" (dict "root" . "component" $routines) | indent 6 }} + policyTypes: + - Ingress + - Egress + # No ingress rules, which under an Ingress policyType denies all of it. + ingress: [] + egress: + # DNS, or neither the database nor the server resolves and the sweep reads as both being down. + - ports: + - port: 53 + protocol: UDP + - port: 53 + protocol: TCP + {{- if .Values.postgresql.enabled }} + - to: + - podSelector: + matchLabels: + app.kubernetes.io/name: postgresql + ports: + - port: 5432 + protocol: TCP + {{- end }} + {{- /* + The API server, which is where `SERVER_INTERNAL_URL` points and the only thing this pod calls. + A run is handed over rather than performed here, so this rule is the sweep's whole purpose. + */}} + - to: + - podSelector: + matchLabels: +{{ include "openbot.componentSelectorLabels" (dict "root" . "component" "server") | indent 14 }} + ports: + - port: {{ .Values.server.service.port }} + protocol: TCP + {{- /* An external database, for the reason given on the culler's policy above. */}} + {{- with (default .Values.networkPolicy.extraEgress .Values.networkPolicy.routinesExtraEgress) }} +{{ toYaml . | indent 4 }} + {{- end }} +{{- end }} diff --git a/charts/openbot/values.yaml b/charts/openbot/values.yaml index 813bd87f..5af91efd 100644 --- a/charts/openbot/values.yaml +++ b/charts/openbot/values.yaml @@ -426,6 +426,8 @@ networkPolicy: # Left empty it falls back to `extraEgress`, which is where an external database already has to be # named; set it to give the culler a narrower list than the API's. cullerExtraEgress: [] + # The same, for the routines sweep. It reaches the database and the API server and nothing else. + routinesExtraEgress: [] podSecurityContext: runAsNonRoot: false From 768fac8f9ccce58d1d58a39b327ea7a76bb2d8e6 Mon Sep 17 00:00:00 2001 From: Vaibhav Zope Date: Fri, 28 Aug 2026 09:53:55 +0530 Subject: [PATCH 3/4] Count only the workloads a component label names --- scripts/check-new-values-keys.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scripts/check-new-values-keys.ts b/scripts/check-new-values-keys.ts index f87538c1..e7309e45 100644 --- a/scripts/check-new-values-keys.ts +++ b/scripts/check-new-values-keys.ts @@ -357,11 +357,23 @@ for (const { path, component, field } of fieldFallbacks) { /* * Found by its component label rather than by name, because a name is the release name plus a * suffix and this check would then be pinned to both. + * + * Narrowed to the kinds that carry a pod, because the label is not unique to one: a NetworkPolicy + * naming the same component carries it too, and counting that as a second carrier fails a check + * about a CronJob field for a reason that has nothing to do with the field. */ + const WORKLOAD_KINDS = new Set([ + "CronJob", + "DaemonSet", + "Deployment", + "Job", + "StatefulSet", + ]); const carriers = parseAllDocuments(attempt.out) .map((document) => document.toJS() as unknown) .filter( (resource) => + WORKLOAD_KINDS.has(String(valueAt(resource, ["kind"]))) && valueAt(resource, [ "metadata", "labels", From f35083a30bf2c4536bae45eed62095e8e44125d9 Mon Sep 17 00:00:00 2001 From: Vaibhav Zope Date: Fri, 28 Aug 2026 10:17:36 +0530 Subject: [PATCH 4/4] Say the API-server rule's reach, and cut the comments back --- charts/openbot/templates/networkpolicy.yaml | 60 ++------------------- charts/openbot/values.yaml | 8 ++- scripts/check-new-values-keys.ts | 4 +- scripts/check-rendered-chart.ts | 13 +---- 4 files changed, 11 insertions(+), 74 deletions(-) diff --git a/charts/openbot/templates/networkpolicy.yaml b/charts/openbot/templates/networkpolicy.yaml index 0fd0370e..ef80cb8f 100644 --- a/charts/openbot/templates/networkpolicy.yaml +++ b/charts/openbot/templates/networkpolicy.yaml @@ -184,24 +184,7 @@ spec: {{- if and .Values.networkPolicy.enabled (eq .Values.computers.mode "sandbox") .Values.computers.sandbox.culler.enabled }} --- {{- $culler := "culler" -}} -{{/* -The culler, which is the third pod this release runs and was the only one no policy selected. - -A NetworkPolicy applies to the pods its selector matches, and a pod nothing selects is not covered -by the release's policy at all — it keeps the cluster default, which on a cluster with no -default-deny is unrestricted egress. So turning `networkPolicy.enabled` on fenced the API and the -computers and left this one open, which is the wrong way round: it mounts a service account token -bound to create, patch and delete on Sandboxes, and it carries the API's whole environment, -including `KEY_ENCRYPTION_KEY` and `BETTER_AUTH_SECRET`, because it runs the same image with the same -config. It also wakes every five minutes by default. - -Narrower than the API's policy because it does less. It reads the database and it asks the -Kubernetes API server which computers to suspend. It never calls Intelligence, an identity provider -or a Bot, so it gets no rule to the internet — the exception-list egress the API needs is exactly -what this pod should not have. - -No ingress at all: nothing connects to a CronJob. -*/}} +{{/* A pod no policy selects keeps the cluster default, so this one was the release's only unfenced one. */}} apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: @@ -233,13 +216,7 @@ spec: - port: 5432 protocol: TCP {{- end }} - {{- /* - The Kubernetes API server, which is the whole reason this pod holds a token. - - Unscoped unless a deployment says otherwise, for the same reason the API's rule is: the API - server answers on a ClusterIP from a service range no chart can know at template time. Name it - in `networkPolicy.kubernetesApiCidr` and this narrows with it. - */}} + {{- /* Empty kubernetesApiCidr permits these ports to any address, as the server's rule above does. */}} - {{- with .Values.networkPolicy.kubernetesApiCidr }} to: - ipBlock: @@ -250,16 +227,7 @@ spec: protocol: TCP - port: 6443 protocol: TCP - {{- /* - A database that is not the bundled one. - - `postgresql.enabled` off means the rule above did not render, and this pod has no rule to the - internet, so without something here it could not reach its own database and every sweep would - fail on connect. The chart already refuses `networkPolicy.enabled` with an external database - unless `networkPolicy.extraEgress` names it, so that list is where the address already is; - reusing it keeps an operator from having to write the same CIDR twice and keeps an upgrade from - silently breaking the sweep. Set `cullerExtraEgress` to give this pod a narrower list instead. - */}} + {{- /* An external database, which extraEgress already had to name; cullerExtraEgress narrows it. */}} {{- with (default .Values.networkPolicy.extraEgress .Values.networkPolicy.cullerExtraEgress) }} {{ toYaml . | indent 4 }} {{- end }} @@ -268,22 +236,7 @@ spec: {{- if and .Values.networkPolicy.enabled .Values.routines.enabled }} --- {{- $routines := "routines" -}} -{{/* -The routines sweep, which is the other pod a policy has to name. - -Same rule as the culler above: a NetworkPolicy applies to the pods its selector matches, so a -workload nothing selects keeps the cluster default instead of being fenced. This one arrived after -the culler's policy was written, which is the case the coverage check in -`scripts/check-rendered-chart.ts` exists to catch — it fails the render rather than letting a new -component appear unfenced. - -Narrower than the culler's, because this pod holds less. It reads the database to find what is due -and hands each run to the API server over `/internal/routines/run`; it never touches the Kubernetes -API, and `automountServiceAccountToken` is already false on it, so there is no token here to protect -and no rule for the API server to write. It calls nothing outside the cluster. - -No ingress: nothing connects to a CronJob. -*/}} +{{/* Same reason as the culler; this pod holds no token and reaches only the database and the API server. */}} apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: @@ -315,10 +268,7 @@ spec: - port: 5432 protocol: TCP {{- end }} - {{- /* - The API server, which is where `SERVER_INTERNAL_URL` points and the only thing this pod calls. - A run is handed over rather than performed here, so this rule is the sweep's whole purpose. - */}} + {{- /* The API server, where SERVER_INTERNAL_URL points and the only thing this pod calls. */}} - to: - podSelector: matchLabels: diff --git a/charts/openbot/values.yaml b/charts/openbot/values.yaml index 5af91efd..7bd6f2cc 100644 --- a/charts/openbot/values.yaml +++ b/charts/openbot/values.yaml @@ -421,12 +421,10 @@ networkPolicy: # Where a Bot's computer may reach beyond the public internet. A deployment whose Bots must reach # an internal site adds it here, one address at a time, rather than reopening the private ranges. computerExtraEgress: [] - # Where the culler may reach beyond the bundled database and the Kubernetes API. It needs neither - # Intelligence nor an identity provider nor a Bot, so it has no rule to the internet at all. - # Left empty it falls back to `extraEgress`, which is where an external database already has to be - # named; set it to give the culler a narrower list than the API's. + # Extra egress for the culler. Empty falls back to `extraEgress`, where an external database is + # already named; set it to give the culler a narrower list. cullerExtraEgress: [] - # The same, for the routines sweep. It reaches the database and the API server and nothing else. + # The same, for the routines sweep. routinesExtraEgress: [] podSecurityContext: diff --git a/scripts/check-new-values-keys.ts b/scripts/check-new-values-keys.ts index e7309e45..f9e950e3 100644 --- a/scripts/check-new-values-keys.ts +++ b/scripts/check-new-values-keys.ts @@ -358,9 +358,7 @@ for (const { path, component, field } of fieldFallbacks) { * Found by its component label rather than by name, because a name is the release name plus a * suffix and this check would then be pinned to both. * - * Narrowed to the kinds that carry a pod, because the label is not unique to one: a NetworkPolicy - * naming the same component carries it too, and counting that as a second carrier fails a check - * about a CronJob field for a reason that has nothing to do with the field. + * Narrowed to pod-carrying kinds: a NetworkPolicy naming the same component carries the label too. */ const WORKLOAD_KINDS = new Set([ "CronJob", diff --git a/scripts/check-rendered-chart.ts b/scripts/check-rendered-chart.ts index 2885e114..171f899f 100644 --- a/scripts/check-rendered-chart.ts +++ b/scripts/check-rendered-chart.ts @@ -154,17 +154,8 @@ if (serverPolicy) { } } -/** - * Every pod this release runs is covered by a policy, once any policy exists. - * - * A NetworkPolicy applies only to the pods its selector matches, and a pod nothing selects keeps the - * cluster default rather than being denied — so on a release that has policies at all, a workload - * with none is the one workload that is not fenced. That is invisible in a rendered chart and in - * `kubectl get networkpolicy`, because the policies that do exist look right. - * - * Asked of the rendered objects rather than of the templates, because the question is which pods - * came out, not which conditionals were written. - */ +// A pod no policy selects keeps the cluster default, so on a release that has policies it is the +// only unfenced one. Asked of rendered objects, because the question is which pods came out. const policyComponents = new Set( documents .filter((document) => /^kind:\s*NetworkPolicy\s*$/m.test(document))