Replace S3-based file downloads with HTTPS support

Replaces S3 bucket-based file download logic with direct downloads via HTTPS URLs, removing AWS dependency. Updates `evaluation.py`, rewrites `s3_files.py` to handle streaming downloads securely, and adjusts tests and documentation (`CLAUDE.md`, `evaluation_test.py`, `s3_files_test.py`) to reflect the changes.

Author: m-messer

CLAUDE.md

@@ -94,17 +94,18 @@ All source lives in `evaluation_function/`:
 }
 
 # files — optional, works with all modes
-# Downloads objects from S3 into a per-request working directory (the
-# subprocess's cwd) before student code runs. Data files can be read with
-# open()/pandas.read_csv()/etc.; .py files are importable by student code
-# since they're co-located with the generated script. The same files are
-# also available to the answer code when use_answer_as_expected_output /
-# use_answer_as_test_code is set. Requires the S3_FILES_BUCKET env var.
+# Downloads files into a per-request working directory (the subprocess's
+# cwd) before student code runs, given a pre-signed or public HTTPS URL per
+# file (fetched directly with a GET — no AWS credentials needed here). Data
+# files can be read with open()/pandas.read_csv()/etc.; .py files are
+# importable by student code since they're co-located with the generated
+# script. The same files are also available to the answer code when
+# use_answer_as_expected_output/use_answer_as_test_code is set.
 {
     "mode": "demo",
     "files": [
-        {"key": "uploads/<question-id>/data.csv", "filename": "data.csv"},
-        {"key": "uploads/<question-id>/helper.py", "filename": "helper.py"},
+        {"url": "https://.../data.csv?X-Amz-Signature=...", "filename": "data.csv"},
+        {"url": "https://.../helper.py?X-Amz-Signature=...", "filename": "helper.py"},
     ]
 }
 ```
@@ -167,8 +168,7 @@ CI runs on Python 3.12 and uploads JUnit XML results (`.github/workflows/test-li
 | `FUNCTION_ARGS` | `-m,evaluation_function.main` | lf_toolkit runner |
 | `FUNCTION_RPC_TRANSPORT` | `ipc` | lf_toolkit transport |
 | `LOG_LEVEL` | `debug` | Logging verbosity |
-| `AWS_*` / boto3 credentials | Runtime env | Required for S3 plot uploads and `files` downloads |
-| `S3_FILES_BUCKET` | Runtime env | Bucket name (not URI) that `params["files"]` object keys are resolved against |
+| `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 |
 
 Dependencies managed via Poetry; `.venv` is created in-project (`poetry.toml`).
 

evaluation_function/evaluation.py

@@ -11,7 +11,7 @@
 from lf_toolkit.evaluation import Result, Params
 from lf_toolkit.evaluation.image_upload import upload_image, ImageUploadError
 
-from .s3_files import download_files, FileDownloadError
+from .s3_files import download_files
 
 _TIMEOUT = 25
 _UPLOAD_FOLDER = "evaluatePython"
@@ -332,10 +332,7 @@ def evaluation_function(response: Any, answer: Any, params: Params) -> Result:
     file_specs = params.get("files")
     if file_specs:
         files_dir = tempfile.mkdtemp()
-        try:
-            file_warnings = download_files(file_specs, files_dir)
-        except FileDownloadError as e:
-            file_warnings = [str(e)]
+        file_warnings = download_files(file_specs, files_dir)
 
     try:
         if mode == "demo":

evaluation_function/evaluation_test.py

@@ -317,7 +317,7 @@ class TestFileDownloads(unittest.TestCase):
     @patch("evaluation_function.evaluation.download_files")
     def test_demo_mode_can_read_downloaded_file(self, mock_download):
         mock_download.side_effect = _stub_download({"data.csv": "1,2,3"})
-        params = {"mode": "demo", "files": [{"key": "k", "filename": "data.csv"}]}
+        params = {"mode": "demo", "files": [{"url": "https://example.com/k", "filename": "data.csv"}]}
         result = evaluation_function("print(open('data.csv').read())", None, params).to_dict()
 
         self.assertIn("1,2,3", result["feedback"])
@@ -327,7 +327,7 @@ def test_io_test_downloads_once_for_all_tests(self, mock_download):
         mock_download.side_effect = _stub_download({"data.csv": "42"})
         params = {
             "mode": "io_test",
-            "files": [{"key": "k", "filename": "data.csv"}],
+            "files": [{"url": "https://example.com/k", "filename": "data.csv"}],
             "tests": [_test("", "42\n"), _test("", "42\n")],
         }
         result = evaluation_function("print(open('data.csv').read())", None, params).to_dict()
@@ -341,7 +341,7 @@ def test_answer_code_receives_same_files(self, mock_download):
         params = {
             "mode": "io_test",
             "use_answer_as_expected_output": True,
-            "files": [{"key": "k", "filename": "data.csv"}],
+            "files": [{"url": "https://example.com/k", "filename": "data.csv"}],
             "tests": [{"input": ""}],
         }
         code = "print(open('data.csv').read())"
@@ -352,15 +352,15 @@ def test_answer_code_receives_same_files(self, mock_download):
     @patch("evaluation_function.evaluation.download_files")
     def test_missing_file_reported_as_warning(self, mock_download):
         mock_download.return_value = ["File 'data.csv' could not be found."]
-        params = {"mode": "demo", "files": [{"key": "k", "filename": "data.csv"}]}
+        params = {"mode": "demo", "files": [{"url": "https://example.com/k", "filename": "data.csv"}]}
         result = evaluation_function("print('hi')", None, params).to_dict()
 
         self.assertIn("could not be found", result["feedback"])
 
     @patch("evaluation_function.evaluation.download_files")
     def test_import_of_uploaded_module(self, mock_download):
         mock_download.side_effect = _stub_download({"helper.py": "def square(n):\n    return n * n\n"})
-        params = {"mode": "demo", "files": [{"key": "k", "filename": "helper.py"}]}
+        params = {"mode": "demo", "files": [{"url": "https://example.com/k", "filename": "helper.py"}]}
         result = evaluation_function("import helper\nprint(helper.square(4))", None, params).to_dict()
 
         self.assertIn("16", result["feedback"])
@@ -376,31 +376,31 @@ class TestFileAccessSandbox(unittest.TestCase):
     @patch("evaluation_function.evaluation.download_files")
     def test_read_downloaded_file_succeeds(self, mock_download):
         mock_download.side_effect = _stub_download({"data.csv": "hello"})
-        params = {"mode": "demo", "files": [{"key": "k", "filename": "data.csv"}]}
+        params = {"mode": "demo", "files": [{"url": "https://example.com/k", "filename": "data.csv"}]}
         result = evaluation_function("print(open('data.csv').read())", None, params).to_dict()
 
         self.assertIn("hello", result["feedback"])
 
     @patch("evaluation_function.evaluation.download_files")
     def test_write_mode_to_provided_file_blocked(self, mock_download):
         mock_download.side_effect = _stub_download({"data.csv": "hello"})
-        params = {"mode": "demo", "files": [{"key": "k", "filename": "data.csv"}]}
+        params = {"mode": "demo", "files": [{"url": "https://example.com/k", "filename": "data.csv"}]}
         result = evaluation_function("open('data.csv', 'w')", None, params).to_dict()
 
         self.assertIn("read-only", result["feedback"])
 
     @patch("evaluation_function.evaluation.download_files")
     def test_write_new_file_in_run_dir_blocked(self, mock_download):
         mock_download.side_effect = _stub_download({"data.csv": "hello"})
-        params = {"mode": "demo", "files": [{"key": "k", "filename": "data.csv"}]}
+        params = {"mode": "demo", "files": [{"url": "https://example.com/k", "filename": "data.csv"}]}
         result = evaluation_function("open('output.txt', 'w')", None, params).to_dict()
 
         self.assertIn("read-only", result["feedback"])
 
     @patch("evaluation_function.evaluation.download_files")
     def test_pathlib_read_respects_sandbox(self, mock_download):
         mock_download.side_effect = _stub_download({"data.csv": "world"})
-        params = {"mode": "demo", "files": [{"key": "k", "filename": "data.csv"}]}
+        params = {"mode": "demo", "files": [{"url": "https://example.com/k", "filename": "data.csv"}]}
         code = "from pathlib import Path\nprint(Path('data.csv').read_text())"
         result = evaluation_function(code, None, params).to_dict()
 
@@ -409,7 +409,7 @@ def test_pathlib_read_respects_sandbox(self, mock_download):
     @patch("evaluation_function.evaluation.download_files")
     def test_pathlib_write_respects_sandbox(self, mock_download):
         mock_download.side_effect = _stub_download({"data.csv": "world"})
-        params = {"mode": "demo", "files": [{"key": "k", "filename": "data.csv"}]}
+        params = {"mode": "demo", "files": [{"url": "https://example.com/k", "filename": "data.csv"}]}
         code = "from pathlib import Path\nPath('data.csv').write_text('nope')"
         result = evaluation_function(code, None, params).to_dict()
 

evaluation_function/s3_files.py

@@ -1,106 +1,110 @@
 import os
 from typing import TypedDict
+from urllib.parse import urlparse
 
-import boto3
-from botocore.config import Config
-from botocore.exceptions import ClientError
+import requests
 
 _MAX_FILE_BYTES = 5 * 1024 * 1024
 _MAX_TOTAL_BYTES = 20 * 1024 * 1024
 _DOWNLOAD_TIMEOUT = 10
+_CHUNK_SIZE = 65536
 
 
 class FileSpec(TypedDict):
-    key: str
+    url: str
     filename: str
 
 
-class FileDownloadError(Exception):
-    """Raised for whole-request configuration problems (e.g. missing bucket env var)."""
-    pass
-
+def _valid_filename(filename: str) -> bool:
+    if not filename or filename in (".", ".."):
+        return False
+    return os.path.basename(filename) == filename
 
-def _s3_client():
-    return boto3.client(
-        "s3",
-        region_name=os.environ.get("AWS_REGION", "eu-west-2"),
-        config=Config(
-            connect_timeout=_DOWNLOAD_TIMEOUT,
-            read_timeout=_DOWNLOAD_TIMEOUT,
-            retries={"max_attempts": 2},
-        ),
-    )
 
+class _FileTooLarge(Exception):
+    pass
 
-def _get_bucket_name() -> str:
-    bucket = os.environ.get("S3_FILES_BUCKET")
-    if not bucket:
-        raise FileDownloadError("S3_FILES_BUCKET environment variable is not set")
-    return bucket
 
+def _download_one(url: str, target: str, remaining_budget: int) -> int:
+    """Stream url into target. Returns bytes written.
 
-def _valid_filename(filename: str) -> bool:
-    if not filename or filename in (".", ".."):
-        return False
-    return os.path.basename(filename) == filename
+    Raises _FileTooLarge (and removes any partial file) if the download
+    exceeds _MAX_FILE_BYTES or remaining_budget, or requests.RequestException
+    for network/HTTP errors — both handled by the caller.
+    """
+    resp = requests.get(url, stream=True, timeout=_DOWNLOAD_TIMEOUT)
+    resp.raise_for_status()
+
+    content_length = resp.headers.get("Content-Length")
+    cap = min(_MAX_FILE_BYTES, remaining_budget)
+    if content_length is not None and int(content_length) > cap:
+        raise _FileTooLarge()
+
+    written = 0
+    try:
+        with open(target, "wb") as f:
+            for chunk in resp.iter_content(chunk_size=_CHUNK_SIZE):
+                written += len(chunk)
+                if written > cap:
+                    raise _FileTooLarge()
+                f.write(chunk)
+    except _FileTooLarge:
+        if os.path.exists(target):
+            os.unlink(target)
+        raise
+    return written
 
 
 def download_files(files: list[FileSpec], dest_dir: str) -> list[str]:
     """Download each file into dest_dir.
 
-    Returns a list of warning strings for files that were skipped (missing,
-    too large, or errored) — never raises for per-file problems, only for
-    whole-config problems (missing bucket env var).
+    Returns a list of warning strings for files that were skipped (invalid
+    filename/URL, too large, or errored) — never raises.
     """
     if not files:
         return []
 
-    bucket = _get_bucket_name()
-    client = _s3_client()
-
+    real_dest_dir = os.path.realpath(dest_dir)
     warnings: list[str] = []
     total_bytes = 0
 
     for spec in files:
-        key = spec["key"]
+        url = spec["url"]
         filename = spec["filename"]
 
         if not _valid_filename(filename):
             warnings.append(f"File '{filename}' has an invalid filename and was not made available.")
             continue
 
-        real_dest_dir = os.path.realpath(dest_dir)
         target = os.path.realpath(os.path.join(real_dest_dir, filename))
         if os.path.commonpath([target, real_dest_dir]) != real_dest_dir:
             warnings.append(f"File '{filename}' has an invalid filename and was not made available.")
             continue
 
-        try:
-            head = client.head_object(Bucket=bucket, Key=key)
-        except ClientError as e:
-            warnings.append(f"File '{filename}' could not be found or accessed ({e}).")
+        if urlparse(url).scheme != "https":
+            warnings.append(f"File '{filename}' has an invalid URL and was not made available.")
             continue
 
-        size = head.get("ContentLength", 0)
-        if size > _MAX_FILE_BYTES:
-            warnings.append(
-                f"File '{filename}' exceeds the {_MAX_FILE_BYTES // (1024 * 1024)}MB size limit "
-                "and was not made available."
-            )
-            continue
-        if total_bytes + size > _MAX_TOTAL_BYTES:
+        remaining_budget = _MAX_TOTAL_BYTES - total_bytes
+        if remaining_budget <= 0:
             warnings.append(
                 f"File '{filename}' was skipped because it would exceed the total "
                 f"{_MAX_TOTAL_BYTES // (1024 * 1024)}MB size limit for this run."
             )
             continue
 
         try:
-            client.download_file(bucket, key, target)
-        except ClientError as e:
+            written = _download_one(url, target, remaining_budget)
+        except _FileTooLarge:
+            warnings.append(
+                f"File '{filename}' exceeds the {_MAX_FILE_BYTES // (1024 * 1024)}MB size limit "
+                "and was not made available."
+            )
+            continue
+        except requests.exceptions.RequestException as e:
             warnings.append(f"File '{filename}' could not be downloaded ({e}).")
             continue
 
-        total_bytes += size
+        total_bytes += written
 
     return warnings