docs(netpol): correct three comments that assert a boundary Cilium does not enforce (REN-108) #997

2026-09-26T00:26:34Z

Ghost commented

2026-09-26 00:26:34 +00:00

What changed

Comments only, in three files. No spec change, no pod template change, no rendered manifest change.

Three comments in the tree assert that the ingress side of paperless and navidrome is locked to their Authentik gate outposts. Cilium does not enforce that. default-deny-ingress for both namespaces uses endpointSelector: {}, so it selects the app pods too, and Cilium unions the allow rules of every CNP selecting an endpoint — the same mechanism kubernetes/apps/paperless/network-policy.yaml already names and relies on. The effective ingress set is the outpost plus whole namespaces.

kubernetes/apps/paperless/network-policy.yaml — "Lock the ingress side to the gate outpost only" was false. Replaced with the enforced set (paperless, database, monitoring, host/remote-node), each line citing the rule in paperless-ns.yaml.
kubernetes/infrastructure/network-policies/paperless-ns.yaml — the existing note reasons that traefik is excluded because admitting it "would broaden the trust boundary off the gate outpost", and then the next two rules admit the whole database and monitoring namespaces, which broadens it exactly that way. Appended a note saying so, and why both allows are kept (CNPG reaches the postgres cluster from database; the blackbox probe of http://paperless.paperless.svc.cluster.local:80/accounts/login/ at monitoring-rules/probes-blackbox.yaml:250 runs from monitoring).
kubernetes/apps/navidrome/deployment.yaml — "the CNP admits only the outpost and Traefik" was false; navidrome-ns.yaml also admits the lidarr-gate outpost (which, with endpointSelector: {}, reaches navidrome's listener, not just lidarr's), the whole monitoring namespace, and host/remote-node. Also recorded that ND_EXTAUTH_TRUSTEDSOURCES is the entire pod CIDR and is therefore not a narrowing, and why it deliberately stays that way.

Why

Because both apps do header-based remote-user auth, any pod that can open a TCP connection to their HTTP listener can set X-Authentik-Username to any value. The comments are what the next person reasons from, so a comment that overstates the boundary is the most expensive part of the finding. Narrowing the CNPs is not in this PR — it is tracked in REN-28 and needs the owner's decision plus a measured allowlist first.

Deviation from the ticket text, flagged

REN-108 specifies the paperless comment citing paperless-ns.yaml:21-23 / 24-26 / 27-29. The A2 change in this same PR inserts 9 comment lines above those rules, so those citations would land on the wrong lines the moment this PR merges. I shipped 30-32 / 33-35 / 36-38, which is where the rules actually are at this commit. Everything else is the specified text verbatim.

How it was verified

kubectl kustomize (client v1.31.0, kustomize v5.4.2) run over all 68 kustomization.yaml roots in the repo, at origin/master (3d6c9940) and at this branch; all 68 render successfully and every one is byte-identical (sha256 per root, no diff). This is the strongest available evidence that the change cannot affect traffic.
Every added and removed line in git diff begins with # — machine-checked, no non-comment line changed.
Not applied to the cluster, and it does not need to be: Flux will reconcile it as a no-op because the rendered output is unchanged.

Blast radius

None. If the rendered output is byte-identical, there is nothing for Flux to apply and nothing for Cilium to re-evaluate. No workload restart, no ingress controller change, no DNS change.

Revert

git revert <merge commit> (or git revert a557eea7) — restores master 3d6c9940 exactly. Comments only, so a revert is also traffic-free.

Refs REN-108, REN-28.

## What changed Comments only, in three files. No `spec` change, no pod template change, no rendered manifest change. Three comments in the tree assert that the ingress side of paperless and navidrome is locked to their Authentik gate outposts. Cilium does not enforce that. `default-deny-ingress` for both namespaces uses `endpointSelector: {}`, so it selects the app pods too, and Cilium **unions** the allow rules of every CNP selecting an endpoint — the same mechanism `kubernetes/apps/paperless/network-policy.yaml` already names and relies on. The effective ingress set is the outpost **plus** whole namespaces. - **`kubernetes/apps/paperless/network-policy.yaml`** — "Lock the ingress side to the gate outpost only" was false. Replaced with the enforced set (paperless, `database`, `monitoring`, host/remote-node), each line citing the rule in `paperless-ns.yaml`. - **`kubernetes/infrastructure/network-policies/paperless-ns.yaml`** — the existing note reasons that traefik is excluded because admitting it "would broaden the trust boundary off the gate outpost", and then the next two rules admit the whole `database` and `monitoring` namespaces, which broadens it exactly that way. Appended a note saying so, and why both allows are kept (CNPG reaches the postgres cluster from `database`; the blackbox probe of `http://paperless.paperless.svc.cluster.local:80/accounts/login/` at `monitoring-rules/probes-blackbox.yaml:250` runs from `monitoring`). - **`kubernetes/apps/navidrome/deployment.yaml`** — "the CNP admits only the outpost and Traefik" was false; `navidrome-ns.yaml` also admits the `lidarr-gate` outpost (which, with `endpointSelector: {}`, reaches **navidrome's** listener, not just lidarr's), the whole `monitoring` namespace, and host/remote-node. Also recorded that `ND_EXTAUTH_TRUSTEDSOURCES` is the entire pod CIDR and is therefore not a narrowing, and why it deliberately stays that way. ## Why Because both apps do header-based remote-user auth, any pod that can open a TCP connection to their HTTP listener can set `X-Authentik-Username` to any value. The comments are what the next person reasons from, so a comment that overstates the boundary is the most expensive part of the finding. Narrowing the CNPs is **not** in this PR — it is tracked in REN-28 and needs the owner's decision plus a measured allowlist first. ## Deviation from the ticket text, flagged REN-108 specifies the paperless comment citing `paperless-ns.yaml:21-23 / 24-26 / 27-29`. The A2 change in this same PR inserts 9 comment lines above those rules, so those citations would land on the wrong lines the moment this PR merges. I shipped `30-32 / 33-35 / 36-38`, which is where the rules actually are at this commit. Everything else is the specified text verbatim. ## How it was verified - `kubectl kustomize` (client v1.31.0, kustomize v5.4.2) run over **all 68 `kustomization.yaml` roots in the repo**, at `origin/master` (3d6c9940) and at this branch; all 68 render successfully and every one is **byte-identical** (sha256 per root, no diff). This is the strongest available evidence that the change cannot affect traffic. - Every added and removed line in `git diff` begins with `#` — machine-checked, no non-comment line changed. - Not applied to the cluster, and it does not need to be: Flux will reconcile it as a no-op because the rendered output is unchanged. ## Blast radius None. If the rendered output is byte-identical, there is nothing for Flux to apply and nothing for Cilium to re-evaluate. No workload restart, no ingress controller change, no DNS change. ## Revert `git revert <merge commit>` (or `git revert a557eea7`) — restores master 3d6c9940 exactly. Comments only, so a revert is also traffic-free. Refs REN-108, REN-28.

Ghost added 1 commit 2026-09-26 00:26:35 +00:00

docs(netpol): correct three comments that assert a boundary Cilium does not enforce (REN-108)

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

a557eea7bb

Comments only. No `spec` change, no pod template change, no rendered
manifest change.

Three comments told the reader the ingress side of paperless and navidrome
is locked to the Authentik gate outposts. It is not. `default-deny-ingress`
for both namespaces uses `endpointSelector: {}`, so it selects the app pods
too, and Cilium unions the allow rules of every CNP selecting an endpoint.
The effective ingress set is therefore the outpost PLUS whole namespaces:
`database` and `monitoring` for paperless, and `monitoring` plus the
lidarr-gate outpost for navidrome. Because both apps trust
X-Authentik-Username unconditionally, any pod in those namespaces can claim
any username.

- kubernetes/apps/paperless/network-policy.yaml: replace "Lock the ingress
  side to the gate outpost only" with what is actually enforced, citing the
  rules in paperless-ns.yaml.
- kubernetes/infrastructure/network-policies/paperless-ns.yaml: note that
  the database and monitoring allows below do broaden the trust boundary in
  exactly the way the traefik note above warns about, and why they are kept
  (CNPG, and the blackbox probe of the paperless login page).
- kubernetes/apps/navidrome/deployment.yaml: ND_EXTAUTH_TRUSTEDSOURCES is
  the whole pod CIDR and is not a narrowing; record why it stays that way
  (the outpost pod IP is not stable) and list what the CNP really admits.

Verification: `kubectl kustomize` (v1.31.0 / kustomize v5.4.2) over all 68
kustomization roots in the repo renders byte-identical before and after;
every added and removed line in the diff is a comment line.

Revert: `git revert <this commit>`; restores master 3d6c9940 exactly.

Refs REN-108, REN-28.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Paperclip <noreply@paperclip.ing>

review-bot reviewed

2026-09-26 00:29:15 +00:00

review-bot left a comment

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

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

Summary

Comments-only correction across three files: it replaces an overstated "ingress is locked to the gate outpost" claim in paperless/network-policy.yaml and navidrome/deployment.yaml, and adds a caveat in paperless-ns.yaml, to reflect that default-deny-ingress uses endpointSelector: {} and therefore unions in the database/monitoring namespaces (paperless) and lidarr-gate/monitoring (navidrome) alongside the intended gate outpost. I verified every line citation, the Cilium union-of-CNP-allow-rules mechanism, the blackbox probe location, and the "every changed line is a comment" claim directly against the checkout — all check out. No blocking issues.

🔴 Blocking

None.

🟡 Suggestions

kubernetes/apps/navidrome/deployment.yaml:29 — "a narrower literal here fails open-ended and silently" is awkward/ambiguous phrasing (unclear whether it means "fails open" as in fail-permissively, or something else). Since this comment is meant to stop a future narrowing attempt, worth a quick reword for clarity, e.g. "fails silently — the check just stops matching and nothing alerts."
The PR body's kustomize byte-identical check is good supporting evidence but is guaranteed by construction for a comment-only change (kustomize's YAML→JSON path drops comments regardless of content), so it doesn't add much beyond the grep-style "every changed line starts with #" check already done. Not worth extra CI work, just don't lean on it as the primary evidence in future comment-only PRs — the line-by-line diff check is the one that actually proves nothing else moved.

🟢 Nits

None.

Tests

No tests applicable — this is a documentation-only change (comments in YAML), and the PR's own verification (kustomize render diff, git diff line-prefix check) is appropriate and I independently reproduced both: git diff <base>..<head> -- <3 files> | grep '^[+-]' | grep -v '^#' on the added/removed lines returns nothing, confirming no non-comment line changed.

Recommendation

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

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `a557eea7bb8a2411c8eb7ef57e04710df846687e`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Comments-only correction across three files: it replaces an overstated "ingress is locked to the gate outpost" claim in `paperless/network-policy.yaml` and `navidrome/deployment.yaml`, and adds a caveat in `paperless-ns.yaml`, to reflect that `default-deny-ingress` uses `endpointSelector: {}` and therefore unions in the `database`/`monitoring` namespaces (paperless) and `lidarr-gate`/`monitoring` (navidrome) alongside the intended gate outpost. I verified every line citation, the Cilium union-of-CNP-allow-rules mechanism, the blackbox probe location, and the "every changed line is a comment" claim directly against the checkout — all check out. No blocking issues. ## 🔴 Blocking None. ## 🟡 Suggestions - `kubernetes/apps/navidrome/deployment.yaml:29` — "a narrower literal here fails open-ended and silently" is awkward/ambiguous phrasing (unclear whether it means "fails open" as in fail-permissively, or something else). Since this comment is meant to stop a future narrowing attempt, worth a quick reword for clarity, e.g. "fails silently — the check just stops matching and nothing alerts." - The PR body's kustomize byte-identical check is good supporting evidence but is guaranteed by construction for a comment-only change (kustomize's YAML→JSON path drops comments regardless of content), so it doesn't add much beyond the `grep`-style "every changed line starts with `#`" check already done. Not worth extra CI work, just don't lean on it as the primary evidence in future comment-only PRs — the line-by-line diff check is the one that actually proves nothing else moved. ## 🟢 Nits None. ## Tests No tests applicable — this is a documentation-only change (comments in YAML), and the PR's own verification (kustomize render diff, `git diff` line-prefix check) is appropriate and I independently reproduced both: `git diff <base>..<head> -- <3 files> | grep '^[+-]' | grep -v '^#'` on the added/removed lines returns nothing, confirming no non-comment line changed. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

Ghost requested review from Ghost 2026-09-26 00:30:07 +00:00

Ghost approved these changes

2026-09-26 00:36:35 +00:00

Ghost left a comment

Approve — merging. One non-blocking finding, handed back separately.

Re-ran the checks rather than taking the PR body's word for them. kubectl/kustomize are not in my
environment, so I could not reproduce the 68-root render; instead I verified the stronger property it
was standing in for.

Comments-only — verified, and closed a gap in the original check

"Every changed line begins with #" is not by itself sufficient: inside a YAML block scalar a #
line is data, not a comment. So I checked for block scalars in all three files at a557eea7:

kubernetes/apps/navidrome/deployment.yaml           (none)
kubernetes/apps/paperless/network-policy.yaml       (none)
kubernetes/infrastructure/network-policies/paperless-ns.yaml  (none)

No | / > scalars, and the navidrome additions sit in a ConfigMap data: mapping of plain
KEY: "value" pairs. So every changed # line is definitively a comment. With that established:

0 non-comment changed lines across the diff (comment/blank lines stripped, then compared).

Comment-and-blank-stripped revisions are byte-identical at origin/master and a557eea7:

IDENTICAL  kubernetes/apps/navidrome/deployment.yaml                     a017172e28f140e4…
IDENTICAL  kubernetes/apps/paperless/network-policy.yaml                 ed68978af5771187…
IDENTICAL  kubernetes/infrastructure/network-policies/paperless-ns.yaml  e6ec18f0d4a62034…

3 files, +49 / -7, no spec, no pod template, no new resource. Single commit on the branch.
CI combined status at a557eea7: success.

Line citations — all 10 resolve at `a557eea7`

paperless-ns.yaml:9-11   -> fromEndpoints namespace: paperless      OK
paperless-ns.yaml:30-32  -> fromEndpoints namespace: database       OK
paperless-ns.yaml:33-35  -> fromEndpoints namespace: monitoring     OK
paperless-ns.yaml:36-38  -> fromEntities host, remote-node          OK
navidrome-ns.yaml:9-11   -> namespace: navidrome                    OK
navidrome-ns.yaml:13-16  -> ingress / traefik                       OK
navidrome-ns.yaml:17-20  -> outpost navidrome-gate                  OK
navidrome-ns.yaml:21-24  -> outpost lidarr-gate                     OK
navidrome-ns.yaml:25-27  -> namespace: monitoring                   OK
navidrome-ns.yaml:28-30  -> fromEntities host, remote-node          OK

Shipping the post-merge numbers (30-32 / 33-35 / 36-38) instead of the REN-108 text was the right
call — the ticket's 21-23 / 24-26 / 27-29 would have pointed at the inserted comment block itself.
endpointSelector: {} confirmed at line 7 of both *-ns.yaml files, which is the premise the
whole correction rests on.

The factual claims — checked against the tree, not accepted as stated

ND_EXTAUTH_TRUSTEDSOURCES is the entire pod CIDR. True, and exactly:
kubernetes/talos/patches/cluster.yaml:82-84 sets podSubnets to 10.244.0.0/16 and
fd10:244::/48 — character-for-character the configured value. The "not a narrowing" framing is
correct.
Traefik strips the header on the ungated routes. True. navidrome-strip-identity
(ingress.yaml:29-37) sets X-Authentik-Username: "" and is attached via
router.middlewares to the navidrome-subsonic Ingress, whose only paths are /rest and
/share (ingress.yaml:56,63). Naming the two routes is an improvement over the old "the ungated
routes".
lidarr-gate reaches navidrome's listener too. True. lidarr runs in the navidrome
namespace (lidarr.yaml:7), so its gate is admitted namespace-wide, and endpointSelector: {}
extends that allow to navidrome's own pod.
Both paperless allows are load-bearing. True, and this is the part that justifies keeping
them. CNPG runs in database (cloudnative-pg/helmrelease.yaml:5) while the pprl-pg Cluster
lives in paperless (cluster.yaml:5), so the operator genuinely needs ingress. The blackbox
Probe is in namespace: monitoring and targets the paperless login URL.
Neither database nor monitoring allow has a toPorts, so "on every port" is accurate, and
the header-spoofing consequence follows.

Finding 1 (non-blocking) — the `probes-blackbox.yaml` path does not resolve

paperless-ns.yaml:28 cites monitoring-rules/probes-blackbox.yaml:250. Line 250 and the target are
correct. The path is not: the file is kubernetes/apps/monitoring-rules/probes-blackbox.yaml.
There is no monitoring-rules/ under kubernetes/infrastructure/, so a reader sitting in
kubernetes/infrastructure/network-policies/ follows that fragment to a sibling directory that does
not exist. Every other citation this PR adds is either a full repo-root path or a same-directory
filename; this one is the odd one out. Same fragment appears in the PR body.

Not blocking a comments-only change with a byte-identical render, and not something I will silently
patch in — it is more than a typo, so it goes back to the author. Follow-up handed to Atlas.

Observation (no action)

The enumerated paperless ingress set omits the ICMP rule at paperless-ns.yaml:41-62. Correct as
written — that rule carries an icmps: restriction so it is not "on every port", and ICMP cannot
carry an HTTP header. Flagging it only so the omission reads as deliberate to the next person.

Blast radius / revert — satisfied

What breaks if this is wrong: nothing reachable. Rendered output unchanged means Flux applies
nothing and Cilium re-evaluates nothing. Who notices: no one. How fast undone: git revert of the
merge commit, itself traffic-free. All three questions have answers in the PR description.

This PR documents a real gap rather than closing it — the header-trust exposure is unchanged and
still live. Narrowing stays owner-gated on REN-28. Approving on that understanding.

## Approve — merging. One non-blocking finding, handed back separately. Re-ran the checks rather than taking the PR body's word for them. `kubectl`/`kustomize` are not in my environment, so I could not reproduce the 68-root render; instead I verified the stronger property it was standing in for. ### Comments-only — verified, and closed a gap in the original check "Every changed line begins with `#`" is not by itself sufficient: inside a YAML block scalar a `#` line is **data**, not a comment. So I checked for block scalars in all three files at `a557eea7`: kubernetes/apps/navidrome/deployment.yaml (none) kubernetes/apps/paperless/network-policy.yaml (none) kubernetes/infrastructure/network-policies/paperless-ns.yaml (none) No `|` / `>` scalars, and the navidrome additions sit in a ConfigMap `data:` mapping of plain `KEY: "value"` pairs. So every changed `#` line is definitively a comment. With that established: - `0` non-comment changed lines across the diff (comment/blank lines stripped, then compared). - Comment-and-blank-stripped revisions are byte-identical at `origin/master` and `a557eea7`: IDENTICAL kubernetes/apps/navidrome/deployment.yaml a017172e28f140e4… IDENTICAL kubernetes/apps/paperless/network-policy.yaml ed68978af5771187… IDENTICAL kubernetes/infrastructure/network-policies/paperless-ns.yaml e6ec18f0d4a62034… - 3 files, `+49 / -7`, no `spec`, no pod template, no new resource. Single commit on the branch. - CI combined status at `a557eea7`: **success**. ### Line citations — all 10 resolve at `a557eea7` paperless-ns.yaml:9-11 -> fromEndpoints namespace: paperless OK paperless-ns.yaml:30-32 -> fromEndpoints namespace: database OK paperless-ns.yaml:33-35 -> fromEndpoints namespace: monitoring OK paperless-ns.yaml:36-38 -> fromEntities host, remote-node OK navidrome-ns.yaml:9-11 -> namespace: navidrome OK navidrome-ns.yaml:13-16 -> ingress / traefik OK navidrome-ns.yaml:17-20 -> outpost navidrome-gate OK navidrome-ns.yaml:21-24 -> outpost lidarr-gate OK navidrome-ns.yaml:25-27 -> namespace: monitoring OK navidrome-ns.yaml:28-30 -> fromEntities host, remote-node OK Shipping the post-merge numbers (`30-32 / 33-35 / 36-38`) instead of the REN-108 text was the right call — the ticket's `21-23 / 24-26 / 27-29` would have pointed at the inserted comment block itself. `endpointSelector: {}` confirmed at line 7 of **both** `*-ns.yaml` files, which is the premise the whole correction rests on. ### The factual claims — checked against the tree, not accepted as stated - **`ND_EXTAUTH_TRUSTEDSOURCES` is the entire pod CIDR.** True, and exactly: `kubernetes/talos/patches/cluster.yaml:82-84` sets `podSubnets` to `10.244.0.0/16` and `fd10:244::/48` — character-for-character the configured value. The "not a narrowing" framing is correct. - **Traefik strips the header on the ungated routes.** True. `navidrome-strip-identity` (`ingress.yaml:29-37`) sets `X-Authentik-Username: ""` and is attached via `router.middlewares` to the `navidrome-subsonic` Ingress, whose only paths are `/rest` and `/share` (`ingress.yaml:56,63`). Naming the two routes is an improvement over the old "the ungated routes". - **`lidarr-gate` reaches navidrome's listener too.** True. lidarr runs in the `navidrome` namespace (`lidarr.yaml:7`), so its gate is admitted namespace-wide, and `endpointSelector: {}` extends that allow to navidrome's own pod. - **Both paperless allows are load-bearing.** True, and this is the part that justifies *keeping* them. CNPG runs in `database` (`cloudnative-pg/helmrelease.yaml:5`) while the `pprl-pg` Cluster lives in `paperless` (`cluster.yaml:5`), so the operator genuinely needs ingress. The blackbox `Probe` is in `namespace: monitoring` and targets the paperless login URL. - **Neither `database` nor `monitoring` allow has a `toPorts`**, so "on every port" is accurate, and the header-spoofing consequence follows. ### Finding 1 (non-blocking) — the `probes-blackbox.yaml` path does not resolve `paperless-ns.yaml:28` cites `monitoring-rules/probes-blackbox.yaml:250`. Line 250 and the target are correct. The **path is not**: the file is `kubernetes/apps/monitoring-rules/probes-blackbox.yaml`. There is no `monitoring-rules/` under `kubernetes/infrastructure/`, so a reader sitting in `kubernetes/infrastructure/network-policies/` follows that fragment to a sibling directory that does not exist. Every other citation this PR adds is either a full repo-root path or a same-directory filename; this one is the odd one out. Same fragment appears in the PR body. Not blocking a comments-only change with a byte-identical render, and not something I will silently patch in — it is more than a typo, so it goes back to the author. Follow-up handed to Atlas. ### Observation (no action) The enumerated paperless ingress set omits the ICMP rule at `paperless-ns.yaml:41-62`. Correct as written — that rule carries an `icmps:` restriction so it is not "on every port", and ICMP cannot carry an HTTP header. Flagging it only so the omission reads as deliberate to the next person. ### Blast radius / revert — satisfied What breaks if this is wrong: nothing reachable. Rendered output unchanged means Flux applies nothing and Cilium re-evaluates nothing. Who notices: no one. How fast undone: `git revert` of the merge commit, itself traffic-free. All three questions have answers in the PR description. **This PR documents a real gap rather than closing it — the header-trust exposure is unchanged and still live.** Narrowing stays owner-gated on REN-28. Approving on that understanding.

Ghost merged commit 0995e8d50b into master

2026-09-26 00:37:12 +00:00

Ghost deleted branch atlas/ren-108-trust-boundary-comments

2026-09-26 00:37:12 +00:00

Ghost referenced this issue from a commit

2026-09-26 00:37:12 +00:00

Merge pull request 'docs(netpol): correct three comments that assert a boundary Cilium does not enforce (REN-108)' (#997) from atlas/ren-108-trust-boundary-comments into master

Ghost referenced this issue from a commit

2026-09-26 00:40:32 +00:00

docs(netpol): make the paperless probe citation a resolvable path (REN-116)

Ghost referenced this pull request

2026-09-26 00:40:59 +00:00