From 2fca018addfbbf33fa5d38a7551732224c723632 Mon Sep 17 00:00:00 2001 From: John Cardinal Date: Fri, 23 Oct 2026 19:58:31 -0700 Subject: [PATCH] Security review: F5, F1 and F18 also fixed in 8.3.0; findings gate the release John's decision: 8.3.0 is not released until every finding is fixed or explicitly accepted. The review therefore runs before the release, not after. Co-Authored-By: Claude Opus 5.5 --- devdocs/modernization/NET10-PORT.md | 6 +++--- devdocs/modernization/SECURITY-REVIEW.md | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/devdocs/modernization/NET10-PORT.md b/devdocs/modernization/NET10-PORT.md index 2574c9ed..2f0adbd8 100644 --- a/devdocs/modernization/NET10-PORT.md +++ b/devdocs/modernization/NET10-PORT.md @@ -12,7 +12,7 @@ under "Deferred", not into the branch. | Branch | `net10-port` in `raven` (and `raven-client` and `raven-test-integration` and `raven-launcher`) | | Baseline tag | `net8-baseline`, on the last commit before the port | | Release version | 8.3.0 | -| Release timing | Tentatively within two weeks of 2026-10-23, after John's move of Sockeye and the ayanova.com business server to a new droplet (weekend of 2026-10-24) has settled. The case database's "next release" cases (including the SuperUser delete bug) must be done first. After 8.3.0: a security review of the whole codebase and an MCP interface for AyaNova 8 | +| Release timing | Tentatively within two weeks of 2026-10-23, after John's move of Sockeye and the ayanova.com business server to a new droplet (weekend of 2026-10-24) has settled. The case database's "next release" cases (including the SuperUser delete bug) must be done first. The security review of the whole codebase (`SECURITY-REVIEW.md`) runs before 8.3.0 and its findings gate the release (John, 2026-10-23); after 8.3.0: an MCP interface for AyaNova 8 | ## Where this plan comes from @@ -188,10 +188,10 @@ Found during the port, out of scope. One line each. - **Standalone install needs the Visual C++ runtime (pre-existing, same in 8.2.4; John, 2026-10-23: own case, fixed with the "next release" cases before 8.3.0):** the bundled PostgreSQL (`local-postgres\bin\pg_ctl.exe`, `postgres.exe`) needs `VCRUNTIME140.dll`, which neither the installer nor `local-postgres` provides. A Windows without it (a new PC, Windows Sandbox) shows "pg_ctl.exe - System Error: VCRUNTIME140.dll was not found" and AyaNova never starts. Confirmed fix: copying `vcruntime140.dll`, `vcruntime140_1.dll` and `msvcp140.dll` (x64, 14.44) into `local-postgres\bin` made PostgreSQL start. Ship those three in `dist\win-x64\postgres-standalone\bin` (app-local, allowed by Microsoft's redistribution terms) rather than running `vc_redist.x64.exe` (it crashed Windows Sandbox mid-install) - Standalone `config.json` (launcher template) has no `AYANOVA_JWT_SECRET`, so every boot logs a warning and generates a random one (logins are lost on restart). Pre-existing; noted from the Sandbox test logs. **John, 2026-10-23: own case, fixed with the "next release" cases before 8.3.0** - Restore manual (`ops-restore.md`) covers only the default `postgres` setup. A customer running AyaNova under a dedicated, non-superuser role who restores as `postgres` without `--role` ends up with tables owned by `postgres` and a failed integrity check at boot (found in the section 4 role test). Add a short note: `pg_restore --no-owner --no-privileges --role=`. Pre-existing; after the release -- **For the security review after 8.3.0:** the LAN/IIS (`dist\assets\lan-install-config.json`) and Linux server (`dist\assets\linux-server\config.json`) templates ship `AYANOVA_JWT_SECRET` set to a placeholder that is printed in the manual. The manual tells users to change it (optional on Linux), but an install that keeps it signs logins with a publicly known secret. Consider generating a random secret at install time or refusing to boot with the placeholder. Pre-existing +- **Security review F1, fixed in 8.3.0:** the LAN/IIS (`dist\assets\lan-install-config.json`) and Linux server (`dist\assets\linux-server\config.json`) templates ship `AYANOVA_JWT_SECRET` set to a placeholder that is printed in the manual. The manual tells users to change it (optional on Linux), but an install that keeps it signs logins with a publicly known secret. Consider generating a random secret at install time or refusing to boot with the placeholder. Pre-existing - Standalone and LAN installers share the same Inno Setup `AppId` (`{B2BB79DB-0690-497C-B92B-955F7B468429}`) although they are different products (`AyaNova` vs `AyaNova server`): installing one over the other would be treated as the same application. Pre-existing; not tested. Both also install to the same folder (`{autopf}\ayanova`) and use port 7575, so the two flavours could never sit side by side on one machine anyway; the shared `AppId` only changes what happens when one flavour is installed **over** the other (e.g. an evaluator moving from standalone to LAN on the same PC): Setup treats it as an upgrade of the same program. Changing either `AppId` now would make that flavour's next installer miss existing installs (second Add/Remove entry over the same folder). Standalone is thought to be used almost only for evaluation (John). **John, 2026-10-23: keep both `AppId`s as they are**; the Windows install pages now say only one type per computer, uninstall the other first (backup and restore to switch) - Sockeye 8.1.0 has the NLog 6 archive naming problem fixed here for AyaNova: archives are `log-sockeye_NN.txt`, old `-0` to `-8` archives are never deleted, and its log viewer still sorts by name (it also filters on `ayanova.txt`, not `sockeye.txt`, so the current file is listed twice). Same fix applies: `ArchiveSuffixFormat = "-{0}"` and the date sort -- **Security review: customer contact isolation (John, 2026-10-23: urgent, fixed with the "next release" cases before 8.3.0; case numbers to come).** Findings F13 to F17 in `devdocs/modernization/SECURITY-REVIEW.md`, confirmed on the local server as a seeded customer contact: any customer's work order by id (and, by code, its customer report and attachments); other customers' service requests; the full customer list with phones and emails through pick lists; the whole service schedule; notification subscriptions created for another user. Hosted and self-hosted perpetual installations are equally important. Fix direction in the review: block customer-type users by default except the routes the customer UI uses, then add a "belongs to my customer" check to each allowed route that takes an id. F5 (2FA without password), F1 (placeholder JWT secret) and F18 (pick-list search text in SQL) are recommended for 8.3.0 too, awaiting John's decision +- **Security review: customer contact isolation (John, 2026-10-23: urgent, fixed with the "next release" cases before 8.3.0; case numbers to come).** Findings F13 to F17 in `devdocs/modernization/SECURITY-REVIEW.md`, confirmed on the local server as a seeded customer contact: any customer's work order by id (and, by code, its customer report and attachments); other customers' service requests; the full customer list with phones and emails through pick lists; the whole service schedule; notification subscriptions created for another user. Hosted and self-hosted perpetual installations are equally important. Fix direction in the review: block customer-type users by default except the routes the customer UI uses, then add a "belongs to my customer" check to each allowed route that takes an id. F5 (2FA without password), F1 (placeholder JWT secret) and F18 (pick-list search text in SQL) are fixed in 8.3.0 too (John, 2026-10-23). **8.3.0 is not released until every security review finding is fixed or explicitly accepted by John** ("all holes must be plugged"); the review's remaining passes therefore come before the release - **SuperUser can be deleted (serious, pre-existing, own case; John's decision 2026-10-23: fixed with the other "next release" cases before 8.3.0 ships, not under the port):** `UserBiz.ValidateCanDelete` (`biz/UserBiz.cs:1055`) only refuses a delete when the user has related records; nothing protects user id 1, although updates to it are blocked (`UserBiz.cs:946`). On the droplet, whose sample data left SuperUser with no related records, the integration test `UserWithActivityShouldNotBeDeleteable` (`DELETE User/1`) succeeded and removed the account; `AYANOVA_SET_SUPERUSER_PW` cannot bring it back (it only resets an existing user 1). Same code in 8.2.4; likely also why `superuser` could not log in on the reused local database at the first baseline attempt. Any user with delete rights on Users can do this - `RunProgram.Run` (and its `RunWindows`/`RunLinuxShell`/`RunOSX` helpers) has no callers since the backup moved to `RunNoShell`. It runs through `cmd.exe /C` or `/bin/sh -c` and logs its full command line on error: delete it rather than reuse it - **Own case after the port (John): automated provisioning from Sockeye.** Goal: from a trial subscription server request, one menu item in Sockeye (after John's manual approval) provisions the whole server through the DigitalOcean API, ready for login: droplet, DNS, firewall, setup, licence. Replaces today's manual steps (header comments of the setup script), which are slow and error-prone. Not part of this port diff --git a/devdocs/modernization/SECURITY-REVIEW.md b/devdocs/modernization/SECURITY-REVIEW.md index ad6c6762..f872b537 100644 --- a/devdocs/modernization/SECURITY-REVIEW.md +++ b/devdocs/modernization/SECURITY-REVIEW.md @@ -208,7 +208,9 @@ Status: *candidate* (seen in code, not yet confirmed), *confirmed*, *fixed in ca **Release decisions.** John, 2026-10-23: **F13 to F17 are urgent and are fixed in 8.3.0**, as "next release" cases (case numbers to come). Hosted and self-hosted perpetual installations matter equally; how many use customer contact logins is unknown, so every installation is -assumed to. F5, F1 and F18 are recommended for 8.3.0 as well; awaiting John's decision. +assumed to. **F5, F1 and F18 are fixed in 8.3.0 as well** (John, 2026-10-23). John: "I won't +release until it's secure, all holes must be plugged": 8.3.0 waits for the findings of every +pass to be fixed or explicitly accepted by John. ### F5 2FA step skips the password (pass 2, confirmed on the local server)