docs: correct 35 verified inaccuracies against privacytracker main - #3
Merged
Conversation
Every claim here was checked against the source, then re-checked by three independent review passes. Highlights rather than the full list: Commands that did not work - All three audit_log SQL snippets failed with "no such column: at". The real schema is id (UUID) / created_at / action / actor_ip / user_agent / detail / success. Rewritten, and the undocumented success column documented. - The five audit event codes named in hardening (admin_token_failed and friends) exist nowhere in the source. Real values are dot-namespaced (admin_token.login.invalid). An operator watching for the documented string would have missed a brute-force run entirely. - The "Settings → Diagnostics → Audit log" UI does not exist, on three pages. audit_log is write-only — one INSERT, no SELECTs, no route, no panel. Security properties stated wrongly - "privacytracker has no rate limit of its own", in four places, with advice to add a proxy because of it. A per-key sliding-window limiter runs on 56 of 110 routes. Now documented with the real per-route limits, and the proxy advice reframed around volumetric DoS, which is still true. - Missing admin token returns 401, not the documented 403. The 403 is the inverse case — token absent from config. - CSRF failures return "Cross-origin mutation rejected", not origin_mismatch, which appears nowhere in the tree. - "Read-only endpoints have neither layer" — seven read prefixes require the token when network-exposed, and two of them require it on loopback as soon as the token is configured. - audit_log was described as append-only with no route able to delete it. Two admin-token routes clear the whole table, so the actor you are investigating after a token leak can erase their own trail. Said plainly now, with the off-host replication advice that follows from it. Product model - Focus goals are monitor/cleanup, not understand/declutter — renamed in the source, with the rename comments still in place. Also the migration is six steps, not five, and flag.focus.workflow (the real audit-bundle gate) was undocumented. - AI lens keys, verdict values, snapshot settings and the restore parameter were all wrong; the documented ?confirm=RESTORE is inert while the parameter that matters, allowUntrusted, was missing. - The Tauri page described a sidecar supervisor that does not exist. The real mechanism is inverted: the sidecar watches Tauri's PID and self-exits. Not addressed: scripts/sync-changelog.mjs has been failing on every scheduled run since the initial import, which is why changelog.mdx has never updated. CONTRIBUTING forbids hand-editing it, so that needs a fix in the workflow.
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
2 participants
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.
Every claim in this diff was checked against the product source, then put through three independent review passes. The passes mattered — see below.
How this was produced
The recurring failure was replacing an over-broad claim with a differently over-broad one — a header only one guard sets described as a property of the whole limiter, a condition silently dropped. Worth knowing when reviewing: the risk in this diff is not the facts, which are cited, but generalisations around them.
node scripts/check-docs.mjspasses.Reviewing this
The commit message lists the substantive corrections. If you only read part of the diff, read the security sections — several properties were documented more strongly than the code supports, which is the failure mode that actually costs someone something.
The three that would have cost you most
Every
audit_logSQL snippet failed. Three copy-paste queries againstat/event/ip; the real columns arecreated_at/action/actor_ip. Anyone investigating an incident gotno such column: at.The docs said the app has no rate limiter. Four places, with advice to put a proxy in front because it has none. A per-key sliding-window limiter runs on 56 of 110 routes. The proxy advice is still right for volumetric DoS — the reason given was wrong.
audit_logwas called append-only with no route able to delete it.POST /api/admin/start-overtruncates it andPOST /api/backup/restorereplaces it — both admin-token routes. So the actor you are investigating after a token leak can erase their own trail. That is now stated, along with the off-host replication that follows from it.One thing I could not fix here
scripts/sync-changelog.mjshas been exiting 1 on every scheduled run since the initial import, which is whychangelog.mdxhas never updated. CONTRIBUTING forbids hand-editing that file, so this needs a fix in the sync workflow rather than in the docs.