Document K8s worker identity and simplify deploy env guidance

[gummy789j]

• Add SECURITY.md section on Kubernetes worker process identity
• Remove redundant backend/.env.production; point gce-setup.sh at .env.example
• Replace k8s instancer security TODO with pointers to SECURITY.md
• Minor: reorder TYPE_CHECKING imports in MCP API key middleware

[github-actions]

Code Review Report

Project: evmbench-mcp-server
PR: main → chore/security-k8s-deploy-cleanup
Review Date: 2026-04-01
Reviewer: AI Code Reviewer (Code Review Skill v1.0.0)

---

PR Overview

Branch Information

Property	Value
From Branch	main
To Branch	chore/security-k8s-deploy-cleanup
Commits	1
Files Changed	5
Lines Added	+19
Lines Removed	-76

Commit History

Hash	Message
ecf9426	Document K8s worker identity and simplify deploy env guidance

---

Review Summary

Verdict

Verdict: ✅ Approve (with suggestions)

Findings at a Glance

	Critical	Major	Minor	Suggestion
Count	0	0	2	2

Summary

This is a focused security housekeeping PR with a single commit covering three coherent concerns: (1) transparent documentation of the known root-privilege issue in the Kubernetes instancer, (2) removal of the backend/.env.production template file that was tracked in version control, and (3) corresponding reference updates in the GCE deployment script.

The overall direction is positive. Deleting .env.production removes a duplicate template that could cause confusion with a real production secrets file. The new SECURITY.md section gives operators an honest, actionable description of the root-user risk and a clear hardening path. The auth.py and k8s.py changes are cosmetic clean-ups that improve readability without altering behaviour.

Two minor issues and two suggestions are raised below. None are blockers. The most actionable item is ensuring the GCE deployment script provides sufficiently strong guidance when it directs users to copy a file explicitly labelled "local development defaults" onto a production server.

---

Change Summary

1. Security Documentation — SECURITY.md

File	Change Type	Description
SECURITY.md	Modified	New section "Kubernetes worker process identity" added (+12 lines)

Purpose: Proactively discloses that K8s worker pods currently run as root (because the base image does not define a non-root USER), explains why this matters for untrusted code execution, lists operator-available mitigations, and describes the hardening path when the image is updated.

---

2. Production Env Template Removal — backend/.env.production

File	Change Type	Description
backend/.env.production	Deleted	64-line production env template removed from version control

Purpose: Eliminates a duplicate template file. The surviving backend/.env.example already covers all required variables (with inline comments and generation hints). Tracking a production-flavoured template in source control carries risk of operators accidentally trusting stale or incomplete placeholder values.

---

3. K8s Backend Code Cleanup — backend/instancer/backends/k8s.py

File	Change Type	Description
backend/instancer/backends/k8s.py	Modified	Replaces 7-line block of commented-out non-root settings with a single comment and clean V1SecurityContext()

Purpose: Removes the confusing # TODO block with dead commented-out code. Replaces it with a brief, authoritative comment that cross-references SECURITY.md for context, making the intentional state clear to future readers.

---

4. Auth Module Import Re-ordering — backend/api/mcp/auth.py

File	Change Type	Description
backend/api/mcp/auth.py	Modified	if TYPE_CHECKING: block moved above the module-level constant comment

Purpose: Cosmetic import ordering improvement — groups imports before runtime constants, following standard Python convention. No behavioural change.

---

5. GCE Setup Script Reference Update — deploy/gce-setup.sh

File	Change Type	Description
deploy/gce-setup.sh	Modified	Two references updated from .env.production → .env.example

Purpose: Keeps the deployment guide consistent after the deletion of .env.production. Both the prerequisite comment and the inline VM command are updated.

---

Detailed Findings

---

Minor

[MN-01] GCE deploy script directs users to a file labelled "local development defaults"

Property	Value
Severity	Minor
Category	Documentation / Security
File	deploy/gce-setup.sh : Lines 11, 101

Description

After the deletion of backend/.env.production, the GCE setup script now instructs operators to copy backend/.env.example onto the production VM before deploying. The header of .env.example explicitly states:

# Local development defaults.
# This file contains placeholder credentials/tokens that are sufficient to run
# the stack on your own machine (they are intentionally NOT secure).

Several values in .env.example are unsuitable for production without modification: SECRETS_TOKEN_RO='dev-secret', SECRETS_TOKEN_WO='dev-secret-wo', BACKEND_MCP_API_KEY=dev-secret, BACKEND_JWT_SECRET=DO_NOT_USE_ME, and OAI_PROXY_BASE_URL='http://oai.loc:8084' (an internal dev hostname that will not resolve in a GCE environment).

A user who follows the script mechanically and only partially edits the file could silently deploy with one or more weak/dev values in production.

Code

# deploy/gce-setup.sh  line 11
#   - backend/.env copied from .env.example and filled with production secrets

# deploy/gce-setup.sh  line 101
echo "     cp .env.example .env   # then edit with real secrets"

Recommendation

Add a more prominent inline warning in gce-setup.sh that the file is a dev template and that every DO_NOT_USE_ME / dev-secret / CHANGE_ME_* value must be replaced before starting the stack. Alternatively, restore a separate backend/.env.production.example that carries production-appropriate variable names/values (e.g., a placeholder BACKEND_OAI_KEY_MODE=direct instead of the proxy default), with CHANGE_ME_* markers that are visually distinct from the dev values.

Example diff for gce-setup.sh:

echo "     cp .env.example .env"
echo "     # !! Replace ALL dev placeholders (DO_NOT_USE_ME, dev-secret, etc.) !!"
echo "     # !! OAI_PROXY_BASE_URL must point to your real proxy host, not oai.loc !!"
echo "     #    then edit with real secrets"

---

[MN-02] Pod-level security_context uses container-level type V1SecurityContext instead of V1PodSecurityContext

Property	Value
Severity	Minor
Category	Correctness
File	backend/instancer/backends/k8s.py : Lines 226-228

Description

The spec field of V1PodSpec expects its security_context to be of type V1PodSecurityContext (pod-level), not V1SecurityContext (container-level). These two types have different fields: e.g., fsGroup and supplemental_groups are pod-level fields only available on V1PodSecurityContext.

Although the Kubernetes Python client is dynamically typed and will not raise a runtime TypeError, the serialised pod manifest may not include any securityContext at the pod level (or may emit an unexpected structure), causing silent no-ops when operators later try to add fields like fsGroup.

This pre-existed main, but the PR directly modifies this line by simplifying the instantiation. Since the PR's stated hardening path (SECURITY.md) depends on setting runAsNonRoot / fsGroup via this exact context object, using the correct type now prevents a subtle future bug.

Code

# backend/instancer/backends/k8s.py  line 226-228  (after PR)
spec=client.V1PodSpec(
    automount_service_account_token=False,
    # Pod runs as root until the worker image supports a non-root USER; see SECURITY.md
    security_context=client.V1SecurityContext(),   # ← should be V1PodSecurityContext

Recommendation

Switch to the correct pod-level type so that future hardening fields serialise correctly:

security_context=client.V1PodSecurityContext(),

When the worker image gains a non-root USER, the fields to add are:

security_context=client.V1PodSecurityContext(
    run_as_non_root=True,
    run_as_user=65534,
    run_as_group=65534,
    fs_group=65534,
),

---

Suggestions

[S-01] Workers run as root — consider a tracking issue

File: backend/instancer/backends/k8s.py, SECURITY.md

Description: The SECURITY.md addition transparently documents that worker pods run as root and provides a hardening path. However, there is no corresponding tracking issue or milestone reference in either the SECURITY.md text or the inline comment. This makes it easy for the remediation to slip through future prioritisation.

Suggestion: Add a TODO(#<issue-number>) reference in the inline comment once a tracking issue exists. Example:

# Pod runs as root until the worker image supports a non-root USER; see SECURITY.md
# and TODO(#42). Enable runAsNonRoot/runAsUser/fsGroup when the image does.

---

[S-02] SECRETSVC_TOKEN is injected as a plain environment variable rather than a K8s Secret

File: backend/instancer/backends/k8s.py : Line 197

Description: The read-only secrets-service token is passed to worker pods as a plain env var. This is called out in an existing inline comment (# technically, we should pass the secretsvc token as a secret ref instead). This is not changed by the PR, but the PR's stated goal is security hardening, making it a natural related item.

Suggestion: Follow through on the inline TODO by projecting the token via a K8s Secret mounted as an env var (secretKeyRef) rather than a literal string, so the value is managed by the control plane and does not appear in pod spec manifests or audit logs. Track this alongside S-01.

---

Positive Observations

Area	Observation
Security transparency	The new SECURITY.md section is candid, well-structured, and actionable — it documents both why the limitation exists and exactly what operators must do to resolve it when the image changes
Dead-code removal	Replacing 7 lines of commented-out code with a single explanatory comment and a cross-reference to documentation is the right trade-off; it removes clutter without losing context
Import hygiene	Moving TYPE_CHECKING imports above runtime constants follows the conventional Python import order and removes a minor inconsistency
Secret surface reduction	Deleting backend/.env.production from version control removes a file that could be mistaken for a live secrets file, reducing the risk of accidental exposure or trust of stale values
Existing hardening retained	automount_service_account_token=False, namespace isolation per job, and egress network policies (all unchanged by this PR) remain in place and represent strong baseline isolation

---

Checklist Results

Category	Items Checked	Pass	Fail	N/A	Notes
Correctness	4	3	1	4	MN-02: wrong security context type
Security	8	6	0	2	Dev template in prod guide (MN-01); root worker documented but unresolved (S-01)
Performance	7	0	0	7	No performance-relevant changes
Code Quality	6	6	0	0	Dead-code removal and import ordering are positive
Testing	5	0	0	5	No test changes; no new logic requiring tests
Documentation	4	4	0	0	SECURITY.md addition is clear and thorough
Compatibility	5	5	0	0	No API or schema changes
Observability	4	0	0	4	No observability changes

---

Disclaimer

This is an automated code review. It supplements but does not replace human review. The reviewer analysed only the diff between main and chore/security-k8s-deploy-cleanup. Runtime behaviour, integration testing, and deployment impact are not covered.

---

Report generated by Code Review Skill v1.0.0
Date: 2026-04-01