27b2611a206cf19d3389f13ae8bc119695e5a3c0
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 hermes
|
bbd9fe17e5 |
fix(analysis): exclude suspect runs from every average, not just attenuation
Review finding, and it was right. The PR claimed that runs with impossible readings were "excluded from the averages". That was only true of the attenuation average. Run 12's bad final gravity was still poisoning three other statistics: - **Brew time** - its 56 days were folded into the average, making a typical brew look like 13.7 days when it is 10.9. - **Time to 1.010** - its 56 days did the same to that figure (8.4d). - **The history page's ABV** - an 18.64% ABV, which made run 12 the "strongest" beer in the history and pulled the average up. - **The fermentation curve** - its curve plunges from 1.042 to 0.900, which dragged the median line down and stretched the min-max envelope to cover a gravity below water. One bad reading was reaching four different statistics. What changed ------------ - The suspect runs are computed once and excluded from every aggregate: the attenuation, brew-time and time-to-1.010 averages, the fastest and slowest brews, the history page's ABV average, and its strongest and lightest. - `/api/runs` now carries a `suspect` flag, and the charts skip the flagged runs - the attenuation chart and the fermentation curve. - The suspect run stays in the table, with a "not counted" chip and a tinted row, so it can be corrected. It is excluded from the figures, not hidden. The card header says "16 brews (15 counted)" and the prose says how many brews the figures are based on. Tests ----- 133 (up from 127). New coverage: the suspect run is excluded from the brew time and time-to-1.010 averages, it is still listed for review, it is flagged in the API, it is not the strongest brew in the history, and the charts filter it out. The `logged_in` fixture moved to conftest.py, where both test modules can use it. |
||
|
hermes
|
1828dcb81d |
fix(security): address the independent review's blocking findings
The independent reviewer returned "SIGNIFICANT CHANGES NEEDED" on the security
review, with the code review itself approved. This addresses the three
blocking items plus the legitimate medium-severity ones.
Blocking items
--------------
1. Weak password hashing (SHA-256 -> scrypt)
The custom salted SHA-256 helper is replaced with
`werkzeug.security.generate_password_hash` / `check_password_hash`, which
use scrypt by default. The hash carries its own salt and is deliberately
slow to compute, so it is not attackable with a GPU.
The legacy plaintext accounts still work, but a successful legacy login now
**rewrites the row in place** with a proper hash. The plaintext credential
disappears from the database the first time the account is used.
Verified against the live database: the `beer` account's plaintext
`beer_pass` was replaced with a scrypt hash on first login, and login with
the hash still succeeds.
2. Plaintext password persistence
Fixed by the migrate-on-login above. There is no longer a code path that
leaves a password in plaintext.
3. Hardcoded secret defaults
`SECRET_KEY` no longer has a default. The app raises `ImproperlyConfigured`
and refuses to start when it is missing or still set to a placeholder
value. A known session-signing key would let anyone forge a login cookie,
so a misconfigured deployment must fail loudly.
The Dockerfile no longer ships a placeholder either — the variable is simply
not set, so a deploy that forgets it fails immediately and visibly.
Note: Werkzeug 3 removed `ImproperlyConfigured` and Flask has never
exported one, so the app defines its own as a `RuntimeError` subclass.
Medium items
------------
4. NoSQL injection risk
Every value that reaches a Mongo query is now explicitly coerced first:
run ids and dates go through `_as_id` / `_as_date`, which turn anything
that is not a plain string or number into a string (or an empty string).
Without this, a caller passing `{"$ne": ""}` would have been interpreted
by Mongo as a query operator rather than as a value.
Two tests cover this: one drives the login path with an operator payload,
one checks the coercion helper directly.
5. Dependency pinning
`requirements.txt` now pins exact versions of the runtime dependencies, and
`requirements-dev.txt` pins the test dependencies. The image is rebuilt on
every merge, so an unpinned dependency could change behaviour underneath a
deploy with no commit to show for it.
6. Template logic
The `sg / 1000` arithmetic has been moved out of the Jinja templates and
into the domain layer — `run_summary()` now provides `og_decimal_str`,
`fg_decimal_str` and `min_sg_decimal`, pre-formatted. No template does
arithmetic on the data any more.
7. Health check depth
`/healthz` stays a shallow **liveness** probe that never touches Mongo — a
database outage must not make the kubelet restart a healthy process. A new
`/readyz` **readiness** probe pings Mongo and returns 503 when it is
unreachable, so a pod with no database is pulled out of the Service
endpoints instead of serving errors. The RUNBOOK documents wiring both.
Low items
---------
8. Mongo aggregation pipeline for `runs()`
Not changed. The dataset is 47 documents across 18 runs and will stay in
that range for years — the app exists to record a handful of readings per
brew. Moving the grouping into an aggregation pipeline would add real
complexity for no measurable gain, and the grouping in Python is what makes
the run aggregation straightforward to test. Noted for whenever the data
volume makes it matter.
9. Kubernetes Secret for the credentials
Not changed here, and deliberately so. The live Deployment carries
`MONGO_PASS` as a plain environment value. Replacing it means replacing
hand-provisioned live values, which is an operator decision. The RUNBOOK
now documents the Secret manifest to use when that is done, and includes a
ready-to-paste probe configuration.
10. Rate limiting on `/login`, Flask-Talisman, password complexity
Not added. This is a single-user home-brew tracker on a LAN, reachable
through a NodePort. Rate limiting and security-header middleware are
production-grade hardening that adds dependencies and configuration
surface without addressing a realistic threat here — the credentials are
`beer`/`beer_pass` on a service only the operator and their own scanning
can reach. The one change that materially matters — the session-signing
key — is now enforced.
11. Least-privilege Mongo user
Not changed. The app connects as `root` today, matching the existing
deployment. Creating a scoped user is a cluster-side operation and
belongs with the credential work in item 9.
Tests
-----
73 tests (up from 64). New coverage: legacy password migration, hash
verification, empty-credential rejection, the NoSQL operator payload, the
secret-key guards, and the readiness probe in both states.
|
||
|
hermes
|
24cdd452c5 |
feat(ui): rebuild the interface with a proper design system
The old UI was two pages of raw pandas tables with no navigation, no styling and no way to see a run's comments. This replaces it with a small, testable application and a beer-themed design. What changed ------------ Restructure - The domain logic now lives in `brewing.py` as pure functions over plain data, with no Flask or Mongo dependency. The gravity maths, the ABV formula and the run-aggregation rules are unit-testable on their own. - `app_factory.py` replaces the module-level app, and the routes move into blueprints under `views/`. A request-scoped Mongo repository is created per request and closed at teardown. - `mongo/repository.py` replaces `build_db.py` / `query_db.py` / `get_conn.py`. Reading a document no longer requires a second query to fetch the fields. Interface - A dark, beer-themed design system: warm amber `#be7f39` on near-black, a condensed display face (Anton) for headings and Inter for body text. Both fonts and the chart library are vendored into `static/`, so the container needs no outbound network at runtime — the old templates pulled Bootstrap from a CDN. - A real navigation bar, a login page, a brew-process page with a proper form, and a run-detail page listing every reading with its comment. - The run summary tables now show the beer name and the latest comment, which the old pandas tables did not. - Native form controls (checkbox, number spinners, date icon, scrollbars) are restyled for the dark theme. Correctness - The alcohol calculation now uses the standard homebrew formula, `(OG - FG) x 131.25`. The old code used `(max - min) / 7.36 + 0.5`, which is not a recognised brewing formula and overstated every result. - Gravity points and decimal readings now convert correctly in both directions (`1039` <-> `1.039`). - A run's display name resolves to the most-used label across its readings, because `beer_type` is stored per-reading and drifts across a run. - Re-submitting a reading for the same run and day updates it in place rather than creating a duplicate. Security / production - The container ran the Werkzeug debugger with `FLASK_DEBUG=1`, exposing an interactive console on any unhandled error. It now runs waitress with debugging off. - `SECRET_KEY` comes from the environment. - The app secret is no longer a hard-coded string in source. Housekeeping - Removes the bokeh/duckdb/pandas dependency chain and the static file it generated at import time; removes the dead `add_user.py` and `test.py`. - Splits `requirements.txt` (runtime) from `requirements-dev.txt` (tests). - Adds a pytest suite (64 tests) and a `test` job to the CI workflow so the image is only built when the tests pass. - Adds a README describing the data model and a RUNBOOK for the cluster side. Data compatibility ------------------ The existing `beer_db.beer_data` documents are read as they are and are not migrated or rewritten. No collection is dropped, and no document is modified by this change other than through the explicit "record a reading" form, which keeps the original `(date, beer_run_id)` uniqueness rule. |