diff --git a/DEPLOY-runbook-2026-08-04.md b/DEPLOY-runbook-2026-08-04.md new file mode 100644 index 0000000..53234ec --- /dev/null +++ b/DEPLOY-runbook-2026-08-04.md @@ -0,0 +1,284 @@ +# Deploy runbook — WP Suite + +**For:** IT / whoever administers the Docker host and Portainer +**From:** n.siegfried@prime-controls.com +**Revised:** 2026-08-05 — **this replaces the 2026-08-04 version.** Same procedure, +but the deploy now carries a second database migration and a new admin screen. If +you already have the earlier copy, work from this one instead. +**Expected duration:** 10–15 minutes, including the backup +**Expected downtime:** under a minute, while containers are recreated + +--- + +## Fill these in before handing this over + +| Thing | Value | +|---|---| +| Docker host (SSH target) | `________________` | +| Stack name in Portainer | `________________` | +| Site URL | `https://________________` | +| Stack directory on the host (holds `docker-compose.yml` / `backups/`) | `________________` | + +Container names are fixed by the compose file and are the same on every host: +`nginx_webserver`, `wp_api`, `wp_db`, `wp_db_backup`. + +--- + +## What this deploy changes + +Front-end and nginx changes, a new admin screen, plus pending database migrations +that run automatically. Three things make it more than a routine restart: + +1. **The nginx config and the entire `html/` directory are baked into the + container image at build time.** A plain restart deploys nothing — the stack + must be re-pulled and re-built. +2. **A pending migration rewrites existing rows** in the `users.role` column + (`b41c7ae90d52`, values `user` → `project_user`). That is why step 1 is a backup + and not optional. If a previous deploy already applied it, it will not run again — + step 0 tells you which of these you are actually about to run. +3. **A second migration adds new columns** (`a7c31f9e5b02`: an archive timestamp on + projects, and two default-membership fields on users). This one is additive and + has database defaults for existing rows, so it does not rewrite anything. + +For the people using the app, the visible changes are: projects can now be +**archived** from the Admin Console (they disappear from the pickers and go +read-only, and can be brought back), certain users can be set to join **every new +project automatically**, and the Admin Console has been rebuilt so the user table +fits on screen. + +Migrations run themselves when the `wp_api` container starts. There is nothing +to type and **no new environment variables** — do not change the stack's +environment variables. + +--- + +## Step 0 — Record the current state (needed for rollback) + +SSH to the Docker host and run: + +```bash +docker exec wp_api alembic -c server/alembic.ini current +docker inspect nginx_webserver --format 'nginx image: {{.Image}}' +docker inspect wp_api --format 'api image: {{.Image}}' +``` + +**Copy the output into your ticket.** Also note the Git commit the Portainer +stack is currently on (Portainer → the stack → the Git reference / last-updated +commit). Without these, rollback is guesswork. + +The first command prints the migration the database is currently on. Use it to see +which migrations this deploy will actually run: + +| `alembic current` shows | What will run | What that means | +|---|---|---| +| `c93f2b1d7e04` or earlier | both migrations | The `users.role` rewrite is included — the backup in step 1 matters most in this case. | +| `d15b8c4ef207` | only `a7c31f9e5b02` | The `users.role` rewrite already happened on an earlier deploy. This one is additive only. | +| `a7c31f9e5b02` | nothing | The database is already up to date; this is a code-only deploy. | + +Take the backup either way. + +--- + +## Step 1 — Back up the database + +On the Docker host: + +```bash +docker exec wp_db_backup /scripts/db-backup.sh +``` + +This triggers the stack's existing backup sidecar once, on demand. Expected +output ends with a line like: + +``` +[db-backup] wrote 1.4M /backups/wpsuite-20260804-141233Z.sql.gz.enc +``` + +Confirm the file is on the host (substitute the stack directory): + +```bash +ls -lt /backups | head -3 +``` + +**Record that filename.** Do not continue until you have seen the `wrote …` +line and the file in that listing. + +- A `.sql.gz.enc` extension means backups are encrypted — expected and correct. +- A `.sql.gz` extension plus a `WARNING: BACKUP_ENC_PASSPHRASE not set` line + means backups are unencrypted. Not a blocker for this deploy; report it back. +- **No SSH access?** Portainer → **Containers** → `wp_db_backup` → **Console** → + connect with `/bin/sh`, then run `/scripts/db-backup.sh`. Same result: the + dump lands on the host, because `/backups` is a bind mount. + +--- + +## Step 2 — Redeploy the stack in Portainer + +1. Portainer → **Stacks** → select the stack. +2. **Pull and redeploy** — with re-pull / re-build **enabled**. +3. Wait for it to report success. + +A plain "restart" or "stop/start" will **not** deploy this change. See "What +this deploy changes" above. + +--- + +## Step 3 — Confirm the containers came up + +```bash +docker ps --filter name=nginx_webserver --filter name=wp_api --filter name=wp_db +``` + +All three must be `Up`, and `wp_db` should show `(healthy)`. Then check the API +applied its migrations cleanly: + +```bash +docker logs wp_api --tail 40 +``` + +You are looking for Alembic `Running upgrade …` lines followed by gunicorn +starting up, and **no** traceback. The last one should end at `a7c31f9e5b02`. The +API deliberately refuses to start if a migration fails, so a restarting `wp_api` +container means the migration failed — go to Rollback. + +Confirm the database landed on the new revision: + +```bash +docker exec wp_api alembic -c server/alembic.ini current +``` + +Expected: `a7c31f9e5b02 (head)`. + +Then verify nginx's own view of its config: + +```bash +docker exec nginx_webserver nginx -t +``` + +Expected: `syntax is ok` / `test is successful`. + +--- + +## Step 4 — Confirm the response headers + +```bash +curl -sI https:///work-package-suite.html | grep -Ei 'cache-control|content-security-policy' +``` + +Add `-k` if the site uses an internal or self-signed certificate. + +**Both lines must come back.** Expected, approximately: + +``` +cache-control: no-cache, must-revalidate +content-security-policy: default-src 'self'; script-src 'self' 'unsafe-inline'; ... +``` + +If the `content-security-policy` line is **missing** while `cache-control` is +present, the deploy is bad — go to Rollback and send me the nginx log. (This is +the specific regression this deploy fixes; the two headers must coexist.) + +Also confirm the API is reachable through the proxy: + +```bash +curl -s https:///api/health # → {"ok": true} +``` + +--- + +## Step 5 — Hard-reload once in a browser + +Open the site and press **Ctrl+Shift+R** (Cmd+Shift+R on macOS) once. The app +uses a service worker; a normal reload can serve the previous version and make a +good deploy look broken. + +Sanity checks — all four should take under a minute: + +1. Log in. The home page offers to select or create a project. +2. Open **Admin Console** (the link is on the home page; you need an admin account). + The user table should read as **one line per user** — if rows are three lines tall + and the table spills outside its white card, you are still on the old cached + files: hard-reload again. +3. Two new cards are present and load: **Projects**, and **Default members on new + projects**. Both should list rows, not an error. +4. In the **Projects** card, click **Archive** on a project you don't mind hiding + (a `DEMO-` one if there is one), confirm the prompt, then tick **Show archived** — + it should reappear marked `archived`. Click **Unarchive** to put it back. That + round trip proves the new migration and the new endpoint are both live. + +**Deploy complete.** Please report back: the step 0 output (including which +migrations ran), the backup filename, and the two header lines from step 4. + +--- + +## Rollback + +Pick the case that matches. + +### Case A — nginx won't start, or the CSP header is missing + +The database is untouched by this, so this is a code-only rollback. In Portainer, +redeploy the stack pinned to the **previous Git commit** recorded in step 0 +(Portainer → the stack → change the Git reference to that commit → Pull and +redeploy). Then re-run step 3 and step 4. + +**Before you do:** grab the log, because it is what I need to fix this. + +```bash +docker logs nginx_webserver --tail 100 +``` + +Send me that output. If the container is in a restart loop the log still works. + +### Case B — `wp_api` is restarting / a migration failed + +```bash +docker logs wp_api --tail 100 +``` + +Send me that output. **Do not restore the database and do not roll the API back +without contacting me first.** Which migration got as far as committing decides what +is safe, and they are not the same: + +- **`a7c31f9e5b02`** (the new columns) is additive. If only this one ran, rolling + the API back to the previous image is safe on its own — the old code simply + ignores the extra columns. Nothing needs converting. +- **`b41c7ae90d52`** (the `users.role` rewrite) is not. If that one committed, + rolling the API back without converting those values back **will break logins**. + That conversion is a one-line command, but it has to match what actually ran. + +The `alembic current` output from step 0, plus the `Running upgrade …` lines in the +log above, are exactly what tells us which case you are in — please include both. + +Reach me at n.siegfried@prime-controls.com. + +### Case C — restoring the backup (only if I ask for it) + +Destructive: this drops and recreates the current schema and data. For an +encrypted dump, on the Docker host, in the `backups` directory: + +```bash +export BACKUP_ENC_PASSPHRASE='' +openssl enc -d -aes-256-cbc -pbkdf2 -pass env:BACKUP_ENC_PASSPHRASE \ + -in wpsuite-.sql.gz.enc \ + | gunzip \ + | docker exec -i wp_db psql -U wpsuite -d wpsuite +unset BACKUP_ENC_PASSPHRASE +``` + +For an unencrypted dump, drop the `openssl` stage and pipe `gunzip` straight +into `psql`. Substitute the real values if `POSTGRES_USER` / `POSTGRES_DB` are +not `wpsuite`. + +--- + +## Notes + +- Do not add or change environment variables for this deploy. +- Do not run `docker compose down -v` — the `-v` flag deletes the `pgdata` + volume and with it the entire database. +- `docker compose …` commands are avoided throughout this runbook on purpose: + for a Portainer-managed Git stack the compose project lives under Portainer's + own data directory, so `docker compose` from an SSH session usually can't find + it. The `docker exec ` form used here works from any directory. +- Full background documentation: `DEPLOYMENT.md` in the repository. diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index e8b7b85..489480b 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -209,11 +209,11 @@ users on the same project see the same server-stored SOP and Work Packages. | Table | Holds | Key columns | |-------|-------|-------------| -| `projects` | top-level construction projects | `name`, `number`, `client`, `division`, `site`, `sample`, `data` | +| `projects` | top-level construction projects | `name`, `number`, `client`, `division`, `site`, `sample`, `archived_at`, `data` | | `sops` | project SOP baselines | `project_id` → projects, `name`, `number`, `complete`, `data` (full SOP JSON) | | `work_packages` | individual IWPs | `project_id` → projects, `sop_id` → sops, `parent_id` (split instances), `number`, `subject`, `type`, `status`, `assignee_id` (owner), `issued_at`, `archived_at`, `data` (full WP JSON) | | `comments` | feedback from any page | `source`, `sop_id`, `wp_id`, `step`, `author`, `text`, `extra` | -| `users` | login accounts | `username`, `password_hash` (bcrypt), `role`, `full_name`, `email`, `is_active`, login-lockout + `token_version` fields | +| `users` | login accounts | `username`, `password_hash` (bcrypt), `role`, `full_name`, `email`, `is_active`, `auto_add_projects` + `auto_add_role` (default membership on new projects), login-lockout + `token_version` fields | | `project_members` | per-project access control | `user_id` → users, `project_id` → projects | | `audit_log` | append-only activity trail | `actor`, `action`, `entity_type`, `entity_id`, `project_id`, `summary`, `detail` | | `notifications` | in-app record + email outbox | `user_id`, `kind`, `wp_id`, `subject`, `status` (pending / sent / failed / skipped) | @@ -224,18 +224,23 @@ column; frequently-listed fields are promoted to real columns for filtering. ### Endpoints (summary) -Projects `GET/POST /api/projects`, `GET/DELETE /api/projects/{id}` · +Projects `GET/POST /api/projects`, `GET/DELETE /api/projects/{id}`, +`POST /api/projects/{id}/archive` · SOPs `GET/POST /api/sops`, `GET /api/sops/latest`, `GET/DELETE /api/sops/{id}` · Work Packages `GET/POST /api/wps`, `GET/DELETE /api/wps/{id}`, `POST /api/wps/{id}/issue`, `POST /api/wps/{id}/status`, `POST /api/wps/{id}/archive`, `GET /api/wps/metrics` · Comments `POST /api/comments` (and `/api/feedback`), `GET /api/comments` · Auth `POST /api/auth/login` / `logout`, `GET /api/auth/me`, admin user management -under `/api/auth/users` · Admin-only `GET/PUT /api/settings`, +under `/api/auth/users` (including `POST /api/auth/users/{id}/auto-add`) · +Admin-only `GET/PUT /api/settings`, `POST /api/settings/test-email`, `GET /api/notifications`, `GET /api/projects/{id}/members`. -List/latest/metrics accept a `project_id` (and `sop_id`) filter. Full reference -and request shapes: `/api/docs` and [`server/README.md`](server/README.md). +List/latest/metrics accept a `project_id` (and `sop_id`) filter. `GET /api/projects` +and `GET /api/wps` both take `archived=exclude|only|all` and **default to +`exclude`** — anything that needs to see archived rows (the admin console, the demo +cleanup) must ask for them. Full reference and request shapes: `/api/docs` and +[`server/README.md`](server/README.md). --- @@ -416,8 +421,9 @@ not get a second bar. - Switching project reloads the current page with `?project=`; every page already resolves its project from that parameter. - Search calls `GET /api/search?q=`, which is **scoped to the caller's projects** - (`scope_to_access`) and hides archived work packages. LIKE wildcards in the query - are escaped, so searching `100%` matches a literal `100%`. Two-character minimum. + (`scope_to_access`) and hides archived work packages, archived projects, and + anything belonging to an archived project. LIKE wildcards in the query are escaped, + so searching `100%` matches a literal `100%`. Two-character minimum. - Ctrl/Cmd-K focuses the field from anywhere. ## Schema migrations (Alembic) @@ -470,6 +476,53 @@ the project's members plus app admins, each with their effective role on that project. A project with nobody assigned shows only the admins, which is why assigning people is the first step on a new job. +### Default members on new projects + +Memberships are also created automatically. **Admin console → Default members on +new projects** flags accounts (`users.auto_add_projects`) that belong on every job — +the PM who runs them all, the QC lead — with the role they should hold there +(`users.auto_add_role`, sharing `project_members.role`'s value space, `''` = +inherit the account's own). + +- It applies **only to projects created after the flag is set**. Nothing is + back-filled onto existing jobs; use **Project access** for those. +- App admins are skipped (they already reach every project) and the flag is cleared + if an account is promoted to admin. Inactive accounts are skipped. +- Runs in `add_default_members()` on the `is_new` branch of `upsert_project`, so it + covers every route into project creation — the home page, the sample project, the + demo seeder. An update never re-runs it. +- If the creator is themselves a flagged member, the membership created for them as + creator carries their `auto_add_role`, so they aren't silently downgraded on the + one job they started. +- Audit-logged once per project as `project_access_granted` with + `detail.reason = "auto_add_projects"`. + +## Archiving a project + +A finished job is archived rather than deleted: `projects.archived_at`, set from +**Admin console → Projects** (or `POST /api/projects/{id}/archive`, which needs +Project Admin **on that project**, same bar as deleting it). + +An archived project is **hidden and frozen**: + +- It leaves the home picker, the app-bar switcher and global search, because + `GET /api/projects` defaults to `archived=exclude`. +- It is still readable by id, so a deep link renders it — with a read-only banner + from `wp-chrome.js` — and the admin console still lists it under + `?archived=all`. +- Every write that lands on it is refused with **409** by + `require_project_writable()`: saving a project, SOP or work package, deleting + either, issuing, status changes, WP archiving, and comments on its WPs/SOPs. + Moving a work package *into* or *out of* an archived project is refused too. + 409 rather than 403 is deliberate — nobody lacks a permission, the project's state + is the objection, and the browser outbox (`html/project-data.js`) retires 4xx ops + instead of retrying them forever. +- Unarchiving and **deleting** stay allowed: unarchive is the one write an archived + project must accept, and archive-then-delete is a normal sequence. + +Nothing is removed, and unarchiving restores all of it. `server/smoketest.py` +asserts the whole round trip. + ## Asset freshness (why the app can't run half-updated) A page must never run against a stylesheet or script from a previous deploy. Three diff --git a/KNOWN-ISSUES.md b/KNOWN-ISSUES.md new file mode 100644 index 0000000..aa88274 --- /dev/null +++ b/KNOWN-ISSUES.md @@ -0,0 +1,148 @@ +# Known issues — Work Package Suite + +Defects and limitations we know about and have decided not to fix yet. An entry +here is a commitment to a decision, not a bug tracker: it says what is wrong, what +it costs, why it is still open, and what closing it takes. + +Anything genuinely urgent does not belong here — it belongs in the next deploy. + +Close an entry by deleting it in the same commit that fixes it. + +| # | Issue | Severity | Raised | Status | +|---|-------|----------|--------|--------| +| 1 | XSS via SOP discipline names in the WP creator | Medium (internal), High if externally reachable | 2026-08-05 | Open | +| 2 | Archived projects: the two big apps don't grey out their own controls | Low | 2026-08-05 | Open | + +--- + +## 1. XSS via SOP discipline names in the WP creator + +**Files:** `html/wp-creation-app.js` lines 684, 723, 727, 729 · escaping helper at +line 63 +**Predates:** the 2026-08-05 archive/admin-console work. Not introduced by it. + +### What is wrong + +Discipline names are rendered into inline event handlers escaped with `esc()`, +which maps `'` to `'`. That is correct for text and wrong here. The browser +decodes entities in an attribute value **before** the JavaScript parser sees it, so +`'` becomes a bare `'` inside the handler's string literal and closes it early. + +```js +// html/wp-creation-app.js:684 — esc() is not sufficient for a handler argument +onchange="toggleDiscipline('${esc(d)}',this.checked)" +``` + +Escaping for an inline handler has to happen in this order: **backslash, then +quote** (for the JS string literal), **then HTML** (for the attribute carrying it). +`esc()` only does the last part. + +### How it is reached + +1. `gov_disciplines` (`html/work-package-suite.html:219`) is a free-text field. Its + value is comma-split with no validation at `work-package-suite-app.js:1224`. +2. It is saved into `sops.data` and syncs to the server via `ProjectData.pushSOP`. +3. Every other member of that project pulls it with `pullProject()` and renders it + in the WP creator — so this is **stored** and **cross-user**, and it fires on + page load rather than needing the victim to click anything. + +Any **project_user** on the job can set it while the SOP is a draft (after the SOP +is marked complete it takes project_admin). The victim is anyone who opens the WP +creator for that project, which includes administrators. + +### What it costs + +**The likely cost is a broken screen, not an attack.** A discipline named +`Owner's Equipment` — an ordinary thing to type — produces a syntax error in the +handler, so the discipline pill and its scope-step buttons silently stop +responding. No error message, nothing a field user can diagnose. + +**The security ceiling is project_user → admin.** The session cookie is HttpOnly so +the token cannot be read, but the injected code does not need it: it runs in the +victim's page and can call any API the victim can, including +`POST /api/auth/users/{id}/role`. + +Two controls that look like they would contain this do not: + +- **CSP does not mitigate it.** `nginx-wp-suite.conf:58` serves + `script-src 'self' 'unsafe-inline'`, and `'unsafe-inline'` is what permits inline + event handlers in the first place. +- **The CSRF gate does not mitigate it.** `_csrf_ok` (`server/app.py:67`) only + requires a same-origin `Origin`, and code running inside our own page is + same-origin. + +### Why it is still open + +The suite is internal, behind a login, on the corporate network, with a small set +of named employee accounts and no anonymous input path. Exploiting it means an +employee deliberately attacking colleagues, and the audit log carries their name on +the SOP edit. The accidental-breakage case is far more likely to be met than the +malicious one. + +**Re-rate this as High and fix it immediately if any of these become true:** the +suite is exposed outside the corporate network, accounts are issued to +subcontractors or clients, or self-registration is added. + +### What closing it takes + +Small — roughly half an hour. The helper already exists; it was added to the SOP +builder on 2026-08-05 for the same bug in custom constraint names: + +```js +// html/work-package-suite-app.js:1120 +function escHandlerArg(v){ return escAttr(String(v==null?'':v).replace(/\\/g,'\\\\').replace(/'/g,"\\'")); } +``` + +1. Add the same helper to `html/wp-creation-app.js` alongside `esc()`. +2. Use it at lines 684, 723, 727 and 729 in place of `esc(d)`. +3. Sweep the other inline handlers in that file for the same pattern. The remaining + ones interpolate server-generated ids that `check_id()` already constrains to a + safe charset, or hardcoded enum values, so they are not currently reachable — + converting them anyway keeps the pattern from coming back. +4. Confirm with a discipline named `Owner's Equipment`: the pill must respond to + clicks and the name must display intact. + +The equivalent fix on the admin side is `jsq()` in `html/admin.js` — same ordering, +same reasoning, worth reading before starting. + +--- + +## 2. Archived projects: the two big apps don't grey out their own controls + +**Files:** `html/wp-creation-app.js`, `html/work-package-suite-app.js` +**Raised:** 2026-08-05, with the project-archiving work. + +### What is wrong + +Archiving a project freezes it server-side — every write returns 409 (see +`require_project_writable` in `server/app.py`, and the *Archiving a project* +section of `DEPLOYMENT.md`). The front end tells the user, but does not stop them: +`wp-chrome.js` shows a read-only banner and sets `data-wp-archived="1"` on the +document element, and nothing reads that attribute yet. So on an archived project +the WP creator and the SOP builder still present working Save and Issue buttons. + +### What it costs + +Low, and it fails safe — the server refuses the write, so nothing is corrupted and +no data is lost. The cost is wasted effort and a confusing moment: someone deep- +linked to an archived job can fill in a form and only learn it was refused when the +sync indicator reports the change did not save. + +Reaching an archived project at all takes a deep link or a stale tab, since it is +gone from every picker, switcher and search — which is why this is a rough edge +rather than a defect. + +### Why it is still open + +Gating every control in two large single-page apps is materially bigger than the +archive feature itself, and the server is the real enforcement boundary either way. +The banner plus the sync indicator were judged enough for a first release. + +### What closing it takes + +`data-wp-archived` is already on the document element for exactly this purpose. +Either add `[data-wp-archived]` rules in `wp-chrome.css` that disable and dim the +save/issue controls, or add a boot check in each app that disables them and shows a +read-only notice inline. Decide separately how the embedded creator +(`wp-creation-index.html`) surfaces it, since it runs in an iframe where the shared +app bar — and therefore the banner — is deliberately skipped. diff --git a/html/admin.html b/html/admin.html index 4fbdcbc..43a6214 100644 --- a/html/admin.html +++ b/html/admin.html @@ -14,58 +14,198 @@ @@ -82,34 +222,34 @@