Phase 1 — hardening (deferred from 1a review) #7

New issue

Closed

opened 2026-10-08 07:30:19 +00:00 by agent-pi · 1 comment

agent-pi commented

2026-10-08 07:30:19 +00:00

Collaborator

Hardening items identified during the Phase 1a final review that were deliberately deferred, to be addressed as part of Phase 1b where they touch the same code.

Items: login throttling; session cookie Secure flag (for TLS deployment); logout CSRF shape (GET logout); worker crash-recovery for jobs left 'running'; complete_job/fail_job status='running' guards; index on jobs.status; POLL_INTERVAL_SECONDS as a Setting; run_worker_cycle bounded batching before checkpoint; transactional migration scripts (matters from migration 2 onward); and the identified test coverage gaps (empty/unicode passwords, malformed cookie halves, get_db close-on-exception, server.main()/worker.main()).

Hardening items identified during the Phase 1a final review that were deliberately deferred, to be addressed as part of Phase 1b where they touch the same code. Items: login throttling; session cookie Secure flag (for TLS deployment); logout CSRF shape (GET logout); worker crash-recovery for jobs left 'running'; complete_job/fail_job status='running' guards; index on jobs.status; POLL_INTERVAL_SECONDS as a Setting; run_worker_cycle bounded batching before checkpoint; transactional migration scripts (matters from migration 2 onward); and the identified test coverage gaps (empty/unicode passwords, malformed cookie halves, get_db close-on-exception, server.main()/worker.main()).

agent-pi added this to the Phase 1 — Core milestone

2026-10-08 07:30:19 +00:00

agent-pi referenced this issue

2026-10-08 07:41:25 +00:00

Phase 1b — Jellyfin Connect & Sync #3

agent-pi referenced this issue

2026-10-08 09:45:09 +00:00

Phase 1b: Jellyfin connect and movie sync #8

agent-pi commented

2026-10-15 06:46:59 +00:00

Author

Collaborator

Audit against the current code (post 1b/1c/1d), so we only implement what actually remains

Already addressed during later phases:

✅ transactional migration scripts (migration 2 + transactional migrate)
✅ index on jobs.status (owned by migration 2)
✅ worker crash-recovery: recover_stale_jobs wired into run_worker startup
✅ complete_job/fail_job status = 'running' guards (verified in db/jobs.py)
✅ poll_interval_seconds is a Settings field (XIQUE_POLL_INTERVAL_SECONDS)

Still missing:

❌ login throttling — no attempt tracking in web/routes/auth.py
❌ session cookie Secure flag — set_cookie calls (session + CSRF) carry only httponly/samesite
❌ logout is GET /logout with no CSRF — state change on GET
❌ test gaps: empty/unicode passwords, malformed cookie halves, get_db close-on-exception, server.main()/worker.main() entry points

Plan of attack (one PR, stacked after the #11 logging PR so failure logs exist while testing)

RED: tests for all four gaps.
Cookie flags: new Settings.session_cookie_secure: bool = False (XIQUE_COOKIE_SECURE); applied to session + CSRF cookies; documented for TLS deployments.
Logout: POST /logout with CSRF token, nav form in base.html; GET returns 405.
Login throttling: sqlite-backed failed-attempt window keyed on (username, source ip) — e.g. 5 failures / 15 min locks the pair out for 15 min; generic error so it does not leak which factor failed; cleanup of expired rows.
Test gaps: the four named coverages.

Key files: src/xique/config.py, src/xique/web/routes/auth.py, src/xique/web/deps.py, src/xique/db/users.py, templates/base.html, README.md, tests/.

**Audit against the current code (post 1b/1c/1d), so we only implement what actually remains** Already addressed during later phases: - ✅ transactional migration scripts (migration 2 + transactional `migrate`) - ✅ index on `jobs.status` (owned by migration 2) - ✅ worker crash-recovery: `recover_stale_jobs` wired into `run_worker` startup - ✅ `complete_job`/`fail_job` `status = 'running'` guards (verified in `db/jobs.py`) - ✅ `poll_interval_seconds` is a `Settings` field (`XIQUE_POLL_INTERVAL_SECONDS`) Still missing: - ❌ login throttling — no attempt tracking in `web/routes/auth.py` - ❌ session cookie `Secure` flag — `set_cookie` calls (session + CSRF) carry only `httponly`/`samesite` - ❌ logout is `GET /logout` with no CSRF — state change on GET - ❌ test gaps: empty/unicode passwords, malformed cookie halves, `get_db` close-on-exception, `server.main()`/`worker.main()` entry points **Plan of attack** (one PR, stacked after the #11 logging PR so failure logs exist while testing) 1. RED: tests for all four gaps. 2. Cookie flags: new `Settings.session_cookie_secure: bool = False` (`XIQUE_COOKIE_SECURE`); applied to session + CSRF cookies; documented for TLS deployments. 3. Logout: `POST /logout` with CSRF token, nav form in `base.html`; GET returns 405. 4. Login throttling: sqlite-backed failed-attempt window keyed on (username, source ip) — e.g. 5 failures / 15 min locks the pair out for 15 min; generic error so it does not leak which factor failed; cleanup of expired rows. 5. Test gaps: the four named coverages. Key files: `src/xique/config.py`, `src/xique/web/routes/auth.py`, `src/xique/web/deps.py`, `src/xique/db/users.py`, `templates/base.html`, `README.md`, `tests/`.

agent-pi referenced this issue from a pull request that will close it,

2026-10-15 07:32:08 +00:00

Login throttling, Secure cookie flag, POST-only logout, and coverage gaps #14

agent-pi closed this issue

2026-10-16 08:56:38 +00:00