Repository navigation
Align project structure and Supabase runtime - #2
Merged
Merged
Conversation
Add telemetry upload routes, gaze estimation, templated reports, and model asset boundaries while aligning local and Render setup with the current Supabase configuration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reviewer's GuideThis PR aligns the documented and deployed architecture by adding authenticated telemetry APIs, integrating gaze/head-pose analysis, templating reports, formalizing model and upload directories, and migrating Supabase/runtime configuration to the current environment-variable scheme while preserving legacy compatibility. Sequence diagram for authenticated telemetry upload and frame retrievalsequenceDiagram
actor User
participant API
participant Storage
participant SessionWS
participant Supabase
User->>API: upload_screenshot(upload)
API->>API: get_current_user()
API->>Storage: _save_image(upload, kind)
API->>SessionWS: publish_session_message(session_id, telemetry_frame)
API-->>User: accepted
User->>API: latest_frame(session_id, type)
API->>API: get_current_user()
API->>Supabase: select sessions
Supabase-->>API: session ownership
API-->>User: FileResponse(latest frame)
Sequence diagram for MediaPipe gaze and head-pose analysissequenceDiagram
participant FaceDetection
participant MediaPipe
participant GazeTracking
FaceDetection->>MediaPipe: face_landmarks
FaceDetection->>GazeTracking: estimate_head_pose(face_landmarks, width, height)
GazeTracking-->>FaceDetection: pose
FaceDetection->>GazeTracking: estimate_gaze(face_landmarks)
GazeTracking-->>FaceDetection: direction, ratio, off_screen
FaceDetection-->>FaceDetection: store pose and gaze results
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Fixed security issues:
- render.yaml (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="server/api/uploads.py" line_range="69-76" />
<code_context>
+ return {"status": "accepted", "session_id": upload.session_id, "filename": path.name}
+
+
+@router.post("/screenshot")
+async def upload_screenshot(upload: ImageUpload, user=Depends(get_current_user)):
+ return await _store_and_publish(upload, "screenshot")
+
+
+@router.post("/webcam")
+async def upload_webcam(upload: ImageUpload, user=Depends(get_current_user)):
+ return await _store_and_publish(upload, "webcam")
+
+
</code_context>
<issue_to_address>
**🚨 issue (security):** The upload endpoints never verify that the authenticated user owns `session_id`, so any authenticated user can write screenshot or webcam frames into another user's session; the latest-frame endpoint only performs the ownership check when a database client is available and therefore also permits cross-session access in database-less deployments.
**Triggers:** When a user submits a valid token while naming another user's session ID, or when Supabase is unavailable.
**Suggested fix:** Resolve the session and enforce ownership for every upload and retrieval, rejecting the request when the session cannot be verified rather than bypassing authorization when `db` is false.
</issue_to_address>
### Comment 2
<location path="server/api/uploads.py" line_range="50-54" />
<code_context>
+ if not content:
+ raise HTTPException(status_code=422, detail="Image payload is empty")
+
+ directory = settings.SCREENSHOTS_DIR if kind == "screenshot" else settings.WEBCAM_DIR
+ directory.mkdir(parents=True, exist_ok=True)
+ extension = _EXTENSIONS.get(match.group("format").lower(), ".bin")
+ path = directory / f"{upload.session_id}-{uuid.uuid4().hex}{extension}"
+ path.write_bytes(content)
+ return path
+
</code_context>
<issue_to_address>
**🚨 issue (security):** `session_id` is interpolated directly into filesystem paths and glob patterns without validation, allowing path separators and `..` components to escape the telemetry directory; uploads can write outside the intended directory and latest-frame queries can enumerate and return files outside it.
**Triggers:** When an authenticated request supplies a session ID containing path traversal components.
**Suggested fix:** Validate session IDs against the application's identifier format and resolve the resulting path, rejecting any path that is not contained by the configured telemetry directory.
</issue_to_address>
### Comment 3
<location path="server/api/uploads.py" line_range="59-66" />
<code_context>
+
+
+async def _store_and_publish(upload: ImageUpload, kind: str) -> dict:
+ path = _save_image(upload, kind)
+ publish_session_message(upload.session_id, {
+ "type": "telemetry_frame",
+ "session_id": upload.session_id,
+ "kind": kind,
+ "path": path.name,
+ })
+ return {"status": "accepted", "session_id": upload.session_id, "filename": path.name}
</code_context>
<issue_to_address>
**issue (bug_risk):** The image is written successfully before Redis publishing occurs, but a Redis connection or publish failure propagates out of the request; the client receives an error even though the frame remains on disk, creating orphaned telemetry files and preventing the documented upload acknowledgement.
**Triggers:** When Redis is unavailable or the publish operation fails after the filesystem write succeeds.
**Suggested fix:** Handle publish failure explicitly, either removing the saved file on failure or returning an accepted response while dispatching/retrying publication asynchronously.
```suggestion
path = _save_image(upload, kind)
try:
publish_session_message(upload.session_id, {
"type": "telemetry_frame",
"session_id": upload.session_id,
"kind": kind,
"path": path.name,
})
except Exception:
path.unlink(missing_ok=True)
raise
return {"status": "accepted", "session_id": upload.session_id, "filename": path.name}
```
</issue_to_address>
Comment on lines
+69
to
+76
| @router.post("/screenshot") | ||
| async def upload_screenshot(upload: ImageUpload, user=Depends(get_current_user)): | ||
| return await _store_and_publish(upload, "screenshot") | ||
|
|
||
|
|
||
| @router.post("/webcam") | ||
| async def upload_webcam(upload: ImageUpload, user=Depends(get_current_user)): | ||
| return await _store_and_publish(upload, "webcam") |
There was a problem hiding this comment.
🚨 issue (security): The upload endpoints never verify that the authenticated user owns session_id, so any authenticated user can write screenshot or webcam frames into another user's session; the latest-frame endpoint only performs the ownership check when a database client is available and therefore also permits cross-session access in database-less deployments.
Triggers: When a user submits a valid token while naming another user's session ID, or when Supabase is unavailable.
Suggested fix: Resolve the session and enforce ownership for every upload and retrieval, rejecting the request when the session cannot be verified rather than bypassing authorization when db is false.
Comment on lines
+50
to
+54
| directory = settings.SCREENSHOTS_DIR if kind == "screenshot" else settings.WEBCAM_DIR | ||
| directory.mkdir(parents=True, exist_ok=True) | ||
| extension = _EXTENSIONS.get(match.group("format").lower(), ".bin") | ||
| path = directory / f"{upload.session_id}-{uuid.uuid4().hex}{extension}" | ||
| path.write_bytes(content) |
There was a problem hiding this comment.
🚨 issue (security): session_id is interpolated directly into filesystem paths and glob patterns without validation, allowing path separators and .. components to escape the telemetry directory; uploads can write outside the intended directory and latest-frame queries can enumerate and return files outside it.
Triggers: When an authenticated request supplies a session ID containing path traversal components.
Suggested fix: Validate session IDs against the application's identifier format and resolve the resulting path, rejecting any path that is not contained by the configured telemetry directory.
Comment on lines
+59
to
+66
| path = _save_image(upload, kind) | ||
| publish_session_message(upload.session_id, { | ||
| "type": "telemetry_frame", | ||
| "session_id": upload.session_id, | ||
| "kind": kind, | ||
| "path": path.name, | ||
| }) | ||
| return {"status": "accepted", "session_id": upload.session_id, "filename": path.name} |
There was a problem hiding this comment.
issue (bug_risk): The image is written successfully before Redis publishing occurs, but a Redis connection or publish failure propagates out of the request; the client receives an error even though the frame remains on disk, creating orphaned telemetry files and preventing the documented upload acknowledgement.
Triggers: When Redis is unavailable or the publish operation fails after the filesystem write succeeds.
Suggested fix: Handle publish failure explicitly, either removing the saved file on failure or returning an accepted response while dispatching/retrying publication asynchronously.
Suggested change
| path = _save_image(upload, kind) | |
| publish_session_message(upload.session_id, { | |
| "type": "telemetry_frame", | |
| "session_id": upload.session_id, | |
| "kind": kind, | |
| "path": path.name, | |
| }) | |
| return {"status": "accepted", "session_id": upload.session_id, "filename": path.name} | |
| path = _save_image(upload, kind) | |
| try: | |
| publish_session_message(upload.session_id, { | |
| "type": "telemetry_frame", | |
| "session_id": upload.session_id, | |
| "kind": kind, | |
| "path": path.name, | |
| }) | |
| except Exception: | |
| path.unlink(missing_ok=True) | |
| raise | |
| return {"status": "accepted", "session_id": upload.session_id, "filename": path.name} |
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved critical security, authentication, runtime, and telemetry integration issues require fixes and human review.
Pull request overview
Aligns the project structure and Supabase runtime configuration while adding telemetry ingestion, gaze analysis, and templated reporting.
Changes:
- Added telemetry upload, latest-frame, gaze, and head-pose functionality.
- Added Jinja2 reporting and model artifact documentation.
- Updated configuration, startup scripts, deployment, documentation, and frontend dependencies.
File summaries
| File | Description |
|---|---|
start.sh |
Creates report upload directories. |
server/workers/report_worker.py |
Renders reports through Jinja2 templates. |
server/supabase_client.py |
Uses server-only Supabase credentials. |
server/setup_database.py |
Updates Supabase setup guidance. |
server/services/weights/README.md |
Documents model artifact storage. |
server/services/gaze_tracking.py |
Adds gaze and head-pose estimation. |
server/services/face_detection.py |
Integrates gaze analysis with face detection. |
server/services/__init__.py |
Exposes gaze helpers. |
server/requirements.txt |
Adds Jinja2. |
server/reports/templates/session_report.html |
Adds the session report template. |
server/migrations/supabase_schema.sql |
Updates Supabase key guidance. |
server/config.py |
Adds current Supabase configuration variables. |
server/api/uploads.py |
Adds telemetry upload and latest-frame APIs. |
server/api/router_v2.py |
Registers upload routes. |
server/.env.example |
Documents runtime environment variables. |
scripts/start-worker.ps1 |
Loads worker environment settings. |
scripts/start-dev.ps1 |
Uses the server environment template. |
scripts/setup-supabase.ps1 |
Updates setup instructions. |
render.yaml |
Updates managed deployment variables. |
README.md |
Updates architecture and setup documentation. |
examguard-pro/package-lock.json |
Synchronizes frontend dependencies. |
Review details
Files not reviewed (1)
- examguard-pro/package-lock.json: Generated file
Suppressed comments (9)
README.md:112
- The new README repeats
server/services/weights/as the active model location, but object detection still readsserver/models/weights/yolo11s.ptand the build downloads into that other directory. This makes the setup instructions ineffective; align the README with the loader and build path.
Model weights belong in `server/services/weights/` and are intentionally not
committed. The MediaPipe face landmarker bundle remains at
`server/services/face_landmarker.task` for compatibility.
scripts/start-dev.ps1:15
- This copies
server/.env.exampleverbatim, but the new example contains active placeholder Supabase and PostgreSQL values. On first startup, Settings therefore initializes a fake Supabase client and constructs a bogus PostgreSQL URL instead of using the local fallback, so API/worker calls fail until the file is manually edited; keep optional integration values commented or stop auto-copying the template.
if (-not (Test-Path (Join-Path $ServerDir ".env"))) {
Copy-Item (Join-Path $ServerDir ".env.example") (Join-Path $ServerDir ".env")
Write-Host "Created server\.env from server\.env.example" -ForegroundColor Yellow
scripts/start-worker.ps1:39
- Because
server/.env.exampledefinesCELERY_BROKER_URLandCELERY_RESULT_BACKENDas localhost, the loader sets them before this block. If a managed environment supplies onlyREDIS_URL, these variables are already non-empty and never inherit it, so the worker connects to localhost despite the configured Redis URL. Skip these keys while loading or treat the local defaults as unset.
if (-not $env:CELERY_BROKER_URL) {
$env:CELERY_BROKER_URL = $env:REDIS_URL
}
if (-not $env:CELERY_RESULT_BACKEND) {
$env:CELERY_RESULT_BACKEND = $env:REDIS_URL
server/.env.example:8
start-dev.ps1copies this file verbatim and immediately starts the API. These uncommented placeholders makeSUPABASE_URL/key non-empty, sosupabase_clientinitializes an invalid client instead of using the no-Supabase fallback; the template's fakePG_HOSThas the same problem for DB setup. Keep optional values commented/empty or make startup validate them before copying.
SUPABASE_URL=https://your-project-ref.supabase.co
SUPABASE_PUBLISHABLE_KEY=sb_publishable_your-public-key
SUPABASE_SECRET_KEY=sb_secret_your-server-only-key
SUPABASE_JWKS_URL=https://your-project-ref.supabase.co/auth/v1/.well-known/jwks.json
server/api/uploads.py:62
_store_and_publishtrusts the client-suppliedsession_idand is called by both upload routes and the legacy route without loading the session or checking ownership. A student can therefore post telemetry into any known session, contaminating its stored frames and session broadcast; enforce the same student/proctor authorization used by the sessions and events APIs before saving.
async def _store_and_publish(upload: ImageUpload, kind: str) -> dict:
path = _save_image(upload, kind)
publish_session_message(upload.session_id, {
"type": "telemetry_frame",
"session_id": upload.session_id,
server/api/uploads.py:65
- The published message is
telemetry_framewith only a filename inpath, but the dashboard'sSessionDetailhandler only consumeslive_framemessages withframe_typeand frame data. Upload notifications are therefore ignored andliveFramesnever updates; the UI falls back to the separately protected URL. Emit the established message shape or update the frontend and URL handling together.
publish_session_message(upload.session_id, {
"type": "telemetry_frame",
"session_id": upload.session_id,
"kind": kind,
"path": path.name,
})
server/api/uploads.py:112
latest_frameis protected byHTTPBearer, but the dashboard uses this endpoint as an<img src>and its token is only attached by the Axios client. Browser image requests cannot send that Bearer header, so the fallback feed URLs return 401. Use a short-lived signed URL or fetch the image through the authenticated API client and create a blob URL.
@router.get("/latest/{session_id}")
async def latest_frame(
session_id: str,
type: str = "screenshot",
user=Depends(get_current_user),
server/api/uploads.py:54
- Every accepted frame is written as a new local file, but no retention or cleanup removes these files; the existing cleanup worker only scrubs database screenshot references. With the extension's periodic captures, per-session disk usage grows indefinitely and local Render storage is not durable. Bound retention/overwrite or persist frames through object storage before relying on this in production.
directory = settings.SCREENSHOTS_DIR if kind == "screenshot" else settings.WEBCAM_DIR
directory.mkdir(parents=True, exist_ok=True)
extension = _EXTENSIONS.get(match.group("format").lower(), ".bin")
path = directory / f"{upload.session_id}-{uuid.uuid4().hex}{extension}"
path.write_bytes(content)
server/services/gaze_tracking.py:15
199is not the MediaPipe chin landmark; the chin is landmark152. BecausePOSE_MODEL_POINTS[1]is the chin coordinate, this mismatched correspondence makessolvePnPfit the wrong facial point and produces incorrect pitch/yaw/roll values. Use landmark152here.
POSE_LANDMARKS = (1, 199, 33, 263, 61, 291)
- Files reviewed: 20/21 changed files
- Comments generated: 8
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| class ImageUpload(BaseModel): | ||
| session_id: str | ||
| timestamp: int | None = None | ||
| image_data: str = Field(min_length=16) |
| directory = settings.SCREENSHOTS_DIR if kind == "screenshot" else settings.WEBCAM_DIR | ||
| directory.mkdir(parents=True, exist_ok=True) | ||
| extension = _EXTENSIONS.get(match.group("format").lower(), ".bin") | ||
| path = directory / f"{upload.session_id}-{uuid.uuid4().hex}{extension}" |
Comment on lines
+117
to
+121
| if db: | ||
| session = db.table("sessions").select("id, student_id").eq("id", session_id).execute() | ||
| if not session.data: | ||
| raise HTTPException(status_code=404, detail="Session not found") | ||
| if user.get("role") == "student" and session.data[0].get("student_id") != user["id"]: |
|
|
||
| async def _store_and_publish(upload: ImageUpload, kind: str) -> dict: | ||
| path = _save_image(upload, kind) | ||
| publish_session_message(upload.session_id, { |
Comment on lines
+69
to
+71
| @router.post("/screenshot") | ||
| async def upload_screenshot(upload: ImageUpload, user=Depends(get_current_user)): | ||
| return await _store_and_publish(upload, "screenshot") |
Comment on lines
+81
to
+85
| """Accept the extension's legacy combined capture payload. | ||
|
|
||
| Storage and inference are deliberately separate: the upload is acknowledged | ||
| immediately while the existing Celery event pipeline handles analysis. | ||
| """ |
Comment on lines
+80
to
+81
| left_iris = landmarks[473] | ||
| right_iris = landmarks[468] |
| Place downloaded inference artifacts in this directory during local or image | ||
| build setup. Large binaries are intentionally ignored by Git. | ||
|
|
||
| - `yolov8n.pt` — optional Ultralytics object detector weights. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The repository structure and local/runtime configuration had drifted from the documented ExamGuard Pro architecture. Telemetry capture routes, gaze estimation, report templates, model asset boundaries, and current Supabase key configuration needed to be represented consistently across development and deployment.
What changed
server/services/weightsandserver/reports/templatesstructure and ensured report upload directories are created at startup.Validation
git diff --checkpassed.Notes
--pool=solowhen required by local multiprocessing permissions.Summary by Sourcery
Align the project structure and runtime configuration with the current ExamGuard Pro architecture while adding telemetry ingestion, gaze analysis, and templated reporting.
New Features:
Bug Fixes:
Enhancements:
Build:
Deployment:
Documentation: