pal-e-docs

Review: Add PSA-restricted securityContext to bases/standard (#137)

review-1118-2026-04-30 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

The bug is real and the validation note correctly identifies the PSA admission failure. However, the proposed fix as written has three concrete blast-radius problems that will break sister services or fail to actually resolve the streamlit case. Issue body needs revision before dispatching a dev agent.

Template Completeness

  • [x] Type (Bug)
  • [x] Lineage (validation-12-2026-04-30)
  • [x] Repo (forgejo_admin/pal-e-deployments)
  • [x] What Broke
  • [x] Repro Steps
  • [x] Expected Behavior
  • [x] Environment
  • [x] Acceptance Criteria
  • [x] Related

Traceability

  • [x] story:admin-row-crud label — story note story-westside-admin-admin-row-crud verified on project-westside-admin user-stories table
  • [x] arch:k8s-deploy label — arch note MISSING: search_notes(query="arch-k8s-deploy") returns zero results. Per platform-wide precedent (review-933-2026-04-10, review-935-2026-04-11, review-972-2026-04-11, review-283-2026-03-27) this is treated as a known waiver, not a per-ticket blocker. Tracked debt for a future arch-k8s-deploy note.
  • [x] type:bug — correctly classified
  • [x] blocks:deploy-chain — correct
  • [x] Forgejo issue #137 — open, accessible

File Targets

  • [x] bases/standard/deployment.yaml — verified exists at the path. Currently has zero securityContext blocks. Container is named app, port 8000, image app-image.
  • [x] overlays/westside-admin/prod/deployment-patch.yaml — verified, no securityContext.
  • [ ] overlays/westside-streamlit/prod/deployment-patch.yaml — ISSUE: contains the issue body's claim is wrong. Streamlit overlay already defines BOTH pod-level securityContext (runAsNonRoot: true, runAsUser: 1000, runAsGroup: 1000, fsGroup: 1000, seccompProfile.type: RuntimeDefault) AND container-level (allowPrivilegeEscalation: false, capabilities.drop: [ALL]). Yet validation note shows streamlit pods are also blocked. The issue body must explain why streamlit's existing securityContext is insufficient (likely root cause: kustomize strategic merge replaces vs. merges arrays at containers, OR a kustomize patch ordering issue, OR streamlit-streamlit's PSA never actually rejected streamlit and validation note conflated the two namespaces). This must be diagnosed before the fix.
  • [x] Base path is correct: 14 overlays consume ../../../bases/standard (basketball-api, gcal-scheduler, mcd-tracker-app, mcd-tracker, pal-e-app, pal-e-docs, pal-e-mail, pal-e-production, platform-validation, twitch-2k-wager, westside-admin, westside-ai-assistant, westside-streamlit, westsidekingsandqueens). pal-e-docs-api and the three playgrounds/email do not.

Repo Placement

Correct — the fix is in pal-e-deployments. No multi-repo concern.

Dependencies

Board context: #1117 (westside-admin#12 validation bug) is in qa — already merged. #1118 was surfaced by that validation. No upstream board blockers.
The fix DOES affect 14 sibling services because they all consume the same base. This is the central blast-radius concern below.

Acceptance Criteria

AC1, AC4, AC5 are testable. AC2/AC3 (deployment 0/1 -> 1/1) depend on AC4 being true (no regression elsewhere). AC4 specifically calls out basketball-api, westsidekingsandqueens, pal-e-app, pal-e-docs — this is the right list to test, but see Blast Radius for what the fix needs to do to satisfy AC4.

Blast Radius — THREE BLOCKERS

Blocker 1: runAsNonRoot: true at base will crash 9 of 14 sibling services at pod start. Audit of Dockerfiles in each consuming service:
  • Has USER directive (safe under runAsNonRoot: true): westside-admin (USER node, UID 1000), westside-streamlit (USER streamlit, UID 1000), twitch-2k-wager (USER appuser).
  • NO USER directive (runs as root, will fail with CreateContainerConfigError when runAsNonRoot: true is enforced): basketball-api, gcal-scheduler, mcd-tracker, mcd-tracker-app, pal-e-app, pal-e-docs, pal-e-mail, westside-ai-assistant, westsidekingsandqueens.
  • Not audited (no Dockerfile pulled in this review): pal-e-production, platform-validation (platform-validation Dockerfile shows no USER — runs as root).
Issue body claims "the fields are valid in any namespace; they only become required under restricted." This is true for the other three fields but FALSE for runAsNonRoot: true: that field actively rejects root containers at runtime regardless of PSA labels. AC4 will fail for at least 9 services.
Blocker 2: basketball-api init container. basketball-api/prod/deployment-patch.yaml defines an initContainer (copy-gmail-oauth using busybox). PSA-restricted requires every container in the pod to satisfy securityContext, including initContainers. A base-level container[0] securityContext does not propagate to initContainers, and basketball-api lives in a PSA-relaxed namespace today — so this is not a regression for basketball-api specifically, but if any future base consumer adopts PSA-restricted (M1 path is exactly this), the init container will fail. Must be addressed in scope or explicitly deferred.
Blocker 3: Streamlit's existing patch already defines securityContext but pods still fail. Without diagnosing why, adding the same fields at the base will not fix streamlit. Two plausible root causes the dev agent must investigate:
  • Kustomize strategic-merge container override: Streamlit's deployment-patch.yaml uses containers: [{name: app, securityContext: {...}, ...}] with no JSON patch op — strategic merge by name should merge with the base, but if the base now also defines a container[0] securityContext under a different field shape, the merge result may drop fields silently.
  • Patch ordering / target mismatch: The streamlit overlay's JSON patches rename container[0]/name from app to streamlit's container name AFTER the strategic merge — verify the rename does not drop securityContext.

Decomposition Assessment

One file target, one repo. Under the 5-minute / 3-file / 5-AC threshold. No decomposition needed if the scope is corrected. If the agent must also patch each consuming overlay's image to add a USER directive or per-overlay securityContext, scope balloons to 9+ repos — at that point, decomposition required.

Recommendation

  • [BODY] Diagnose why streamlit pods fail despite an existing securityContext. Add the diagnosis to the "What Broke" section. Without this, the fix will not unblock streamlit and AC3 cannot pass.
  • [BODY] Replace the proposed "add the four fields to the base" approach with a scoped alternative. Two options:
  • [BODY] If Option B is chosen, decompose into a parent ticket + 9 child Dockerfile PRs via skill-decompose-ticket. Block the base change until all child PRs land.
  • [BODY] Pod-level vs. container-level: PSA-restricted treats runAsNonRoot and seccompProfile.type as satisfied if set at EITHER level (per upstream k8s PSA docs — admission checks pod-level fallback). allowPrivilegeEscalation and capabilities.drop are container-level only. The streamlit overlay's split (pod-level for runAsNonRoot+seccomp, container-level for the rest) is the canonical pattern and should be the recommended shape, not the issue's all-container-level proposal.
  • [BODY] Remove the line "PSA-relaxed namespaces are unaffected (the fields are valid in any namespace; they only become required under restricted)" — runAsNonRoot: true rejects root containers regardless of PSA labels. This sentence misleads the dev agent into thinking AC4 is automatically satisfied.
  • [BODY] Add an explicit AC: "kubectl get pods -A after merge shows zero CreateContainerConfigError events across all 14 base consumers."
  • [SCOPE] arch:k8s-deploy backing note still missing platform-wide. Not blocking per precedent.

Summary

The bug is real, the validation note is sound, the proposed file target is correct. The fix shape proposed in the issue body has three concrete blast-radius problems: (1) breaks 9 services that run as root, (2) doesn't address basketball-api's initContainer, (3) doesn't actually unblock streamlit because streamlit already has the securityContext yet still fails. Reviewer recommends the per-overlay patch alternative (Option A) as the smallest fix that satisfies AC2; AC3 needs a separate streamlit-specific diagnosis ticket. Refine issue body, then re-review.