Merge branch 'main' into feature/file_upload

Reconcile the file-upload feature with main's security refactor, GCS plot
backend, and nsjail sandboxing.

- evaluation.py / preview.py: use the shared check_code_safety() from the
  new security.py; evaluation_function runs it as a pre-execution gate on
  the resolved student code (post _resolve_submission).
- security.py: keep `open` and `pathlib` allowed (main's version blocked
  them) — student code needs them to read files from params["files"] / the
  response payload; runtime _safe_open still blocks writes into the files
  dir. security_test.py updated to match.
- Dockerfile: take main's python:3.12 base (bundles shimmy + nsjail).
- CLAUDE.md: merged pipeline/env docs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VSE4TC3eTVreuQhADdtDaj

Author: m-messer

.github/workflows/production-deploy.yml

@@ -41,6 +41,10 @@ jobs:
     uses: lambda-feedback/evaluation-function-workflows/.github/workflows/deploy.yml@main
     with:
       template-repository-name: 'lambda-feedback/evaluation-function-boilerplate-python'
+      # gcp -> Cloud Run gen2 (see gcp_deploy.yml). gen2 gives full-Linux
+      # compatibility so shimmy's nsjail sandbox (SANDBOX_ENABLED=true in the
+      # Dockerfile) can create user namespaces -- AWS Lambda cannot.
+      build-platforms: "gcp"
       environment: "production"
       version-bump: ${{ inputs.version-bump }}
       branch: ${{ inputs.branch }}

.github/workflows/staging-deploy.yml

@@ -59,7 +59,10 @@ jobs:
     uses: lambda-feedback/evaluation-function-workflows/.github/workflows/deploy.yml@main
     with:
       template-repository-name: "lambda-feedback/evaluation-function-boilerplate-python"
-      build-platforms: "aws"
+      # gcp -> Cloud Run gen2 (see gcp_deploy.yml). gen2 gives full-Linux
+      # compatibility so shimmy's nsjail sandbox (SANDBOX_ENABLED=true in the
+      # Dockerfile) can create user namespaces -- AWS Lambda cannot.
+      build-platforms: "gcp"
       environment: "staging"
       lfs: false
     secrets:

CLAUDE.md

@@ -9,7 +9,7 @@ All source lives in `evaluation_function/`:
 | File | Role |
 |------|------|
 | `main.py` | IPC server entry point; registers `evaluation_function` and `preview_function` with lf_toolkit |
-| `evaluation.py` | Core evaluation pipeline: security check → subprocess execution → output comparison → S3 plot upload → structured feedback |
+| `evaluation.py` | Core evaluation pipeline: security check → subprocess execution → output comparison → plot upload (GCS/S3 via lf_toolkit) → structured feedback |
 | `preview.py` | AST-based pre-execution security validator (`_SecurityVisitor`) |
 | `s3_files.py` | Downloads `params["files"]` objects from S3 into the per-request working directory |
 | `dev.py` | CLI wrapper for local manual testing |
@@ -22,7 +22,7 @@ All source lives in `evaluation_function/`:
    - **`demo`**: execute code with no stdin; return stdout/plots as `output` feedback (no pass/fail)
    - **`io_test`**: for each test in `params["tests"]`, execute with `test["input"]` as stdin and compare stdout against `test["expected_output"]`; upload matplotlib plots on pass or fail
    - **`unit_test`**: append `params["test_code"]` + unit-runner harness to student code; execute once; parse JSON results; supports plain `test_*` functions, `unittest.TestCase` subclasses, and Hypothesis-based tests
-4. Upload any captured matplotlib figures to S3 (`_UPLOAD_FOLDER = "evaluatePython"`)
+4. Upload any captured matplotlib figures via `lf_toolkit` `upload_image` (`_UPLOAD_FOLDER = "evaluatePython"`); backend is GCS or S3 per `IMAGE_UPLOAD_BACKEND`
 5. Return a `Result` with feedback tags: `pass`, `fail`, `hidden_fail`, `error`, `output`, `summary`
 
 ### Request shape
@@ -156,16 +156,19 @@ docker build -t evaluatepython .
 # Cross-platform (CI uses linux/x86_64):
 docker build --platform=linux/x86_64 .
 
-# Run the server locally (port 8080)
-docker run -it --rm -p 8080:8080 evaluatepython
+# Run the server locally (port 8080).
+# --privileged is required: the image enables shimmy's nsjail sandbox
+# (SANDBOX_ENABLED=true), and nsjail needs it to create namespaces.
+docker run -it --rm --privileged -p 8080:8080 evaluatepython
 ```
 
 ## Tests
 
-Two test files, run with `pytest`:
+Three test files, run with `pytest`:
 
-- `evaluation_function/evaluation_test.py` — integration tests covering: all modes (demo, io_test, unit_test), all pass, partial fail, hidden test failure, runtime error, matplotlib plot capture, Hypothesis support
+- `evaluation_function/evaluation_test.py` — integration tests covering: all modes (demo, io_test, unit_test), all pass, partial fail, hidden test failure, runtime error, matplotlib plot capture, Hypothesis support, the pre-execution security gate
 - `evaluation_function/preview_test.py` — unit tests covering: valid Python, syntax errors, dangerous imports, dangerous builtins, dunder access
+- `evaluation_function/security_test.py` — unit tests for `check_code_safety` (the shared AST blocklist used by both `preview_function` and `evaluation_function`)
 
 CI runs on Python 3.12 and uploads JUnit XML results (`.github/workflows/test-lint.yml`).
 
@@ -177,14 +180,20 @@ CI runs on Python 3.12 and uploads JUnit XML results (`.github/workflows/test-li
 | `MPLBACKEND` | `Agg` | Set at subprocess runtime to suppress GUI |
 | `FUNCTION_COMMAND` | `python` | lf_toolkit runner |
 | `FUNCTION_ARGS` | `-m,evaluation_function.main` | lf_toolkit runner |
-| `FUNCTION_RPC_TRANSPORT` | `ipc` | lf_toolkit transport |
+| `FUNCTION_RPC_TRANSPORT` | `stdio` | shimmy↔worker transport (stdio so it survives the sandbox mount namespace) |
 | `LOG_LEVEL` | `debug` | Logging verbosity |
-| `AWS_*` / boto3 credentials | Runtime env | Required for S3 plot uploads. Not needed for `params["files"]` downloads — those are fetched via plain HTTPS GET from a pre-signed/public URL |
+| `IMAGE_UPLOAD_BACKEND` | `gcs` | Plot upload backend in lf_toolkit (`gcs` set in Dockerfile; override to `s3` on the service to use AWS) |
+| `GCS_BUCKET` | Runtime env | Target bucket for matplotlib plot uploads; set per-environment on the Cloud Run service. Auth is via the runtime service account (ADC) — no keys |
+| `AWS_*` / `S3_BUCKET_URI` | Runtime env | Only for the legacy S3 plot-upload backend (`IMAGE_UPLOAD_BACKEND=s3`). Not needed for `params["files"]` / response-payload file downloads — those are plain HTTPS GETs from a pre-signed/public URL |
+| `SANDBOX_ENABLED` | `true` | Wrap the worker in shimmy's nsjail sandbox (needs `--privileged` at run time) |
+| `SANDBOX_SECCOMP` | `true` | nsjail seccomp syscall filter |
+| `SANDBOX_RO_BINDS` | `/usr:/lib:/lib64:/bin:/sbin:/etc:/app` | Read-only bind mounts visible inside the jail |
+| `SANDBOX_TMPFS` | `/tmp` | Writable tmpfs inside the jail (student scripts, plot dirs, MPLCONFIGDIR) |
 
 Dependencies managed via Poetry; `.venv` is created in-project (`poetry.toml`).
 
 ## Deployment
 
 - Push to `main` triggers GitHub Actions (`.github/workflows/`) which builds and deploys to Lambda Feedback automatically
 - The function name is declared in `config.json` as `EvaluationFunctionName: "evaluatePython"` (lowerCamelCase)
-- The base Docker image is `ghcr.io/lambda-feedback/evaluation-function-base/python:test-sandbox-3.12`
+- The base Docker image is `ghcr.io/lambda-feedback/evaluation-function-base/python:3.12` (bundles shimmy + nsjail; sandboxing is enabled via the `SANDBOX_*` env vars in the Dockerfile, not by the base tag)

Dockerfile

@@ -1,4 +1,4 @@
-FROM ghcr.io/lambda-feedback/evaluation-function-base/python:latest AS builder
+FROM ghcr.io/lambda-feedback/evaluation-function-base/python:3.12 AS builder
 
 RUN pip install poetry==1.8.3
 
@@ -12,7 +12,7 @@ COPY pyproject.toml poetry.lock ./
 RUN --mount=type=cache,target=$POETRY_CACHE_DIR \
     poetry install --without dev --no-root
 
-FROM ghcr.io/lambda-feedback/evaluation-function-base/python:latest
+FROM ghcr.io/lambda-feedback/evaluation-function-base/python:3.12
 
 ENV VIRTUAL_ENV=/app/.venv \
     PATH="/app/.venv/bin:$PATH"
@@ -25,13 +25,60 @@ RUN python -m compileall -q .
 # Copy the evaluation function to the app directory
 COPY evaluation_function ./evaluation_function
 
-# Command to start the evaluation function with
-ENV FUNCTION_COMMAND="python"
+# How lf_toolkit's runner launches the worker.
+ENV FUNCTION_COMMAND="python" \
+    FUNCTION_ARGS="-m,evaluation_function.main"
 
-# Args to start the evaluation function with
-ENV FUNCTION_ARGS="-m,evaluation_function.main"
+# RPC transport: stdio, not ipc. The sandboxed worker runs in an nsjail mount
+# namespace where the /tmp/eval.sock IPC rendezvous is fragile; stdio sidesteps
+# it. lf_toolkit logs to stderr, so stdout stays clean for the RPC framing.
+ENV FUNCTION_RPC_TRANSPORT="stdio"
 
-# The transport to use for the RPC server
-ENV FUNCTION_RPC_TRANSPORT="ipc"
+# --- Sandboxed execution of untrusted student code (shimmy + nsjail) ---
+# Always on. shimmy wraps the worker -- and every `python` subprocess it spawns
+# per submission -- in nsjail: run as nobody, a read-only bind-mounted rootfs,
+# namespace isolation. Defence in depth on top of the AST gate in
+# evaluation_function/security.py, which rejects unsafe imports/builtins before
+# any code runs.
+#
+# Requires a base image that ships /usr/sbin/nsjail AND a shimmy build with the
+# sandbox fixes (--keep_env, PATH resolution of FUNCTION_COMMAND, --cwd fallback,
+# kafel seccomp, namespace toggles). With stock shimmy the worker fails to start.
+#
+# Run-time: the container must run --privileged (or --cap-add SYS_ADMIN) -- nsjail
+# needs CAP_SYS_ADMIN for unshare(CLONE_NEWNS). Running as uid 0, nsjail's "auto"
+# userns handling then drops CLONE_NEWUSER (a nested userns' unprivileged gid_map
+# write fails); --user still drops the worker to nobody. Network stays up so
+# matplotlib plots can be uploaded to object storage (see below).
+#
+# SANDBOX_RO_BINDS / _RW_BINDS: shimmy splits these env vars on COMMA. Bind "/"
+# read-only (whole rootfs -- arch-independent, where an explicit list would need
+# /lib64 only on x86_64, and a missing bind source is fatal to nsjail), then
+# re-mount /tmp read-write. Not SANDBOX_TMPFS: nsjail's tmpfs defaults to 4 MiB,
+# too small for matplotlib's font cache + plot output.
+#
+# SANDBOX_DISABLE_CLONE_NEWPID: a nested PID namespace breaks worker thread
+# creation on some hosts ("pthread_create ... Invalid argument"); the mount and
+# user namespaces still isolate the filesystem and privileges.
+#
+# Network stays up so matplotlib plots (see evaluation.py::_upload_plots) can be
+# pushed to object storage. On GCP we use lf_toolkit's GCS backend
+# (IMAGE_UPLOAD_BACKEND=gcs): the worker authenticates with Application Default
+# Credentials via the Cloud Run runtime service account -- no static keys -- and
+# needs GCS_BUCKET set on the service (staging/prod differ). To fall back to S3,
+# override IMAGE_UPLOAD_BACKEND=s3 on the service and set S3_BUCKET_URI / AWS_*.
+#
+# No seccomp (nsjail has no built-in default policy; the fixed shimmy takes a
+# kafel policy via SANDBOX_SECCOMP_STRING / _POLICY_FILE if wanted) and no
+# rlimits (the RPC worker is long-lived and shared; per-run limits are the
+# _TIMEOUT in evaluation.py).
+ENV SANDBOX_ENABLED="true" \
+    SANDBOX_RO_BINDS="/" \
+    SANDBOX_RW_BINDS="/tmp" \
+    SANDBOX_DISABLE_CLONE_NEWPID="true"
+
+# Plot upload backend (lf_toolkit). GCS_BUCKET is supplied per-environment on the
+# Cloud Run service.
+ENV IMAGE_UPLOAD_BACKEND="gcs"
 
 ENV LOG_LEVEL="debug"

README.md

@@ -12,11 +12,13 @@ Push to `main` triggers GitHub Actions which automatically builds and deploys to
 ### Run the Docker Image
 
 ```bash
-docker run -it --rm -p 8080:8080 ghcr.io/lambda-feedback/evaluatepython:latest
+docker run -it --rm --privileged -p 8080:8080 ghcr.io/lambda-feedback/evaluatepython:latest
 ```
 
 The image includes [Shimmy](https://github.com/lambda-feedback/shimmy), which listens for HTTP requests on port 8080 and forwards them to the evaluation function.
 
+`--privileged` (or `--cap-add SYS_ADMIN`) is required: the image runs student code inside Shimmy's [nsjail](https://github.com/google/nsjail) sandbox (`SANDBOX_ENABLED=true`), and nsjail needs those privileges to create its namespaces. The sandbox runs the worker as `nobody` with a minimal bind-mounted filesystem and seccomp filtering; untrusted imports/builtins are additionally rejected before execution by the AST check in `evaluation_function/security.py`.
+
 ### Evaluation Modes
 
 The function supports three modes, set via `params.mode`.
@@ -80,10 +82,12 @@ Add `"pep8_feedback": true` to any mode to append a style check to the feedback.
 ```
 evaluation_function/main.py             # IPC server entry point
 evaluation_function/evaluation.py       # core evaluation pipeline (all three modes)
-evaluation_function/preview.py          # AST-based security validator
+evaluation_function/security.py         # shared AST blocklist (check_code_safety)
+evaluation_function/preview.py          # pre-submission validator (syntax + security)
 evaluation_function/dev.py              # CLI wrapper for local testing
 evaluation_function/evaluation_test.py  # integration tests
 evaluation_function/preview_test.py     # preview/security tests
+evaluation_function/security_test.py    # check_code_safety unit tests
 config.json                             # deployment configuration
 ```
 
@@ -144,9 +148,11 @@ docker build --platform=linux/x86_64 -t evaluatepython .
 ### Running the Docker Image
 
 ```bash
-docker run -it --rm -p 8080:8080 evaluatepython
+docker run -it --rm --privileged -p 8080:8080 evaluatepython
 ```
 
+`--privileged` is required for the nsjail sandbox (see [Run the Docker Image](#run-the-docker-image)). To run without it for local debugging, disable the sandbox: add `-e SANDBOX_ENABLED=false`.
+
 ## Deployment to Lambda Feedback
 
 The function name is declared in [`config.json`](config.json) as `"evaluatePython"` (lowerCamelCase). Pushing to `main` triggers automated deployment via GitHub Actions.

docs/dev.md

@@ -7,6 +7,12 @@
 - **`evaluation_function`** — run and grade student code
 - **`preview_function`** — static AST-based security check (called before execution)
 
+Both share the AST blocklist in `security.py` (`check_code_safety`). `preview_function`
+runs it pre-submission; `evaluation_function` also runs it as a hard gate — if the
+submission trips the blocklist it is **not executed** and an `error` item is returned.
+At the container level, shimmy additionally wraps execution in an nsjail sandbox
+(`SANDBOX_*` env in the `Dockerfile`).
+
 ---
 
 ## `evaluation_function`
@@ -173,7 +179,9 @@ The check runs in-process via `pycodestyle.Checker` with `max_line_length=200` (
 
 ## `preview_function`
 
-Called before evaluation. Parses the student code as an AST and checks for security violations.
+Called before evaluation. Parses the student code as an AST and checks for security
+violations via `security.check_code_safety`. The same check gates `evaluation_function`,
+so the blocklist below applies to both entry points.
 
 ### Blocked constructs
 

evaluation_function/evaluation.py

@@ -14,6 +14,7 @@
 from lf_toolkit.evaluation.image_upload import upload_image, ImageUploadError
 
 from .s3_files import download_files
+from .security import check_code_safety
 
 _TIMEOUT = 25
 _UPLOAD_FOLDER = "evaluatePython"
@@ -379,6 +380,14 @@ def evaluation_function(response: Any, answer: Any, params: Params) -> Result:
 
     code, file_specs = _resolve_submission(response, params)
 
+    violations = check_code_safety(code)
+    if violations:
+        result.add_feedback(
+            "error",
+            "Unsafe code detected -- not executed:\n" + "\n".join(f"- {v}" for v in violations),
+        )
+        return result
+
     files_dir = None
     try:
         file_warnings: list[str] = []

evaluation_function/evaluation_test.py

@@ -593,4 +593,40 @@ def test_pep8_appended_to_io_test_result(self):
         params = {**_params(_test("5\n", "25\n")), "pep8_feedback": True}
         result = evaluation_function(_SQUARE_CODE, None, params).to_dict()
         self.assertTrue(result["is_correct"])
-        self.assertIn("No style issues found", result["feedback"])
+        self.assertIn("No style issues found", result["feedback"])
+
+
+class TestSecurityGate(unittest.TestCase):
+
+    def test_blocked_import_not_executed(self):
+        result = evaluation_function(
+            "import os\nprint(os.getcwd())", None, {"mode": "demo"}
+        ).to_dict()
+        self.assertFalse(result["is_correct"])
+        self.assertIn("Unsafe code detected", result["feedback"])
+        self.assertIn("os", result["feedback"])
+
+    def test_blocked_builtin_not_executed(self):
+        result = evaluation_function(
+            "eval('1 + 1')", None, _params(_test("", ""))
+        ).to_dict()
+        self.assertIn("Unsafe code detected", result["feedback"])
+        self.assertNotIn("tests passed", result["feedback"])
+
+    def test_dunder_access_blocked_in_unit_test(self):
+        result = evaluation_function(
+            "().__class__.__bases__", None, _unit_params(_SQUARE_TESTS)
+        ).to_dict()
+        self.assertIn("Unsafe code detected", result["feedback"])
+
+    def test_safe_stdlib_import_still_runs(self):
+        result = evaluation_function(
+            "import math\nprint(math.sqrt(16))", None, {"mode": "demo"}
+        ).to_dict()
+        self.assertIn("4.0", result["feedback"])
+        self.assertNotIn("Unsafe", result["feedback"])
+
+    def test_syntax_error_passes_gate_and_surfaces_downstream(self):
+        result = evaluation_function("def f(:\n", None, {"mode": "demo"}).to_dict()
+        self.assertNotIn("Unsafe code detected", result["feedback"])
+        self.assertIn("SyntaxError", result["feedback"])

evaluation_function/preview.py

@@ -2,55 +2,19 @@
 from typing import Any
 from lf_toolkit.preview import Result, Params, Preview
 
-_BLOCKED_MODULES = {
-    "os", "sys", "subprocess", "socket", "urllib", "http",
-    "requests", "shutil", "ftplib", "smtplib",
-    "ctypes", "multiprocessing", "threading", "importlib",
-    "pickle", "builtins",
-}
-
-_BLOCKED_BUILTINS = {"exec", "eval", "compile", "__import__"}
-
-
-class _SecurityVisitor(ast.NodeVisitor):
-    def __init__(self):
-        self.violations: list[str] = []
-
-    def visit_Import(self, node):
-        for alias in node.names:
-            root = alias.name.split(".")[0]
-            if root in _BLOCKED_MODULES:
-                self.violations.append(f"import of '{root}' is not allowed")
-        self.generic_visit(node)
-
-    def visit_ImportFrom(self, node):
-        if node.module:
-            root = node.module.split(".")[0]
-            if root in _BLOCKED_MODULES:
-                self.violations.append(f"import of '{root}' is not allowed")
-        self.generic_visit(node)
-
-    def visit_Call(self, node):
-        if isinstance(node.func, ast.Name) and node.func.id in _BLOCKED_BUILTINS:
-            self.violations.append(f"use of '{node.func.id}()' is not allowed")
-        self.generic_visit(node)
-
-    def visit_Attribute(self, node):
-        if node.attr.startswith("__") and node.attr.endswith("__"):
-            self.violations.append(f"access to '{node.attr}' is not allowed")
-        self.generic_visit(node)
+from .security import check_code_safety
 
 
 def preview_function(response: Any, params: Params) -> Result:
+    code = str(response)
     try:
-        tree = ast.parse(str(response))
+        ast.parse(code)
     except SyntaxError as e:
         return Result(preview=Preview(feedback=f"SyntaxError: {e.msg} (line {e.lineno})"))
 
-    visitor = _SecurityVisitor()
-    visitor.visit(tree)
-    if visitor.violations:
-        lines = "\n".join(f"- {v}" for v in visitor.violations)
+    violations = check_code_safety(code)
+    if violations:
+        lines = "\n".join(f"- {v}" for v in violations)
         return Result(preview=Preview(feedback=f"Unsafe code detected:\n{lines}"))
 
     return Result(preview=Preview(feedback="Valid Python syntax."))