Compare commits

...

4 Commits

Author SHA1 Message Date
CC Worker
115ecd2351 test(exam): mock service-role files read in source-pdf download test (S4-8.1)
Some checks failed
api-ci-deploy / test-build-deploy (push) Has been cancelled
The download path now resolves the files row via SupabaseServiceRoleClient (to
sidestep the cabinet_memberships RLS recursion); the test must mock it like the
upload test does. Test-only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-06 22:56:45 +00:00
CC Worker
a37bcaa935 fix(exam): source-pdf download reads files row via service role (S4-8.1 merge-gate fix 2)
Pre-merge smoke caught a second issue: the source_file_id download path read `files`
as-the-user, tripping a PRE-EXISTING broken RLS policy on cabinet_memberships
(42P17 infinite recursion). Authz is already enforced (template fetch + source
visibility), and source_file_id is the template's own file, so resolve the row via
service role (documented exception, same as the catalogue lookup). Flagged the
cabinet_memberships RLS recursion separately as infra bug E8.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-06 22:54:24 +00:00
CC Worker
c0775f3be1 fix(exam): source-PDF upload uses shared cc.users bucket (S4-8.1 merge-gate fix)
Pre-merge live smoke on .94 caught 'Bucket not found': the upload wrote to a
per-institute bucket cc.institutes.<id>.private that isn't provisioned on dev.
Use the shared SOURCE_BUCKET_FALLBACK (cc.users); institute is namespaced in the
storage path + enforced by the files-row RLS. Per-institute buckets are a future
multi-tenant concern. Catalogue path + cross-institute 404 already verified green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-06 22:51:51 +00:00
CC Worker
c58df6715c feat(exam): template source PDF at create + GET /templates/{id}/source-pdf (S4-8.1)
Recovered from cc-worker WIP that was left uncommitted in the dev-centre clone
(card t_0055b89b). Multipart source_pdf upload at create -> source_file_id;
source-pdf download endpoint resolves from exam_id (catalogue) or source_file_id.
NOT yet human-reviewed/merged; preserving + verifying so it isn't clobbered.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-06 22:29:32 +00:00
2 changed files with 265 additions and 6 deletions

View File

@ -13,11 +13,15 @@ join keys (spec §2).
from __future__ import annotations
import os
from typing import Any, Dict, List, Optional
import uuid
from typing import Any, Dict, List, Optional, Tuple
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException
from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request, UploadFile
from fastapi.responses import Response
from modules.database.services.exam_projection import project_template, project_template_safe
from modules.database.supabase.utils.client import SupabaseServiceRoleClient
from modules.database.supabase.utils.storage import StorageAdmin
from modules.logger_tool import initialise_logger
from routers.exam.dependencies import ExamContext, get_exam_context, lookup_exam_code
from routers.exam.schemas import (
@ -30,6 +34,9 @@ logger = initialise_logger(__name__, os.getenv("LOG_LEVEL"), os.getenv("LOG_PATH
router = APIRouter()
SOURCE_CABINET_NAME = "Exam Marker Template Sources"
SOURCE_BUCKET_FALLBACK = "cc.users"
# ─── helpers ─────────────────────────────────────────────────────────────────
@ -55,12 +62,19 @@ def _fetch_template_or_404(ctx: ExamContext, template_id: str) -> Dict[str, Any]
def _require_owner(ctx: ExamContext, template: Dict[str, Any]) -> None:
"""Writes are limited to the owning teacher (R2.4). RLS also enforces this; we pre-check
so a colleague who can *read* the template gets a clean 403 instead of a silent no-op."""
"""Writes are limited to the owning teacher (R2.4)."""
if template.get("teacher_id") != ctx.user_id:
raise HTTPException(status_code=403, detail="Only the template owner can modify it")
def _require_source_visibility_or_404(ctx: ExamContext, template: Dict[str, Any]) -> None:
"""Template source reads must not leak existence across institutes or non-owners."""
if template.get("teacher_id") != ctx.user_id:
raise HTTPException(status_code=404, detail="Template not found")
if template.get("institute_id") not in ctx.institute_ids:
raise HTTPException(status_code=404, detail="Template not found")
def _template_has_recorded_marks(ctx: ExamContext, template_id: str) -> bool:
"""True if any mark_entry exists for a batch of this template (→ destructive PUT is unsafe)."""
batches = _rows(
@ -75,25 +89,148 @@ def _template_has_recorded_marks(ctx: ExamContext, template_id: str) -> bool:
return bool(marks)
def _parse_storage_loc(storage_loc: str) -> Tuple[str, str]:
bucket, sep, path = (storage_loc or "").partition("/")
if not bucket or not sep or not path:
raise ValueError(f"Invalid storage_loc: {storage_loc!r}")
return bucket, path
def _lookup_exam_storage_loc(exam_id: str) -> Optional[str]:
try:
sb = SupabaseServiceRoleClient().supabase
res = sb.table("eb_exams").select("storage_loc").eq("id", exam_id).limit(1).execute()
row = _first(res)
return row.get("storage_loc") if row else None
except Exception as exc:
logger.warning(f"storage_loc lookup failed for exam_id={exam_id}: {exc}")
return None
async def _parse_create_template_request(request: Request) -> tuple[CreateTemplateRequest, Optional[UploadFile]]:
content_type = request.headers.get("content-type", "")
if "multipart/form-data" in content_type:
form = await request.form()
payload: Dict[str, Any] = {}
for key in ("title", "subject", "exam_id", "exam_code", "source_file_id", "page_count", "institute_id"):
value = form.get(key)
if value is not None and value != "":
payload[key] = value
upload = form.get("source_pdf")
if upload is not None and not hasattr(upload, "read"):
raise HTTPException(status_code=400, detail="source_pdf must be a file upload")
if upload is not None and payload.get("source_file_id"):
raise HTTPException(status_code=400, detail="Use either source_file_id or source_pdf, not both")
return CreateTemplateRequest(**payload), upload
try:
data = await request.json()
except Exception as exc:
raise HTTPException(status_code=400, detail=f"Invalid request body: {exc}")
return CreateTemplateRequest(**data), None
async def _upload_template_source_file(
ctx: ExamContext,
institute_id: str,
upload: UploadFile,
) -> str:
file_bytes = await upload.read()
if not file_bytes:
raise HTTPException(status_code=400, detail="Uploaded PDF is empty")
if upload.content_type and upload.content_type != "application/pdf":
raise HTTPException(status_code=400, detail="Uploaded file must be a PDF")
service = SupabaseServiceRoleClient()
storage = StorageAdmin()
cabinet_name = SOURCE_CABINET_NAME
existing = _first(
service.supabase.table("file_cabinets")
.select("id")
.eq("user_id", ctx.user_id)
.eq("name", cabinet_name)
.limit(1)
.execute()
)
if existing:
cabinet_id = existing["id"]
else:
created_cabinet = _first(
service.supabase.table("file_cabinets")
.insert({"user_id": ctx.user_id, "name": cabinet_name})
.execute()
)
if not created_cabinet:
raise HTTPException(status_code=500, detail="Failed to create upload cabinet")
cabinet_id = created_cabinet["id"]
file_id = str(uuid.uuid4())
safe_name = os.path.basename(upload.filename or "template.pdf")
# Use the shared users bucket (exists on all envs). Per-institute private buckets
# (cc.institutes.<id>.private) are a future multi-tenant provisioning concern and are NOT
# created on dev .94 — using one here failed with "Bucket not found". The institute is already
# namespaced in the storage path + enforced by RLS on the files row.
bucket = SOURCE_BUCKET_FALLBACK
storage_path = f"exam-marker/{institute_id or 'noinst'}/{cabinet_id}/{file_id}/{safe_name}"
try:
storage.upload_file(bucket, storage_path, file_bytes, "application/pdf", upsert=True)
except Exception as exc:
raise HTTPException(status_code=500, detail=f"Storage upload failed: {exc}")
inserted = _first(
service.supabase.table("files").insert(
{
"id": file_id,
"cabinet_id": cabinet_id,
"name": safe_name,
"path": storage_path,
"bucket": bucket,
"mime_type": "application/pdf",
"uploaded_by": ctx.user_id,
"size_bytes": len(file_bytes),
"source": "classroomcopilot-web",
"is_directory": False,
"relative_path": safe_name,
"processing_status": "uploaded",
}
).execute()
)
if not inserted:
raise HTTPException(status_code=500, detail="Failed to create file record")
return file_id
# ─── templates ───────────────────────────────────────────────────────────────
@router.post("/templates")
async def create_template(
body: CreateTemplateRequest,
request: Request,
ctx: ExamContext = Depends(get_exam_context),
) -> Dict[str, Any]:
body, upload = await _parse_create_template_request(request)
institute_id = ctx.resolve_institute(body.institute_id)
if body.exam_id and body.source_file_id:
raise HTTPException(status_code=400, detail="Use either exam_id or source_file_id, not both")
exam_code = body.exam_code
if body.exam_id and not exam_code:
exam_code = lookup_exam_code(body.exam_id)
source_file_id = body.source_file_id
if upload is not None:
source_file_id = await _upload_template_source_file(ctx, institute_id, upload)
row = {
"title": body.title,
"subject": body.subject,
"exam_id": body.exam_id,
"exam_code": exam_code,
"source_file_id": body.source_file_id,
"source_file_id": source_file_id,
"page_count": body.page_count,
"institute_id": institute_id,
"teacher_id": ctx.user_id,
@ -109,6 +246,23 @@ async def create_template(
return created
@router.get("/catalogue")
async def list_catalogue_papers() -> Dict[str, Any]:
"""Lightweight exam-board paper catalogue for the create dialog."""
try:
sb = SupabaseServiceRoleClient().supabase
res = (
sb.table("eb_exams")
.select("id, exam_code, spec_code, paper_code, tier, session, type_code, storage_loc")
.eq("type_code", "QP")
.order("exam_code")
.execute()
)
return {"papers": _rows(res)}
except Exception as exc:
raise HTTPException(status_code=502, detail=f"Could not load catalogue papers: {exc}")
@router.get("/templates")
async def list_templates(
include_archived: bool = False,
@ -148,6 +302,56 @@ async def get_template(
}
@router.get("/templates/{template_id}/source-pdf")
async def get_template_source_pdf(
template_id: str,
ctx: ExamContext = Depends(get_exam_context),
) -> Response:
template = _fetch_template_or_404(ctx, template_id)
_require_source_visibility_or_404(ctx, template)
bucket: Optional[str] = None
path: Optional[str] = None
if template.get("exam_id"):
storage_loc = _lookup_exam_storage_loc(template["exam_id"])
if not storage_loc:
raise HTTPException(status_code=404, detail="Template source not found")
try:
bucket, path = _parse_storage_loc(storage_loc)
except ValueError:
raise HTTPException(status_code=404, detail="Template source not found")
elif template.get("source_file_id"):
# Resolve the file row via service role (authz already done above: the caller proved they
# can see this template, and source_file_id is the template's own file). Reading `files`
# as-the-user trips a pre-existing broken RLS policy on cabinet_memberships
# (42P17 infinite recursion) — documented service-role exception, like the catalogue lookup.
file_row = _first(
SupabaseServiceRoleClient().supabase.table("files")
.select("bucket, path, mime_type, name")
.eq("id", template["source_file_id"])
.limit(1)
.execute()
)
if not file_row or not file_row.get("bucket") or not file_row.get("path"):
raise HTTPException(status_code=404, detail="Template source not found")
bucket = file_row["bucket"]
path = file_row["path"]
else:
raise HTTPException(status_code=404, detail="Template source not found")
if not bucket or not path:
raise HTTPException(status_code=404, detail="Template source not found")
try:
pdf_bytes = StorageAdmin().download_file(bucket, path)
except Exception as exc:
logger.warning(f"Template source download failed for template {template_id}: {exc}")
raise HTTPException(status_code=404, detail="Template source not found")
return Response(content=pdf_bytes, media_type="application/pdf")
@router.put("/templates/{template_id}")
async def replace_template(
template_id: str,
@ -293,6 +497,7 @@ async def neo4j_sync(
# ─── questions (granular edit path, R5.2) ────────────────────────────────────
@router.patch("/questions/{question_id}")
async def patch_question(
question_id: str,

View File

@ -136,6 +136,19 @@ class FakeSupabase:
return FakeQuery(self.store, name)
class _FakeStorageAdmin:
def upload_file(self, *args, **kwargs):
return None
def download_file(self, bucket_id, file_path):
return b"%PDF-1.7 fake"
class _FakeServiceRoleClient:
def __init__(self, store):
self.supabase = FakeSupabase(store)
def make_client(user_id=TEACHER, institute_ids=(INST_A,), store=None):
store = store if store is not None else {}
app = FastAPI()
@ -169,6 +182,47 @@ def test_create_template_sets_owner_and_institute():
assert row["status"] == "draft"
def test_create_template_accepts_uploaded_source_pdf(monkeypatch):
store = {}
client, store = make_client(store=store)
monkeypatch.setattr(templates_mod, "StorageAdmin", _FakeStorageAdmin)
monkeypatch.setattr(templates_mod, "SupabaseServiceRoleClient", lambda: _FakeServiceRoleClient(store))
resp = client.post(
"/api/exam/templates",
data={"title": "AQA Physics 1H", "subject": "Physics"},
files={"source_pdf": ("paper.pdf", b"%PDF-1.7 test", "application/pdf")},
)
assert resp.status_code == 200
row = resp.json()
assert row["source_file_id"] is not None
assert store["files"][0]["id"] == row["source_file_id"]
assert store["files"][0]["uploaded_by"] == TEACHER
def test_get_template_source_pdf_from_uploaded_file(monkeypatch):
store = {
"exam_templates": [{
"id": "t1",
"title": "p",
"status": "draft",
"institute_id": INST_A,
"teacher_id": TEACHER,
"source_file_id": "f1",
}],
"files": [{"id": "f1", "bucket": "cc.users", "path": "exam-marker/cab1/f1/paper.pdf", "name": "paper.pdf"}],
}
client, _ = make_client(store=store)
monkeypatch.setattr(templates_mod, "StorageAdmin", _FakeStorageAdmin)
# The download resolves the files row via service role (sidesteps the broken cabinet_memberships
# RLS recursion) — mock it to the same fake store, like the upload test does.
monkeypatch.setattr(templates_mod, "SupabaseServiceRoleClient", lambda: _FakeServiceRoleClient(store))
resp = client.get("/api/exam/templates/t1/source-pdf")
assert resp.status_code == 200
assert resp.headers["content-type"].startswith("application/pdf")
assert resp.content.startswith(b"%PDF-1.7")
def test_create_template_rejects_foreign_institute():
client, _ = make_client(institute_ids=(INST_A,))
resp = client.post("/api/exam/templates", json={"title": "X", "institute_id": INST_B})