From 2c415a1712b4351e183d79a09193bdb6b9155bd4 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Mon, 3 Aug 2026 14:57:38 -0400 Subject: [PATCH] fix(installer): correct a false security claim, and clear the should-fix list CLIENT IP / SPOOFABILITY. docs/geenforce-api-cutover.md claimed that removing the IIS rewrite rule made the allowlist fail closed and that it does NOT become spoofable. The opposite is true. IIS never sets X-Forwarded-For on its own; the rule is the only thing that does. Remove it and IIS still forwards whatever X-Forwarded-For the CALLER sent, waitress trusts it because it arrives from 127.0.0.1, and remote_addr becomes attacker-controlled - so a token-less caller can fetch manifests from anywhere on the network. The document and the _trusted_client_ip docstring now say so, waitress runs with --trusted-proxy-count=1, and stage 5 checks the rule is actually live rather than assuming it. The wizard question is rephrased to something an operator can verify with their network team instead of guessing at. NON-ASCII. The style gate only ever checked .py/.vue/.js/.ts, so documentation accumulated em-dashes, arrows and box-drawing characters against this repo's own convention - including in files added this week. Cleaned, and the gate now uses INCLUDES_ALL so Markdown, JSON and YAML are covered. PLUGIN DEFAULTS. The wizard pre-ticked measuringtools and printedparts, both of which ship default_enabled=false, so every site taking the defaults installed and enabled them against their manifests. Inno has no JSON parser so the list must be hardcoded, but tests/test_installer_defaults.py now fails when it drifts. UPGRADES. The payload copy merges, so a plugin dropped from a site's profile kept its code forever - which defeats a lean build and leaves core's optional-import guards succeeding for a plugin the site no longer has. Stale plugin directories are now deregistered and removed before the copy. add-plugin used 'plugin install', which for the five default_enabled=false plugins left them installed but DISABLED - and printed a green success line anyway. It now goes through apply-profile, and the success line is gated on the exit code. Invoke-Flask records its own exit status, because $LASTEXITCODE keeps a stale value when flask.exe is missing and no native command runs. CHARSET. The utf8mb4 compiler hook lived inline in migrations/env.py, so it covered the CORE chain only: plugin baselines inherited the server default, which on a latin1 server means two charsets in one database. It is now shopdb/utils/mysql_charset.py, imported by both, and preflight reports the database's default charset. BACKUP HONESTY. The dump was described as 'all of your asset data'. Uploaded branding and floor-map images live in instance\ on disk, not in the database, so a restore from the .sql alone comes back with no map. backup now archives instance\ alongside it and says both are needed. VERSIONING. AppVersion was hardcoded at 0.9.0 while the product, the frontend and the newest tag said 0.7.0 - and 0.9.0 collides with a retired contract version. Both builders now generate version.iss from shopdb/__init__.py. Smaller: rollback overwrites .env before deleting it, as uninstall already did; appcmd unlocks are scoped to this site's location rather than server-wide, with the wide unlock as a fallback; DEVELOPMENT-SETUP says Python 3.14; the README plugin list gains printedparts; prune-schema --force is documented as first-provisioning-only; HTTPS is documented as not-the-default with the steps to add it; the DBA SQL is on the wizard's database page; the features page says unticking does not remove an installed feature; and the installer README states that bundle-lock cannot vouch for the exe itself - that needs signing or an out-of-band hash, neither of which is wired up. --- CHANGELOG.md | 50 ++++++++++- README.md | 43 +++++----- deploy/windows/installer/.gitignore | 1 + deploy/windows/installer/README.md | 23 ++++- deploy/windows/installer/ShopDBFlask.iss | 51 +++++++++--- deploy/windows/installer/build-installer.ps1 | 11 +++ deploy/windows/installer/build-installer.sh | 11 +++ deploy/windows/installer/shopdb-admin.ps1 | 84 +++++++++++++++++-- deploy/windows/installer/shopdb-install.ps1 | 88 +++++++++++++++++++- deploy/windows/web.config | 2 +- docs/DEPLOY-WINDOWS-IIS.md | 10 ++- docs/DEVELOPMENT-SETUP.md | 4 +- docs/INSTALL-WINDOWS-IIS.md | 2 +- docs/INSTALL-WINDOWS.md | 42 +++++----- docs/OPERATE-WINDOWS.md | 62 +++++++++++--- docs/geenforce-api-cutover.md | 27 +++++- migrations/PRODUCTION_MIGRATION_GUIDE.md | 8 +- migrations/env.py | 25 ++---- plugins/geenforce/api/routes.py | 23 +++-- scripts/check-naming-and-style.sh | 6 +- shopdb/cli/__init__.py | 25 ++++++ shopdb/plugins/alembic_template.py | 8 ++ shopdb/utils/mysql_charset.py | 40 +++++++++ tests/test_installer_defaults.py | 55 ++++++++++++ 24 files changed, 582 insertions(+), 119 deletions(-) create mode 100644 shopdb/utils/mysql_charset.py create mode 100644 tests/test_installer_defaults.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0393589..47529ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,54 @@ ADR-007 and ADR-002. ### Added +- Air-gapped Windows installer (`deploy/windows/installer/`). One `.exe` per + site, built from that site's plugin profile, containing Python, the wheels, + the SPA, the IIS modules and optionally MySQL. Operator docs: + `docs/INSTALL-WINDOWS.md` and `docs/OPERATE-WINDOWS.md`, both shipped onto the + server. `docs/INSTALL-WINDOWS-IIS.md` and `docs/DEPLOY-WINDOWS-IIS.md` are now + reference-only, for hand-built servers. +- `bundle-lock.json`: an exact sha256 + size record of the installer's + third-party payload (wheels, Python installer, IIS MSIs). Verified as set + equality by both builders and again on the server before anything runs; there + is no install-time override. Regenerate with `refresh-bundle-lock.ps1`. +- CycloneDX SBOM (`sbom.cdx.json`) generated on every build from + `requirements.txt` and `package-lock.json`, covering both ecosystems, staged + into the application tree so an air-gapped server can answer "do we carry this + component" locally: `shopdb-admin.ps1 verify -Path `. +- `shopdb-admin.ps1`, the operator console: status, start/stop/restart, logs, + health check, backup, plugins, verify. `check -Json` emits secret-free + structured state for pasting into a support ticket or an AI assistant. +- `build-installer.ps1`, the whole build natively on Windows, so a work PC needs + no Bash. Shares `scripts/resolve_plugin_closure.py` with `build-site.sh`. +- URL Rewrite is bundled and the wizard asks where client IPs come from + (`-ClientIpSource direct|proxy`). Without the rule IIS sends no + `X-Forwarded-For` at all and every client reads as 127.0.0.1. + +### Changed + +- `requirements.txt` and `requirements-dev.txt` are compiled `--universal + --generate-hashes`. Installs run under `pip --require-hashes`, so a wheel whose + sha256 is not listed is refused. The dev lockfile is constrained to the + production pins; the two had drifted. +- Python 3.14 across the Dockerfile, CI, `web.config` and the docs, which + previously declared four different versions. +- The naming/style gate covers Markdown, JSON and YAML, not just code. + +### Fixed + +- `plugins/employees` imported `shopdb.core.models` directly, failing the + contract-surface test on `main` since the dashboard employee-name resolver + landed. +- `scripts/build-site.sh` copied all of `deploy/` into its own output directory, + which recursed when the output was staged inside it - the documented Windows + build could not complete. +- Plugin baseline migrations inherited the MySQL server's default charset: the + utf8mb4 compiler hook lived in `migrations/env.py` and so covered the core + chain only. It is now `shopdb/utils/mysql_charset.py`, imported by both. + `flask db-utils preflight` reports the database's default charset. + +### Added + - Configurable site timezone: a `site_timezone` site setting (default `America/New_York`, public-readable), editable in Settings > Site > Localization. Notification start/end times are entered and displayed in this @@ -53,7 +101,7 @@ ADR-007 and ADR-002. - Shopfloor kiosk header text is readable (light on the dark navy header). -### Added +### Added (continued) - Collector-driven PC -> printer relationships. The computers collector schema gained optional `defaultprinter` (string) and `printers` (array of strings) diff --git a/README.md b/README.md index b0535ba..61b7d34 100644 --- a/README.md +++ b/README.md @@ -36,26 +36,26 @@ ShopDB tracks and manages: ``` shopdb-flask/ -├── shopdb/ # Flask application -│ ├── core/ -│ │ ├── api/ # REST API endpoints -│ │ ├── models/ # SQLAlchemy models -│ │ ├── schemas/ # Validation schemas -│ │ └── services/ # Business logic -│ ├── plugins/ # Plugin system -│ └── utils/ # Shared utilities -├── frontend/ # Vue 3 application -│ ├── src/ -│ │ ├── api/ # API client -│ │ ├── components/ # Reusable components -│ │ ├── views/ # Page components -│ │ ├── router/ # Route definitions -│ │ └── stores/ # Pinia stores -│ └── public/ # Static assets -├── plugins/ # Bundled and external plugins -├── migrations/ # Alembic migration chain (flask db upgrade) -├── scripts/ # Import and utility scripts -└── tests/ # Test suite ++-- shopdb/ # Flask application +| +-- core/ +| | +-- api/ # REST API endpoints +| | +-- models/ # SQLAlchemy models +| | +-- schemas/ # Validation schemas +| | `-- services/ # Business logic +| +-- plugins/ # Plugin system +| `-- utils/ # Shared utilities ++-- frontend/ # Vue 3 application +| +-- src/ +| | +-- api/ # API client +| | +-- components/ # Reusable components +| | +-- views/ # Page components +| | +-- router/ # Route definitions +| | `-- stores/ # Pinia stores +| `-- public/ # Static assets ++-- plugins/ # Bundled and external plugins ++-- migrations/ # Alembic migration chain (flask db upgrade) ++-- scripts/ # Import and utility scripts +`-- tests/ # Test suite ``` ## Naming Conventions @@ -224,7 +224,7 @@ Query parameters for list endpoints: ShopDB supports plugins for extending functionality. See `CONTRIBUTING.md` for plugin development guidelines. -The image bundles twelve plugins; only the ones a site installs are loaded: +The image bundles thirteen plugins; only the ones a site installs are loaded: - **computers** - Shopfloor PCs and workstations, collector fleet ingest - **employees** - Employee directory @@ -234,6 +234,7 @@ The image bundles twelve plugins; only the ones a site installs are loaded: - **knowledgebase** - Documentation and troubleshooting guides - **network** - Network devices and subnets - **notifications** - Shopfloor notifications and recognition feed +- **printedparts** - 3D-printed part catalogue, kiosk issue tracking and stock alerts - **printers** - Extended printer management with Zabbix integration - **slides** - TV/kiosk slideshows - **usb** - CMMC USB check-in/out tracking diff --git a/deploy/windows/installer/.gitignore b/deploy/windows/installer/.gitignore index 1b43d65..5a3b04a 100644 --- a/deploy/windows/installer/.gitignore +++ b/deploy/windows/installer/.gitignore @@ -3,3 +3,4 @@ bundle/ Output/ # Generated by build-installer.sh from the staged bundle. plugins.iss +version.iss diff --git a/deploy/windows/installer/README.md b/deploy/windows/installer/README.md index 4344b51..7dbf084 100644 --- a/deploy/windows/installer/README.md +++ b/deploy/windows/installer/README.md @@ -94,6 +94,23 @@ Windows wheelhouse. A Linux-only resolve had silently omitted `colorama`, a win32-only dependency of `click` - which in hash-checking mode is a hard error rather than a quiet omission. +### Verifying the installer itself + +`bundle-lock.json` is **inside** the thing it describes, so it proves the payload +was not altered between build and install - not that the `.exe` you received is +the one that was built. That needs something out of band. Two options, in order +of preference: + +1. **Authenticode-sign the `.exe`** with a GE code-signing certificate. Windows + then shows a real publisher instead of "Unknown", which is also what stops an + operator learning to click through the SmartScreen warning. +2. **Publish a sha256 per release** through a different channel than the file + itself, and have the receiving site check it: + `Get-FileHash ShopDBFlask_Installer_*.exe -Algorithm SHA256` + +Neither is wired up yet. Until one is, an installer is only as trustworthy as +the share it arrived on. + ### Changing what ships ```powershell @@ -145,10 +162,10 @@ Every build stages a CycloneDX 1.6 SBOM at `sbom.cdx.json`, inside the application tree, so it installs onto the server with the app. Both ecosystems, in one document: -- **Python** — every pin in `requirements.txt`, with the sha256 the installer +- **Python** - every pin in `requirements.txt`, with the sha256 the installer enforces. Environment markers are ignored: a `sys_platform == 'win32'` dependency still installs on the target. -- **npm** — every package in `frontend/package-lock.json`. Build-only packages +- **npm** - every package in `frontend/package-lock.json`. Build-only packages are marked `scope: excluded` rather than dropped, so "not here" is distinguishable from "not looked for". @@ -161,7 +178,7 @@ shopdb-admin.ps1 verify -Path leaflet # is that component here, at what versio ``` Generated by `scripts/generate_sbom.py` from files that are already pinned and -committed, so it is a translation rather than a scan — no network, no extra +committed, so it is a translation rather than a scan - no network, no extra toolchain on the build box, and byte-identical output for the same inputs. It is deliberately not in `bundle-lock.json`: its provenance is git, not the payload. diff --git a/deploy/windows/installer/ShopDBFlask.iss b/deploy/windows/installer/ShopDBFlask.iss index 17c81f7..7bbecd2 100644 --- a/deploy/windows/installer/ShopDBFlask.iss +++ b/deploy/windows/installer/ShopDBFlask.iss @@ -18,11 +18,16 @@ #endif #define AppName "ShopDB-Flask" -; Pre-release. This has not shipped, so it is 0.x by definition. It becomes -; 1.0.0 when a real site installs from it successfully - not before. -; Do NOT bump this to work around a locked output file: the BUILD STAMP below -; makes every compile a unique filename, which is what that problem needed. -#define AppVersion "0.9.0" +; The PRODUCT version, generated into version.iss by the builder from +; shopdb/__init__.py. It was hardcoded, and had drifted to 0.9.0 while the +; product, the frontend and the newest tag all said 0.7.0 - so the delivered exe, +; its Add/Remove Programs entry and the version stamp written onto the server all +; disagreed with the code inside it, and 0.9.0 collided with a retired contract +; version. Never hardcode it here again. +; +; Do NOT bump a version to work around a locked output file: the BUILD STAMP +; below makes every compile a unique filename, which is what that problem needed. +#include "version.iss" #define AppPublisher "GE Aerospace" #define BundleDir "bundle" ; Single source for the install directory - used by DefaultDirName and by the @@ -239,9 +244,18 @@ end; // True when a fresh install should tick this by default. The five omitted here // are specialised; a site that wants them can tick them. +// Pre-tick to match each plugin's own manifest. These five ship +// "default_enabled": false, so a site gets them only by asking. The list used to +// omit measuringtools and printedparts, which meant the wizard installed and +// enabled them against their manifests on every site that took the defaults - +// creating tables nobody asked for. +// +// Keep this in step with the manifests. tests/test_installer_defaults.py fails +// if they drift. function PluginDefault(const Name: String): Boolean; begin - Result := (Name <> 'usb') and (Name <> 'employees') and (Name <> 'geenforce'); + Result := (Name <> 'usb') and (Name <> 'employees') and (Name <> 'geenforce') + and (Name <> 'measuringtools') and (Name <> 'printedparts'); end; // Comma-separated list of what the operator ticked. @@ -372,8 +386,10 @@ begin // have and ticking a new box adds it. PluginPage := CreateInputOptionPage(PreflightPage.ID, 'Features', 'Which parts of ShopDB-Flask does this site use?', - 'Everything is included in this installer. Unticked features are simply not ' + - 'set up, and their database tables are not created.', + 'Unticked features are simply not set up, and their database tables are not ' + + 'created. On an upgrade this shows what the site already has - ticking a new ' + + 'box adds it, but UNTICKING one does NOT remove a feature that is already ' + + 'installed. Removing is a deliberate step, not a side effect of an upgrade.', False, True); PluginNames := StringSplit('{#AvailablePlugins}', [','], stAll); for I := 0 to GetArrayLength(PluginNames) - 1 do @@ -394,7 +410,11 @@ begin 'Existing database', 'Connection details', 'The database and application user must already exist. Your DBA creates ' + 'them; the installer does not, so that it never needs administrative ' + - 'rights on your database server.'); + 'rights on your database server. If they need the exact statements:' + #13#10 + + ' CREATE DATABASE shopdb_flask CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;' + #13#10 + + ' CREATE USER ''shopdb''@''%'' IDENTIFIED BY '''';' + #13#10 + + ' GRANT ALL PRIVILEGES ON shopdb_flask.* TO ''shopdb''@''%'';' + #13#10 + + 'The utf8mb4 charset matters - the default on older servers is latin1.'); DbDetailsPage.Add('Host:', False); DbDetailsPage.Add('Port:', False); DbDetailsPage.Add('Database:', False); @@ -445,12 +465,17 @@ begin // wrong behind a proxy (REMOTE_ADDR is the proxy, so the real client IP is // discarded). Hence a question rather than a default. ClientIpPage := CreateInputOptionPage(SitePage.ID, - 'Client addresses', 'How does this server see who is connecting?', + 'Client addresses', 'Does anything sit between your users and this server?', 'ShopDB records the address of every request, and some features decide what ' - + 'to show based on it. Choose whichever describes this server.', + + 'to show based on it. If you are not sure, choose the first option - it is ' + + 'correct for a server users reach directly.', True, False); - ClientIpPage.Add('Clients connect to this server directly (installs URL Rewrite)'); - ClientIpPage.Add('A proxy or load balancer sits in front of this server'); + ClientIpPage.Add('No - users reach this server directly (installs URL Rewrite)'); + // Phrased as "already adds the visitor's address" rather than "is a proxy": + // the operator can check that with their network team, whereas "is there a + // proxy" invites a guess, and guessing wrong here silently discards the real + // client address on every request. + ClientIpPage.Add('Yes - a load balancer or gateway that already adds the visitor''s address'); ClientIpPage.SelectedValueIndex := 0; end; diff --git a/deploy/windows/installer/build-installer.ps1 b/deploy/windows/installer/build-installer.ps1 index 5a0f672..080632f 100644 --- a/deploy/windows/installer/build-installer.ps1 +++ b/deploy/windows/installer/build-installer.ps1 @@ -220,6 +220,17 @@ if (Test-Path $aliasFile) { $aliasBuilt = (Get-Content $aliasFile -TotalCount 1) "@ | Set-Content -Path (Join-Path $PSScriptRoot 'plugins.iss') -Encoding ASCII Say " $shipped" 'White' +# The product version, read from the code rather than restated in the .iss. +Step 'Writing version.iss' +$initText = Get-Content (Join-Path $RepoRoot 'shopdb\__init__.py') -Raw +if ($initText -notmatch "(?m)^__version__\s*=\s*'([^']+)'") { Die 'could not read __version__ from shopdb\__init__.py' } +$appVersion = $Matches[1] +@" +; GENERATED by build-installer.ps1 from shopdb/__init__.py - do not edit. +#define AppVersion "$appVersion" +"@ | Set-Content -Path (Join-Path $PSScriptRoot 'version.iss') -Encoding ASCII +Say " $appVersion" 'White' + # --- 5. installer scripts --------------------------------------------------- # From THIS directory, which is the reviewed copy under version control. They # used to be copied from a downloads folder, so the logic that shipped was not diff --git a/deploy/windows/installer/build-installer.sh b/deploy/windows/installer/build-installer.sh index f20a0d9..9062dc2 100755 --- a/deploy/windows/installer/build-installer.sh +++ b/deploy/windows/installer/build-installer.sh @@ -59,6 +59,17 @@ cat > "$HERE/plugins.iss" < Writing version.iss" +APPVERSION=$(sed -n "s/^__version__ = '\\(.*\\)'/\\1/p" "$REPO/shopdb/__init__.py" | head -1) +[ -n "$APPVERSION" ] || { echo "could not read __version__ from shopdb/__init__.py"; exit 1; } +cat > "$HERE/version.iss" < {1}" -f ((Get-Item $zip).Length / 1MB), (Split-Path $zip -Leaf)) 'Green' + } catch { + Say (" could not archive instance\: {0}" -f $_.Exception.Message) 'Yellow' + Say ' copy it by hand - it holds branding and floor-map images' 'Yellow' + } + } + + Say '' + Say ' Store BOTH files off this server. The .sql holds the records; the' 'Yellow' + Say ' .zip holds uploaded branding and floor-map images, which the' 'Yellow' + Say ' database does not. A restore needs both.' 'Yellow' + Say '' + Say ' Contains user password hashes - treat it as sensitive.' 'Yellow' } else { Say ' backup is empty or truncated - do NOT rely on it' 'Red' Remove-Item $file -Force -ErrorAction SilentlyContinue @@ -525,17 +549,30 @@ function Show-Sessions { } +# Set by every Invoke-Flask call. Callers MUST test this rather than +# $LASTEXITCODE: when flask.exe is missing no native command runs at all, so +# $LASTEXITCODE keeps whatever value it had from something earlier - which reads +# as success and made a command that did nothing report that it had worked. +$script:LastFlaskExit = 0 + function Invoke-Flask { param([string[]] $Arguments) $flask = Join-Path $AppRoot 'venv\Scripts\flask.exe' - if (-not (Test-Path $flask)) { Say ' application not installed' 'Red'; return $null } + if (-not (Test-Path $flask)) { + Say ' application not installed' 'Red' + $script:LastFlaskExit = 127 + return $null + } Push-Location $AppRoot $env:FLASK_APP = 'shopdb' # The app logs plugin startup to STDERR even on success; with EAP=Stop that # becomes a terminating error and a healthy command looks like a failure. $prev = $ErrorActionPreference $ErrorActionPreference = 'Continue' - try { & $flask @Arguments 2>&1 } + try { + & $flask @Arguments 2>&1 + $script:LastFlaskExit = $LASTEXITCODE + } finally { $ErrorActionPreference = $prev; Pop-Location } } @@ -578,10 +615,47 @@ function Add-Plugin { return } - Say (" installing {0}..." -f $Name) - Invoke-Flask @('plugin','install',$Name) | ForEach-Object { Say " $_" } + # apply-profile, not plugin install. Five plugins ship default_enabled=false, + # so `install` alone left them installed-but-disabled: the command printed a + # green success line and the feature did not appear anywhere in the UI. + # apply-profile installs AND enables, and pulls in the dependency closure. + Say (" adding {0}..." -f $Name) + $profilePath = Join-Path $AppRoot 'site-profile.json' + $applied = $false + if (Test-Path $profilePath) { + try { + $profile = Get-Content $profilePath -Raw | ConvertFrom-Json + $wanted = @($profile.plugins) + if ($wanted -notcontains $Name) { $wanted += $Name } + $profile.plugins = $wanted + $profile | ConvertTo-Json -Depth 6 | Set-Content -Path $profilePath -Encoding UTF8 + Invoke-Flask @('plugin','apply-profile',$profilePath) | ForEach-Object { Say " $_" } + $applied = ($script:LastFlaskExit -eq 0) + } catch { Say (" could not update site-profile.json: {0}" -f $_.Exception.Message) 'Yellow' } + } + if (-not $applied) { + # No profile on disk, or apply-profile failed: fall back, but enable + # explicitly so the outcome is the same either way. + Invoke-Flask @('plugin','install',$Name) | ForEach-Object { Say " $_" } + $installed = ($script:LastFlaskExit -eq 0) + Invoke-Flask @('plugin','enable',$Name) | ForEach-Object { Say " $_" } + $applied = $installed -and ($script:LastFlaskExit -eq 0) + } + if (-not $applied) { + # Do NOT print the green line on a failure. It used to be unconditional, + # so a failed add reported success and the operator went looking for a + # feature that was never enabled. + Say (" {0} was NOT added - see the output above" -f $Name) 'Red' + return + } + Say ' applying its database migrations...' Invoke-Flask @('plugin','upgrade-all') | ForEach-Object { Say " $_" } + if ($script:LastFlaskExit -ne 0) { + Say ' migrations FAILED - the feature is installed but its tables are not' 'Red' + Say ' do not use it until this is resolved; restore a backup if needed' 'Red' + return + } Say ' restarting so its routes register...' Restart-App Say (" {0} added" -f $Name) 'Green' diff --git a/deploy/windows/installer/shopdb-install.ps1 b/deploy/windows/installer/shopdb-install.ps1 index 8125f79..feae3a8 100644 --- a/deploy/windows/installer/shopdb-install.ps1 +++ b/deploy/windows/installer/shopdb-install.ps1 @@ -890,6 +890,42 @@ what is on this server, or restore a backup taken before the upgrade. } catch { } } + # Plugin directories present on the server but NOT in the new bundle are + # removed BEFORE the copy. The copy merges rather than replaces, so a plugin + # dropped from a site's profile kept its code on disk forever - which defeats + # the point of a lean build (ADR-013) and, worse, leaves the core's + # `try: from plugins.X.models import ...` guards succeeding for a plugin this + # site no longer has, so SQLAlchemy still builds the relationship and queries + # still join a table that may have been pruned. + if (-not $WhatIfOnly) { + $newPlugins = @() + $srcPlugins = Join-Path $AppSource 'plugins' + if (Test-Path $srcPlugins) { + $newPlugins = @(Get-ChildItem $srcPlugins -Directory | Select-Object -ExpandProperty Name) + } + $livePlugins = Join-Path $AppRoot 'plugins' + if ($newPlugins.Count -gt 0 -and (Test-Path $livePlugins)) { + foreach ($dir in (Get-ChildItem $livePlugins -Directory)) { + if ($newPlugins -notcontains $dir.Name) { + Write-Log "removing '$($dir.Name)': not in this build" 'WARN' + # Uninstall FIRST, while the code is still importable - the + # registry entry outlives the directory otherwise, and the app + # then fails to load a plugin it still believes is installed. + try { + if (Test-Path $Flask) { + Push-Location $AppRoot + $env:FLASK_APP = 'shopdb' + & $Flask plugin uninstall $dir.Name 2>&1 | + ForEach-Object { Write-Log " $_" } + Pop-Location + } + } catch { Write-Log " could not deregister $($dir.Name): $($_.Exception.Message)" 'WARN' } + Remove-Item $dir.FullName -Recurse -Force -ErrorAction SilentlyContinue + } + } + } + } + Write-Log "copying application payload to $AppRoot" if (-not $WhatIfOnly) { Copy-Item (Join-Path $AppSource '*') -Destination $AppRoot -Recurse -Force @@ -1533,9 +1569,21 @@ has to come from the bundle either way. # Handler sections are locked server-wide by default. Without unlocking, # IIS returns 500.19 the moment it reads the app's web.config. + # + # `appcmd unlock config /section:X` unlocks it for EVERY site on the machine, + # which on a shared server hands every other application the ability to + # define its own handlers. Delegate to our own location instead, and fall + # back to the server-wide unlock only if that is refused - a 500.19 the + # operator cannot diagnose is worse than a wider delegation, but it should be + # the second choice, not the first. + $ourLocation = if ($MountAlias) { "$ParentSite/$($MountAlias.Trim('/'))" } else { $SiteName } foreach ($section in @('system.webServer/handlers','system.webServer/httpPlatform')) { - Write-Log "unlocking $section" - if (-not $WhatIfOnly) { + if ($WhatIfOnly) { Write-Log "would unlock $section for $ourLocation"; continue } + Write-Log "unlocking $section for $ourLocation" + $scoped = & $appcmd unlock config "$ourLocation" /section:$section 2>&1 + $scoped | ForEach-Object { Write-Log " $_" } + if ($LASTEXITCODE -ne 0) { + Write-Log " scoped unlock refused; unlocking $section server-wide" 'WARN' & $appcmd unlock config /section:$section 2>&1 | ForEach-Object { Write-Log " $_" } } } @@ -1783,6 +1831,29 @@ public class ShopdbSmokeTestCertPolicy : ICertificatePolicy { } } + # Is the client-IP rule actually in effect? It is the control that makes + # remote_addr trustworthy: without it IIS forwards whatever X-Forwarded-For + # the CALLER sent, waitress trusts it because it arrives from loopback, and + # the GE-Enforce IP allowlist becomes bypassable from anywhere on the network. + # It does not fail closed. Verified here rather than assumed, because the + # failure is silent and looks exactly like a working server. + if ($ClientIpSource -eq 'direct') { + $liveCfg = Join-Path $AppRoot 'web.config' + $cfgText = if (Test-Path $liveCfg) { Get-Content $liveCfg -Raw } else { '' } + $ruleLive = ($cfgText -match '') -and ($cfgText -notmatch 'SHOPDB-CLIENTIP-BEGIN') + $moduleThere = Test-Path (Join-Path $env:windir 'system32\inetsrv\rewrite.dll') + if ($ruleLive -and $moduleThere) { + Write-Log 'client-IP rule is live; remote_addr is trustworthy' 'OK' + } else { + Write-Log 'CLIENT-IP RULE IS NOT IN EFFECT' 'WARN' + if (-not $moduleThere) { Write-Log ' URL Rewrite module is not installed' 'WARN' } + if (-not $ruleLive) { Write-Log ' web.config does not have an active block' 'WARN' } + Write-Log ' every client will be recorded as 127.0.0.1, and the GE-Enforce IP' 'WARN' + Write-Log ' allowlist is BYPASSABLE - a caller can send its own X-Forwarded-For' 'WARN' + Write-Log ' re-run with -ClientIpSource direct, or use token authentication' 'WARN' + } + } + # The self-hosted API documentation. It is the thing an operator - or the # assistant an operator is asking - reaches for on a server with no internet, # and it is served from a file that has to have been staged into the build. @@ -1854,7 +1925,18 @@ function Invoke-Rollback { } 'apppool' { Remove-WebAppPool -Name $item.Id -ErrorAction SilentlyContinue; Write-Log " removed app pool $($item.Id)" } 'firewall' { Remove-NetFirewallRule -DisplayName $item.Id -ErrorAction SilentlyContinue; Write-Log " removed firewall rule" } - 'file' { Remove-Item $item.Id -Force -ErrorAction SilentlyContinue; Write-Log " removed $($item.Id)" } + 'file' { + # Overwrite before deleting, same as uninstall does. A rolled + # back install left .env - which holds the database password + # in plaintext by design - recoverable on disk. + try { + if ((Split-Path $item.Id -Leaf) -eq '.env') { + Set-Content -Path $item.Id -Value ('0' * 512) -Encoding UTF8 -Force + } + } catch { } + Remove-Item $item.Id -Force -ErrorAction SilentlyContinue + Write-Log " removed $($item.Id)" + } 'dir' { Write-Log " left directory $($item.Id) in place (remove manually if wanted)" 'WARN' } 'python' { Write-Log " left Python install in place at $($item.Id)" 'WARN' } 'service' { diff --git a/deploy/windows/web.config b/deploy/windows/web.config index 6aeffd1..8d0a5c8 100644 --- a/deploy/windows/web.config +++ b/deploy/windows/web.config @@ -28,7 +28,7 @@ **Not the route for a new site.** Sister sites install from the Windows -> installer — one `.exe`, no manual IIS work: **[INSTALL-WINDOWS.md](INSTALL-WINDOWS.md)**. +> installer - one `.exe`, no manual IIS work: **[INSTALL-WINDOWS.md](INSTALL-WINDOWS.md)**. > > This is the **manual** procedure for the West Jefferson server, which was built > by hand against its existing MySQL 5.6 and predates the installer. Keep it for @@ -140,12 +140,18 @@ Cleaner than a hand list: declare the set once in a site profile and apply it: ```powershell venv\Scripts\flask plugin apply-profile deploy\site-profile.json # install + enable the chosen set, in dependency order venv\Scripts\flask plugin upgrade-all -venv\Scripts\flask plugin prune-schema --yes --force # lean DB: drop tables of plugins this site did NOT install (ADR-014) +venv\Scripts\flask plugin prune-schema --yes --force # FIRST PROVISIONING ONLY - see the warning below ``` (Alternatively copy the dev box's `instance/plugins.json` to `APP_ROOT\instance\` to reproduce the exact set, then just run `flask plugin upgrade-all`.) +> **`prune-schema --force` is for first provisioning only.** It drops the tables +> of plugins this site did not install *even when they hold rows*. On a site that +> already has data, run `flask plugin prune-schema` with no flags first and read +> what it says it would drop. Re-running with `--force` after a feature has been +> used deletes that feature's records with no prompt and no backup. + ## 6. Create the IIS site + web.config This describes the own-site method (the app gets its own IIS site + port). To diff --git a/docs/DEVELOPMENT-SETUP.md b/docs/DEVELOPMENT-SETUP.md index db1be6f..1f5a6f3 100644 --- a/docs/DEVELOPMENT-SETUP.md +++ b/docs/DEVELOPMENT-SETUP.md @@ -23,7 +23,7 @@ sane, then use manual for day-to-day work. | Need | Version | Check | | --- | --- | --- | -| Python | 3.13 (64-bit) - matches CI; prod images run 3.12 | `python --version` | +| Python | 3.14 (64-bit) - matches CI, the container image and the Windows installer wheelhouse | `python --version` | | Node.js | 18+ | `node --version` | | MySQL | 8.0 (or Docker, below) | `mysql --version` | | Git | any recent | `git --version` | @@ -33,7 +33,7 @@ reopen the terminal so PATH updates): ```powershell winget install Git.Git # includes Git Bash (naming hook needs it) -winget install Python.Python.3.13 +winget install Python.Python.3.14 winget install OpenJS.NodeJS.LTS # LTS; may install v24, fine for this SPA winget install Microsoft.VisualStudioCode winget install Oracle.MySQL # or Docker.DockerDesktop for the DB diff --git a/docs/INSTALL-WINDOWS-IIS.md b/docs/INSTALL-WINDOWS-IIS.md index 8ba14da..9cb9031 100644 --- a/docs/INSTALL-WINDOWS-IIS.md +++ b/docs/INSTALL-WINDOWS-IIS.md @@ -1,7 +1,7 @@ # ShopDB - Windows + IIS install runbook > **Not the route for a new site.** Sister sites install from the Windows -> installer — one `.exe`, no manual IIS work: **[INSTALL-WINDOWS.md](INSTALL-WINDOWS.md)**. +> installer - one `.exe`, no manual IIS work: **[INSTALL-WINDOWS.md](INSTALL-WINDOWS.md)**. > > This document is the **manual** procedure, kept for reference and for > hand-built servers that predate the installer. Note that the installer will not diff --git a/docs/INSTALL-WINDOWS.md b/docs/INSTALL-WINDOWS.md index 0005980..36d22b0 100644 --- a/docs/INSTALL-WINDOWS.md +++ b/docs/INSTALL-WINDOWS.md @@ -5,7 +5,7 @@ and get a working application. Nothing here needs an internet connection, and yo do not need to know IIS, Python or MySQL. If you are looking after an existing hand-built server, see -[DEPLOY-WINDOWS-IIS.md](DEPLOY-WINDOWS-IIS.md) instead — that is the manual +[DEPLOY-WINDOWS-IIS.md](DEPLOY-WINDOWS-IIS.md) instead - that is the manual procedure, and the installer will not adopt a server it did not build. --- @@ -17,9 +17,9 @@ You need **four things**. The installer supplies everything else. | | What | How to check | |---|---|---| | 1 | Windows Server 2019 or newer | `winver` | -| 2 | The **IIS Web Server role** installed | Server Manager → Manage → Add Roles → Web Server (IIS). Or run the PowerShell below. | -| 3 | Administrator rights on the box | Right-click PowerShell → "Run as administrator" works | -| 4 | A decision about the database — see [Which database?](#which-database) | — | +| 2 | The **IIS Web Server role** installed | Server Manager -> Manage -> Add Roles -> Web Server (IIS). Or run the PowerShell below. | +| 3 | Administrator rights on the box | Right-click PowerShell -> "Run as administrator" works | +| 4 | A decision about the database - see [Which database?](#which-database) | - | Installing IIS, if it is missing (this needs no internet): @@ -35,10 +35,10 @@ time. Two options. Pick before you start, because they ask different questions. -- **Use the bundled MySQL** — the installer puts MySQL 8.0 on this server and +- **Use the bundled MySQL** - the installer puts MySQL 8.0 on this server and creates the database for you. Choose this when the server has no database today. Simplest option, nothing to arrange in advance. -- **Use an existing MySQL** — the database already exists somewhere, and you have +- **Use an existing MySQL** - the database already exists somewhere, and you have a hostname, a database name, a username and a password for it. Choose this if your site already runs MySQL, or a DBA looks after it. @@ -51,29 +51,29 @@ bundled option. 1. Copy the installer `.exe` onto the server. It is one file and needs no network. -2. **Right-click it → Run as administrator.** Without this it cannot configure +2. **Right-click it -> Run as administrator.** Without this it cannot configure IIS, and it will tell you so. 3. Work through the wizard. The pages are: | Page | What it wants | If unsure | |---|---|---| -| **Server check** | Nothing — it reports what it found | Fix anything red, then "Check again". You cannot continue while something is red, and nothing has been changed yet. | +| **Server check** | Nothing - it reports what it found | Fix anything red, then "Check again". You cannot continue while something is red, and nothing has been changed yet. | | **Features** | Which parts of the product this site uses | The defaults are fine. You can add more later; removing needs a new installer. | -| **Database** | Bundled or existing — see above | Bundled | +| **Database** | Bundled or existing - see above | Bundled | | **Database details** | Host, port, name, user, password | Only asked for the existing-database option | | **Address** | How people reach the site | See [Own address or subpath?](#own-address-or-subpath) | | **Client addresses** | Whether a proxy sits in front | See [Client addresses](#client-addresses) | | **Location** | Where to install | `C:\shopdb-flask` is fine | 4. The install takes a few minutes. Most of it is Python and the database schema. -5. At the end you get the address to open. **Write it down** — it is also on the +5. At the end you get the address to open. **Write it down** - it is also on the Start Menu as "Open ShopDB-Flask". ### Own address or subpath? -- **Its own address** — `http://yourserver:8090/`. Choose this on a server that +- **Its own address** - `http://yourserver:8090/`. Choose this on a server that is not already running a website. Simplest. -- **Under this server's existing address** — `http://yourserver/shopdb/`. Choose +- **Under this server's existing address** - `http://yourserver/shopdb/`. Choose this when the server already serves something else and you do not want a second port or a new DNS name. This is what West Jefferson uses. @@ -85,9 +85,9 @@ web interface has the address compiled into it. The application records who connects, and some features decide what to show based on it. The wizard asks one question: -- **Clients connect to this server directly** — the normal answer. Pick this +- **Clients connect to this server directly** - the normal answer. Pick this unless you know otherwise. -- **A proxy or load balancer sits in front** — pick this only if your network +- **A proxy or load balancer sits in front** - pick this only if your network team has told you traffic reaches this server through something else first. Getting this wrong is not dangerous, but the site will record every visitor as @@ -102,7 +102,7 @@ Open the address the installer gave you. With no users in the database yet, the page offers to **create the first administrator**, then runs a short setup wizard for site details, features and the floor map. -That first account is a normal administrator account. Use a real password — +That first account is a normal administrator account. Use a real password - this is the account that creates everyone else. --- @@ -118,7 +118,7 @@ cd C:\shopdb-flask You want to see the site started, the pool started, and `responding : yes`. -Day-to-day tasks — restarting, backups, logs, upgrades — are in +Day-to-day tasks - restarting, backups, logs, upgrades - are in [OPERATE-WINDOWS.md](OPERATE-WINDOWS.md). --- @@ -130,9 +130,9 @@ is still there. That is deliberate: it means re-running is able to pick up where it stopped. 1. Read the error. It names the cause and what to do about it. -2. Fix that, then **run the same installer again**. Re-running is safe — it skips +2. Fix that, then **run the same installer again**. Re-running is safe - it skips what is already done and does not touch your database or `.env`. -3. If you would rather start clean, remove it from **Settings → Apps** first. +3. If you would rather start clean, remove it from **Settings -> Apps** first. The full log is at: @@ -155,7 +155,7 @@ real state rather than a description: That prints one structured block covering the version, how the site is published, IIS state, database reachability, Python version, installed features and any errors. Paste it in. **It contains no passwords.** The install log is also safe -to share — the installer keeps secrets out of it deliberately. +to share - the installer keeps secrets out of it deliberately. Offline API reference for this server is served at `/api/docs` on the site itself, and `docs\` in the install directory holds these runbooks. @@ -174,7 +174,7 @@ Run a newer installer over the top. It: Nothing else is required. See [UPGRADE.md](UPGRADE.md). -> **Before your first upgrade:** confirm `mysqldump` is available — the console's +> **Before your first upgrade:** confirm `mysqldump` is available - the console's > health check reports it. Without it the pre-upgrade backup is skipped, and that > is the one you would want if a migration went wrong. It ships with the bundled > database option; for an existing remote database, ask for `mysqlclient\` to be @@ -184,7 +184,7 @@ Nothing else is required. See [UPGRADE.md](UPGRADE.md). ## Removing it -**Settings → Apps → ShopDB-Flask**, or Add/Remove Programs. +**Settings -> Apps -> ShopDB-Flask**, or Add/Remove Programs. That removes the website, the application pool, the firewall rule and the application directory. It deliberately **does not** drop the database and does diff --git a/docs/OPERATE-WINDOWS.md b/docs/OPERATE-WINDOWS.md index 9e5c3f2..dc8e61b 100644 --- a/docs/OPERATE-WINDOWS.md +++ b/docs/OPERATE-WINDOWS.md @@ -11,7 +11,7 @@ C:\shopdb-flask\shopdb-admin.ps1 The Start Menu folder **ShopDB-Flask** has shortcuts for the common tasks. Run it with no arguments for a menu, or pass a command directly. It needs -Administrator — it will ask, except for `open`. +Administrator - it will ask, except for `open`. --- @@ -20,11 +20,11 @@ Administrator — it will ask, except for `open`. | Command | What it does | Safe at any time | |---|---|---| | `status` | Is it published, running, responding; database and table count | yes | -| `restart` | Recycles the application pool. **Use this after any config change.** | yes — drains requests rather than cutting them off | +| `restart` | Recycles the application pool. **Use this after any config change.** | yes - drains requests rather than cutting them off | | `stop` / `start` | Takes the site down / brings it back | yes, but `stop` makes it unavailable | | `logs` | Last lines of the application and install logs | yes | | `check` | Full health check | yes | -| `check -Json` | The same, machine-readable — see [Getting help](#getting-help) | yes | +| `check -Json` | The same, machine-readable - see [Getting help](#getting-help) | yes | | `verify` | Which build this is, and whether what is installed still matches it | yes | | `sessions` | IIS worker processes and memory | yes | | `plugins` | Which features are installed, which are available | yes | @@ -50,7 +50,7 @@ Examples: ``` Writes to `C:\ProgramData\ShopDB-Flask\backups` unless you pass a directory. The -dump is **verified complete** before it is reported as good — a truncated backup +dump is **verified complete** before it is reported as good - a truncated backup is deleted rather than left to be discovered later. Two things to know: @@ -85,7 +85,7 @@ go backwards, and restores if a migration fails. See Only features **shipped in this build** can be added. Each site's installer is built for that site's chosen feature set, so a feature nobody asked for is not on -the server at all — adding it means a new installer built from an updated +the server at all - adding it means a new installer built from an updated profile. `plugins` shows you which is which. --- @@ -119,7 +119,7 @@ immediately if it is not: .\shopdb-admin.ps1 restart ``` -**4. Check the database is reachable** — `status` reports the host and whether it +**4. Check the database is reachable** - `status` reports the host and whether it could count tables. A site that starts but shows no data is usually a database problem, not an application one. @@ -129,7 +129,7 @@ problem, not an application one. .\shopdb-admin.ps1 verify ``` -This flags packages that no longer match what shipped — which usually means +This flags packages that no longer match what shipped - which usually means somebody ran a `pip install` on the server by hand. --- @@ -147,13 +147,13 @@ whether it responds, database host and reachability, Python version, installed features, and any errors. **It contains no passwords** and is safe to paste into a chat window or a ticket. -The install log is also safe to share — secrets are deliberately kept out of it. +The install log is also safe to share - secrets are deliberately kept out of it. Offline reference on the server itself: -- `/api/docs` on the site — the full API reference, self-hosted, no internet. -- `C:\shopdb-flask\docs\` — these runbooks. -- `C:\shopdb-flask\sbom.cdx.json` — every component this build contains. +- `/api/docs` on the site - the full API reference, self-hosted, no internet. +- `C:\shopdb-flask\docs\` - these runbooks. +- `C:\shopdb-flask\sbom.cdx.json` - every component this build contains. --- @@ -178,12 +178,50 @@ Nothing found means this server does not carry it. --- +## Adding HTTPS + +**The installer publishes over HTTP.** It has no certificate to use and no way to +get one on an air-gapped server, so it does not pretend otherwise. On an internal +network behind the site firewall that is often accepted; confirm it against your +own policy rather than assuming. + +If you installed **under an existing site** (the subpath option) and that site +already has a certificate, you are already on HTTPS - nothing to do. + +For a site of its own, once you have a certificate in the machine store: + +```powershell +Import-Module WebAdministration + +# 1. Add the binding. Get the thumbprint from the certificate you imported. +New-WebBinding -Name shopdb-flask -Protocol https -Port 443 +$cert = Get-ChildItem Cert:\LocalMachine\My | Where-Object { $_.Subject -like '*yourserver*' } +Get-Item "IIS:\SslBindings\0.0.0.0!443" -EA SilentlyContinue | Remove-Item -EA SilentlyContinue +New-Item "IIS:\SslBindings\0.0.0.0!443" -Value $cert + +# 2. Open the port. +New-NetFirewallRule -DisplayName "shopdb-flask 443" -Direction Inbound ` + -Protocol TCP -LocalPort 443 -Action Allow +``` + +Then **update `CORS_ORIGINS` in `C:\shopdb-flask\.env`** to the `https://` address +and restart: + +```powershell +.\shopdb-admin.ps1 restart +``` + +That last step is not optional. `CORS_ORIGINS` is an exact origin match, so a +site reached over `https://` while `.env` still says `http://` loads the page and +then fails every data request - which looks like the application is broken rather +than a configuration mismatch. + ## Where things live | | | |---|---| | Application | `C:\shopdb-flask` | -| Configuration and secrets | `C:\shopdb-flask\.env` (locked down — do not loosen) | +| Configuration and secrets | `C:\shopdb-flask\.env` (locked down - do not loosen) | | Application logs | `C:\shopdb-flask\logs` | | Install logs | `C:\ProgramData\ShopDB-Flask\logs` | | Backups | `C:\ProgramData\ShopDB-Flask\backups` | diff --git a/docs/geenforce-api-cutover.md b/docs/geenforce-api-cutover.md index 7364547..56e64ea 100644 --- a/docs/geenforce-api-cutover.md +++ b/docs/geenforce-api-cutover.md @@ -133,9 +133,30 @@ in place: A client that somehow hits waitress directly is not a trusted proxy, so its `remote_addr` is its own real peer address. Either way, real client IP. -If the IIS rule is ever removed, the allowlist stops matching (fails closed to -401 for token-less clients) - it does NOT become spoofable, but the fleet -falls back to cached manifests. Keep the rule on. +**If the IIS rule is ever removed, the allowlist becomes SPOOFABLE.** This +document previously claimed the opposite; it was wrong, and the reasoning matters. + +IIS does not set `X-Forwarded-For` on its own - the rewrite rule is the only +thing that does. Remove the rule and IIS still *forwards* whatever +`X-Forwarded-For` the caller sent. waitress trusts that header because it arrives +from `127.0.0.1`, which is IIS, and sets `remote_addr` from it. So a caller who +sends `X-Forwarded-For: 10.134.48.5` gets `remote_addr = 10.134.48.5`, matches +the allowlist and fetches manifests token-less from anywhere on the network. + +The rule is not a nicety that improves logging. It is the control that makes +`remote_addr` trustworthy, and everything downstream - the allowlist, the +dashboard visitor-location lookup, per-host login rate limiting - depends on it. + +Three consequences worth stating plainly: + +- The Windows installer enables the rule when told IIS faces clients directly + (`-ClientIpSource direct`), installs URL Rewrite from the bundle to make that + possible offline, and its stage-5 check fails if the rule is not live. +- On a hand-built server, verify it: `deploy/windows/web.config` must have the + `` block ACTIVE, not inside the `SHOPDB-CLIENTIP` comment markers. +- Behind a real reverse proxy the rule is the wrong answer, because `REMOTE_ADDR` + is then the proxy. There, the proxy must set `X-Forwarded-For` itself and be + the only thing that can reach IIS. `-ClientIpSource proxy` covers that case. --- diff --git a/migrations/PRODUCTION_MIGRATION_GUIDE.md b/migrations/PRODUCTION_MIGRATION_GUIDE.md index 3f3874a..7a68acf 100644 --- a/migrations/PRODUCTION_MIGRATION_GUIDE.md +++ b/migrations/PRODUCTION_MIGRATION_GUIDE.md @@ -13,10 +13,10 @@ This guide documents the process for migrating data from the legacy VBScript Sho ### New System (Flask) - Core `assets` table (unified asset registry) - Plugin-specific extension tables: - - `equipment` (equipmenttypeid → equipmenttypes) - - `computers` (computertypeid → computertypes) - - `printers` (printertypeid → printertypes) - - `network_device` (networkdevicetypeid → networkdevicetypes) + - `equipment` (equipmenttypeid -> equipmenttypes) + - `computers` (computertypeid -> computertypes) + - `printers` (printertypeid -> printertypes) + - `network_device` (networkdevicetypeid -> networkdevicetypes) ## Key Mappings diff --git a/migrations/env.py b/migrations/env.py index 0c5f7bc..0b4049e 100644 --- a/migrations/env.py +++ b/migrations/env.py @@ -4,27 +4,12 @@ from logging.config import fileConfig from flask import current_app from alembic import context -from sqlalchemy.ext.compiler import compiles -from sqlalchemy.schema import CreateTable - -# Force every table the migrations create on MySQL to utf8mb4 + DYNAMIC row -# format. Without this a fresh `flask db upgrade` inherits the server default -# charset, so a box whose default is latin1 (common on older MySQL) silently -# builds a latin1 schema that drifts from the utf8mb4 production target. The -# DYNAMIC row format also keeps utf8mb4 indexes under the 767-byte prefix limit -# on pre-5.7 InnoDB. Scoped to the mysql dialect so the SQLite test DB is -# untouched. -@compiles(CreateTable, "mysql") -def _mysql_create_table_utf8mb4(element, compiler, **kw): - sql = compiler.visit_create_table(element, **kw) - if "CHARSET" not in sql.upper(): - sql = sql.rstrip().rstrip(";") - sql += ( - " ENGINE=InnoDB DEFAULT CHARSET=utf8mb4" - " COLLATE=utf8mb4_unicode_ci ROW_FORMAT=DYNAMIC" - ) - return sql +# Registers the compiler hook that forces utf8mb4 + DYNAMIC on MySQL. Imported +# for the side effect, and shared with the PLUGIN chains via +# shopdb/plugins/alembic_template.py - it used to live here, so it covered core +# only and plugin baselines inherited the server default charset. +import shopdb.utils.mysql_charset # noqa: F401 # this is the Alembic Config object, which provides # access to the values within the .ini file in use. diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index 63f958a..3a245bc 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -52,12 +52,23 @@ def _trusted_client_ip(): Uses request.remote_addr, NOT the raw X-Forwarded-For header. Proxies APPEND to X-Forwarded-For, so its first hop is attacker-controlled: parsing it (as _client_ip does for rate-limiting) would let any caller send - 'X-Forwarded-For: ' and bypass the token. remote_addr cannot - be forged here - behind IIS the URL-Rewrite rule overwrites X-Forwarded-For - with the real TCP peer and waitress (--trusted-proxy=127.0.0.1 - --trusted-proxy-headers=x-forwarded-for) derives remote_addr from it; a - client hitting waitress directly is not a trusted proxy, so its remote_addr - is its own real peer address. Either way remote_addr is the true client. + 'X-Forwarded-For: ' and bypass the token. + + DEPLOYMENT DEPENDENCY, not a property of this function. remote_addr is only + trustworthy because of what sits in front: + + - Behind IIS, the URL-Rewrite rule OVERWRITES X-Forwarded-For with the real + TCP peer, and waitress (--trusted-proxy=127.0.0.1 + --trusted-proxy-headers=x-forwarded-for) derives remote_addr from it. + - Remove that rule and IIS still forwards whatever X-Forwarded-For the + CALLER sent. waitress trusts it because it comes from 127.0.0.1, so + remote_addr becomes attacker-controlled and this allowlist is bypassable + from anywhere on the network. It does NOT fail closed. + - A client reaching waitress directly is not a trusted proxy, so there + remote_addr is its own peer address. + + The rule is the control. See docs/geenforce-api-cutover.md; the Windows + installer enables it and verifies it is live at the end of an install. """ return request.remote_addr or '' diff --git a/scripts/check-naming-and-style.sh b/scripts/check-naming-and-style.sh index 39aa0f5..a791bcf 100755 --- a/scripts/check-naming-and-style.sh +++ b/scripts/check-naming-and-style.sh @@ -53,8 +53,12 @@ INCLUDES_ALL=( --include='*.yml' ) +# INCLUDES_ALL, not INCLUDES_CODE: the check only ever covered .py/.vue/.js/.ts, +# so documentation was free to accumulate em-dashes, arrows and smart quotes - +# and did, including in files this repo's own convention forbids them in. +# Markdown, JSON and YAML are now covered too. echo "==> Checking for non-ASCII characters..." -NON_ASCII=$(grep -rPn '[^\x00-\x7F]' "${EXCLUDES[@]}" "${INCLUDES_CODE[@]}" . 2>/dev/null || true) +NON_ASCII=$(grep -rPn '[^\x00-\x7F]' "${EXCLUDES[@]}" "${INCLUDES_ALL[@]}" . 2>/dev/null || true) if [ -n "$NON_ASCII" ]; then echo "FAIL: non-ASCII characters found (em-dashes, smart quotes, arrows, emojis):" echo "$NON_ASCII" diff --git a/shopdb/cli/__init__.py b/shopdb/cli/__init__.py index eb395f7..88230c0 100644 --- a/shopdb/cli/__init__.py +++ b/shopdb/cli/__init__.py @@ -98,6 +98,31 @@ def preflight(): elif version: ok('MySQL 5.7+/8.0 - no extra index flags needed') + # The database's own default charset. Tables are forced to utf8mb4 by the + # compiler hook (shopdb/utils/mysql_charset.py), so a latin1 default does not + # break the schema - but anything created OUTSIDE that path inherits it, and + # a database created without an explicit CHARSET is the usual cause. Cheap to + # check here, invisible until characters come back mangled. + if version: + try: + row = db.session.execute(text( + 'SELECT DEFAULT_CHARACTER_SET_NAME FROM information_schema.SCHEMATA ' + 'WHERE SCHEMA_NAME = DATABASE()')).fetchone() + charset = row[0] if row else None + if charset is None: + warn('Could not read the database default charset', + 'Not fatal - tables are forced to utf8mb4 regardless.') + elif str(charset).lower() == 'utf8mb4': + ok('Database default charset is utf8mb4') + else: + warn(f'Database default charset is {charset}, not utf8mb4', + 'Tables this application creates are forced to utf8mb4, so the ' + 'schema is correct. Fix the default so anything created outside ' + 'the migrations matches: ALTER DATABASE CHARACTER SET ' + 'utf8mb4 COLLATE utf8mb4_unicode_ci;') + except Exception as exc: + warn('Could not read the database default charset', f'({exc})') + click.echo('') if failures: click.echo(click.style(f'{len(failures)} blocker(s). Fix them before installing.', fg='red', bold=True)) diff --git a/shopdb/plugins/alembic_template.py b/shopdb/plugins/alembic_template.py index 2ae8698..213a15c 100644 --- a/shopdb/plugins/alembic_template.py +++ b/shopdb/plugins/alembic_template.py @@ -34,6 +34,14 @@ from typing import Iterable from alembic import context from sqlalchemy import MetaData, pool +# Registers the compiler hook that forces utf8mb4 + DYNAMIC on MySQL. Imported +# for the side effect. It used to live inline in migrations/env.py, so it applied +# to the CORE chain only: a plugin's baseline tables were created at the server's +# default charset while core's were utf8mb4, on the same database. On a server +# defaulting to latin1 that difference is invisible until a join between the two +# stops using an index, or a character comes back mangled. +import shopdb.utils.mysql_charset # noqa: F401,E402 + logger = logging.getLogger('alembic.env.plugin') # Explicit table-ownership map. Adding tables to a plugin requires updating diff --git a/shopdb/utils/mysql_charset.py b/shopdb/utils/mysql_charset.py new file mode 100644 index 0000000..b5f071e --- /dev/null +++ b/shopdb/utils/mysql_charset.py @@ -0,0 +1,40 @@ +"""Force every table created on MySQL to utf8mb4 + DYNAMIC row format. + +Importing this module installs a SQLAlchemy compiler hook. It is a side effect on +purpose: the hook has to be registered before any CreateTable is compiled, and +every place that creates tables needs it. + +WHY. Without it a CREATE TABLE inherits the SERVER's default charset. A MySQL box +defaulting to latin1 - common on older installs, and the West Jefferson 5.6 +server is one - silently builds a latin1 schema that drifts from the utf8mb4 +production target. Nothing fails at create time; it surfaces later as mangled +characters, or as a join between a utf8mb4 and a latin1 column that cannot use an +index. + +DYNAMIC row format is the other half: it keeps utf8mb4 indexes under the 767-byte +prefix limit on pre-5.7 InnoDB, which is what error 1071 ("key too long") is. + +This lived inline in migrations/env.py, so it covered the CORE chain only. Plugin +chains (ADR-008) run through shopdb/plugins/alembic_template.py, which never +imported it - so a plugin's baseline tables were created at the server default +while core's were utf8mb4, on the same database. Both import this now. + +Scoped to the mysql dialect, so the SQLite test database is untouched. +""" +from sqlalchemy.ext.compiler import compiles +from sqlalchemy.schema import CreateTable + +TABLE_SUFFIX = ( + " ENGINE=InnoDB DEFAULT CHARSET=utf8mb4" + " COLLATE=utf8mb4_unicode_ci ROW_FORMAT=DYNAMIC" +) + + +@compiles(CreateTable, "mysql") +def _mysql_create_table_utf8mb4(element, compiler, **kw): + sql = compiler.visit_create_table(element, **kw) + # An explicit CHARSET in the model's __table_args__ wins - this is a default, + # not an override. + if "CHARSET" not in sql.upper(): + sql = sql.rstrip().rstrip(";") + TABLE_SUFFIX + return sql diff --git a/tests/test_installer_defaults.py b/tests/test_installer_defaults.py new file mode 100644 index 0000000..1d479eb --- /dev/null +++ b/tests/test_installer_defaults.py @@ -0,0 +1,55 @@ +"""The installer wizard's pre-ticked feature list must match the plugin manifests. + +The wizard cannot read manifest.json - Inno's Pascal Script has no JSON parser - +so the default set is a hardcoded list in ShopDBFlask.iss. It drifted: two plugins +that ship "default_enabled": false were pre-ticked, so every site taking the +defaults installed and enabled them against their own manifests. + +A hardcoded list is fine; a hardcoded list nobody checks is not. +""" +import json +import re +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[1] +ISS = REPO / 'deploy' / 'windows' / 'installer' / 'ShopDBFlask.iss' +PLUGINS = REPO / 'plugins' + +pytestmark = pytest.mark.skipif(not ISS.exists(), reason='installer not in this tree') + + +def manifests_defaulting_off(): + off = set() + for manifest in PLUGINS.glob('*/manifest.json'): + try: + data = json.loads(manifest.read_text()) + except (ValueError, OSError): + continue + if data.get('default_enabled') is False: + off.add(manifest.parent.name) + return off + + +def wizard_excludes(): + """The names PluginDefault returns False for.""" + body = re.search(r'function PluginDefault.*?\nend;', ISS.read_text(), re.S) + assert body, 'PluginDefault not found in ShopDBFlask.iss' + return set(re.findall(r"Name <> '([a-z_]+)'", body.group(0))) + + +def test_wizard_defaults_match_the_manifests(): + off = manifests_defaulting_off() + assert off, 'no plugin declares default_enabled false - has the field moved?' + assert wizard_excludes() == off, ( + 'ShopDBFlask.iss PluginDefault disagrees with the manifests.\n' + ' manifests default_enabled=false: %s\n' + ' wizard leaves unticked: %s' % (sorted(off), sorted(wizard_excludes()))) + + +def test_every_wizard_exclusion_is_a_real_plugin(): + """A typo in the .iss list silently pre-ticks the plugin it meant to exclude.""" + for name in wizard_excludes(): + assert (PLUGINS / name / 'manifest.json').exists(), ( + '%s is excluded by the wizard but has no manifest' % name)