Feat/db constraints secure cookies - #1
Merged
Conversation
Handlers were constructor closures over *sql.DB, and main.go listed every route by hand. Turn them into methods on a Handlers struct holding the shared dependencies, and let each resource file own its own sub-router. Router() is now the only place that knows URL prefixes, so a resource file never repeats its own mount path. Two routing details worth calling out: - Bind /static to GET and HEAD explicitly. http.FileServer ignores the request method, so a single catch-all Handle answered POST, DELETE and TRACE with the file body. - Canonicalise paths with CleanPath and StripSlashes, so "/clients//" and "/clients/1/edit/" reach their handler instead of the catch-all. Route parity with the previous net/http mux was verified route by route: same paths, same status codes, same Location headers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two data-integrity problems and one transport one, plus the migration machinery the first of them needed. Foreign keys were never actually on. The pragma was issued as a statement after connecting, which configures only the connection that happened to serve it, and database/sql pools connections freely; every other one ran with enforcement off. Deleting a client therefore left its tasks behind, pointing at a row that no longer existed, where they rendered with a blank name and still counted toward every hours total. The pragmas move into the DSN, so no connection can miss them. Turning enforcement on changes what a delete means. Tasks are the records this app exists to keep, so their foreign keys stay RESTRICT and the refusal surfaces as db.ErrInUse, which the handlers report as a 409 rather than a 500 full of SQL. The client/task-type join table is bookkeeping instead, so it cascades; that needed a table rebuild, since SQLite can't ALTER a constraint. Periods sit between the two: they only group hours, so deleting one detaches its tasks in a transaction rather than refusing. Expressing that rebuild meant replacing the old startup schema block and its list of ALTERs, which could only ever add columns. Migrations are now embedded .sql files applied in order and recorded in schema_migrations. Migration 0001 is the schema as it already stands, written idempotently so existing databases are recognised as migrated instead of rebuilt. Session cookies were never marked Secure, so the token travelled in cleartext over any plain-HTTP hop. It defaults on now, since the app is normally reached through a TLS-terminating proxy, and a deployment that never considered it should land on the safe setting; plain-HTTP setups opt out with TRACKER_SECURE_COOKIES=false, which compose.yml does. Login and logout share one helper so their attributes can't drift apart. Environment variables gain a TRACKER_ prefix. This is a breaking change for existing deployments: DB_PATH becomes TRACKER_DB_PATH and JWT_SECRET becomes TRACKER_JWT_SECRET. The bundled Dockerfile and compose.yml are updated; anything overriding them needs the same rename. WAL stays opt-in behind TRACKER_SQLITE_WAL, since it leaves -wal and -shm files next to the database. Capping the pool at one connection, which SQLite's single-writer rule wants anyway, removes the contention that would otherwise be the reason to reach for it. Verified against a copy of a live database: 927 tasks and their 1063.5 hours preserved, all 52 assignments carried across the rebuild, both cascade clauses in place, no foreign key violations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The port was the one setting hardcoded in Config, so the only way to move the server was to remap it outside the process. It reads TRACKER_PORT now, alongside the other settings. An out-of-range or unparseable value falls back to 8080 with a warning rather than failing to start, matching how the boolean settings already behave. Zero is rejected with the rest: it would ask the kernel for an arbitrary free port, which is never what a server someone has to reach was meant to do. Config is read once into a package-level variable instead of per command. Load logs as it falls back, and every command builds its flags at startup, so a single malformed value used to print its warning five times. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.