Merge pull request 'fix(vpn-ingress): new-peer.sh counted its own commented-out example as an allocated address' (#64) from fix/new-peer-ignores-template-comments into main

Reviewed-on: #64

decisions/2026-10-06-new-peer-ignores-template-comments.md

@@ -0,0 +1,73 @@
+---
+date: 2026-10-06
+branch: fix/new-peer-ignores-template-comments
+title: new-peer.sh refused the first VPN address because its duplicate check matched its own commented-out example
+---
+
+## What changed
+
+The operator's first real enrolment failed:
+
+```
+$ ./infra/vpn-ingress/new-peer.sh keith-phone 2
+new-peer.sh: 10.90.0.2/32 is already allocated in platform/vpn-ingress/peers.yaml. Pick another octet.
+```
+
+**Nothing was allocated.** `peers.yaml` had zero peers — the pod's own init container reported
+`wg0.conf assembled: 0 peer(s)`. The duplicate-address check grepped the WHOLE file, and
+`peers.yaml` line 66 is a **commented-out example stanza** I wrote, containing literally
+`#   AllowedIPs = 10.90.0.2/32`. So `.2` — the first address anybody would ask for, and the one my
+own runbook uses in its example — was permanently un-allocatable, with a message asserting the
+opposite of the truth.
+
+Both halves fixed, because either alone leaves the trap half-set:
+
+- **`infra/vpn-ingress/new-peer.sh`**: the check strips comment lines before matching
+  (`grep -vE '^[[:space:]]*#' | grep -q …`). That is the correct predicate rather than a patch: a
+  real stanza's `AllowedIPs` line is never commented, while both the template and every peer's
+  `# <name> — <owner>` label always are.
+- **`platform/vpn-ingress/peers.yaml`**: the example now uses placeholders that cannot match
+  anything real — `# <device-name> — <owner>, enrolled <YYYY-MM-DD>` and
+  `AllowedIPs = 10.90.0.<octet>/32`. A future grep cannot be fooled by it either.
+
+The bug class is worth naming: **a documentation example that is byte-identical to real data will
+eventually be parsed as real data.** The same shape caused two earlier defects in this component —
+`flux-postbuild-substitution` flagged a `${...}` written inside a comment *explaining* that hazard,
+and `no-plaintext-secrets` flagged the `.sops.yaml.example` because the kustomization named it in a
+comment. Three instances now; the rule is to make examples structurally unmatchable, not merely
+commented out.
+
+No sensitive paths (`platform/vpn-ingress/**`, `infra/vpn-ingress/**`, `decisions/**`), so this
+merges on a green gate with no override.
+
+## What I verified
+
+Run in the pinned toolbox against the live tree, 2026-10-06.
+
+- **`.2` now works**, and it went through the REAL key path: `new-peer.sh keith-phone 2` rendered a
+  profile with `Address 10.90.0.2/32`, `AllowedIPs = 10.90.0.2/32` in the printed stanza, full
+  tunnel, at `.cache/wg-peers/keith-phone.conf` (0600). The throwaway keypair was deleted after.
+- **This incidentally closes a gap left open in #60**: the profile could only have been rendered by
+  deriving the server's public key via `sops -d --extract … | wg pubkey` from
+  `wg-server-key.sops.yaml` — so that pipeline is now proven **against the operator's real key**,
+  not just against a placeholder it was expected to reject.
+- **A real duplicate is still refused.** Appending an uncommented
+  `AllowedIPs = 10.90.0.5/32` stanza and asking for octet 5 produced the correct refusal; the tree
+  was restored afterwards and `git diff --stat` showed only the intended template edit.
+- **Zero active allocations remain**: `grep -vE '^[[:space:]]*#' peers.yaml | grep -c AllowedIPs`
+  → `0`.
+- **Toolbox gate**: rc 3, with `spec-immutability`, `decisions-log`, `lint`, `l0`, `render-golden`,
+  `l1` and `approval-policy` all pass; rc 3 is the two environmental skips (`gitea-settings`,
+  conformance L2/L3). **Not green. Reported as rc 3.**
+
+## What I did NOT verify
+
+- **No peer is enrolled by this branch.** The keypair I generated was a throwaway and was deleted;
+  the operator must run the script themselves, since the device's private key must exist only on
+  the device. So the end of the enrolment path — a stanza merged into `peers.yaml`, Flux rolling
+  the DaemonSet, and `wg show` listing a peer — is still unexercised.
+- **The duplicate-NAME warning** (`grep -qi "^ *# *${NAME} "`) deliberately still inspects
+  comments, because per-peer labels are comments. It is no longer tripped by the template, but I
+  did not test it against a real enrolled peer.
+- **Nothing about client traffic.** The WAN port-forward is still not configured and there is still
+  no off-LAN vantage point, so the tunnel has carried no packet.

infra/vpn-ingress/new-peer.sh

@@ -133,7 +133,13 @@ CLIENT_ADDR="${CLIENT_BASE}.${OCTET}"
 # --- refuse a collision, because the failure is otherwise invisible -----------------------------
 # Two devices sharing an address does not error anywhere: the server routes that address to
 # whichever peer handshook most recently, so BOTH devices appear to work intermittently.
-if [ -r "$PEERS_FILE" ] && grep -q "AllowedIPs *= *${CLIENT_ADDR}/32" "$PEERS_FILE"; then
+# COMMENT LINES ARE NOT ALLOCATIONS. This used to grep the whole file and so matched the
+# COMMENTED-OUT EXAMPLE STANZA in peers.yaml, which made `.2` — the first address anyone would
+# ask for — permanently un-allocatable with a message claiming it was already taken. Strip
+# comments first: a real stanza's `AllowedIPs` line is never commented, while both the template
+# and every peer's `# <name> — <owner>` label are.
+if [ -r "$PEERS_FILE" ] \
+   && grep -vE '^[[:space:]]*#' "$PEERS_FILE" | grep -q "AllowedIPs *= *${CLIENT_ADDR}/32"; then
   die "${CLIENT_ADDR}/32 is already allocated in platform/vpn-ingress/peers.yaml. Pick another octet."
 fi
 if [ -r "$PEERS_FILE" ] && grep -qi "^ *# *${NAME} " "$PEERS_FILE"; then

platform/vpn-ingress/peers.yaml

@@ -60,10 +60,16 @@ data:
   #
   # COPY THIS TEMPLATE, uncomment, one block per device:
   #
-  #   # pixel-8 — Keith's phone, enrolled 2026-10-05
+  #   # <device-name> — <owner>, enrolled <YYYY-MM-DD>
   #   [Peer]
   #   PublicKey = <the public key new-peer.sh printed>
-  #   AllowedIPs = 10.90.0.2/32
+  #   AllowedIPs = 10.90.0.<octet>/32
+  #
+  # The placeholders above are deliberately NOT valid values. An earlier version of this template
+  # used a real-looking `10.90.0.2/32`, and new-peer.sh's duplicate-address check — which grepped
+  # the whole file — matched it, so `.2` could never be allocated and the script insisted it was
+  # already taken. The check now ignores comments; keeping the example unmatchable too means a
+  # future grep cannot be fooled either.
   #
   # Keep the `# <name> — <owner>, enrolled <date>` comment line. It is the only place a key is
   # tied to a human-recognisable device; `wg show` can only ever print the key itself, so without