Review security and data handling

Lesson 9
45 min
Module 3: Review generated work

Lesson 9 of 15 · AI-Assisted Software DevelopmentReview security and data handling

Course overview

1. Establish a safe working setup

2. Write actionable tasks

3. Review generated work

4. Debug and test with assistance

5. Deliver maintainable work

6. Case studies

What you will be able to do

Find what a change does with input, secrets, files and deletions. Follow each value from where it enters the program to where it is used. The tests rarely cover these paths, so your reading must.

Before you startWhat to know and to have readyAbout 45 minutes

What you need to know

The review checklist, from Inspect the diff.
Environment variables, the Settings class and field(repr=False), from Configuration and secrets.
Log levels, from Exceptions and logging.
A sandbox and a disposable copy, from Context and boundaries.

What you need on your computer

The project ~/projects/ticket-cleaner-assist, with 34 passed and a clean working tree. Its environment stays active for the whole lesson.
Less than 1 MB of free space, for a throwaway copy of the snapshot start.
No account, no key and no money. The token in this lesson is made up.

A cleaner with a bin bag

You ask a cleaner: “Please throw away the old papers in the box on my desk.” When you come back, the box is empty. Your passport was in it, under the papers. The cleaner did what you said, in a way that you did not mean.

On the same day, the cleaner made a list of everything on the desk, for you. They pinned it to the front door. The list includes the PIN from the note in your drawer.

Neither action was dishonest. Both used something of yours where it does damage. Your words became “everything in the box”, and your private note became public text.

The problem: a helpful change

Candidate E adds two things to cli.py. The course team constructed it for this lesson. It is a short change, 9 lines, and it looks helpful. Here are its added lines, without most of the context:

@@ -1,5 +1,6 @@
 import argparse
 import logging
+import shutil
@@ -28,6 +29,11 @@ def make_parser() -> argparse.ArgumentParser:
+    parser.add_argument(
+        "--clean",
+        action="store_true",
+        help="delete old reports before writing the new one",
+    )
@@ -50,6 +56,7 @@ def main(argv: list[str] | None = None) -> int:
+    logger.debug("settings: %s", vars(settings))
@@ -63,6 +70,8 @@ def main(argv: list[str] | None = None) -> int:
     summary = build_summary(tickets, rejected, args.status)
+    if args.clean:
+        shutil.rmtree(args.output.parent, ignore_errors=True)
     write_json(summary, args.output)

All 24 tests of the snapshot start pass, and ruff check . prints All checks passed!. No test uses --clean or the debug level, so no test runs the new lines.

The course team ran it in throwaway copies and measured two results:

--clean --output summary.json deleted the project copy itself: 44 files became 1, the new summary.json. It also deleted the .git folder, with every commit. Afterwards, git status printed fatal: not a git repository, and Git could not bring anything back.
LOG_LEVEL=DEBUG printed the secret token on the screen: 'api_token': 'demo-token-not-real'.

The help text promised “delete old reports”. The code deletes a whole folder, and the user chooses which one.

The idea: follow each value to where it is used

A security review asks one question many times: where does this value come from, and where does it go? A value from outside the program is not under your control. It can come from an argument, a file, an environment variable or the network. Follow it to the place where it is used, and check what the worst value can do there.

Four places need the most care. They are the “Security and data” part of the checklist:

Input validation. Before a value is used as a path, a command or a number, the code checks it. A path from the command line can point anywhere.
Secrets. A secret, such as a token or a password, is never printed, logged, saved in a file or sent anywhere except where it belongs.
File permissions. The program runs with your permissions, so it can read, change and delete everything that you can. A file with a secret should be readable by you only.
Destructive operations. An operation that deletes or overwrites data cannot be undone. It touches only what the user asked for, only what the program made, and it asks first when the loss would hurt.

A tiny example: the parent of three paths

shutil.rmtree(folder) deletes a folder and everything inside it. Candidate E gives it args.output.parent, the folder that contains the output file. What is that folder for three possible outputs? Ask Python:

python -c "from pathlib import Path; print(Path('out/summary.json').parent)"
python -c "from pathlib import Path; print(Path('summary.json').parent)"
python -c "from pathlib import Path; print(Path('summary.json').parent.resolve())"

out
.
~/projects/ticket-cleaner-throwaway

The parent of summary.json is ., the current folder: the project. So --clean --output summary.json means “delete the project”. The diagram follows the value through the code:

A table for constructed candidate E, which runs shutil.rmtree on the parent of --output with ignore_errors=True. Three rows. --output reports/summary.json, the default: the parent is reports, so --clean deletes every older report in reports. --output out/summary.json: the parent is out; measured, out had last-week.json, and afterwards only summary.json. --output summary.json: the parent is the current folder, the project itself; measured in a throwaway copy, 44 files became 1. At the bottom: a value from the command line becomes the folder that shutil.rmtree deletes. — Follow the value of --output: the user's choice of file decides which folder candidate E deletes, with no check and no question.

Even the default deletes more than the help text says. A report of last week, which someone saved in reports/ on purpose, is gone. And ignore_errors=True hides every problem, so the user sees nothing unusual.

Try it in a throwaway copy

You will run candidate E, so you need a sandbox: a copy that you can lose. Run candidate E only in the throwaway copy, never in ticket-cleaner-assist. Never run --clean with an output in the current folder: it deletes the folder and its .git, with every commit.

Make a throwaway copy of start next to your project, and make it a Git repository. Keep your project’s environment active: the copy needs the same packages.
- macOS and Linux
- Windows
Terminal window
cd ~/projects cp -R nextia-learning-labs/C15/snapshots/start ticket-cleaner-throwaway cd ticket-cleaner-throwaway git init -q && git add -A && git commit -qm "Start"
Terminal window
cd ~\projects Copy-Item -Recurse nextia-learning-labs\C15\snapshots\start ticket-cleaner-throwaway cd ticket-cleaner-throwaway git init -q; git add -A; git commit -qm "Start"

Throwaway copy only. Check that you are in it: pwd must end with ticket-cleaner-throwaway. Then apply candidate E, and run the checks:

git apply ../nextia-learning-labs/C15/candidates/E-clean-and-debug.patch
git diff --stat
python -m pytest

 ticket_cleaner/cli.py | 9 +++++++++
 1 file changed, 9 insertions(+)
============================== 24 passed in 0.04s ==============================

Make an “old” report in a folder out, as a user would:
Terminal window
```
python -m ticket_cleaner data/tickets.csv --output out/last-week.json
```
Then list the folder: ls out on macOS and Linux, Get-ChildItem out in PowerShell. It shows last-week.json.
Throwaway copy only. --clean deletes a folder. Use exactly --output out/summary.json, never an output in the current folder. Run the new option, and list the folder again:
Terminal window
```
python -m ticket_cleaner data/tickets.csv --clean --output out/summary.json
ls out
```
Output (the end)
```
15 rows: 9 valid, 6 rejected.
Report: out/summary.json
summary.json
```
last-week.json is gone. Nothing printed a warning.

Do not try --output summary.json, not even in the throwaway copy. The course team measured it for you: the folder lost everything, .git too. A commit protects you from your own edits, but not from a program that deletes the repository.

Larkfield: Kwame follows the token

Kwame reviews the second addition. He starts with his usual question: “What can it touch?” The debug line touches the settings, and the settings hold the token.

In the Python course, the Settings class protected the token with field(repr=False). Here is a tiny example with a made-up token. print(s) and vars(s) show the same object in two ways:

TICKETS_API_TOKEN=demo-token-not-real python -c "from ticket_cleaner.config import load_settings; s = load_settings(); print(s); print(vars(s))"

Settings(tickets_url='https://raw.githubusercontent.com/Nextia-Intelligence/nextia-learning-labs/main/C02/data/tickets.json', log_level='WARNING')
{'tickets_url': 'https://raw.githubusercontent.com/Nextia-Intelligence/nextia-learning-labs/main/C02/data/tickets.json', 'api_token': 'demo-token-not-real', 'log_level': 'WARNING'}

On Windows, set the variable as in step 5 below, then run the python -c part. The first line comes from the class’s own description, and repr=False leaves the token out. vars() returns every field as a dictionary and does not ask the class. So it goes around the protection. Candidate E logs vars(settings).

Throwaway copy only, made-up token only. Run the program with the debug level. On macOS and Linux, set both variables for one command:

macOS and Linux
Windows

TICKETS_API_TOKEN=demo-token-not-real LOG_LEVEL=DEBUG python -m ticket_cleaner data/tickets.csv --output out/summary.json

$env:TICKETS_API_TOKEN = "demo-token-not-real"; $env:LOG_LEVEL = "DEBUG"
python -m ticket_cleaner data/tickets.csv --output out/summary.json
Remove-Item Env:TICKETS_API_TOKEN, Env:LOG_LEVEL

DEBUG ticket_cleaner: settings: {'tickets_url': 'https://raw.githubusercontent.com/Nextia-Intelligence/nextia-learning-labs/main/C02/data/tickets.json', 'api_token': 'demo-token-not-real', 'log_level': 'DEBUG'}

The token TICKETS_API_TOKEN=demo-token-not-real goes from the terminal into load_settings and the Settings class, where api_token has field(repr=False). Two branches. Green: print(settings) or logger.debug with settings shows Settings with tickets_url and log_level only, because repr=False hides the token. Red: logger.debug with vars(settings) shows a dictionary with api_token demo-token-not-real, because vars reads every field. From the red branch, the token goes to the terminal, a saved log file, a test or build log, and a bug report that someone pastes into a chat. — Where the token goes. The class hides it; vars() goes around the class, and a debug log carries it to places that many people read.

Debug logs travel. People turn on the debug level exactly when something is wrong, and then they share the output to get help. So a secret in a debug line will, sooner or later, reach someone who should not have it. If a real token leaks, the fix is not to delete the line from the screen. The owner of the service must cancel the token and make a new one.

File permissions: your program is you

Every program that you run has your permissions. That is why shutil.rmtree() could delete the project: you could, so the program could. A review cannot change that, but it can keep the program inside its own folders.

Permissions also protect secrets at rest. If you keep the token in a .env file, check who can read it. On macOS and Linux, in the throwaway copy:

printf "TICKETS_API_TOKEN=demo-token-not-real\n" > .env
ls -l .env
chmod 600 .env
ls -l .env

-rw-r--r--@ .env
-rw-------@ .env

rw-r--r-- means that you can read and write the file, and every other user of the computer can read it. After chmod 600, only you can. (The @ is macOS’s mark for extra file information; Linux does not show it.) git status does not list .env, because .gitignore names it. On Windows, files in your user folder are normally readable only by your account and the administrators; the course did not test Windows.

Put the throwaway copy back to start. git clean deletes files that Git does not track, so look first with -n, which only lists them:
Terminal window
```
git restore .
git clean -n -d
git clean -fd
```
Output
```
Would remove out/
Removing out/
```
git status --short now prints nothing. .env stays, because Git ignores it, so git clean does not touch it. You delete the whole copy after the module practice.

git clean -fd is a destructive operation too. The -n run is the “ask first” step that candidate E did not have.

What Amira does with candidate E

She rejects it. Neither addition was in any task, so the scope check alone is enough. The security findings show why that rule exists. Her review names each finding with its evidence:

Destructive operation, no input validation: shutil.rmtree(args.output.parent, ignore_errors=True) deletes any folder that --output points to; --output summary.json deletes the project, .git included.
Secret in a log: logger.debug("settings: %s", vars(settings)) prints the token at the debug level, around field(repr=False).

If Grace ever asks for old reports to be removed, that is a new task with its own brief. Its criteria could be: delete only the .json files that the program wrote in reports/, and list them and ask first. A test runs it in a temporary folder. If a debug log of the settings is useful, it logs settings, not vars(settings), and a test checks that the token is not in the output.

When to review for security, and what it costs

Do this review for every change that reads input, touches files, uses the network, handles a secret or deletes anything. That is most changes to a command-line program. It takes a few minutes: list the values that come from outside, and follow each one.

It has limits. You find what you follow, and a subtle problem can hide two functions away. For a program that handles customer data or runs on a server, a security specialist like Kwame reviews it as well. The checklist does not replace that review. It makes sure that the obvious paths were followed every time.

Common mistake

A destructive change tested in the real project

Symptom: Tomás wants to see what --clean does, so he applies candidate E in his own ticket-cleaner-assist and runs it with --output summary.json. The folder is empty except for summary.json. git status prints fatal: not a git repository.

Cause: the change deletes the parent folder of the output, and .git was inside it. Git cannot restore what it no longer has. His commits on the branch are gone too, unless he pushed them somewhere.

Fix: there is no fix in the folder. He copies the snapshot of the module that he finished last and does the lesson steps again, as the reset steps say. A pushed branch, as in the Git course, would have kept his commits. Next time, he runs an unknown change only in a throwaway copy, and with an output folder that he can lose.

Your turn: an old risk in the existing code

Candidate E is not the only place where --output matters. The code from the Python course writes the report wherever --output points. In the throwaway copy, before you run it, predict: what happens with this command?

python -m ticket_cleaner data/tickets.csv --output data/web-form-export.csv

Run it, then run git status --short and head -3 data/web-form-export.csv. Which line of the checklist does it break? What would you propose?

Show the answer

The program overwrites Grace’s export with the JSON report, and it says so calmly: Report: data/web-form-export.csv.

 M data/web-form-export.csv
{
  "status": "open",
  "valid_records": 9,

It breaks “No file is deleted, and no data file is changed” and “Nothing is deleted or overwritten without the user asking”. The user did ask, by mistake. A proposal for a separate task: refuse an output inside data/, with exit code 2 and a clear message. Or refuse to overwrite a file that is not a .json report. This is old code, not an assistant’s change. A review of the risk applies to every line that you own. Put the copy back with git restore ..

Module practice: compare two candidates

The module’s practice task: compare two candidate patches, and reject the one with a subtle defect. Both candidates add the --category option to start, and both pass their own tests. One is the real bounded session of Module 2. The other, candidate C, was constructed for the course: short, tidy and formatted.

Read both in the explorer first. Predict which one fails, and which acceptance criterion it breaks. Then run the hidden checks.

Explore

Two candidates, both green on their own tests

Candidate 1

real: a recorded session (the task brief)

git diff --stat: 5 files, +145 −4
Its own tests: 35 passed in 0.05s
ruff format --check: fail

Show the code in ticket_cleaner/

diff --git a/ticket_cleaner/cli.py b/ticket_cleaner/cli.py
--- a/ticket_cleaner/cli.py
+++ b/ticket_cleaner/cli.py
@@ -28,6 +28,12 @@ def make_parser() -> argparse.ArgumentParser:
         choices=sorted(STATUSES),
         help="which tickets to count (default: open)",
     )
+    parser.add_argument(
+        "--category",
+        action="append",
+        metavar="NAME",
+        help="count only this category; repeat for more (case-insensitive)",
+    )
     parser.add_argument(
         "--output",
         type=Path,
@@ -62,7 +68,7 @@ def main(argv: list[str] | None = None) -> int:
     for record in rejected:
         logger.warning("row %d rejected: %s", record.row, "; ".join(record.problems))
 
-    summary = build_summary(tickets, rejected, args.status)
+    summary = build_summary(tickets, rejected, args.status, categories=args.category)
     write_json(summary, args.output)
     logger.info("wrote %s", args.output)
     print(f"{len(rows)} rows: {len(tickets)} valid, {len(rejected)} rejected.")
diff --git a/ticket_cleaner/report.py b/ticket_cleaner/report.py
--- a/ticket_cleaner/report.py
+++ b/ticket_cleaner/report.py
@@ -21,10 +21,20 @@ def average_priority(tickets: list[Ticket]) -> float | None:
     return round(sum(ticket.priority for ticket in tickets) / len(tickets), 1)
 
 
-def build_summary(tickets: list[Ticket], rejected: list[Rejected], status: str) -> dict:
-    """Return the report as a dictionary that can be saved as JSON."""
+def build_summary(
+    tickets: list[Ticket],
+    rejected: list[Rejected],
+    status: str,
+    categories: list[str] | None = None,
+) -> dict:
+    """Return the report, optionally selecting normalized category names."""
     selected = filter_by_status(tickets, status)
-    return {
+    chosen_categories = None
+    if categories is not None:
+        chosen_categories = sorted({category.strip().lower() for category in categories})
+        category_names = set(chosen_categories)
+        selected = [ticket for ticket in selected if ticket.category in category_names]
+    summary = {
         "status": status,
         "valid_records": len(tickets),
         "selected": len(selected),
@@ -32,3 +42,6 @@ def build_summary(tickets: list[Ticket], rejected: list[Rejected], status: str)
         "average_priority": average_priority(selected),
         "rejected": [{"row": r.row, "problems": r.problems} for r in rejected],
     }
+    if chosen_categories is not None:
+        summary["categories"] = chosen_categories
+    return summary

Candidate 2

constructed for the course

git diff --stat: 3 files, +24 −0
Its own tests: 25 passed in 0.04s
ruff format --check: pass

Show the code in ticket_cleaner/

diff --git a/ticket_cleaner/cli.py b/ticket_cleaner/cli.py
--- a/ticket_cleaner/cli.py
+++ b/ticket_cleaner/cli.py
@@ -28,6 +28,12 @@ def make_parser() -> argparse.ArgumentParser:
         choices=sorted(STATUSES),
         help="which tickets to count (default: open)",
     )
+    parser.add_argument(
+        "--category",
+        action="append",
+        metavar="NAME",
+        help="count only this category; repeat for more",
+    )
     parser.add_argument(
         "--output",
         type=Path,
@@ -59,10 +65,15 @@ def main(argv: list[str] | None = None) -> int:
         return 1
 
     tickets, rejected = split_records(rows)
+    if args.category:
+        wanted = {name.strip().lower() for name in args.category}
+        tickets = [ticket for ticket in tickets if ticket.category in wanted]
     for record in rejected:
         logger.warning("row %d rejected: %s", record.row, "; ".join(record.problems))
 
     summary = build_summary(tickets, rejected, args.status)
+    if args.category:
+        summary["categories"] = sorted(wanted)
     write_json(summary, args.output)
     logger.info("wrote %s", args.output)
     print(f"{len(rows)} rows: {len(tickets)} valid, {len(rejected)} rejected.")

The results of the 8 hidden checks for each candidate.

The 8 hidden acceptance checks
Hidden check	Candidate 1	Candidate 2
--category billing counts only the billing tickets	pass	pass
--category billing --category shipping counts both	pass	pass
--category ' Billing ' (capital letter, spaces) still finds billing	pass	pass
the report lists the chosen categories, sorted, in small letters	pass	pass
without --category, the report is exactly as before	pass	pass
valid_records (9) and rejected (6) still describe the whole file	pass	fail
an unknown category gives 0 selected and exit code 0	pass	pass
--help describes --category	pass	pass

Candidate 1 is real (the recorded session with the task brief); candidate 2 is candidate C, constructed for the course. Results of the 8 hidden acceptance checks, from the course team's runs.

Text description of the explorer

Two candidates for the --category option, both as patches for start. Candidate 1 is real, from the recorded session with the task brief: 5 files, +145 −4; its own tests give 35 passed; ruff format --check fails. Candidate 2 is candidate C, constructed: 3 files, +24 −0; its own tests give 25 passed; ruff format --check passes. Each card can show the code in ticket_cleaner/.

The table of the 8 hidden acceptance checks: candidate 1 passes all 8. Candidate 2 passes 7 and fails one: “valid_records (9) and rejected (6) still describe the whole file”.

Now find the failing behaviour yourself, in the throwaway copy:

Apply candidate C, and run its checks:

git apply ../nextia-learning-labs/C15/candidates/C-filter-before-summary.patch
git diff --stat
python -m pytest
ruff format --check .

 README.md             |  1 +
 tests/test_cli.py     | 12 ++++++++++++
 ticket_cleaner/cli.py | 11 +++++++++++
 3 files changed, 24 insertions(+)
============================== 25 passed in 0.03s ==============================
15 files already formatted

Every check that the author ran is green.

Run criterion 5 of the brief: valid_records and rejected do not change with --category.
Terminal window
```
python -m ticket_cleaner data/tickets.csv --category billing
```
Output
```
15 rows: 3 valid, 6 rejected.
Report: reports/summary.json
```
The report has "valid_records": 3. The same command in your project prints 15 rows: 9 valid, 6 rejected. and gives "valid_records": 9.

Put the copy back, and delete it:

macOS and Linux
Windows

git restore . && git clean -fd
cd ..
rm -rf ticket-cleaner-throwaway

git restore .; git clean -fd
cd ..
Remove-Item -Recurse -Force ticket-cleaner-throwaway

Check the folder name before you press Enter.

Completion check: explain the defect of candidate C in your own words, and point to a behaviour that fails. Then write a test that fails on candidate C and passes on the accepted change.

Show a worked solution

The defect. Candidate C filters in cli.py, before the summary: tickets = [ticket for ticket in tickets if ticket.category in wanted]. Then it gives the filtered list to build_summary(). But build_summary() counts valid_records from the list it receives. So with --category billing, the report says that the file has 3 valid tickets, the billing tickets of every status. The file has 9. The printed line is wrong too: 15 rows: 3 valid, 6 rejected. adds up to 9 rows, not 15.

The failing behaviour. python -m ticket_cleaner data/tickets.csv --category billing gives "valid_records": 3. Criterion 5 expects 9. A spreadsheet that shows “valid tickets” would now change with the team that looks at it.

Why the tests missed it. Candidate C’s only new test checks selected, by_category and categories. It never looks at valid_records. The test checks what the author thought about, not what the brief asks.

A test for the defect, for example in tests/test_cli.py:

def test_category_does_not_change_the_whole_file_counts(tmp_path):
    output = tmp_path / "billing.json"
    assert main(["data/tickets.csv", "--category", "billing", "--output", str(output)]) == 0
    summary = json.loads(output.read_text(encoding="utf-8"))
    assert summary["valid_records"] == 9
    assert len(summary["rejected"]) == 6

The course team ran it on both:

E       assert 3 == 9
============================== 1 failed in 0.03s ===============================

============================== 1 passed in 0.02s ===============================

Your project already has the same check one level down: test_categories_do_not_change_the_whole_file_counts in tests/test_report.py. The decision: reject candidate C, with this command and this test as the evidence.

Module check

You have finished the module when you can show these on your own project:

git log --oneline shows eight commits on feature/category-option. The last two are Add the --category option, after review and Add a checklist for reviewing a generated change.
python -m pytest prints 34 passed, and ruff format --check . prints 21 files already formatted.
python -m ticket_cleaner data/tickets.csv --category billing --category shipping gives "by_category": {"billing": 2, "shipping": 1} and "categories": ["billing", "shipping"].

On macOS and Linux, this command prints nothing, because your project matches the snapshot end-of-m03:

diff -rq . ../nextia-learning-labs/C15/snapshots/end-of-m03 -x .git -x .venv -x reports -x __pycache__ -x .pytest_cache -x .ruff_cache

You can name, for each of candidates A, C, D and E, the reason to reject it and the evidence.

Reset. If your project is broken, follow the reset steps of Context and boundaries with the snapshot end-of-m03 in place of end-of-m01. The last line of python -m pytest is then 34 passed.

Knowledge check

Check your understanding

Name the four parts of a security and data review, with the line of candidate E that each finding points to, or 'no finding'.

Show the answer

Input validation: shutil.rmtree(args.output.parent, ignore_errors=True) uses a path from the command line with no check.
Secrets: logger.debug("settings: %s", vars(settings)) prints the token at the debug level.
File permissions: no line changes permissions. But the program runs with yours, so rmtree() can delete anything that you can, .git included.
Destructive operations: the same rmtree() line deletes a whole folder without asking, and ignore_errors=True hides every problem.

Check your understanding

The Settings class has field(repr=False). Why did candidate E still print the token, and what would a safe debug line log?

Show the answer

repr=False only changes the class’s own description, which print(settings) and "%s" % settings use. vars(settings) returns the object’s fields as a dictionary and does not ask the class, so the token is in it. A safe debug line logs settings itself, or only the fields that are not secret, such as settings.tickets_url. A test can run the program at the debug level with a made-up token and check that the token is not in the output.

Check your understanding

Predict: with candidate E, what does python -m ticket_cleaner data/tickets.csv --clean delete, with no --output? Explain from the code.

Show the answer

The default output is reports/summary.json, so args.output.parent is reports. shutil.rmtree() deletes the whole reports/ folder, with every older report in it, and then write_json() makes the folder again with only the new summary.json. Nothing asks or warns. It is the least damaging case, and it still deletes more than the help text says. Any file that someone saved in reports/ is gone.

Recap

This lesson

A security review follows each value from outside the program to where it is used: input, secrets, file permissions and destructive operations.
Candidate E (constructed) passed all 24 tests. --clean deleted the folder of --output: with summary.json, the project and .git. Its debug line printed the token, because vars() goes around field(repr=False).
Run an unknown change only in a throwaway copy. Ask first before you delete: git clean -n before git clean -fd.

Module 3: review generated work

Review every diff line by line with docs/review-checklist.md: scope, behaviour and contracts, dependencies, security and data, and the checks that you run yourself.
A passing test run is not a review. Candidates A, C, D and E all failed for reasons that their own tests did not show.
Before any pip install: the package exists, it is needed, it is maintained and fits, and its licence fits.
Your project matches end-of-m03: the accepted --category option and the checklist, 34 passed.

Next, in Module 4, you go back to Grace’s bug. You reproduce it first, and you ask the assistant for evidence instead of a quick rewrite.

Next: Reproduce first