Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 151 additions & 0 deletions GSOC_PROPOSAL_DRAFT.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
# GSoC 2026 Proposal: EduAid Backend Hardening

**Proposal Title:** Hardening EduAid: Security, Scalability, and Reliability Improvements

## Summary
EduAid has real problems that get in the way of running it in production. File uploads have security holes. AI processing ties up the server whenever more than one person uses it at the same time. Memory usage runs past 15GB in a configuration where shared model instances could bring it under 5GB.

This project fixes all three. I will patch the file handling layer to close the directory traversal vulnerability and enforce proper cleanup, wire up Celery and Redis so heavy AI work runs in background workers instead of blocking request threads, and consolidate model loading so the same instance is shared across generator classes rather than loaded fresh each time.

When this is done, EduAid will handle concurrent users without timeouts, have no known file handling or injection vulnerabilities, and run with significantly lower memory overhead.

This proposal is based on prior hands-on analysis of the EduAid backend, including reporting a directory traversal vulnerability (Issue #502).

## Motivation
The backend works fine for local testing but falls apart under real usage. File uploads accept anything, including filenames designed to escape the upload directory. AI inference runs synchronously inside Flask request handlers, so the third concurrent user typically times out waiting. Five generator classes each call `from_pretrained` independently, loading duplicate copies of the same models into memory.

A classroom of 30 students hitting the question generation endpoint simultaneously will produce timeouts, not questions. One malformed filename can write to arbitrary paths on the server. And the memory situation makes the hosting bill higher than it needs to be.

Students are the ones who pay for unreliable tools. Timeouts during studying are frustrating in a specific way that is different from a general SaaS outage. None of the work I want to do later—better document parsing, smarter AI features, horizontal scaling—is worth building on top of a backend with security gaps and resource leaks. So I want to fix the foundation first.

## Personal Motivation and Suitability
I found the directory traversal vulnerability by reading through how files move through the system. I traced an upload from the point it hits the endpoint all the way through to cleanup, and noticed that temp files only get deleted when the operation succeeds. Failures leave files behind with no cleanup scheduled. I reported this as Issue #502.

I like problems where you can tell whether the fix worked. Close a security hole and verify the attack vector is gone. Add async processing and run a load test. Reduce memory usage and measure before and after. Infrastructure work with that kind of feedback loop is the kind I find satisfying.

I have built backend systems before and have run into the specific failure modes EduAid has: temp files accumulating until disk fills up, blocking operations causing cascading timeouts, models loaded redundantly because nobody wired up a shared instance. I have already read enough of the codebase to understand how the components connect. I am not guessing at the scope.

## Tech Stack
Python, Flask, Celery, Redis, PyTorch, Transformers, PyMuPDF, `python-docx`, `yt-dlp`, Werkzeug, pytest, Docker

## Implementation Timeline

Testing is not a phase at the end. For each fix, I write the test that proves the problem exists, then I write the fix, then I confirm the test passes. Milestones below are checkpoints where I can show something working, not just code committed.

### Week 1: Security fixes
I will start by reproducing every issue I found, including Issue #502, to confirm exact scope before touching anything.

Secure file handling comes first. Every filename goes through `werkzeug.utils.secure_filename`, and the resulting path is validated with `os.path.abspath` to confirm it stays inside the uploads directory. Anything that escapes gets rejected outright.

```python
import os
from werkzeug.utils import secure_filename

UPLOAD_DIR = os.path.abspath("/app/uploads")

def safe_save(file, filename):
name = secure_filename(filename)
dest = os.path.abspath(os.path.join(UPLOAD_DIR, name))
if not dest.startswith(UPLOAD_DIR + os.sep):
raise ValueError("Path traversal attempt blocked")
file.save(dest)
return dest
```

I write the path traversal test before writing this function, so I know exactly what the fix needs to defeat. The test uploads a file named `../../etc/passwd` and asserts it gets rejected with a 400.

The `yt-dlp` injection fix runs the same week. Right now `video_id` comes from user input and goes into a subprocess call with `shell=True`. I sanitize the parameter and switch to argument list mode so shell metacharacters are never interpreted:

```python
import re
import subprocess

if not re.fullmatch(r'[A-Za-z0-9_-]{6,16}', video_id):
raise ValueError("Invalid video ID")
subprocess.run(["yt-dlp", video_id, "-o", "output.mp4"])
```

Test first here too. I pass `; rm -rf /tmp/test` as a video ID and confirm it gets blocked before touching the subprocess call.

File upload validation also goes in this week: a whitelist of PDF and DOCX only, size limits at both the application layer and Nginx, and payload checks that reject oversized requests before Flask ever sees them.
Comment on lines +57 to +70

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Align this section with the code shipped in this PR.

This paragraph describes the current backend as using shell=True and says the upload whitelist will be “PDF and DOCX only,” but backend/server.py already uses list-mode subprocess.run(...) and the implemented allowlist is txt, pdf, docx. Updating the proposal text will keep reviewers from reading two different baselines in the same PR.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@GSOC_PROPOSAL_DRAFT.md` around lines 57 - 70, The proposal text is out of
sync with the shipped code: update the paragraph so it reflects that the backend
already uses argument-list mode (subprocess.run([...]) with the video_id
variable passed as an element) instead of shell=True, and that the implemented
upload allowlist contains txt, pdf, and docx (not just PDF and DOCX); mention
the validation and size limits as implemented (application-layer checks plus
upstream Nginx limits) and remove the example implying shell=True or a different
allowlist.


**Milestone:** Path traversal test passes. Shell injection test passes. A reviewer can run both in under a minute.

### Week 2: Reliability and file handling
The cleanup problem is straightforward but needs consistent treatment everywhere. Context managers and `try/finally` blocks wrap every operation that touches temp files. Whether the operation succeeds or crashes, the file gets deleted. Celery task failure hooks get the same treatment so background failures do not leave orphaned files.

UUIDs replace the current filename scheme. Each request gets its own isolated subdirectory, which eliminates the race condition in subtitle processing where concurrent requests write to the same location and try to figure out which file belongs to them by comparing timestamps. With per-request directories that problem disappears.

Bare `except` blocks that catch everything and do nothing become proper logging that captures what operation failed, what the inputs were, and what the exception was. HTTP status codes get used correctly so callers can tell what went wrong.

A periodic background scan finds and removes temp files older than a configurable threshold, catching anything that slips through during unexpected process termination.

**Milestone:** Force a failure mid-upload and confirm the temp file is gone. Repeat for three different failure points in the pipeline.

### Weeks 3 to 5: Asynchronous processing
This is the biggest architectural change and the one with the most ways to go wrong unexpectedly. Three weeks, with real buffer for debugging Celery and Flask interactions, which in my experience never go smoothly the first time.

Right now document parsing and AI inference run inside Flask request handlers. Under load, requests queue up and the ones at the back time out. The fix: heavy endpoints validate input, enqueue a Celery task, and return a task ID immediately. The actual work runs in a background worker.

**API migration.** Moving to async is a breaking change for existing callers. I will handle this with a `/v2/` namespace alongside the existing synchronous endpoints. The old endpoints stay functional but return a `Deprecation` response header pointing to the migration guide. New `/v2/task/status/<task_id>` and `/v2/task/result/<task_id>` endpoints expose progress and results for polling.

One operational concern I want to address directly: what happens to a request actively being processed on the old synchronous endpoint when a deployment occurs? For long-running requests this is a real problem. I will add a short draining period to the deployment process so the load balancer stops routing new traffic to the old endpoint while in-flight requests finish. After a timeout, they fail gracefully and the client gets a `503 Service Unavailable` with a `Retry-After` header. That is simpler than trying to migrate in-flight state across the v1/v2 boundary.

Celery and Redis setup. Redis serves as both the message broker and result backend. Retry logic uses exponential backoff with a cap so tasks cannot loop indefinitely. Stuck tasks are detectable through timeout tracking rather than silently sitting in "processing" forever.

**Rate limiting.** Flask-Limiter with Redis will cap request rates at around 5 document processing requests per minute per IP as a floor. There is one problem with hard per-IP limits in a classroom context worth acknowledging: if 30 students are all behind the same school NAT, they share a single IP address. A strict per-IP limit blocks all of them after the fifth request. My approach is to apply the per-IP floor as burst protection, use a higher threshold of around 20 requests per minute per IP, and pair it with a global queue depth limit so no single source can flood the worker pool regardless of auth status. For deployments with authentication, the limit applies per authenticated user instead.

SIGTERM handlers go in for graceful shutdowns so workers finish their current task and update state before exiting. Nothing appears stuck after a deployment.

After each major piece—Celery setup, rate limiting, graceful shutdown—I run a concurrent load test with 10 simultaneous clients before moving on. The load testing is not saved for the end.

**Milestone end of week 5:** Upload a document, receive a task ID, poll for completion, fetch the result. Full async round trip working. Old synchronous endpoint still responds and returns the deprecation header.

### Weeks 6 to 7: Model optimization
Five generator classes each call `from_pretrained` independently. MCQGen loads t5-large. BoolQGen loads its own copy of t5-large. ShortQGen loads t5-base. The result is over 15GB of memory for something that could run under 5GB with shared instances.

I will build a centralized model manager that loads each model once per worker process and hands the same instance to whoever requests it. All redundant `from_pretrained` calls get replaced with requests to the manager.

**PyTorch and Celery forking.** This is the part most writeups on Celery with ML models skip. Python's default `fork()` does not safely duplicate CUDA contexts. If models are loaded before the fork, GPU memory can become corrupted or deadlocked in child workers. The correct pattern is to load models after the fork using Celery's `worker_process_init` signal:

```python
from celery.signals import worker_process_init
from model_manager import ModelManager

manager = None

@worker_process_init.connect
def init_models(**kwargs):
global manager
manager = ModelManager()
manager.preload(["t5-large", "t5-base"])
```

If CPU-only inference is used the forking constraint is relaxed, but I will enforce the signal-based pattern regardless so the code is safe whether or not a GPU is present.

Loading is lazy by default: models load on first request rather than at worker startup. This speeds up deployment and lets workers that only handle certain task types skip loading models they will never use.

**Milestone:** Memory usage measured before and after with an identical workload. Both numbers documented.

### Week 8: Buffer and consolidation
I built this week in deliberately. Weeks 3 through 7 carry real complexity and something will take longer than expected. Unexpected Celery and Flask interaction bugs, Redis connection edge cases under load, model manager thread safety—these are all places I expect to spend extra time that is hard to budget precisely upfront.

Whatever spilled from earlier phases gets resolved here. Edge cases found during integration testing get fixed. The security tests, async workflow tests, and memory benchmarks I have been running incrementally throughout get consolidated into a single suite that a reviewer can run with one command.

### Week 9: Finalization
Mentor feedback from the full review gets incorporated. Documentation covers the new async architecture, how to deploy with Celery and Redis, what the v2 API changes are, the migration path from the old synchronous endpoints, and what the updated system requirements are. The implementation gets cleaned up and submitted.

## Extended Work (if core finishes early)
* **Intelligent caching.** Before creating a task, hash the input (URL or PDF checksum) and check Redis for a cached result from an identical previous request. In classroom settings where 50 students process the same assigned video, this reduces compute from 50 full processing runs to one.
* **Improved document processing.** The current approach pipes everything through `get_text` and splits on periods with regex, which destroys structure. Bullet points become fragments, code blocks get mangled, paragraphs split in the wrong places. A proper parser would preserve headings, lists, and code blocks and chunk on semantic boundaries so the questions generated are about coherent content rather than sentence fragments.

## Logistics and Availability
- **Time zone:** IST (UTC+5:30)
- **Availability:** 25 to 30 hours per week for the full GSoC duration.
- **Known conflicts:** None at this time.
- I am available for the full GSoC duration and can dedicate approximately 25 to 30 hours per week to this project. I am comfortable collaborating asynchronously and will actively communicate progress, blockers, and updates with mentors. I will also keep a public progress log throughout the project.

## Why This Project Matters
EduAid is for students. A security hole in a student tool is not just a technical problem. It is a breach of trust with people who are trying to learn. Timeouts that interrupt studying are direct interference with someone's work, not an abstract SLA number.

The goal is a backend students can actually depend on. Getting there means fixing what is broken before adding anything new.
65 changes: 50 additions & 15 deletions backend/server.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from flask import Flask, request, jsonify
from flask_cors import CORS
from werkzeug.exceptions import RequestEntityTooLarge
from pprint import pprint
import nltk
import subprocess
Expand Down Expand Up @@ -31,6 +32,23 @@
CORS(app)
print("Starting Flask App...")

# 1. Add MAX_CONTENT_LENGTH config (5 MB limit)
app.config['MAX_CONTENT_LENGTH'] = 5 * 1024 * 1024

# 2. Add ALLOWED_EXTENSIONS validation
ALLOWED_EXTENSIONS = {'txt', 'pdf', 'docx'}

def allowed_file(filename):
if '.' not in filename:
return False

extension = filename.rsplit('.', 1)[1].lower()
return extension in ALLOWED_EXTENSIONS

@app.errorhandler(RequestEntityTooLarge)
def handle_file_too_large(error):
return jsonify({"error": "File size exceeds the 5MB limit"}), 413

SERVICE_ACCOUNT_FILE = './service_account_key.json'
SCOPES = ['https://www.googleapis.com/auth/documents.readonly']

Expand Down Expand Up @@ -495,6 +513,10 @@ def upload_file():
if file.filename == '':
return jsonify({"error": "No selected file"}), 400

# 3. Reject invalid files EARLY
if not allowed_file(file.filename):
return jsonify({"error": "File type not allowed. Supported types: txt, pdf, docx"}), 400

Comment on lines +516 to +519

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that the raw client filename still reaches the filesystem save path.
sed -n '352,384p' backend/Generator/main.py
echo
rg -n -C2 'process_file|file\.filename|secure_filename|allowed_file' backend/server.py backend/Generator/main.py

Repository: AOSSIE-Org/EduAid

Length of output: 3015


Sanitize the filename before handing it to FileProcessor.

The allowlist only checks the extension. A name like ../../tmp/payload.txt still passes the validation and reaches backend/Generator/main.py:371 where os.path.join(self.upload_folder, file.filename) saves the file using the raw, unsanitized filename. This leaves the upload path traversal vulnerability open.

Suggested fix
+from werkzeug.utils import secure_filename
+
 `@app.route`('/upload', methods=['POST'])
 def upload_file():
     if 'file' not in request.files:
         return jsonify({"error": "No file part"}), 400

     file = request.files['file']

     if file.filename == '':
         return jsonify({"error": "No selected file"}), 400

-    if not allowed_file(file.filename):
+    safe_name = secure_filename(file.filename)
+    if not safe_name or not allowed_file(safe_name):
         return jsonify({"error": "File type not allowed. Supported types: txt, pdf, docx"}), 400
+    file.filename = safe_name

     content = file_processor.process_file(file)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# 3. Reject invalid files EARLY
if not allowed_file(file.filename):
return jsonify({"error": "File type not allowed. Supported types: txt, pdf, docx"}), 400
from werkzeug.utils import secure_filename
# (other code in the function...)
# 3. Reject invalid files EARLY
safe_name = secure_filename(file.filename)
if not safe_name or not allowed_file(safe_name):
return jsonify({"error": "File type not allowed. Supported types: txt, pdf, docx"}), 400
file.filename = safe_name
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 516 - 519, The filename is not sanitized
before being passed to FileProcessor, allowing path traversal via file.filename;
sanitize the name (e.g., use werkzeug.utils.secure_filename or at minimum strip
path separators and enforce os.path.basename and reject empty results)
immediately after the allowed_file check and before calling FileProcessor, and
also update FileProcessor where it constructs the path with upload_folder to
always use the sanitized filename (and reject filenames that are absolute or
contain ..); reference allowed_file, file.filename, FileProcessor, and
upload_folder and ensure both the upload entrypoint and the save logic use the
same sanitized value.

content = file_processor.process_file(file)

if content:
Expand Down Expand Up @@ -539,22 +561,35 @@ def get_transcript():
if not video_id:
return jsonify({"error": "No video ID provided"}), 400

subprocess.run(["yt-dlp", "--write-auto-sub", "--sub-lang", "en", "--skip-download",
"--sub-format", "vtt", "-o", f"subtitles/{video_id}.vtt", f"https://www.youtube.com/watch?v={video_id}"],
check=True, capture_output=True, text=True)

# Find the latest .vtt file in the "subtitles" folder
subtitle_files = glob.glob("subtitles/*.vtt")
if not subtitle_files:
return jsonify({"error": "No subtitles found"}), 404

latest_subtitle = max(subtitle_files, key=os.path.getctime)
transcript_text = clean_transcript(latest_subtitle)
transcript_text = None
latest_subtitle = None

# Optional: Clean up the file after reading
os.remove(latest_subtitle)

return jsonify({"transcript": transcript_text})
try:
subprocess.run(["yt-dlp", "--write-auto-sub", "--sub-lang", "en", "--skip-download",
"--sub-format", "vtt", "-o", f"subtitles/{video_id}.vtt", f"https://www.youtube.com/watch?v={video_id}"],
check=True, capture_output=True, text=True)
Comment on lines +567 to +570

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that raw video_id is interpolated into filesystem paths/globs.
sed -n '558,592p' backend/server.py
echo
rg -n -C2 'video_id|subtitles/\{video_id\}|glob\.glob' backend/server.py

Repository: AOSSIE-Org/EduAid

Length of output: 2738


Validate video_id before using it in paths or glob patterns.

video_id from request.args.get('videoId') is used directly in the yt-dlp output path (line 569) and in the cleanup glob pattern (line 589) without validation. Characters like ../ or glob metacharacters (*, ?, [) can escape the intended directory or match unrelated files.

Suggested fix
 `@app.route`('/getTranscript', methods=['GET'])
 def get_transcript():
     video_id = request.args.get('videoId')
     if not video_id:
         return jsonify({"error": "No video ID provided"}), 400
+    if not re.fullmatch(r"[A-Za-z0-9_-]{11}", video_id):
+        return jsonify({"error": "Invalid video ID"}), 400

     transcript_text = None
     latest_subtitle = None
🧰 Tools
🪛 Ruff (0.15.9)

[error] 568-568: subprocess call: check for execution of untrusted input

(S603)


[error] 568-569: Starting a process with a partial executable path

(S607)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 567 - 570, Validate and sanitize the video_id
before use: ensure the value from request.args.get('videoId') matches a strict
allowlist (e.g., regex like /^[A-Za-z0-9_-]{11}$/ or your permitted pattern) and
return a 400 error if it fails; reject any value containing path separators,
dots sequences, or glob metacharacters. Use the validated/sanitized identifier
when constructing the output path for the subprocess.run call (reference
variable video_id and the subprocess.run invocation) by joining with a safe
directory (e.g., using pathlib.Path("subtitles") / safe_name) and avoid
constructing glob patterns with the raw input—either build the exact filename
from the sanitized id for cleanup or sanitize before passing to glob. Ensure all
places that reference the original video_id (the subprocess output -o argument
and the cleanup glob usage) use the validated safe_name instead of the raw
input.


# Find the latest .vtt file in the "subtitles" folder
subtitle_files = glob.glob("subtitles/*.vtt")
if not subtitle_files:
return jsonify({"error": "No subtitles found"}), 404

latest_subtitle = max(subtitle_files, key=os.path.getctime)
transcript_text = clean_transcript(latest_subtitle)
Comment on lines +572 to +578

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Stop picking the “latest” subtitle from a shared directory.

This is still race-prone. Two concurrent requests can both write into subtitles/, then whichever file has the newest ctime gets read and deleted by both handlers. That can return the wrong transcript or break the other request. Use a request-scoped temp directory (or an exact output path you already know) instead of scanning subtitles/*.vtt.

Suggested direction
+import shutil
+import uuid
+
 `@app.route`('/getTranscript', methods=['GET'])
 def get_transcript():
     video_id = request.args.get('videoId')
     if not video_id:
         return jsonify({"error": "No video ID provided"}), 400

     transcript_text = None
     latest_subtitle = None
+    request_dir = os.path.join("subtitles", uuid.uuid4().hex)
+    os.makedirs(request_dir, exist_ok=True)

     try:
+        output_template = os.path.join(request_dir, "%(id)s.%(ext)s")
         subprocess.run(["yt-dlp", "--write-auto-sub", "--sub-lang", "en", "--skip-download",
-                    "--sub-format", "vtt", "-o", f"subtitles/{video_id}.vtt", f"https://www.youtube.com/watch?v={video_id}"],
+                    "--sub-format", "vtt", "-o", output_template, f"https://www.youtube.com/watch?v={video_id}"],
                    check=True, capture_output=True, text=True)

-        subtitle_files = glob.glob("subtitles/*.vtt")
+        subtitle_files = glob.glob(os.path.join(request_dir, "*.vtt"))
         if not subtitle_files:
             return jsonify({"error": "No subtitles found"}), 404

         latest_subtitle = max(subtitle_files, key=os.path.getctime)
         transcript_text = clean_transcript(latest_subtitle)

         return jsonify({"transcript": transcript_text})

     finally:
-        if latest_subtitle and os.path.exists(latest_subtitle):
-            app.logger.info(f"Deleting temporary subtitle file: {latest_subtitle}")
-            os.remove(latest_subtitle)
-        elif not latest_subtitle:
-            for stray in glob.glob(f"subtitles/{video_id}*.vtt"):
-                if os.path.exists(stray):
-                    app.logger.info(f"Deleting stray temporary subtitle file: {stray}")
-                    os.remove(stray)
+        shutil.rmtree(request_dir, ignore_errors=True)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/server.py` around lines 572 - 578, The code currently finds the
newest file via subtitle_files = glob.glob("subtitles/*.vtt") and
latest_subtitle = max(...), which is race-prone; instead make the subtitle
output path request-scoped (e.g., create a unique temp dir or unique filename
per request) and read that exact path rather than scanning the shared
"subtitles" folder. Update the code that generates the .vtt to accept/return a
deterministic path (or use tempfile.mkdtemp()/NamedTemporaryFile) and then call
clean_transcript() with that specific path (replace the glob/max logic around
subtitle_files/latest_subtitle), and ensure cleanup happens only for that
request's file to avoid cross-request deletion.


return jsonify({"transcript": transcript_text})

finally:
# Guarantee cleanup runs even if subprocess or extraction fails
if latest_subtitle and os.path.exists(latest_subtitle):
app.logger.info(f"Deleting temporary subtitle file: {latest_subtitle}")
os.remove(latest_subtitle)
elif not latest_subtitle:
# Fallback cleanup: If yt-dlp crashed halfway but still emitted partial files
for stray in glob.glob(f"subtitles/{video_id}*.vtt"):
if os.path.exists(stray):
app.logger.info(f"Deleting stray temporary subtitle file: {stray}")
os.remove(stray)

if __name__ == "__main__":
os.makedirs("subtitles", exist_ok=True)
Expand Down