IR-20261112-d6: take the reviewer token out of sbx-lab and sbx-gits (D6) #956

Open
pi-landing wants to merge 5 commits from plane/f-r2-d6-sbx-reviewer-out into live
Collaborator

D6 (F-R2-POD-ENV): the reviewer token leaves the sandboxes — infra-core half

SUPERSEDED BY THE SPLIT BELOW (3c, 11:48:30Z). The paragraphs that used to
open this body said both sandboxes lose the reviewer credential in this PR. They
no longer do: sbx-lab loses it here; sbx-gits keeps sbx-gits-ld-reviewer
and gains its own posting identity instead.
The removal of
sbx-gits-ld-reviewer is #957, HELD.

REVIEW 3c / FIX ROUND 2 (cd, 12:32:39Z) FOLDED INTO THE SPLIT

N1 (tf-plan deny, R30 path — MUST show in the CI plan). The tf-plan policy
holds path "*" {read, list, sudo} and prod PR branches run that plan, so the D6
objects were reachable through the plan route even with the stores namespaced and
eso-read denying them. Homelab/prod#1216 (same run) now adds deny on
kv/data/ld-fleet/*, kv/metadata/ld-fleet/*, kv/data/sbx-gits/gits-reviewer
and kv/metadata/sbx-gits/gits-reviewer to tf-plan. Filtered plan at
pr-d6/plan-3296-filtered.txt (prod run 3296 job 0, head b6a3078c):
Plan: 7 to add, 2 to change, 0 to destroy — vault_policy.tf_plan updated
in-place
with both deny blocks visible.

Wording, exactly as ruled (12:33:02Z) — this sentence now appears wherever the
old "outside every sandbox's reach" claim was:

Outside every sandbox's direct reach; CI path residual R20 until <the earlier of the CI-isolation step or pi-landing leaving sbx-lab, ~11-20>

N2 (split shape, binding). At this head: sbx-gits-ld-reviewer is KEPT here
(its removal is #957, HELD); gits-reviewer is ADD-only
(store/ES/mount/SA); and this PR carries its own eso-read deny and the
tf-plan deny on kv/data + kv/metadata sbx-gits/gits-reviewer.

N3 (the RUN 2 route, agreed). Order, exactly:

  1. ESO force-sync the reviewer ExternalSecret; confirm Ready=True and a
    changed Secret resourceVersion (read the version, never the value).
  2. rollout restart ld-runtime (LD_REVIEWER_TOKEN is an env secretKeyRef,
    captured at container start).
  3. ONE relay call, OK on the new token.
  4. Revoke the old token.
  5. ONE relay call AFTER the revoke — the only valid proof. A relay call made
    before the revoke proves nothing: both tokens are ld-reviewer, so it can
    pass on the old env.

The old-path readers the revoke cuts (read at this PR head,
terraform/openbao.tf: the only policies granting kv/data/ld/forgejo-reviewer
are those two; everything else on that path is the ESO role
eso-ld-forgejo-reviewer, whose sandbox consumers are exactly the
ExternalSecrets this change-set moves):

reader policy/role what replaces it
ld-landing-narrow vault_policy.ld_landing_narrow — exact kv/data/ld/forgejo-reviewer + kv/data/pi/forgejo-landing the landing worker keeps using it for pi/forgejo-landing (untouched); its reviewer-path grant stops being usable once the object is revoked, which is the intent — pi-landing posts, it does not review
ld-reviewer-post vault_policy.ld_reviewer_post — exact kv/data/ld/forgejo-reviewer kv/ld-fleet/ld-runtime-reviewer through the ld-fleet namespaced store: the runtime's panel posts as ld-reviewer from ld-fleet, which is the whole point of D6
ESO role eso-ld-forgejo-reviewer (four ESs: ld-fleet/ld-read-forgejo-token, ld-fleet/ld-runtime-forgejo-reviewer, sbx-lab/sbx-lab-ld-reviewer, sbx-gits/sbx-gits-ld-reviewer) eso-ld-* the two ld-fleet ESs move to kv/ld-fleet/* (prod#1216); sbx-lab-ld-reviewer is deleted here; sbx-gits-ld-reviewer is deleted by #957 after the GITS proof rows

SPLIT (3c 11:47:44Z + 11:48:30Z): the sbx-gits half is an ADD, not a removal

New head: 839da5c7. Two new commits on top of 0b3e4c7e (nothing was
force-pushed and nothing was amended):

commit what it is
7d0c9ac1 restores the sandbox-GITS half of d7cfde80/0b3e4c7e, so this PR's net diff is the sbx-lab half only (3 files, -78/+1) plus, from the commit below, the gits addition
839da5c7 add-only gits-reviewer for the GITS seats (see below)

Why the gits deletion is not in this PR

ref/seat-clean/bin/seat-post-comment.sh in sbx-gits-0 posts GITS gate
verdicts with /run/secrets/ld/identities/ld-reviewer/token. Deleting that
credential today stops GITS seats from posting, so it is swapped, not
removed
: the seats get their own identity, which can post and cannot approve.

What this PR adds for sbx-gits (839da5c7)

  • kv path kv/sbx-gits/gits-reviewer (username, token);
  • an openbao.tf policy reading only that path (eso-sbx-gits-gits-reviewer)
    and a role bound to sbx-gits/eso-openbao-gits-reviewer only — in
    Homelab/prod#1216, same run;
  • the eso_read deny on BOTH kv/data/sbx-gits/gits-reviewer and
    kv/metadata/sbx-gits/gits-reviewer
    (in addition to the ld-fleet/* denies).
    Both paths, because metadata read/list is the refresh route: denying only
    data would leave the object enumerable and refreshable through the generic
    openbao store;
  • a namespaced SecretStore openbao-sbx-gits-gits-reviewer in sbx-gits
    (core/manifests/apps/sbx-gits/secretstores.yaml, with ServiceAccount
    eso-openbao-gits-reviewer, automountServiceAccountToken: false). No
    ClusterSecretStore may read this path
    : another namespace cannot resolve a
    namespaced store at all (review 3c B1, applied to this path);
  • ExternalSecret/Secret sbx-gits-gits-reviewer, mounted read-only at
    /run/secrets/ld/identities/gits-reviewer/
    in sbx-gits-0 (same pattern as
    the ld-reviewer mount, 0440 files; optional: true, so the pod starts before
    RUN 3 writes the kv object);
  • the epic harness: manifest[5] now checks the new ES (store, key,
    username/token), that the store is NAMESPACED and that no ClusterSecretStore
    appears in that file, Retain 6 → 7; manifest[5b] adds the third read-only
    mount. Both PASS locally on 839da5c7.

The sbx-lab half is unaffected

core/manifests/apps/sbx-lab/{externalsecrets,statefulset,README} — the
ExternalSecret sbx-lab-ld-reviewer, its volume and its read-only mount are gone,
exactly as before the split.

What is HELD: #957

Homelab/infra-core#957 (head 19f4ac13, based on this branch) removes
sbx-gits-ld-reviewer, its mount, its env and its target Secret (by GC). Do not
merge it until a GITS seat has posted through /run/secrets/ld/identities/gits-reviewer
and the ExternalSecret is force-synced Ready.

The sequence

  1. terraform apply (prod#1216 creates the policy + role) → merge prod#1216.
  2. Merge this PR → ArgoCD syncs: SA + namespaced store + ES exist; the ES is
    SecretSyncedError until step 3, the mount is optional, the pod starts and
    the seats keep posting with the old ld-reviewer credential
    .
  3. RUN 3 writes kv/sbx-gits/gits-reviewer; force-sync; the Secret becomes
    Ready; a GITS seat posts a verdict through the new path.
  4. Merge #957 (retarget it to live first).
  5. RUN 2 (the revoke) as scripted.

The three options this split leaves open (El Director's call)

  1. GITS via the relay — the seats post through the ld-fleet runtime
    (infra-core #945 / spec 009(c)); then gits-reviewer is unnecessary and #957
    simply drops sbx-gits-ld-reviewer after the switch.
  2. A GITS-only, comment-scoped identity — what this PR builds
    (gits-reviewer: one path, one namespace, one SA, no merge right, no branch
    rule counting it). Cheapest today; keeps GITS independent of the relay.
  3. Hold — leave sbx-gits-ld-reviewer in place and merge only the sbx-lab
    half. To do that: merge this PR as it stands (the gits half is add-only, so
    nothing is lost by leaving it unmerged too), do not merge #957, and the
    reviewer credential stays where it is in sbx-gits until a ruling lands.

How to land / how to drop

  • Land as it stands (add-only gits half included): merge this PR. Nothing in
    sbx-gits is removed; gits-reviewer appears and stays Ready or
    SecretSyncedError (harmless, optional: true).
  • Land only the sbx-lab half: merge the branch at 7d0c9ac1 — that commit's
    tree is exactly the sbx-lab change and nothing else. git revert 839da5c7
    produces the same tree (it reverts cleanly; nothing was amended or
    force-pushed), so the clean route is: revert 839da5c7 on this branch, land,
    and re-apply it later. A push would be needed either way; no force-push.
  • Drop the gits work later: git revert 839da5c7 (or close #957 if it was
    merged and it turns out the relay is the answer).

Review 3c blockers, restated for this shape

  • B1 (unconditioned ClusterSecretStore): not repeated — the new store is
    NAMESPACED, so no other namespace can resolve it.
  • B2 (generic eso-read wildcard): closed for this path with both denies.
  • B3 (the harness): still green; the checks were extended, not loosened.

CI

Green at 839da5c7; run ids in the comment below.


FIX ROUND 1 (3c, 11:00:57Z) — head 0b3e4c7e (previous d7cfde80).

B3 — the epic harness now passes on this PR's own files

scripts/gits-sbx-epic-test.sh checks [5] and [5b] grep the RAW manifests for
two literals, comments included, so the D6 comments that named the retired
path/variable turned the harness red here. Both comments are reworded (they now
say "the OLD reviewer kv object" and "the reviewer identity variable"); no field
Kubernetes reads ever contained either literal.

Local run on this head:

PASS  manifest[5] identidades reales (R12 + D6): sin ld-reviewer (solo ld-fleet
      lo lee), pi-landing -> openbao-pi-forgejo-landing/pi/forgejo-landing, Retain=5
PASS  manifest[5b] mounts /run/secrets/ld/identities/pi-landing readOnly y
      NINGUN ld-reviewer (D6)

At the pre-fix head those two are FAIL; the 22 failures that remain in both runs
are the harness's own live-comparison and sandbox-environment checks (they expect
a merged head, /home/ld writable, etc.), not this change.

Terraform apply identity, credentials and order

  • Apply identity: the normal route — infra-apply-cluster on live
    (per-run OpenBao token, R25 per-job AWS state creds), or a named human operator
    session. No agent ad-hoc token, no -target.
  • Backend credentials: scoped AWS state-backend creds fetched from OpenBao
    per job into an ephemeral 0700 directory; terraform init -backend-config=../backend.hcl. Nothing pod-mounted.
  • Apply order (binding, both repos together): terraform apply (prod#1216)
    → merge prod#1216 → merge this PR → RUN 1 (write kv/ld-fleet/*)
    → RUN 2 (revoke). Terraform first (the roles must exist), and this PR may
    merge before RUN 1: both ExternalSecrets sit in SecretSyncedError and keep
    their current target data, so no pod loses a mounted file early.
  • Saved plan: CI plans prod#1216 (plan-cluster, green at a68598b1,
    run index #3292) — no credential job was run. This repo's PR carries no
    terraform.

RUN 2 order (script spec)

  1. ESO force-sync the runtime's reviewer ExternalSecret, confirm Ready=True
    and a changed target-Secret resourceVersion;
  2. ld-runtime rollout restart (LD_REVIEWER_TOKEN is an env
    secretKeyRef, read at container start);
  3. one relay call succeeds with the new token;
  4. only then revoke the old one.

The two sandbox mounts are removed here, so a running sandbox pod keeps the old
files until it is recreated — they hold a revoked token after step 4, which is
the point.

Rollback

Before RUN 2: revert both PRs (the old kv object is still populated). After RUN
2 it is roll-forward only
— a revert re-publishes the new token on the old,
unconditioned path, which every namespace can read.

Leftovers recorded, not fixed here

  • the old store openbao-ld-forgejo-reviewer stays unconditioned and
    kv/ld/forgejo-reviewer stays populated until the D6 follow-up (after the
    revoke);
  • ci-reader's old token remains in the three sandbox ExternalSecrets — a
    separate rotation item;
  • forgejo-ext's identity resolver will refuse ld-reviewer in sandboxes until
    #945 / spec 009(c): expected, and the sandbox tooling that needs a verdict now
    goes through the runtime's relay.
## D6 (F-R2-POD-ENV): the reviewer token leaves the sandboxes — infra-core half **SUPERSEDED BY THE SPLIT BELOW (3c, 11:48:30Z).** The paragraphs that used to open this body said both sandboxes lose the reviewer credential in this PR. They no longer do: **`sbx-lab` loses it here; `sbx-gits` keeps `sbx-gits-ld-reviewer` and gains its own posting identity instead.** The removal of `sbx-gits-ld-reviewer` is **#957, HELD**. ## REVIEW 3c / FIX ROUND 2 (cd, 12:32:39Z) FOLDED INTO THE SPLIT **N1 (tf-plan deny, R30 path — MUST show in the CI plan).** The `tf-plan` policy holds `path "*" {read, list, sudo}` and prod PR branches run that plan, so the D6 objects were reachable through the plan route even with the stores namespaced and `eso-read` denying them. `Homelab/prod#1216` (same run) now adds `deny` on `kv/data/ld-fleet/*`, `kv/metadata/ld-fleet/*`, `kv/data/sbx-gits/gits-reviewer` and `kv/metadata/sbx-gits/gits-reviewer` to `tf-plan`. Filtered plan at `pr-d6/plan-3296-filtered.txt` (prod run 3296 job 0, head `b6a3078c`): `Plan: 7 to add, 2 to change, 0 to destroy` — `vault_policy.tf_plan` **updated in-place** with both deny blocks visible. **Wording, exactly as ruled (12:33:02Z)** — this sentence now appears wherever the old "outside every sandbox's reach" claim was: > Outside every sandbox's direct reach; CI path residual R20 until <the earlier of the CI-isolation step or pi-landing leaving sbx-lab, ~11-20> **N2 (split shape, binding).** At this head: `sbx-gits-ld-reviewer` is KEPT here (its removal is **#957, HELD**); `gits-reviewer` is ADD-only (store/ES/mount/SA); and this PR carries its own `eso-read` deny **and** the `tf-plan` deny on `kv/data` + `kv/metadata` `sbx-gits/gits-reviewer`. **N3 (the RUN 2 route, agreed).** Order, exactly: 1. ESO `force-sync` the reviewer ExternalSecret; confirm `Ready=True` and a changed Secret `resourceVersion` (read the version, never the value). 2. `rollout restart` `ld-runtime` (`LD_REVIEWER_TOKEN` is an env `secretKeyRef`, captured at container start). 3. **ONE relay call, OK** on the new token. 4. **Revoke the old token.** 5. **ONE relay call AFTER the revoke — the only valid proof.** A relay call made before the revoke proves nothing: both tokens are `ld-reviewer`, so it can pass on the old env. **The old-path readers the revoke cuts** (read at this PR head, `terraform/openbao.tf`: the only policies granting `kv/data/ld/forgejo-reviewer` are those two; everything else on that path is the ESO role `eso-ld-forgejo-reviewer`, whose sandbox consumers are exactly the ExternalSecrets this change-set moves): | reader | policy/role | what replaces it | |---|---|---| | `ld-landing-narrow` | `vault_policy.ld_landing_narrow` — exact `kv/data/ld/forgejo-reviewer` + `kv/data/pi/forgejo-landing` | the landing worker keeps using it for `pi/forgejo-landing` (untouched); its reviewer-path grant stops being usable once the object is revoked, which is the intent — pi-landing posts, it does not review | | `ld-reviewer-post` | `vault_policy.ld_reviewer_post` — exact `kv/data/ld/forgejo-reviewer` | **`kv/ld-fleet/ld-runtime-reviewer`** through the `ld-fleet` namespaced store: the runtime's panel posts as `ld-reviewer` from `ld-fleet`, which is the whole point of D6 | | ESO role `eso-ld-forgejo-reviewer` (four ESs: `ld-fleet/ld-read-forgejo-token`, `ld-fleet/ld-runtime-forgejo-reviewer`, `sbx-lab/sbx-lab-ld-reviewer`, `sbx-gits/sbx-gits-ld-reviewer`) | `eso-ld-*` | the two `ld-fleet` ESs move to ` kv/ld-fleet/*` (prod#1216); `sbx-lab-ld-reviewer` is deleted here; `sbx-gits-ld-reviewer` is deleted by **#957** after the GITS proof rows | --- ## SPLIT (3c 11:47:44Z + 11:48:30Z): the sbx-gits half is an ADD, not a removal New head: `839da5c7`. Two new commits on top of `0b3e4c7e` (nothing was force-pushed and nothing was amended): | commit | what it is | |---|---| | `7d0c9ac1` | restores the sandbox-GITS half of `d7cfde80`/`0b3e4c7e`, so **this PR's net diff is the sbx-lab half only** (3 files, -78/+1) plus, from the commit below, the gits addition | | `839da5c7` | **add-only** `gits-reviewer` for the GITS seats (see below) | ### Why the gits deletion is not in this PR `ref/seat-clean/bin/seat-post-comment.sh` in `sbx-gits-0` posts GITS gate verdicts with `/run/secrets/ld/identities/ld-reviewer/token`. Deleting that credential today stops GITS seats from posting, so it is **swapped, not removed**: the seats get their own identity, which can post and cannot approve. ### What this PR adds for sbx-gits (`839da5c7`) - kv path **`kv/sbx-gits/gits-reviewer`** (`username`, `token`); - an `openbao.tf` policy reading **only that path** (`eso-sbx-gits-gits-reviewer`) and a role bound to **`sbx-gits/eso-openbao-gits-reviewer` only** — in `Homelab/prod#1216`, same run; - the `eso_read` **deny on BOTH `kv/data/sbx-gits/gits-reviewer` and `kv/metadata/sbx-gits/gits-reviewer`** (in addition to the `ld-fleet/*` denies). Both paths, because metadata read/list is the refresh route: denying only `data` would leave the object enumerable and refreshable through the generic `openbao` store; - a **namespaced `SecretStore`** `openbao-sbx-gits-gits-reviewer` in `sbx-gits` (`core/manifests/apps/sbx-gits/secretstores.yaml`, with ServiceAccount `eso-openbao-gits-reviewer`, `automountServiceAccountToken: false`). **No ClusterSecretStore may read this path**: another namespace cannot resolve a namespaced store at all (review 3c B1, applied to this path); - ExternalSecret/Secret **`sbx-gits-gits-reviewer`**, mounted **read-only at `/run/secrets/ld/identities/gits-reviewer/`** in `sbx-gits-0` (same pattern as the ld-reviewer mount, 0440 files; `optional: true`, so the pod starts before RUN 3 writes the kv object); - the epic harness: manifest[5] now checks the new ES (store, key, username/token), that the store is NAMESPACED and that no ClusterSecretStore appears in that file, Retain 6 → 7; manifest[5b] adds the third read-only mount. Both PASS locally on `839da5c7`. ### The sbx-lab half is unaffected `core/manifests/apps/sbx-lab/{externalsecrets,statefulset,README}` — the ExternalSecret `sbx-lab-ld-reviewer`, its volume and its read-only mount are gone, exactly as before the split. ### What is HELD: #957 `Homelab/infra-core#957` (head `19f4ac13`, based on this branch) removes `sbx-gits-ld-reviewer`, its mount, its env and its target Secret (by GC). **Do not merge it until a GITS seat has posted through `/run/secrets/ld/identities/gits-reviewer` and the ExternalSecret is force-synced Ready.** ### The sequence 1. `terraform apply` (prod#1216 creates the policy + role) → merge **prod#1216**. 2. Merge **this PR** → ArgoCD syncs: SA + namespaced store + ES exist; the ES is `SecretSyncedError` until step 3, the mount is optional, **the pod starts and the seats keep posting with the old `ld-reviewer` credential**. 3. **RUN 3** writes `kv/sbx-gits/gits-reviewer`; force-sync; the Secret becomes Ready; **a GITS seat posts a verdict through the new path**. 4. Merge **#957** (retarget it to `live` first). 5. **RUN 2** (the revoke) as scripted. ### The three options this split leaves open (El Director's call) 1. **GITS via the relay** — the seats post through the `ld-fleet` runtime (infra-core #945 / spec 009(c)); then `gits-reviewer` is unnecessary and #957 simply drops `sbx-gits-ld-reviewer` after the switch. 2. **A GITS-only, comment-scoped identity** — what this PR builds (`gits-reviewer`: one path, one namespace, one SA, no merge right, no branch rule counting it). Cheapest today; keeps GITS independent of the relay. 3. **Hold** — leave `sbx-gits-ld-reviewer` in place and merge only the sbx-lab half. To do that: merge this PR as it stands (the gits half is add-only, so nothing is lost by leaving it unmerged too), do not merge #957, and the reviewer credential stays where it is in `sbx-gits` until a ruling lands. ### How to land / how to drop - **Land as it stands (add-only gits half included):** merge this PR. Nothing in `sbx-gits` is removed; `gits-reviewer` appears and stays Ready or `SecretSyncedError` (harmless, `optional: true`). - **Land only the sbx-lab half:** merge the branch at `7d0c9ac1` — that commit's tree is exactly the sbx-lab change and nothing else. `git revert 839da5c7` produces the same tree (it reverts cleanly; nothing was amended or force-pushed), so the clean route is: revert `839da5c7` on this branch, land, and re-apply it later. A push would be needed either way; **no force-push**. - **Drop the gits work later:** `git revert 839da5c7` (or close #957 if it was merged and it turns out the relay is the answer). ### Review 3c blockers, restated for this shape - **B1** (unconditioned ClusterSecretStore): not repeated — the new store is NAMESPACED, so no other namespace can resolve it. - **B2** (generic `eso-read` wildcard): closed for this path with both denies. - **B3** (the harness): still green; the checks were extended, not loosened. ### CI Green at `839da5c7`; run ids in the comment below. --- **FIX ROUND 1 (3c, 11:00:57Z)** — head `0b3e4c7e` (previous `d7cfde80`). ### B3 — the epic harness now passes on this PR's own files `scripts/gits-sbx-epic-test.sh` checks [5] and [5b] grep the RAW manifests for two literals, comments included, so the D6 comments that named the retired path/variable turned the harness red here. Both comments are reworded (they now say "the OLD reviewer kv object" and "the reviewer identity variable"); no field Kubernetes reads ever contained either literal. Local run on this head: PASS manifest[5] identidades reales (R12 + D6): sin ld-reviewer (solo ld-fleet lo lee), pi-landing -> openbao-pi-forgejo-landing/pi/forgejo-landing, Retain=5 PASS manifest[5b] mounts /run/secrets/ld/identities/pi-landing readOnly y NINGUN ld-reviewer (D6) At the pre-fix head those two are FAIL; the 22 failures that remain in both runs are the harness's own live-comparison and sandbox-environment checks (they expect a merged head, `/home/ld` writable, etc.), not this change. ### Terraform apply identity, credentials and order - **Apply identity:** the normal route — `infra-apply-cluster` on `live` (per-run OpenBao token, R25 per-job AWS state creds), or a named human operator session. No agent ad-hoc token, no `-target`. - **Backend credentials:** scoped AWS state-backend creds fetched from OpenBao per job into an ephemeral 0700 directory; `terraform init -backend-config=../backend.hcl`. Nothing pod-mounted. - **Apply order (binding, both repos together):** `terraform apply` (prod#1216) → **merge prod#1216** → **merge this PR** → **RUN 1** (write `kv/ld-fleet/*`) → **RUN 2** (revoke). Terraform first (the roles must exist), and this PR may merge before RUN 1: both ExternalSecrets sit in `SecretSyncedError` and keep their current target data, so no pod loses a mounted file early. - **Saved plan:** CI plans prod#1216 (`plan-cluster`, green at `a68598b1`, run index #3292) — no credential job was run. This repo's PR carries no terraform. ### RUN 2 order (script spec) 1. ESO **force-sync** the runtime's reviewer ExternalSecret, confirm `Ready=True` and a changed target-Secret `resourceVersion`; 2. **`ld-runtime` rollout restart** (`LD_REVIEWER_TOKEN` is an env `secretKeyRef`, read at container start); 3. **one relay call succeeds** with the new token; 4. **only then revoke** the old one. The two sandbox mounts are removed here, so a running sandbox pod keeps the old files until it is recreated — they hold a revoked token after step 4, which is the point. ### Rollback Before RUN 2: revert both PRs (the old kv object is still populated). **After RUN 2 it is roll-forward only** — a revert re-publishes the new token on the old, unconditioned path, which every namespace can read. ### Leftovers recorded, not fixed here - the old store `openbao-ld-forgejo-reviewer` stays unconditioned and `kv/ld/forgejo-reviewer` stays populated until the D6 follow-up (after the revoke); - `ci-reader`'s old token remains in the three sandbox ExternalSecrets — a separate rotation item; - `forgejo-ext`'s identity resolver will refuse `ld-reviewer` in sandboxes until #945 / spec 009(c): expected, and the sandbox tooling that needs a verdict now goes through the runtime's relay.
IR-20261112-d6: take the reviewer token out of both sandboxes (D6)
All checks were successful
CI Review / helm-validate (pull_request) Successful in 10s
CI Review / receiver-test (pull_request) Successful in 4s
CI Review / runner-manifest-test (pull_request) Successful in 10s
CI Review / alert-receiver-test (pull_request) Successful in 22s
CI Review / record-sql-test (pull_request) Successful in 1m10s
Lint & Validate / yaml-lint (pull_request) Successful in 26s
Lint & Validate / shellcheck (pull_request) Successful in 8s
CI Review / cockpit-test (pull_request) Successful in 3m37s
CI Review / cockpit-e2e (pull_request) Successful in 3m39s
Lint & Validate / terraform-validate (pull_request) Successful in 2m9s
CI Review / dispatcher-test (pull_request) Successful in 4m2s
d7cfde8055
The ld-reviewer credential is the one the `live` branch rule counts, so a seat
that can read it is a seat that can approve its own work. IR-20261112-d6 moves
it to ld-fleet only; this half removes it from the sandboxes.

- sbx-lab: ExternalSecret `sbx-lab-ld-reviewer` deleted, its volume and its
  read-only mount at /run/secrets/ld/identities/ld-reviewer removed.
- sbx-gits: ExternalSecret `sbx-gits-ld-reviewer` deleted, its volume and mount
  removed, and the `LD_REVIEWER_IDENTITY` env (nothing in the repo reads it).
- The target Secrets are not deleted by hand: ESO created them with
  `creationPolicy: Owner`, so each carries ownerReference -> its ExternalSecret
  (verified live: sbx-lab/sbx-lab-ld-reviewer and sbx-gits/sbx-gits-ld-reviewer,
  both owner=ExternalSecret/<name>), and Kubernetes garbage collection removes
  them when ArgoCD prunes the ExternalSecret.
- scripts/gits-sbx-epic-test.sh: the manifest invariant inverts — the reviewer
  ExternalSecret and the ld-reviewer mount must be ABSENT from sbx-gits, and the
  Retain count drops 6 -> 5. READMEs and docs/gits-sbx notes updated to match.

Sandbox tooling is NOT changed here: anything that posts or files a verdict as
ld-reviewer switches to the fleet runtime's relay (infra-core #945, spec
009(c)). That switch-over is that lane's, and this manifest change is its
precondition.

Lab-Run: IR-20261112-d6
IR-20261112-d6: fix round 1 — epic harness goes green on its own manifest checks (review 3c B3)
All checks were successful
CI Review / helm-validate (pull_request) Successful in 9s
CI Review / runner-manifest-test (pull_request) Successful in 10s
CI Review / receiver-test (pull_request) Successful in 5s
CI Review / alert-receiver-test (pull_request) Successful in 25s
Lint & Validate / yaml-lint (pull_request) Successful in 25s
Lint & Validate / shellcheck (pull_request) Successful in 4s
CI Review / record-sql-test (pull_request) Successful in 58s
CI Review / cockpit-e2e (pull_request) Successful in 2m18s
Lint & Validate / terraform-validate (pull_request) Successful in 2m25s
CI Review / cockpit-test (pull_request) Successful in 4m4s
CI Review / dispatcher-test (pull_request) Successful in 4m59s
0b3e4c7e9a
`scripts/gits-sbx-epic-test.sh` checks [5] and [5b] grep the RAW manifest files
for two literals (comment lines included), so the D6 comments that named the
retired reviewer path and the retired identity variable turned the harness red
on this PR's own files:

  [5]  core/manifests/apps/sbx-gits/externalsecrets.yaml  'ld/forgejo-reviewer'
  [5b] core/manifests/apps/sbx-gits/statefulset.yaml      'LD_REVIEWER_IDENTITY'

Both comments are reworded to say the same thing without the literals ("the OLD
reviewer kv object", "the reviewer identity variable"), which is what the check
is for: the fields Kubernetes reads must not name them, and neither should a
comment that the harness cannot distinguish from a field.

Local proof (this commit, scripts/gits-sbx-epic-test.sh):
  PASS  manifest[5] identidades reales (R12 + D6): sin ld-reviewer (solo ld-fleet
        lo lee), pi-landing -> openbao-pi-forgejo-landing/pi/forgejo-landing,
        Retain=5
  PASS  manifest[5b] mounts /run/secrets/ld/identities/pi-landing readOnly y
        NINGUN ld-reviewer (D6)
The remaining failures in that run are the harness's own live-comparison and
sandbox-environment checks, which are identical at the pre-fix head.

Lab-Run: IR-20261112-d6
D6 SPLIT, 3c 2026-11-12T11:44:27Z. `ref/seat-clean/bin/seat-post-comment.sh` in
sbx-gits-0 posts GITS gate verdicts with the token at
/run/secrets/ld/identities/ld-reviewer/token, so deleting that ExternalSecret
today would break GITS seat posting. Only the sbx-lab half merges first.

This commit restores, to their `live` state, exactly the sandbox-GITS half of
d7cfde80/0b3e4c7e:

  core/manifests/apps/sbx-gits/externalsecrets.yaml   the ExternalSecret
                                                      `sbx-gits-ld-reviewer` is
                                                      back
  core/manifests/apps/sbx-gits/statefulset.yaml       its volume, its read-only
                                                      mount and the
                                                      LD_REVIEWER_IDENTITY env
                                                      are back
  core/manifests/apps/sbx-gits/README.md              the identity rows/tests
  docs/gits-sbx/{manifiesto,relevo-E1,sbx-checklist}.md
  scripts/gits-sbx-epic-test.sh                       manifest[5]/[5b] check
                                                      the pre-D6 shape again

What the branch still changes, and all it changes, is the sbx-lab half: the
ExternalSecret `sbx-lab-ld-reviewer` plus its volume and mount are gone (78
lines removed from core/manifests/apps/sbx-lab/). sbx-lab is untouched here.

The sbx-gits deletion comes back as the NEXT commit, alone, so that a single
`git revert <that sha>` puts the branch back to this state (sbx-lab only). Until
the GITS route is ruled (relay / GITS-only comment-scoped identity / hold), this
commit's tree is what lands.

Lab-Run: IR-20261112-d6
IR-20261112-d6: give the GITS seats their own posting identity (add-only)
All checks were successful
CI Review / helm-validate (pull_request) Successful in 9s
CI Review / runner-manifest-test (pull_request) Successful in 9s
CI Review / receiver-test (pull_request) Successful in 4s
CI Review / alert-receiver-test (pull_request) Successful in 20s
CI Review / record-sql-test (pull_request) Successful in 50s
Lint & Validate / shellcheck (pull_request) Successful in 7s
Lint & Validate / yaml-lint (pull_request) Successful in 22s
CI Review / cockpit-test (pull_request) Successful in 2m33s
CI Review / cockpit-e2e (pull_request) Successful in 2m46s
CI Review / dispatcher-test (pull_request) Successful in 3m18s
Lint & Validate / terraform-validate (pull_request) Successful in 1m56s
839da5c741
Operator ruling 3c 11:48:30Z, "split the swap" item (i). `sbx-gits-ld-reviewer`
STAYS in this PR: `ref/seat-clean/bin/seat-post-comment.sh` in sbx-gits-0 posts
GITS gate verdicts with its token, so removing it here would stop GITS seats
posting. D6 adds a GITS-only identity next to it instead, and the removal
travels as a separate, HELD PR.

ADD (nothing removed):

- `secretstores.yaml` (new): ServiceAccount `eso-openbao-gits-reviewer` in
  sbx-gits (`automountServiceAccountToken: false` — ESO mints its token through
  the TokenRequest API) and a NAMESPACED `SecretStore`
  `openbao-sbx-gits-gits-reviewer` that authenticates as it. NO
  ClusterSecretStore may read `kv/sbx-gits/gits-reviewer`: a namespaced store
  cannot be resolved from another namespace at all, which is the fix review 3c
  demanded for the `ld-fleet` paths (B1), applied to this one.
- `externalsecrets.yaml`: ExternalSecret `sbx-gits-gits-reviewer` -> Secret
  `sbx-gits-gits-reviewer`, keys `username`/`token` from
  `kv/sbx-gits/gits-reviewer`, `deletionPolicy: Retain`.
- `statefulset.yaml`: volume + read-only mount at
  /run/secrets/ld/identities/gits-reviewer (same pattern as the ld-reviewer
  mount, 0440 files) and `GITS_REVIEWER_IDENTITY=gits-reviewer`. The volume is
  `optional: true` so the pod starts before RUN 3 writes the kv object.
- kustomization: `secretstores.yaml` listed; the SA and the store carry
  `argocd.argoproj.io/sync-wave: "-1"` so they precede the ExternalSecret.
- `scripts/gits-sbx-epic-test.sh`: manifest[5] now checks the new ES (store,
  key, username/token), that the store is a NAMESPACED SecretStore and that NO
  ClusterSecretStore appears in that file; Retain 6 -> 7. manifest[5b] adds the
  third read-only mount. Both PASS locally on this commit.
- README: the new objects, and that `ld-reviewer` is still here.

The OpenBao half (one-path policy `eso-sbx-gits-gits-reviewer`, a role bound to
`sbx-gits/eso-openbao-gits-reviewer` ONLY, and the `eso_read` deny on BOTH
`kv/data/sbx-gits/gits-reviewer` and `kv/metadata/sbx-gits/gits-reviewer`) is
Homelab/prod #1216, this same run.

SEQUENCE (documented in both PR bodies): (i) this merged and synced -> RUN 3
writes the kv object -> force-sync -> a GITS seat posts through the new path ->
(ii) the HELD removal PR -> RUN 2.

Lab-Run: IR-20261112-d6
IR-20261112-d6: state exactly what the gits-reviewer store does and does not close
All checks were successful
CI Review / helm-validate (pull_request) Successful in 11s
CI Review / receiver-test (pull_request) Successful in 8s
CI Review / runner-manifest-test (pull_request) Successful in 13s
CI Review / alert-receiver-test (pull_request) Successful in 23s
CI Review / record-sql-test (pull_request) Successful in 1m24s
Lint & Validate / shellcheck (pull_request) Successful in 10s
Lint & Validate / yaml-lint (pull_request) Successful in 25s
CI Review / cockpit-test (pull_request) Successful in 3m12s
CI Review / cockpit-e2e (pull_request) Successful in 3m6s
CI Review / dispatcher-test (pull_request) Successful in 3m45s
Lint & Validate / terraform-validate (pull_request) Successful in 2m6s
c5b997847e
Fix round 2 N1 wording (El Director 12:33:02Z). The store header claimed the
path was "reachable from this sandbox and from no other", which is not true
while the CI path exists: prod PR branches plan under `tf-plan` (`path "*"`
read/list/sudo — the corresponding deny is in prod #1216, same run), and the
apply credentials (tf-apply, R20) and the operator identities still read
`kv/data/*`. The agreed sentence replaces the sweeping claim, verbatim:

  Outside every sandbox's direct reach; CI path residual R20 until <the earlier
  of the CI-isolation step or pi-landing leaving sbx-lab, ~11-20>

What the store itself does is stated narrowly: no OTHER sandbox can read
`kv/sbx-gits/gits-reviewer` through THIS namespaced store, because the name is
unresolvable outside `sbx-gits` and the OpenBao role binds
`sbx-gits/eso-openbao-gits-reviewer` only.

Comment-only. No field Kubernetes or OpenBao reads changes.

Lab-Run: IR-20261112-d6
All checks were successful
CI Review / helm-validate (pull_request) Successful in 11s
Required
Details
CI Review / receiver-test (pull_request) Successful in 8s
CI Review / runner-manifest-test (pull_request) Successful in 13s
CI Review / alert-receiver-test (pull_request) Successful in 23s
CI Review / record-sql-test (pull_request) Successful in 1m24s
Lint & Validate / shellcheck (pull_request) Successful in 10s
Required
Details
Lint & Validate / yaml-lint (pull_request) Successful in 25s
Required
Details
CI Review / cockpit-test (pull_request) Successful in 3m12s
CI Review / cockpit-e2e (pull_request) Successful in 3m6s
CI Review / dispatcher-test (pull_request) Successful in 3m45s
Lint & Validate / terraform-validate (pull_request) Successful in 2m6s
Required
Details
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin plane/f-r2-d6-sbx-reviewer-out:plane/f-r2-d6-sbx-reviewer-out
git switch plane/f-r2-d6-sbx-reviewer-out
Sign in to join this conversation.
No reviewers
No labels
operator-review
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Homelab/infra-core!956
No description provided.