From 29e9052bca6b8ddb3115c5fd8b7380998492de00 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Thu, 13 Aug 2026 00:32:23 -0700 Subject: [PATCH 1/2] ADFA-5123: Document docdb SQL-authoring gotchas from ADFA-5088 Add two lessons learned writing SQL migration scripts against documentation.db: a local copy's on-disk schema/content can be stale independent of git history (a stale copy caused a real near-miss in ADFA-5088 - a script validated against it would have silently overwritten curated production content), and the .bail on / guard-table pattern needed for a BEGIN/COMMIT script to actually be atomic against a bad or empty Brotli payload. Co-Authored-By: Claude Sonnet 5 --- docs/documentation-database.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/documentation-database.md b/docs/documentation-database.md index fd72edcc1c..5d378d8de4 100644 --- a/docs/documentation-database.md +++ b/docs/documentation-database.md @@ -9,6 +9,7 @@ This is a **read-only, prebuilt** database — CoGo never creates or migrates it - Installed path: `context.getDatabasePath("documentation.db")` (`Environment.DOC_DB` in `common/.../utils/Environment.java`), i.e. the app's private `databases/` dir. - Bundled as an asset and extracted on install/update by `BundledAssetsInstaller` / `SplitAssetsInstaller`. - **Debug override:** if `/sdcard/Download/documentation.db` exists and is newer than the installed copy, `WebServer` and `ToolTipManager` swap to it at request time (timestamp-compared per request, not just at startup) — a fast way to test a new database on-device without reinstalling. `WebServer`'s debug logging and experiment flags are also file-flag-gated under `/sdcard/Download/` (`CodeOnTheGo.webserver.debug`, `CodeOnTheGo.exp`, `CodeOnTheGo.webserver.cs0`). +- **Don't trust a local copy's on-disk schema or row content as ground truth without checking freshness first.** Any manually downloaded or debug-override copy is independent of git history — a stale one can have a different schema (e.g. missing `UNIQUE(path)` or `templateId`) or be missing rows that already exist in the current, maintained database. A stale copy caused a real near-miss in ADFA-5088: a SQL script validated against it would have silently overwritten curated production tooltip content for several tags. Diff or re-download before authoring SQL against a local copy's state, not just before shipping it. ## Schema @@ -91,6 +92,13 @@ Schema changes and data edits happen **outside this repo**, in `OfflineDocumenta - Tooltip uniqueness is `(categoryId, tag)`; `TooltipButtons.uri` values are validated there against `Content.path` (post `?query`/`#fragment` stripping) before being allowed into the database. - Every edit made through the tool updates `LastChange` for the affected documentation set, which is how `DatabaseVersionResolver`'s debug logging can say what build of the docs is loaded. +### Writing one-off SQL scripts against this database + +Some tickets (e.g. ADFA-5088) ship a one-off `.sql` script under `docs/docdb/` for a `docdb-studio` maintainer to run against the real database, rather than editing it directly through the tool. Two gotchas found writing those scripts: + +- **Keep each `.system` line simple.** The sqlite3 CLI's `.system` dot-command can hit a content-dependent shell-parsing failure when a line chains multiple operators (`;`, `&&`, `||`, parentheses) — it reproduces for some input strings and not others, so it won't necessarily show up in a quick test. Stick to one plain `command | pipe > file` per `.system` line. +- **`.bail on` is required for `BEGIN`/`COMMIT` to actually mean atomic.** Without it, a mid-script SQL error prints to stderr but the script *keeps going* — including reaching the final `COMMIT`, which then persists whatever succeeded before the error (verified empirically, not just documented behavior). `.bail` also can't see `.system` shell failures directly, so a failed or empty Brotli payload (which leaves its target file missing or zero-length) needs its own check: insert its `READFILE()` into a throwaway `CREATE TEMP TABLE` guarded by `NOT NULL CHECK (length(content) > 0)` immediately before the real `Content` insert, turning that failure into a real SQL error `.bail` will catch. See `docs/docdb/ADFA-5088-preference-tooltips.sql` for the working pattern. + ## Known rough edges - A Tier 3 link that points off-device is a bug in the *content*, not the code — the web server and webview will happily follow it. If you see one while working in this area, it's a data problem to report upstream, not a `WebServer` bug to fix here. From a2cc1c001cbcb15bc637dc3581b10cc6374df9ed Mon Sep 17 00:00:00 2001 From: David Schachter Date: Thu, 13 Aug 2026 00:55:43 -0700 Subject: [PATCH 2/2] ADFA-5123: Document the owner-only-workdir temp-file pattern Insecure /tmp filenames (CWE-377) were found and fixed in the ADFA-5088 docdb scripts after this doc's first pass - a fixed, guessable name under world-writable /tmp lets another local user pre-plant a symlink or race the write/read pair. Co-Authored-By: Claude Sonnet 5 --- docs/documentation-database.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/documentation-database.md b/docs/documentation-database.md index 5d378d8de4..566703ad1b 100644 --- a/docs/documentation-database.md +++ b/docs/documentation-database.md @@ -94,10 +94,11 @@ Schema changes and data edits happen **outside this repo**, in `OfflineDocumenta ### Writing one-off SQL scripts against this database -Some tickets (e.g. ADFA-5088) ship a one-off `.sql` script under `docs/docdb/` for a `docdb-studio` maintainer to run against the real database, rather than editing it directly through the tool. Two gotchas found writing those scripts: +Some tickets (e.g. ADFA-5088) ship a one-off `.sql` script under `docs/docdb/` for a `docdb-studio` maintainer to run against the real database, rather than editing it directly through the tool. Gotchas found writing those scripts: - **Keep each `.system` line simple.** The sqlite3 CLI's `.system` dot-command can hit a content-dependent shell-parsing failure when a line chains multiple operators (`;`, `&&`, `||`, parentheses) — it reproduces for some input strings and not others, so it won't necessarily show up in a quick test. Stick to one plain `command | pipe > file` per `.system` line. - **`.bail on` is required for `BEGIN`/`COMMIT` to actually mean atomic.** Without it, a mid-script SQL error prints to stderr but the script *keeps going* — including reaching the final `COMMIT`, which then persists whatever succeeded before the error (verified empirically, not just documented behavior). `.bail` also can't see `.system` shell failures directly, so a failed or empty Brotli payload (which leaves its target file missing or zero-length) needs its own check: insert its `READFILE()` into a throwaway `CREATE TEMP TABLE` guarded by `NOT NULL CHECK (length(content) > 0)` immediately before the real `Content` insert, turning that failure into a real SQL error `.bail` will catch. See `docs/docdb/ADFA-5088-preference-tooltips.sql` for the working pattern. +- **Don't write Brotli payloads to bare `/tmp/*.br` filenames.** A fixed, guessable name directly under world-writable `/tmp` lets another local user pre-plant a symlink or race the write/read pair between the `.system echo | brotli` write and the `READFILE()` read (CWE-377). Create an owner-only working directory instead — `rm -rf` it, then `mkdir -m 700` it (the mode is set atomically at creation, with no window where it's briefly world-accessible) — write every payload under that directory, and remove it again before `COMMIT`. See the same script for the working pattern. ## Known rough edges