Skip to content

feat: add due dates to todos#91

Open
demo7-labworksdev wants to merge 1 commit intomainfrom
feat/due-dates
Open

feat: add due dates to todos#91
demo7-labworksdev wants to merge 1 commit intomainfrom
feat/due-dates

Conversation

@demo7-labworksdev
Copy link
Copy Markdown

Summary

  • Adds due_date support to todos with date picker, display, overdue highlighting (red), and sort by due date
  • Adds HTTP Basic Auth protecting all routes, with rate limiting, constant-time credential comparison, and per-todo ownership checks
  • Auto-migrates existing databases (adds due_date and created_by columns)

Notes

  • Set AUTH_PASSWORD env var before running — app refuses to start without it
  • AUTH_USERNAME defaults to admin; override via env var
  • Legacy todos (created before auth) are assigned to AUTH_USERNAME on startup
  • Auth was added in response to zenable conformance requirements; tracked separately in Add authorization checks to mutating routes #85

Closes #26

🤖 Generated with Claude Code

- due_date column added to schema with auto-migration for existing DBs
- date picker on add form, due date displayed per todo
- overdue items highlighted red, sort by due date or created date
- inline edit form per row (title + due date)
- HTTP Basic Auth on all routes (AUTH_PASSWORD env var required at startup)
- rate limiting, constant-time credential comparison, failed attempt logging
- per-todo ownership via created_by column; users see only their own todos
- legacy todos migrated to AUTH_USERNAME on startup

Closes #26

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@ai-coding-guardrails ai-coding-guardrails bot left a comment

Choose a reason for hiding this comment

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

I've got 2 comments for you to consider

Did you know Zenable also supports GitLab SaaS and self-managed?

Comment on lines 44 to 45
<a href="/toggle/{{ todo.id }}">[toggle]</a>
<a href="/delete/{{ todo.id }}">[delete]</a>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The /toggle/{{ todo.id }} and /delete/{{ todo.id }} actions use GET requests (plain anchor links). This is a CSRF vulnerability — any page can trigger state-changing operations by embedding links or <img src="..."> tags that the authenticated user's browser will follow. These should use POST forms (with a CSRF token if the framework supports it), not GET links.

Why did I show this?

Category: security
Comment Quality: high

Influenced by requirements:

Tools used:

  1. list_changed_files, {'pattern': {'type': 'string', 'value': '**/*.py'}}
  2. list_changed_files, {'pattern': '**/*.py'}

Comment on lines +118 to +119
sort = request.args.get("sort", "created_at")
order_by = "due_date ASC, created_at DESC" if sort == "due_date" else "created_at DESC"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The sort parameter is taken from user input and used directly in an f-string SQL query without validation. Although only two values are used, if the logic ever changes or is copy-pasted, this is a SQL injection vector. The allowed values should be explicitly validated:

Suggested change
sort = request.args.get("sort", "created_at")
order_by = "due_date ASC, created_at DESC" if sort == "due_date" else "created_at DESC"
sort = request.args.get("sort", "created_at")
if sort not in ("due_date", "created_at"):
sort = "created_at"
order_by = "due_date ASC, created_at DESC" if sort == "due_date" else "created_at DESC"
Why did I show this?

Category: security
Comment Quality: high

Based on general best practices

Tools used:

  1. get_file_lines, {'file_path': 'templates/index.html', 'start_line': '1', 'end_line': '200'}
  2. list_changed_files, {'pattern': '**/*.html'}
  3. get_file_lines, {'file_path': 'src/templates/index.html', 'start_line': '1', 'end_line': '200'}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Due Dates

1 participant