Phase 3: Correction — Implementing and Justifying Each Fix

Capsule overview

You have a findings document with 12-18 documented problems. Now comes the part where you prove not only that you can find problems, but that you can solve them correctly. This phase has three components: prioritizing what to fix first, implementing each correction, and justifying why your fix is correct.

The justification matters just as much as the fix. Any developer can change a line of code. A professional developer explains why that change is necessary, why the correction is the right one, and how they verified it works. That's what sets you apart.

Prioritization: What to Fix First

The rule: descending impact

You don't fix in the order you found the problems. You fix in order of impact:

Critical → High → Medium → Low

Why? Because in a real scenario, if you only had time to fix half of the problems, the Critical and High ones are the ones you can't let slide. A security hole in production is an emergency. Incorrect naming is an improvement.

How to organize your fixes

Sort your findings document by severity and group them:

## Corrections to Implement

### Block 1: Critical (implement immediately)
1. Finding #X: Hardcoded JWT secret → config.py
2. Finding #Y: SQL injection in search → routes/users.py
3. Finding #Z: Password stored in plain text → routes/auth.py
4. Finding #W: Endpoint with no authentication → routes/users.py

### Block 2: High (implement next)
5. Finding #A: Incorrect delete logic → services/task_service.py
6. Finding #B: Missing authorization in get_task_by_id → services/task_service.py
7. Finding #C: Insufficient password validation → models.py
8. Finding #D: Statistics include deleted tasks → services/task_service.py

### Block 3: Medium (implement afterwards)
9.  Finding #E: Off-by-one pagination → services/task_service.py
10. Finding #F: Division by zero in statistics → services/task_service.py
11. Finding #G: Insufficient title validation → models.py
12. Finding #H: Filters with SQL injection → services/task_service.py

### Block 4: Low (implement if there's time)
13. Finding #I: Nonexistent import → main.py
14. Finding #J: Missing priority/status validations → models.py
15. Finding #K: Pagination accepts page=0 → routes/tasks.py

Deciding: Regenerate vs Edit

Before you start fixing, decide for each finding whether you'll:

• Edit manually: Change the specific lines of the problem
• Regenerate with Claude Code: Ask Claude Code to regenerate the whole function or block

Decision framework (Module 7)

Examples of when to edit

Finding: Hardcoded JWT secret in config.py
→ EDIT: Change one line from `os.getenv("...", "fallback")` 
  to `os.environ["..."]`. Targeted fix, clear, no risk.

Finding: Password validation accepts 4 chars instead of 8
→ EDIT: Change `if len(v) < 4` to `if len(v) < 8`. 
  One line, the rest of the function is fine.

Examples of when to regenerate

Finding: The get_tasks function has SQL injection + incorrect 
offset + filters built by concatenation
→ REGENERATE: The function has 3 different problems. 
  It's safer to regenerate the whole function with the 
  correct requirements than to patch 3 things.

Finding: The delete flow is a hard delete when it should 
be a soft delete. It affects delete_task + get_tasks + get_user_stats
→ REGENERATE all 3 functions: The change affects multiple 
  functions and the underlying logic is incorrect.

How to document the decision

For each fix, include:

**Regenerate/edit decision:** [Edit / Regenerate]
**Justification:** [Why you chose this option]

Corrections by Category

Below are the corrections for the codebase's problems. Try to implement yours first before consulting these solutions. The solutions are hidden so that you use them as a reference after your own attempt.

Correction: Security — Hardcoded JWT Secret

File: config.py Problem: JWT_SECRET_KEY has a hardcoded fallback that would be used in production if the environment variable doesn't exist. Requirement violated: RF-02.3, RF-06.1

import os


class Settings:
    APP_NAME: str = "TaskFlow API"
    APP_VERSION: str = "1.0.0"
    DEBUG: bool = os.getenv("DEBUG", "false").lower() == "true"

    DATABASE_URL: str = os.getenv("DATABASE_URL", "sqlite:///./taskflow.db")
    DATABASE_PATH: str = "taskflow.db"

    JWT_SECRET_KEY: str = os.environ["JWT_SECRET_KEY"]
    JWT_ALGORITHM: str = "HS256"
    JWT_EXPIRATION_MINUTES: int = 30

    BCRYPT_ROUNDS: int = 12

    DEFAULT_PAGE_SIZE: int = 10
    MAX_PAGE_SIZE: int = 100


settings = Settings()

Correction: Security — Password Stored in Plain Text

File: routes/auth.py Problem: In the register() function, the password is inserted directly into the database without hashing. Requirement violated: RF-01.2

@router.post("/register", response_model=UserResponse)
async def register(user: UserCreate):
    existing = execute_query(
        "SELECT id FROM users WHERE email = ?", (user.email,)
    )
    if existing:
        raise HTTPException(status_code=400, detail="Email already registered")

    hashed = hash_password(user.password)

    user_id = execute_insert(
        "INSERT INTO users (email, name, password) VALUES (?, ?, ?)",
        (user.email, user.name, hashed),
    )

    created_user = execute_query(
        "SELECT id, email, name, created_at FROM users WHERE id = ?",
        (user_id,),
    )

    row = dict(created_user[0])
    return UserResponse(**row)

Correction: Security — SQL Injection in User Search

File: routes/users.py Problem: The /users/search endpoint uses an f-string to build the SQL query and doesn't require authentication. Requirement violated: RF-06.2, RF-02.4

@router.get("/search")
async def search_users(
    query: str,
    current_user: dict = Depends(get_current_user),
):
    results = execute_query(
        "SELECT id, email, name FROM users WHERE name LIKE ?",
        (f"%{query}%",),
    )
    return [dict(row) for row in results]

Correction: Security — SQL Injection in Task Filters

File: services/task_service.py Problem: The get_tasks() function uses f-strings to insert status and priority into the SQL query. Requirement violated: RF-06.2

def get_tasks(
    user_id: int,
    status: Optional[str] = None,
    priority: Optional[str] = None,
    page: int = 1,
    size: int = 10,
) -> dict:
    base_query = "SELECT * FROM tasks WHERE user_id = ?"
    count_query = "SELECT COUNT(*) as total FROM tasks WHERE user_id = ?"
    params = [user_id]

    if status:
        base_query += " AND status = ?"
        count_query += " AND status = ?"
        params.append(status)

    if priority:
        base_query += " AND priority = ?"
        count_query += " AND priority = ?"
        params.append(priority)

    offset = (page - 1) * size
    base_query += f" ORDER BY created_at DESC LIMIT ? OFFSET ?"
    query_params = params + [size, offset]

    with get_db() as conn:
        cursor = conn.cursor()

        cursor.execute(count_query, tuple(params))
        total = cursor.fetchone()["total"]

        cursor.execute(base_query, tuple(query_params))
        rows = cursor.fetchall()

    tasks = [dict(row) for row in rows]
    return {
        "tasks": tasks,
        "total": total,
        "page": page,
        "size": size,
    }

Correction: Logic — Missing Authorization in get_task_by_id

File: services/task_service.py Problem: get_task_by_id() doesn't filter by user_id, letting any authenticated user see any task. Requirement violated: RF-03.4, RF-03.7, RF-05.6

def get_task_by_id(task_id: int, user_id: int) -> Optional[dict]:
    query = """
        SELECT id, title, description, priority, status, 
               user_id, created_at, updated_at
        FROM tasks WHERE id = ?
    """
    rows = execute_query(query, (task_id,))
    if not rows:
        return None

    task = dict(rows[0])

    if task["user_id"] != user_id:
        from fastapi import HTTPException
        raise HTTPException(
            status_code=403,
            detail="You don't have permission to access this task"
        )

    return task

Correction: Logic — Delete is a Hard Delete Instead of a Soft Delete

File: services/task_service.py Problem: delete_task() runs DELETE FROM tasks instead of changing the status to "deleted". Requirement violated: RF-03.6

def delete_task(task_id: int, user_id: int) -> bool:
    existing = get_task_by_id(task_id, user_id)
    if not existing:
        return False

    query = "UPDATE tasks SET status = ?, updated_at = ? WHERE id = ?"
    execute_query(query, ("deleted", datetime.utcnow().isoformat(), task_id))
    return True

Correction: Logic — Statistics Include Deleted Tasks

File: services/task_service.py Problem: get_user_stats() counts every task, including the deleted (soft deleted) ones. Requirement violated: RF-04.2

def get_user_stats(user_id: int) -> dict:
    query = """
        SELECT status, priority FROM tasks 
        WHERE user_id = ? AND status != ?
    """
    rows = execute_query(query, (user_id, "deleted"))

    total = len(rows)
    by_status = {}
    by_priority = {}

    for row in rows:
        row_dict = dict(row)
        status = row_dict["status"]
        priority = row_dict["priority"]

        by_status[status] = by_status.get(status, 0) + 1
        by_priority[priority] = by_priority.get(priority, 0) + 1

    if total == 0:
        completion_percentage = 0.0
    else:
        completion_percentage = (
            by_status.get("completed", 0) / total * 100
        )

    return {
        "total_tasks": total,
        "by_status": by_status,
        "by_priority": by_priority,
        "completion_percentage": round(completion_percentage, 2),
    }

Correction: Edge Case — Division by Zero in Statistics

File: services/task_service.py Problem: get_user_stats() divides by total without checking that it's greater than zero. It crashes for users with no tasks. Requirement violated: RF-04.1 (must return valid statistics)

Note: This correction is already included in the previous statistics correction. The combined solution addresses both problems.

Correction: Edge Case — Off-by-One Pagination

File: services/task_service.py Problem: The offset is computed as page * size instead of (page - 1) * size, causing the first page (page=1) to skip the first records. Requirement violated: RF-03.2

Note: This correction is already included in the get_tasks() correction from the SQL injection section.

Correction: Hallucination — Nonexistent Import

File: main.py Problem: from pydantic_settings import BaseSettings imports a class from a package that isn't in requirements.txt. The pydantic-settings package is separate from pydantic and isn't installed. Requirement violated: N/A (hallucination)

from fastapi import FastAPI
from fastapi.middleware.cors import CORSMiddleware

from config import settings
from database import init_db
from routes import auth, tasks, users

app = FastAPI(
    title=settings.APP_NAME,
    version=settings.APP_VERSION,
    debug=settings.DEBUG,
)

app.add_middleware(
    CORSMiddleware,
    allow_origins=["*"],
    allow_credentials=True,
    allow_methods=["*"],
    allow_headers=["*"],
)


@app.on_event("startup")
async def startup():
    init_db()


app.include_router(auth.router)
app.include_router(tasks.router)
app.include_router(users.router)


@app.get("/health")
async def health_check():
    return {
        "status": "healthy",
        "app": settings.APP_NAME,
        "version": settings.APP_VERSION,
    }

Correction: Hallucination — Nonexistent Function in bcrypt

File: services/auth_service.py Problem: from bcrypt import hashpw, gensalt, checkpw, verify_hash imports verify_hash, a function that doesn't exist in the bcrypt package. The real bcrypt functions are hashpw, gensalt, checkpw, and kdf. Requirement violated: N/A (hallucination)

from bcrypt import hashpw, gensalt, checkpw

Correction: Hallucination — Nonexistent Parameter in fetchall()

File: services/task_service.py Problem: cursor.fetchall(as_dict=True) passes an as_dict parameter that doesn't exist in sqlite3.Cursor.fetchall(). The method doesn't accept any parameters. Requirement violated: N/A (hallucination)

cursor.execute(base_query, tuple(params))
rows = cursor.fetchall()

Correction: Validation — Password Accepts 4 Characters

File: models.py Problem: The password validation accepts a 4-character minimum, but RF-01.4 requires a minimum of 8 characters. Requirement violated: RF-01.4

class UserCreate(BaseModel):
    email: EmailStr
    name: str
    password: str

    @field_validator("password")
    @classmethod
    def validate_password(cls, v):
        if len(v) < 8:
            raise ValueError("Password must be at least 8 characters")
        return v

Correction: Validation — Title Accepts 1 Character

File: models.py Problem: The title validation only checks that it isn't empty (>= 1 character), but RF-05.1 requires a minimum of 3 and a maximum of 100 characters. Requirement violated: RF-05.1

class TaskCreate(BaseModel):
    title: str
    description: Optional[str] = ""
    priority: str = "medium"
    status: str = "pending"

    @field_validator("title")
    @classmethod
    def validate_title(cls, v):
        if len(v) < 3:
            raise ValueError("Title must be at least 3 characters")
        if len(v) > 100:
            raise ValueError("Title must be at most 100 characters")
        return v

    @field_validator("priority")
    @classmethod
    def validate_priority(cls, v):
        allowed = {"low", "medium", "high"}
        if v not in allowed:
            raise ValueError(f"Priority must be one of: {', '.join(allowed)}")
        return v

    @field_validator("status")
    @classmethod
    def validate_status(cls, v):
        allowed = {"pending", "in_progress", "completed"}
        if v not in allowed:
            raise ValueError(f"Status must be one of: {', '.join(allowed)}")
        return v

Correction: Edge Case — Pagination Accepts page=0

File: routes/tasks.py Problem: The page parameter accepts 0 because it has ge=0 instead of ge=1. Requirement violated: RF-05.4

@router.get("/", response_model=TaskListResponse)
async def list_tasks(
    status: Optional[str] = Query(default=None),
    priority: Optional[str] = Query(default=None),
    page: int = Query(default=1, ge=1),
    size: int = Query(default=10, ge=1, le=100),
    current_user: dict = Depends(get_current_user),
):
    result = get_tasks(
        user_id=current_user["user_id"],
        status=status,
        priority=priority,
        page=page,
        size=size,
    )
    return TaskListResponse(**result)

How to Verify Each Fix

After implementing each correction, verify that it works:

Verifying the security fixes

# 1. Verify the app doesn't start without JWT_SECRET_KEY
unset JWT_SECRET_KEY
uvicorn main:app --reload  # Must fail with KeyError

# 2. Verify the password is hashed
export JWT_SECRET_KEY="test-secret"
uvicorn main:app --reload

curl -X POST http://localhost:8000/auth/register \
  -H "Content-Type: application/json" \
  -d '{"email": "verify@test.com", "name": "Verify", "password": "securepass123"}'

sqlite3 taskflow.db "SELECT password FROM users WHERE email='verify@test.com'"
# Must show a bcrypt hash, not "securepass123"

# 3. Verify SQL injection doesn't work
curl -s "http://localhost:8000/users/search?query=test'%20OR%20'1'='1" \
  -H "Authorization: Bearer $TOKEN"
# Must not return every user

# 4. Verify search requires auth
curl -s "http://localhost:8000/users/search?query=test"
# Must return 401, not results

Verifying the logic fixes

# 1. Verify soft delete
curl -X DELETE http://localhost:8000/tasks/1 \
  -H "Authorization: Bearer $TOKEN"

sqlite3 taskflow.db "SELECT id, status FROM tasks WHERE id = 1"
# Must show status = 'deleted', not be empty

# 2. Verify authorization
# With User 2's token, try to see User 1's task
curl -s http://localhost:8000/tasks/1 \
  -H "Authorization: Bearer $TOKEN_USER2"
# Must return 403, not the task

# 3. Verify pagination
curl -s "http://localhost:8000/tasks/?page=1&size=5" \
  -H "Authorization: Bearer $TOKEN"
# Must return the first 5 tasks, skipping none

Verifying the edge case fixes

# 1. Verify statistics with no tasks
curl -s http://localhost:8000/users/stats \
  -H "Authorization: Bearer $TOKEN_NEW_USER"
# Must return {"total_tasks": 0, ...}, not a 500 error

# 2. Verify statistics without deleted tasks
# (after a soft delete)
curl -s http://localhost:8000/users/stats \
  -H "Authorization: Bearer $TOKEN"
# Must not include tasks with status "deleted"

# 3. Verify validations
curl -X POST http://localhost:8000/auth/register \
  -H "Content-Type: application/json" \
  -d '{"email": "weak@test.com", "name": "Weak", "password": "1234"}'
# Must return 422, not 200

curl -X POST http://localhost:8000/tasks/ \
  -H "Content-Type: application/json" \
  -H "Authorization: Bearer $TOKEN" \
  -d '{"title": "AB", "priority": "medium"}'
# Must return 422 (title < 3 characters)

curl -X POST http://localhost:8000/tasks/ \
  -H "Content-Type: application/json" \
  -H "Authorization: Bearer $TOKEN" \
  -d '{"title": "Valid task", "priority": "urgent"}'
# Must return 422 (invalid priority)

Justification Documentation Format

For each correction, document it using this format:

# Correction Justifications — TaskFlow API

## Fix #1: [Finding title]

**Finding:** #[number] — [brief description]
**Severity:** [Critical/High/Medium/Low]
**File:** [file name]
**Affected lines:** [line range]

### What I changed
[Concise description of the change — which lines changed and how]

### Why the original code was incorrect
[Explanation referencing the functional requirement violated.
Use the requirement ID: RF-XX.X]

### Why my correction is correct
[Explanation of why the fix solves the problem.
Include how you verified it works.]

### Regenerate/edit decision
- **Decision:** [Edit / Regenerate]
- **Justification:** [Why you chose this option]

### Verification
- **Method:** [How you verified the fix works]
- **Result:** [What you observed]

---

## Fix #2: [Finding title]
[Same format]

Common Mistakes in This Phase

Mistake 1: Fixing without justifying

Changing a line of code doesn't demonstrate knowledge. Explaining why you change it and why your version is correct does. Every fix without a justification is an incomplete fix.

Mistake 2: Fixing the Low ones before the Critical ones

It's tempting to start with the easy stuff (changing a number from 4 to 8). But in a professional environment, if you only have 30 minutes, the security holes come first. Practice prioritization.

Mistake 3: Not verifying that the fix works

"I changed the line, it must work" is an assumption. Run the endpoint, check the result, confirm the fix solves the problem without introducing others.

Mistake 4: Introducing new bugs while fixing

Every correction has the potential to break something else. After each fix, run the basic tests to confirm you didn't break anything:

# Smoke test after each fix
curl -s http://localhost:8000/health
curl -s -X POST http://localhost:8000/auth/register \
  -H "Content-Type: application/json" \
  -d '{"email": "smoke@test.com", "name": "Smoke", "password": "smoketest123"}'
curl -s -X POST http://localhost:8000/auth/login \
  -H "Content-Type: application/json" \
  -d '{"email": "smoke@test.com", "password": "smoketest123"}'

Mistake 5: Regenerating the whole codebase

If you tell Claude Code "regenerate the whole codebase, corrected", you lose:

• The opportunity to demonstrate that you understand each problem
• The documentation of what changed and why
• The practice of the real skill (surgical correction)

Regenerate specific functions when the framework justifies it. Don't regenerate whole files.

Summary of the Expected Corrections

By the end of this phase, you should have implemented corrections for every problem you found. This is a map of the main corrections by file:

config.py

• ✅ JWT secret with no hardcoded fallback
• ✅ DEBUG not hardcoded as True

main.py

• ✅ Nonexistent import removed

models.py

• ✅ Password validation: 8 characters minimum
• ✅ Title validation: 3-100 characters
• ✅ Priority validation: only low/medium/high
• ✅ Status validation: only pending/in_progress/completed

routes/auth.py

• ✅ Password hashed before storing

routes/tasks.py

• ✅ Pagination: page >= 1

routes/users.py

• ✅ SQL injection fixed in search
• ✅ Authentication added to search

services/task_service.py

• ✅ SQL injection fixed in filters
• ✅ Pagination offset fixed
• ✅ Soft delete implemented
• ✅ Ownership check in get_task_by_id
• ✅ Statistics exclude deleted tasks
• ✅ Division by zero handled

Correction Phase Checkpoint

Before moving on to the delivery and retrospective phase, verify:

• ✅ Every Critical severity finding is fixed
• ✅ Every High severity finding is fixed
• ✅ Most Medium findings are fixed
• ✅ Every correction has a written justification
• ✅ Every correction was verified by running the endpoint
• ✅ For each fix, you documented the regenerate/edit decision
• ✅ You didn't introduce new bugs (smoke tests pass)
• ✅ The corrected codebase runs with no errors

Next capsule: Delivery and Retrospective — The complete delivery format, the self-assessment rubric, and the guiding questions for the retrospective.

Debugging & Code Review with Claude Code — Module 8, Capsule 04 Claude Code Agentic Development Path — Guide #6 of 11