feat(todo): add priority levels to todos

[demo6-labworksdev]

• Add priority field (high/medium/low) with medium as default
• Color-code rows red/yellow/green by priority
• Filter todos by priority via ?priority= query param
• Sort todos by priority via ?sort=priority query param
• DB migration adds priority column to existing tables
• 15 unit tests with 100% coverage

Closes #27

Contributor Comments

Pull Request Checklist

Thank you for submitting a contribution to todo-app!

Please address the following items:

• If you are adding a dependency, please explain how it was chosen.
• If manual testing is needed in order to validate the changes, provide a testing plan and the expected results.
• Validate that documentation is accurate and aligned to any project updates or additions.

[ai-coding-guardrails]

I've got 2 comments for you to consider

Reviewed with 🤟 by Zenable

[ai-coding-guardrails]

The todo.priority value is rendered directly into HTML class names and badge classes without escaping (e.g., class="priority-{{ todo.priority }}"). While the backend validates priority on insert, if a row with an unexpected priority value exists in the DB (e.g., from a direct DB write or a pre-migration row), it would be injected unescaped into the HTML attribute, creating a potential XSS vector. Jinja2 auto-escapes content inside {{ }} in HTML context, but attribute values like class="priority-{{ todo.priority }}" are also auto-escaped by Jinja2 when using .html templates — so this is safe as long as Jinja2 autoescape is enabled (which it is by default for .html files in Flask). However, the sort_by and filter_priority values from query params are rendered unescaped into href attributes on lines 36–43, which could allow injection of arbitrary URL content (e.g., javascript: URIs or breaking out of the attribute) since Jinja2 does NOT sanitize URL schemes in href attributes.

For example, a user could craft ?sort=javascript:alert(1) and the link on line 42 would render as href="/?priority=...&sort=javascript:alert(1)". While clicking would just navigate to a bad URL (not execute JS in most browsers due to the /? prefix), filter_priority on lines 42–43 has no validation on the server side for the sort links, and sort_by on lines 36–39 is passed through directly. Consider validating sort_by against an allowlist (e.g., created_at, priority) before passing it to the template, similar to how filter_priority is validated for DB queries.

Why did I show this?

Category: security
Comment Quality: high

Based on general best practices

Tools used:

1. list_changed_files, {'pattern': {'type': 'string', 'value': '**/*.py'}}
2. list_changed_files, {'pattern': '**/*.py'}
3. list_changed_files, {'pattern': '**/*.py'}
4. get_file_lines, {'file_path': 'src/todo_app/app.py', 'start_line': 1, 'end_line': 200}
5. get_file_lines, {'file_path': 'tests/test_app.py', 'start_line': 1, 'end_line': 200}

[ai-coding-guardrails]

The client fixture mutates the global app_module.DATABASE but only restores it in the teardown code after yield. If an exception occurs during app_module.init_db() (line 19), the teardown code on line 24 is never reached because it is outside the with block and not in a finally clause. This means app_module.DATABASE will be left pointing to the temp path, potentially corrupting other tests.

Use a try/finally or move the restore inside the with block:

Suggested change

	original_db = app_module.DATABASE
	app_module.DATABASE = db_path
	app_module.init_db()
	with app.test_client() as c:
	yield c
	app_module.DATABASE = original_db
	db_path = str(tmp_path / "test_todos.db")
	app.config["TESTING"] = True
	original_db = app_module.DATABASE
	app_module.DATABASE = db_path
	try:
	app_module.init_db()
	with app.test_client() as c:
	yield c
	finally:
	app_module.DATABASE = original_db

Why did I show this?

Category: bug
Comment Quality: high

Based on general best practices

Tools used:

1. get_file_lines, {'file_path': 'todo_app/app.py', 'start_line': 1, 'end_line': 200}
2. list_changed_files, {'pattern': '**/*.py'}