diff --git a/CLAUDE.md b/CLAUDE.md index f820a7a..1e56490 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,7 +116,7 @@ The binding style guide for all current and future charts lives in **README.md ( - Every container offers an additive `env` block rendered through `tpl` with `value`, `secretKeyRef`, and `configMapKeyRef` forms. - Hardening: `automountServiceAccountToken: false`, non-root, `readOnlyRootFilesystem: true`, caps dropped, `allowPrivilegeEscalation: false`; contexts values-configurable (`securityContext.pod` / `securityContext.container`); writable paths as emptyDirs; image-forced deviations MUST be documented with a comment in the chart's `values.yaml`. - Probes: liveness/readiness/startup on every workload (exception: paperless consume CronJob), values-configurable with calibrated defaults. -- **Every chart renders a `NetworkPolicy`, `networkPolicy.enabled` defaults to `true`** (#5); adding one to an existing chart is a **major**. Values: `enabled`, `allowedNamespaces`, `probeSources`, `ingressController`, `egress.extraPorts` — and only the ones that actually reach a rule, because a value that does nothing reads as a control. Each port falls in one of four classes and the class decides the rule: a port a **network** probe addresses stays open (`no from`) until `probeSources` is set, because the kubelet probes from the worker node's IP and no chart can know it; a NodePort-published port stays open (the source is already masqueraded); anything else is restricted to the namespace, `allowedNamespaces` and — for HTTP — `ingressController`; a replica-to-replica bus gets a bare `podSelector`. **`exec` probes are not network probes** (no packet, so the port can be restricted — `valkey`, `minecraft-server`). **Pod ports, never NodePorts.** Never `from: []`. Reasoning in Go template comments, never `#` (those ship to the cluster), and such a comment must not spell out its own closing delimiter. Egress is by **port, never destination** — Canal has no FQDN rule and Calico-specific objects are out of scope — so most dependency ports are the *standard* ones and a dependency elsewhere is blocked; `policyTypes` names only what there is something to restrict (`valkey`, `homeassistant`, `satisfactory` are Ingress-only, each argued in its own file). `ci/run-networkpolicy-test.sh` is the gate. Full rules: README, "NetworkPolicies". +- **Every chart renders a `NetworkPolicy`, `networkPolicy.enabled` defaults to `true`** (#5); adding one to an existing chart is a **major**. Values: `enabled`, `allowedNamespaces`, `probeSources`, `ingressController`, `egress.extraPorts` — and only the ones that actually reach a rule, because a value that does nothing reads as a control. Each port falls in one of four classes and the class decides the rule: a port a **network** probe addresses stays open (`no from`) until `probeSources` is set, because the kubelet probes from the worker node's IP and no chart can know it; a NodePort-published port stays open (the source is already masqueraded); anything else is restricted to the namespace, `allowedNamespaces` and — for HTTP — `ingressController`; a replica-to-replica bus gets a bare `podSelector`. **`exec` probes are not network probes** (no packet, so the port can be restricted — `valkey`, `minecraft-server`). **`allowedNamespaces` is never shipped pre-filled** — a chart must not bake in an assumption about this cluster's namespace layout, so restrictive is the default and filling it is the deployment adaptation (which is why a sweep produces release-side one-liners: that is the procedure, not a defect list). **A policy applies to traffic that came through a Service** — a Service is only destination NAT and the policy acts at the pod interface; what changes is the *source address*, preserved for pod→ClusterIP→pod (this is why the `ingressController` rule works) and masqueraded to a node for NodePort traffic. **And the whole convention depends on one setting it cannot check: Traefik must not run with `hostNetwork`** — measured, its pods have real pod IPs in `10.42.0.0/16`; on host networking they stop being selectable as pods and every web chart goes dark at once while every pod stays `Ready`. **Pod ports, never NodePorts.** Never `from: []`. Reasoning in Go template comments, never `#` (those ship to the cluster), and such a comment must not spell out its own closing delimiter. Egress is by **port, never destination** — Canal has no FQDN rule and Calico-specific objects are out of scope — so most dependency ports are the *standard* ones and a dependency elsewhere is blocked; `policyTypes` names only what there is something to restrict (`valkey`, `homeassistant`, `satisfactory` are Ingress-only, each argued in its own file). `ci/run-networkpolicy-test.sh` is the gate. Full rules: README, "NetworkPolicies". - Resources: requests as values defaults; limits auto-computed by the `_helpers.tpl` helper (`limitFactor` default 3; no CPU limits unless set explicitly). - Replicas per workload (explicit `0` allowed) plus a chart-global `scaleDown` flag that overrides them. - `icon:` mandatory in every `Chart.yaml`; `helm lint --strict` without findings; CI fixtures under `ci//` or a justified exclusion. diff --git a/README.md b/README.md index b83e5be..4ed1ffd 100644 --- a/README.md +++ b/README.md @@ -692,6 +692,10 @@ Every port of a workload falls into one of four, and this is the part to get rig `networkPolicy.enabled`, `.allowedNamespaces`, `.probeSources`, `.ingressController`, `.egress.extraPorts` — and a chart ships **only the ones that reach a rule**. A value that silently does nothing is worse than one that is absent, because it reads as a control: `samba` and `satisfactory` carry `enabled` alone (every inbound rule is open to any source), and `teamspeak-server` has `allowedNamespaces` but no `probeSources`. +**No chart ships `allowedNamespaces` pre-filled, ever.** A chart that named `mc-router`, or `paperless-jk`, or `stalwart` in its defaults would be baking in an assumption about *this* cluster's namespace layout — which is exactly the kind of claim a chart cannot keep true. So the default is the empty list and the chart is **restrictive**; filling it is the **deployment adaptation**, made where the namespace layout is actually visible. That is why a sweep like this produces a list of release-side one-liners: those are the expected procedure, not a defect list, and the breaking-change major on each chart is what announces them. + +`ingressController` is the one exception to that, and only because it names a **cluster component** rather than a tenant: its default is measured below, and it is still overridable. + `ingressController` is a separate value with two keys rather than a line in `allowedNamespaces` for one reason: **its two selectors render as one `from` entry, which is an AND** — those pods in that namespace. Written as two list entries they would be an OR, and putting `kube-system` into `allowedNamespaces` instead would open the port to every pod in the cluster's own namespace. Its default is **measured, not assumed**: `ingress-nginx` is gone from this cluster, the only `IngressClass` is `traefik`, and the controller is the DaemonSet `rke2-traefik` in `kube-system` — which also serves the Gateway API here, so one source covers the Ingress and the HTTPRoute path alike. `ingress.className` says nothing about it: a class names an implementation, not the pods the traffic comes from. #### What these policies do not do — and it belongs in every chart, not just here @@ -718,3 +722,12 @@ Every port of a workload falls into one of four, and this is the part to get rig - **Never `from: []`.** It allows everything, identically to having no key, while reading as a restriction. Write no `from` where open is meant and say so in a template comment. - **Reasoning goes in Go template comments, not `#` comments.** A `#` comment is part of the rendered object: it travels into the cluster, appears in `helm get manifest` and `kubectl get -o yaml`, and lands in every release diff. The render test fails on a rendered `#`. One second-order trap, found while writing the reference implementation: prose inside a template comment must not spell out the comment's own closing delimiter, or the parser ends the comment early and the rest is emitted as manifest content — the render then fails with `did not find expected node content`, pointing at the template and not at the sentence. - **A conditional `containerPort` needs a conditional rule.** `minecraft-server`'s RCON port is rendered only when `server.rcon.enabled` is true; a rule without the same gate names a port the pod does not declare, and the render test rejects it. +- **A policy applies to traffic that arrived through a Service, and that is why the `ingressController` rule works at all.** A `NetworkPolicy` is evaluated at the pod's network interface and a Service is only destination NAT, so going through a ClusterIP does not exempt anything. What the Service *does* change is the **source address**: pod → ClusterIP → pod **preserves** it, which is what lets a `podSelector` match the Traefik pod or a `namespaceSelector` match a consumer's namespace. Traffic from outside through a `NodePort` with the default `externalTrafficPolicy: Cluster` is **masqueraded to a node**, which is why those rules cannot be restricted by source at all and are left open instead (`stalwart`'s template carries that second half; the first half is the load-bearing one). + +#### The one cluster setting this convention depends on and does not control + +**Traefik must not run with `hostNetwork`.** Measured on 2026-10-14: the `rke2-traefik` DaemonSet has no `hostNetwork`, and its pods carry real pod IPs (`10.42.3.120`, `10.42.4.94`, `10.42.0.187` — the pod CIDR is `10.42.0.0/16`, nodes are on entirely different addresses). **Only because of that does a `podSelector` match it**, and the `ingressController` value is built on exactly that. + +If Traefik were ever switched to host networking, its pods would share the node's network namespace, they would no longer be selectable as pods, the `ingressController` entry would match nothing — and **every web chart in this repository would go dark at once**, in the restricted configuration. The policies would still be accepted, still read as correct, and deny all web traffic. + +This is a dependency on a cluster setting this repository does not own, in the same class as the `--enable-certificate-owner-ref` case from #126: nothing here can detect it, and nothing here can fail usefully on it. It is written down in both places someone would look — here, and in the header of `template-chart/templates/xxx.networkpolicy.yaml` — so that whoever changes the ingress controller's networking finds the consequence attached to the thing they are changing. The symptom to recognise: every chart's web traffic stops at once while every pod stays `Ready`, because the probe path does not go through the controller. diff --git a/ci/networkpolicy/gotenberg.yaml b/ci/networkpolicy/gotenberg.yaml new file mode 100644 index 0000000..27237b0 --- /dev/null +++ b/ci/networkpolicy/gotenberg.yaml @@ -0,0 +1,32 @@ +# Overlay for the NetworkPolicy render test (Gitea #5), applied last. +# +# Two code paths this chart's own defaults never reach, and both are held +# against the four assertions on every pull request rather than checked once by +# hand: +# +# 1. networkPolicy.egress.remoteAssets: true - the named switch that lets +# Chromium fetch over 80/443. Its default is OFF, so the rule it renders +# existed in no render at all. It is an EGRESS rule, which is exactly why it +# is worth asserting: assertions 2 and 3 look at INGRESS rules, so a mistake +# on the egress side has no mechanical check except that the render still +# parses and the policy still holds together. +# +# 2. The source-restricted probe rule, which only appears once probeSources is +# non-empty - the same branch ci/networkpolicy/template-chart.yaml exists +# for, and the one the overlay guard demands of any chart that offers the +# value. It carries the expensive mistake: a namespaceSelector and a +# podSelector written as two list entries instead of one would be an OR. +# +# Note what this overlay does NOT do: it changes no release. gotenberg's +# install-test uses ci/gotenberg/test-values.yaml, which this file does not +# touch - the policy installed there keeps the restrictive default, because +# that is the default the convention ships. +networkPolicy: + egress: + remoteAssets: true + # Not the real node addresses of any cluster; documentation CIDRs + # (RFC 5737 TEST-NET-1) so nobody can mistake these for something to copy. + probeSources: + - 192.0.2.11/32 + allowedNamespaces: + - paperless-jk diff --git a/gotenberg/templates/NOTES.txt b/gotenberg/templates/NOTES.txt index aba73bb..b41b9e2 100644 --- a/gotenberg/templates/NOTES.txt +++ b/gotenberg/templates/NOTES.txt @@ -86,14 +86,42 @@ NETWORKPOLICY IS ACTIVE — WHAT IT CAPS, AND WHAT IT DOES NOT {{ .Release.Name }}-{{ .Chart.Name }}-networkpolicy selects this workload's pods. Inbound: TCP 3000 only. Outbound: DNS{{ with .Values.networkPolicy.egress }}{{ with .extraPorts }} plus TCP {{ join ", " . }}{{ end }}{{ end }}. - EGRESS IS DNS ONLY ON PURPOSE, AND IT COSTS SOMETHING. Gotenberg drives a +{{- /* Null-safe on purpose: networkPolicy.egress may be erased to null (the + schema permits it), and a bare .egress.remoteAssets then fails the whole + render with "nil pointer evaluating interface {}.remoteAssets" - an + error naming this file while the operator's value appears nowhere in it. + Same rule as issue #99: an erased block means "the default applies", not + a broken render. */}} +{{- if (.Values.networkPolicy.egress | default dict).remoteAssets }} + REMOTE ASSETS ARE ALLOWED IN THIS RELEASE (networkPolicy.egress.remoteAssets: + true), so Chromium may fetch over TCP 80 and 443 to any destination. Know + what that opens: this service is reachable from the whole cluster by default + (3000 is the probe port), so anything that can POST here can now have + Chromium fetch an arbitrary URL — including one inside the cluster. Switch it + back off if nothing in this release renders HTML that references remote + images. +{{- else }} + EGRESS IS DNS ONLY HERE, AND IT COSTS SOMETHING SILENTLY. Gotenberg drives a headless Chromium, so an egress rule for 80/443 would turn a service the - whole cluster can POST to into a way of fetching arbitrary URLs. The price is - that HTML conversion loses remote assets: inline content converts, images the - page would have fetched silently do not. That degrades a PDF instead of - failing a pod, so nothing will alert on it. If this release converts URLs or - needs remote assets, set networkPolicy.egress.extraPorts: [80, 443] — and - know what it opens. + whole cluster can POST to into a way of fetching arbitrary URLs — which is + why the default is off. + + What it costs: in a rendered HTML document, EMBEDDED images are MIME parts + inside the file and convert fine, but REMOTE images () + are fetched by Chromium at render time and are SILENTLY DROPPED. The PDF is + produced and is simply missing them. Nothing fails, nothing is logged here, + no probe notices. + + If this release needs them — a paperless-ngx rendering mail bodies that + reference remote pictures is the case — set + + networkPolicy: + egress: + remoteAssets: true + + and not extraPorts: [80, 443]. The named value says why the ports are open; + a port list cannot. +{{- end }} THIS CHART HAS NO INGRESS AND ITS CONSUMERS ARE OTHER PODS. While probeSources is empty the 3000 rule has no source selector; narrowing it diff --git a/gotenberg/templates/gotenberg.networkpolicy.yaml b/gotenberg/templates/gotenberg.networkpolicy.yaml index 8648258..8ad7d24 100644 --- a/gotenberg/templates/gotenberg.networkpolicy.yaml +++ b/gotenberg/templates/gotenberg.networkpolicy.yaml @@ -4,16 +4,22 @@ NetworkPolicy for gotenberg (Gitea #5). Gotenberg here is a cluster-internal service: no Ingress, and its consumers are other pods (paperless-ngx converts mail HTML to PDF through it). -EGRESS IS DNS ONLY, AND THAT IS A DELIBERATE, VISIBLE TRADE rather than an -oversight. Gotenberg drives a headless Chromium, so an egress rule for 80 and -443 would let anything that can POST to this service fetch arbitrary URLs -through it - a textbook SSRF pivot, and the pod is reachable from the whole -cluster by default because 3000 is the probe port. Blocking it costs the -remote-asset half of HTML conversion: a converted page keeps its inline content -and loses images it would have fetched, which degrades a PDF rather than -failing a pod. The degradation is silent, which is why it is named here, in -values.yaml and in NOTES.txt; the fix for a deployment that needs it is -networkPolicy.egress.extraPorts: [80, 443], chosen knowingly. +EGRESS IS DNS ONLY BY DEFAULT, AND THAT IS A DELIBERATE, VISIBLE TRADE rather +than an oversight. Gotenberg drives a headless Chromium, so an egress rule for +80 and 443 would let anything that can POST to this service fetch arbitrary +URLs through it - a textbook SSRF pivot, and the pod is reachable from the +whole cluster by default because 3000 is the probe port. Blocking it costs the +remote-asset half of HTML conversion: a converted page keeps its EMBEDDED +content and loses the images it would have FETCHED, which degrades a PDF rather +than failing a pod. The degradation is silent, which is why it is named here, in +values.yaml and in NOTES.txt. + +A deployment that needs those images sets networkPolicy.egress.remoteAssets: +true - a named switch rather than a line in extraPorts, so the trade is a +decision someone made and not a port number nobody can place. Its default is +off, because restrictive-by-default is the convention and the deployment is +where reality is accommodated. The long form of what it buys, and of the one +thing about it that was NOT verified, is on the rule itself. WHAT THIS OBJECT IS: a ceiling on reachability, not an authentication layer. Only the ports declared below are reachable at all - whatever else the process @@ -130,6 +136,49 @@ spec: - protocol: TCP port: 53 {{- with $np.egress }} + {{- if .remoteAssets }} + {{- /* + REMOTE ASSETS: the web, on both ports, to any destination. Off by default, + and the switch exists because the cost of leaving it off is invisible while + the cost of removing it is not. + + WHAT IT BUYS, measured on the live cluster rather than assumed. paperless-ngx + splits a mail into two steps with two separate endpoints, both + cluster-internal: the .eml goes to TIKA for parsing + (PAPERLESS_TIKA_ENDPOINT) and the resulting HTML comes here for rendering + (PAPERLESS_TIKA_GOTENBERG_ENDPOINT). Of the images in that HTML: + + - EMBEDDED images are MIME parts INSIDE the file. Neither Tika nor + Gotenberg needs the network for them, which is why apache-tika stays + DNS-only and why this switch is not needed for a mail that carries its + pictures with it. + - REMOTE images ( in the mail body) are fetched by + CHROMIUM, here, at render time. With this switch off they are silently + dropped: the PDF is produced, it is just missing them. Nothing fails, + nothing is logged by this chart, and no probe notices. + + NOT CHECKED, and not claimed: whether paperless-ngx re-embeds the images + Tika extracted before handing the HTML over. If it does not, embedded + images would be lost as well - but that would be pre-existing paperless + behaviour and not this policy, so do not read the switch as a fix for it. + + WHY IT IS A SWITCH AND NOT THE DEFAULT. Gotenberg drives a headless + Chromium and 3000 is its probe port, so the service is reachable from the + whole cluster by default. Allowing 80/443 outbound therefore turns anything + that can POST here into a way of fetching arbitrary URLs - an SSRF pivot + with the cluster's whole internal address space behind it. That argument is + unchanged by the switch; the switch only makes the trade a NAMED decision + with a default of "no" instead of a line in extraPorts that reads like + nothing in particular. + */}} + - ports: + - protocol: TCP + port: 80 + - protocol: TCP + port: 443 + {{- end }} + {{- end }} + {{- with $np.egress }} {{- with .extraPorts }} {{- /* Ports only, to any destination - see the header. This is the operator's diff --git a/gotenberg/values.schema.json b/gotenberg/values.schema.json index bad0276..e450c7a 100644 --- a/gotenberg/values.schema.json +++ b/gotenberg/values.schema.json @@ -35,6 +35,10 @@ "type": ["object", "null"], "additionalProperties": false, "properties": { + "remoteAssets": { + "description": "Lets Chromium fetch over TCP 80 and 443 to any destination, for the REMOTE images in a rendered HTML document (). Off by default: Gotenberg drives a headless Chromium and 3000 is its probe port, so the service is reachable from the whole cluster, and allowing outbound web access turns anything that can POST here into a way of fetching arbitrary URLs. EMBEDDED images are MIME parts inside the file and need no network at all, which is the case this switch is NOT for. With it off, remote images are silently dropped - the PDF is produced and is simply missing them.", + "type": "boolean" + }, "extraPorts": { "description": "Additional outbound TCP ports, to ANY destination - ports and not destinations, because a NetworkPolicy cannot express a hostname. DNS is always allowed and has no switch. Without an entry here an outbound port the chart does not already derive is blocked, which is the cap's whole purpose and also its upgrade risk.", "type": ["array", "null"], diff --git a/gotenberg/values.yaml b/gotenberg/values.yaml index 619c827..706fb5f 100644 --- a/gotenberg/values.yaml +++ b/gotenberg/values.yaml @@ -141,10 +141,43 @@ networkPolicy: # namespace and allowedNamespaces to the same rule for exactly that reason. probeSources: [] egress: + # REMOTE ASSETS: lets Chromium fetch over TCP 80 and 443, to any + # destination. OFF by default, and both halves of that are deliberate. + # + # WHAT IT BUYS, measured rather than assumed. paperless-ngx splits a mail + # into two steps with two separate cluster-internal endpoints: the .eml goes + # to TIKA for parsing (PAPERLESS_TIKA_ENDPOINT) and the resulting HTML comes + # here for rendering (PAPERLESS_TIKA_GOTENBERG_ENDPOINT). Of the images in + # that HTML: + # - EMBEDDED images are MIME parts INSIDE the file. Neither Tika nor + # Gotenberg needs the network for them - which is why apache-tika stays + # DNS-only, and why a mail that carries its pictures with it needs + # nothing from this switch. + # - REMOTE images ( in the body) are fetched by + # CHROMIUM, here, at render time. With this off they are SILENTLY + # DROPPED: the PDF is produced and is simply missing them. Nothing + # fails, nothing is logged by this chart, no probe notices. + # + # NOT CHECKED, and deliberately not claimed: whether paperless-ngx re-embeds + # the images Tika extracted before handing the HTML over. If it does not, + # embedded images would be lost as well - but that would be pre-existing + # paperless behaviour and not this policy. Do not read this switch as a fix + # for it. + # + # WHY IT IS OFF BY DEFAULT. Gotenberg drives a headless Chromium and 3000 is + # its probe port, so the service is reachable from the whole cluster by + # default. Allowing 80/443 outbound therefore turns anything that can POST + # here into a way of fetching arbitrary URLs - an SSRF pivot with the + # cluster's internal address space behind it. Switching this on is a trade, + # and it should be one somebody made on purpose. + remoteAssets: false # Additional outbound TCP ports. DNS (UDP and TCP 53) is always allowed and # has no switch - a knob whose only other setting breaks every name lookup # in the pod is a trap, not a feature. # + # Use remoteAssets above rather than putting 80 and 443 here: the named + # value says WHY the ports are open, and this list cannot. + # # PORTS, NOT DESTINATIONS, and that is a limit of the object rather than # laziness: a NetworkPolicy matches CIDRs and selectors and never hostnames, # so "only to the database host" cannot be written at all. Each entry diff --git a/template-chart/templates/xxx.networkpolicy.yaml b/template-chart/templates/xxx.networkpolicy.yaml index 512df6c..023f1fd 100644 --- a/template-chart/templates/xxx.networkpolicy.yaml +++ b/template-chart/templates/xxx.networkpolicy.yaml @@ -52,6 +52,37 @@ chart rather than quietly assumed: (Canal, 6/6) and for the CI cluster (k3d, which brings k3s' own policy controller) - verify it rather than reading it off a value. +ONE CLUSTER SETTING THIS WHOLE CONVENTION DEPENDS ON AND CANNOT CHECK: +TRAEFIK MUST NOT RUN WITH hostNetwork. + +Measured on 2026-10-14: the rke2-traefik DaemonSet has no hostNetwork, and its +pods carry real pod IPs (10.42.3.120, 10.42.4.94, 10.42.0.187; the pod CIDR is +10.42.0.0/16, while the nodes sit on entirely different addresses). ONLY +BECAUSE OF THAT does the podSelector in networkPolicy.ingressController match +it. + +Switch the ingress controller to host networking and its pods share the node's +network namespace. They are then not selectable as pods at all, the +ingressController entry matches nothing, and EVERY WEB CHART IN THIS REPOSITORY +GOES DARK AT ONCE in the restricted configuration - while every policy is +still accepted and still reads as correct. The symptom to recognise is exactly +that shape: all web traffic stops together and every pod stays Ready, because +the probe path does not run through the controller. + +Nothing here can detect this and nothing here can fail usefully on it, which is +the same class of dependency as --enable-certificate-owner-ref in #126. It is +written here, on the thing that depends on it, so whoever changes the +controller's networking meets the consequence. + +AND A POLICY DOES APPLY TO TRAFFIC THAT CAME THROUGH A SERVICE, which is why +the ingressController rule works in the first place. A Service is only +destination NAT, and the policy is evaluated at the pod's interface, so a +ClusterIP exempts nothing. What the Service changes is the SOURCE ADDRESS: pod +-> ClusterIP -> pod PRESERVES it, so a podSelector or a namespaceSelector can +still match the sender. Traffic arriving from outside through a NodePort with +the default externalTrafficPolicy: Cluster is MASQUERADED to a node, which is +why such rules cannot be restricted by source at all and are left open instead. + THE PORT NUMBERS HERE ARE POD PORTS, NEVER NODEPORTS. A NetworkPolicy is evaluated at the POD's network interface, after kube-proxy has already translated a NodePort to the targetPort. Writing 30080 where the pod listens