Ship default pod/container security contexts for all workloads

[henzigo]

Closes #2

Stacked on #49 — only the last commit belongs to this PR.

What

Ships default security contexts for all workloads, scoped by what can be proven safe:

Component	Default
all pods except cron	seccompProfile: RuntimeDefault (what Docker always applied by default)
all containers	allowPrivilegeEscalation: false (safe for root and non-root — no_new_privs does not block a master process dropping to worker uids)
nginx	fully unprivileged: uid 101, capabilities: drop ALL, readOnlyRootFilesystem: true + emptyDirs over /var/cache/nginx and /tmp
redis + exporter	fully unprivileged: uid 999 (stock image redis user, gid 1000), drop ALL, read-only root FS — safe because persistence is disabled
kubectl hook (cron-suspend)	drop ALL + no privilege escalation
phing hook Jobs	seccomp + no privilege escalation (same subset as app containers)

The nginx port-80 blocker from the issue is solved via the first option: the health listener moved to webserver.nginx.healthPort (default 8081, values-driven, used by both the templated vhost and the probes), the chart nginx.conf moved the pid file to /tmp/nginx.pid and dropped the user directive. Stock nginx:1.29-alpine stays — no image switch needed.

What is deliberately NOT defaulted (and why)

• runAsNonRoot for application images (php-fpm, storefront, consumers, phing hooks): their user is project-specific and unknown to this generic chart — a wrong guess means CreateContainerConfigError on every pod. The keys exist; projects enable it after verifying their image.
• runAsNonRoot/capabilities for RabbitMQ: existing installs have PVC data owned by the user the broker ran under, and the entrypoint needs root to fix ownership — switching the user on a populated volume bricks the broker. Documented in values.
• cron: out of scope per the issue (crond as root, tracked separately).

Notes

• redis was verified empirically against redis:7.4-alpine (docker run --user 999 → starts and accepts connections; built-in user redis = 999:1000).
• Golden impact: security contexts on all pods/containers, nginx conf changes (+ changed checksum/nginx → forces one nginx rollout on upgrade), health probes 80→8081.
• Custom webserver.nginx.config/projectConfig overrides must adopt the new pid path/health port — called out in deviations entry 24.
• The issue's "verify pods start on a real cluster (PSA restricted)" task remains for the pilot; PSA restricted will additionally require the deferred runAsNonRoot items above.
• New security_contexts_test.yaml suite (10 cases) + 2 infra assertions. 89 unit tests total.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Introduces secure-by-default pod/container security contexts across the Helm charts, while keeping defaults scoped to settings that are expected to be safe for a generic, multi-project deployment package. It also makes nginx run fully unprivileged by moving the health listener to an unprivileged port and adjusting nginx configuration to support a read-only root filesystem.

Changes:

• Add default hardening contexts (pod-level seccompProfile: RuntimeDefault and container-level allowPrivilegeEscalation: false) across workloads, plus fully-unprivileged defaults for nginx and redis.
• Move nginx health listener from port 80 to webserver.nginx.healthPort (default 8081), update probes/ports, and adjust nginx pid location + user directive removal for unprivileged operation.
• Add/update unit tests and regenerate golden snapshots to reflect the new rendered manifests.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/golden/scenarios/production-with-cloudflare/expected/first-deploy.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/production-with-cloudflare/expected/first-deploy-with-demo-data.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/production-with-cloudflare/expected/continuous.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/escaping-env/expected/first-deploy.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/escaping-env/expected/first-deploy-with-demo-data.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/escaping-env/expected/continuous.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/development-with-cloudflare/expected/first-deploy.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/development-with-cloudflare/expected/first-deploy-with-demo-data.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/development-with-cloudflare/expected/continuous.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/development-single-domain/expected/first-deploy.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/development-single-domain/expected/first-deploy-with-demo-data.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/development-single-domain/expected/continuous.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/basic-production/expected/first-deploy.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/basic-production/expected/first-deploy-with-demo-data.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
tests/golden/scenarios/basic-production/expected/continuous.yaml	Golden snapshot updates for security contexts + nginx health port/config changes.
docs/values.md	Notes that hardening defaults now ship per component.
docs/migrating-from-shopsys-deployment.md	Adds deviation entry documenting the new default security-context behavior and nginx/redis specifics.
charts/shopsys-infra/values.yaml	Sets default pod/container security contexts for redis and rabbitmq (seccomp + no privilege escalation; redis runs unprivileged + RO rootfs).
charts/shopsys-infra/tests/infra_test.yaml	Adds infra chart assertions for redis unprivileged/RO rootfs and rabbitmq hardening.
charts/shopsys-infra/templates/deployment-redis.yaml	Applies exporter container securityContext from values.
charts/shopsys-app/values.yaml	Adds default hardening for webserver/storefront/consumers; introduces webserver.nginx.healthPort and unprivileged nginx defaults.
charts/shopsys-app/tests/security_contexts_test.yaml	New test suite validating default security contexts and nginx health port wiring.
charts/shopsys-app/templates/hooks/job-post-deploy.yaml	Adds seccomp RuntimeDefault at pod level and no privilege escalation at container level.
charts/shopsys-app/templates/hooks/job-migrate-application.yaml	Adds seccomp RuntimeDefault at pod level and no privilege escalation at container level.
charts/shopsys-app/templates/hooks/job-cron-suspend.yaml	Adds seccomp RuntimeDefault and drops capabilities for the kubectl hook container.
charts/shopsys-app/templates/deployment-webserver-php-fpm.yaml	Adds emptyDirs for nginx writable paths, applies per-container securityContext, and moves probes/ports to healthPort.
charts/shopsys-app/files/nginx/project-nginx.conf	Switches health server listen port to templated webserver.nginx.healthPort.
charts/shopsys-app/files/nginx/nginx.conf	Removes user directive and moves pid file to /tmp/nginx.pid for unprivileged + RO rootfs operation.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[henzigo]

Implemented in 8e556ff: deviation 24 now ends with an explicit note that these defaults alone do not satisfy the PSA restricted profile — that additionally requires runAsNonRoot and capabilities: {drop: ["ALL"]} on every container, enabled per project, and a real-cluster verification before enforcing restricted on the namespace.

[henzigo]

Review of #50 (diff vs dedicated-service-accounts)

What I reviewed: all 28 changed files — the two nginx conf files, the webserver/redis templates, the three hook Jobs, both charts' values, the new unit-test suite, docs (deviation 24, values.md), and the regenerated golden snapshots.

What I verified (all on this branch):

• ./tests/run-golden-tests.sh — 15/15 pass; helm unittest — 89/89 pass; helm lint both charts clean; kubeconform -strict on all rendered golden output — 591/591 valid.
• Image UID claims confirmed against the actual images: nginx:1.29-alpine → nginx = 101:101; redis:7.4-alpine → redis = 999:1000. Both match the defaults.
• nginx runtime test with the chart-rendered config (extracted from the golden output): --user 101:101 --read-only --cap-drop ALL --security-opt no-new-privileges, writable mounts over /var/cache/nginx and /tmp → master+workers start as nginx, pid lands in /tmp/nginx.pid, /health on 8081 returns 200. The hardened default genuinely works. (Note: it relies on kubelet creating emptyDirs 0777 — correct on Kubernetes; my first attempt with Docker's 0755 tmpfs default failed on mkdir /var/cache/nginx/client_temp, which confirms the emptyDirs are load-bearing.)
• redis runtime test with the chart-rendered redis.conf: --user 999:1000 --read-only --cap-drop ALL → starts, PING → PONG. Persistence is indeed disabled in the shipped config, so the read-only root FS is safe.
• RabbitMQ under no_new_privs: rabbitmq:4.1-management-alpine with --security-opt no-new-privileges boots fine and beam runs as rabbitmq — the root entrypoint dropping privileges is not blocked, confirming the PR's allowPrivilegeEscalation: false rationale (same reasoning holds for php-fpm master → workers).
• Coverage is complete: every pod spec in both charts (webserver, storefront, cron, consumers, redis, rabbitmq, all three hook Jobs) either gets defaults or is explicitly exempted with a documented reason; deviation 24 and values.md were updated; goldens regenerated in the same commit. Nothing else in the charts or deploy scripts still references port 80 or /var/run/nginx.pid, and the Service/Ingress only use 8080, so the health-port move breaks nothing else.

Findings (severity-ranked):

1. Medium — the blessed "revert nginx to root" override CrashLoops at runtime (verified empirically; inline comment on the test). Overriding only runAsNonRoot/runAsUser/readOnlyRootFilesystem deep-merges with the chart default, so capabilities: {drop: [ALL]} survives — nginx as uid 0 without CAP_CHOWN then dies with chown("/var/cache/nginx/client_temp") failed (1: Operation not permitted). The revert recipe must also null out capabilities. Rendering-level unit tests cannot catch this, but the test currently documents a broken recipe.
2. Low — hook Job security contexts are hardcoded, not values-driven (inline comment) — the one place this PR deviates from the repo's "every workload exposes securityContext" convention. No escape hatch if a project's migration image ever needs privilege escalation.
3. Low — cron is excluded even from seccompProfile: RuntimeDefault (inline comment) — seccomp is orthogonal to crond running as root, so the exclusion looks broader than the tracked root issue requires. Possibly deliberate scoping; worth a one-line rationale in values if so.
4. Info — seccomp RuntimeDefault is a real behavior change, not a codification of the status quo: on Kubernetes, pods ran Unconfined by default (unless the kubelet's seccompDefault feature was enabled) — the "what Docker always applied" framing holds for docker-run, not for the legacy k8s deploys. RuntimeDefault is broadly safe for these workloads and deviation 24 documents the default itself, so no change needed — just flagging that the pilot-cluster test in the issue is the actual proof for the PHP images.
5. Nit — nginx-cache/nginx-tmp emptyDirs have no sizeLimit (inline comment): proxy/fastcgi/client-body temp files are now bounded only by node disk.

What I could NOT verify and should stay on the pilot checklist: the PHP application images (php-fpm, phing hook Jobs, consumers) under RuntimeDefault + allowPrivilegeEscalation: false on a real cluster, and the redis-exporter running as uid 999 with a read-only root FS (static binary, low risk). These are correctly left non-defaulted / low-risk by the PR, but only a real deploy proves them.

Overall: careful, well-scoped, well-documented PR — the deliberate non-defaults (app-image runAsNonRoot, RabbitMQ user, cron) all have sound reasoning, the pod-level vs container-level field split is correct throughout, and everything that was defaulted I could verify actually runs.

— Claude Agent (CR)

[henzigo]

Medium (verified): this revert recipe is broken at runtime. These overrides deep-merge with the chart defaults, so capabilities: {drop: ["ALL"]} and allowPrivilegeEscalation: false survive the merge. nginx running as uid 0 with all capabilities dropped fails on startup — I reproduced it with the chart-rendered config:

nginx: [emerg] chown("/var/cache/nginx/client_temp", 101) failed (1: Operation not permitted)

(and even past that, spawning workers needs CAP_SETUID/SETGID). A working revert must also null the capabilities out:

webserver:
  nginx:
    securityContext:
      runAsNonRoot: false
      runAsUser: 0
      runAsGroup: 0
      readOnlyRootFilesystem: false
      capabilities: null

The unit test only proves rendering, so it passes while documenting a recipe that CrashLoops. Suggest adding capabilities: null here (asserting notExists on it), and spelling the full recipe out in deviation 24 — this is exactly the override a migrating project will reach for first.

— Claude Agent (CR)

[henzigo]

Good catch — fixed in 8e556ff. The recipe in this test now includes capabilities: null with a notExists assertion on the rendered key, and deviation 24 spells out the full working override (including why the null is required: maps deep-merge, so the default drop: ["ALL"] survives and root nginx CrashLoops without CAP_CHOWN/CAP_SETUID). I also verified the null-deletion actually works through plain helm template with a values file override — the rendered container securityContext has no capabilities key.

[henzigo]

Low: hook security contexts are hardcoded, unlike every other workload. The repo convention is that each workload exposes securityContext/podSecurityContext through values, and this PR keeps that everywhere except the three hook Jobs (here, job-post-deploy.yaml, job-cron-suspend.yaml), where the contexts are baked into the template. Since the migration/post-deploy Jobs run the project's own application image, a project whose image ever needs privilege escalation (e.g. a setuid helper invoked from a migration) has no knob to turn allowPrivilegeEscalation: false off short of forking the template.

The chosen subset is safe for the stock images (I verified root→worker privilege drops still work under no_new_privs), so this is not a blocker — but consider a deploy.hooks.podSecurityContext/deploy.hooks.securityContext pair (defaulting to exactly what is hardcoded now) for consistency and as an escape hatch.

— Claude Agent (CR)

[henzigo]

Implemented in 8e556ff for the two Jobs that run the project's application image: deploy.hooks.podSecurityContext / deploy.hooks.securityContext now drive migrate-application and post-deploy, defaulting to exactly what was hardcoded (rendered output is byte-identical — golden snapshots unchanged). Covered by new unit tests (defaults on post-deploy + an override test that nulls the pod context and flips allowPrivilegeEscalation).

The cron-suspend kubectl hook deliberately stays hardcoded: it runs the chart-pinned kubectl image whose needs the chart fully knows (kubectl needs no capabilities or privilege escalation), so a values knob there would be surface without a use case — the escape-hatch concern only applies to the project-owned images.

[henzigo]

Low/question: the cron exclusion looks broader than the tracked issue requires. The crond-as-root problem is about the user; seccompProfile: RuntimeDefault at the pod level is orthogonal to running as root (it is literally what Docker applies to root containers by default), and allowPrivilegeEscalation: false is the same root-safe subset the php-fpm container gets in this PR. crond forking cron jobs as root runs fine under both. If leaving cron fully untouched is a deliberate blast-radius choice until the crond rework lands, fine — but a one-line comment on cron.podSecurityContext in values.yaml saying so would stop the next reader (or a PSS audit) from treating it as an oversight.

— Claude Agent (CR)

[henzigo]

It is a deliberate blast-radius choice — cron is the one legacy-root workload and is kept entirely untouched until the crond rework lands (tracked separately, scoped out of the security-context issue). 8e556ff adds the rationale comment above cron.securityContext in values.yaml so a PSS audit or future reader does not mistake it for an oversight; hardening remains opt-in via the existing keys.

[henzigo]

Nit: consider a sizeLimit on these emptyDirs. /var/cache/nginx now hosts client-body/proxy/fastcgi temp files on the node's disk with no bound — a large slow upload or big buffered upstream responses can consume node disk and get the pod evicted late instead of failing fast. Something like emptyDir: {sizeLimit: 1Gi} keeps the failure local to the pod.

For the record, I verified the mounts themselves are correct and sufficient: with the chart-rendered configs, nginx:1.29-alpine as 101:101, read-only root FS, drop ALL + no_new_privs starts cleanly, writes its pid to /tmp/nginx.pid, creates all temp dirs under /var/cache/nginx, and serves /health on 8081 — no fsGroup needed since kubelet creates emptyDirs 0777.

— Claude Agent (CR)

[henzigo]

Not taking this one, for two reasons:

1. The disk exposure is not new. Before this PR nginx wrote the same client-body/proxy/fastcgi temp files to the container's writable overlay layer — also bounded only by node disk. The emptyDirs merely move the write target; adding a default sizeLimit would introduce a NEW failure mode (pod eviction on crossing the limit) that never existed, and for a generic chart any fixed number is a guess — upload profiles and fastcgi buffering vary per project (uploads themselves are already capped by client_max_body_size 32m in the chart config).

2. The knob already exists without new surface: emptyDir usage counts against the pod's ephemeral-storage limits, so a project that wants a local bound can set webserver.nginx.resources.limits.ephemeral-storage — values-driven today.

Thanks for the runtime verification of the mounts — good to have the 0777-emptyDir dependency on record.