Compare commits

...

9 Commits

Author SHA1 Message Date
044862acba Wave 10: task breakdown for the Okta OIDC build 2026-09-02 17:38:35 -07:00
31c548318b D15: retire D13/D14 (LDAPS, never deployed), move straight to Okta OIDC 2026-09-02 17:20:01 -07:00
8f117680b0 Merge branch 'fix/alembic-transaction-per-migration': truthful migration logs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-01 15:35:36 -07:00
6034c08bad Alembic runs one transaction PER MIGRATION, not one for the whole chain
The 2026-08-21 crash at material_items printed 'Running upgrade' lines for
location_nodes and wp_files and then rolled all three back together - env.py
wrapped the entire run in a single transaction. The repair that followed
trusted those lines: material_items was hand-created, the version stamped to
head, and production ran for two days missing two tables it claimed to have.
Found 2026-08-23 when the locations import 500'd on UndefinedTable.

transaction_per_migration=True makes the log truthful: a crash keeps every
step that completed, and a stamp-to-head repair after a crash repairs ONE
migration, not an unknowable prefix of the chain. Verified: the full chain
still applies on a fresh scratch SQLite; the offline --sql render is
unchanged.

The production surgery (creating the two rolled-back tables from the offline
postgres render) is recorded in the session; no version stamp is needed
there - it is already, now truthfully, at head.

Items: BL-027's class, third finding; env.py infrastructure.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-01 15:35:36 -07:00
17cabbd032 Merge branch 'fix/import-row-hazards': imports reject rows, never 500
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-01 15:26:38 -07:00
222c0b1c29 CR-005/D6 fix - a bad CSV row rejects by line number instead of 500ing Postgres
Nick's real location list hit the production import and got 'Internal Server
Error' with no line number - BL-027's class again, three days after the
migration outage: Postgres enforces VARCHAR lengths and refuses control
bytes, SQLite shrugs at both, and the importers were only ever rehearsed on
SQLite. Reproduced both hazards locally (an over-long value and a NUL byte
import cleanly on SQLite; either 500s Postgres wholesale).

Both importers now validate per row, before any INSERT, so every dialect
answers the same way - with the line number and a reason:
- locations: control characters; names over 200; codes over 60; combined
  paths over 200 (checked where the path exists, with read-counts taken
  before the loop so a mid-loop rejection is not counted twice)
- materials: control characters; description/unit/code over 300/20/80

And the client stops lying about it: wp-list-import.js read every response
with r.json(), so a plain-text 500 threw mid-parse and surfaced as 'Could not
reach the server' while the server was answering fine. One tolerant reader
(text -> parse if it parses -> keep status) now serves import, add and patch;
a real error reads 'Import refused - HTTP 500'.

Pins: materials_check +2 (over-long and control-byte rows reject at line,
20/20), locations_check +1 (over-long name rejects at line, 59/59).

Items: CR-005, D6, BL-027 (second instance of its class; the probe-side
dialect guard it proposes is still open).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-09-01 15:26:37 -07:00
e31234beef Merge branch 'docs/bl-026-027': the outage's two lessons, backlogged
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-21 12:20:32 -07:00
6057d05b98 BL-026 / BL-027 - the two lessons of the Aug 21 outage, logged
BL-026: no version stamp - 'is live current?' took mid-outage fingerprinting;
the fix shape is a git SHA baked at build, served by /api/health, shown on
the admin diagnostics card (D13 candidate).

BL-027: migrations rehearsed on SQLite only - the dialect drift class behind
the outage stays unguarded beyond the one pinned instance; the fix shape is
an offline postgresql-dialect render in the runbook and/or a probe.

Items: backlog only, no code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-21 12:20:30 -07:00
64eac0cbbb Merge branch 'fix/material-items-boolean-default': the Postgres deploy hotfix
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-21 12:05:27 -07:00
9 changed files with 250 additions and 5 deletions

View File

@@ -292,7 +292,7 @@ Wave 8 adds these:
```bash
python tests/kitting_check.py # CR-009/010/012 - statuses, owner, delivery 26 checks
python tests/kitting_notify_check.py # CR-011 - kitting mail, coalesced, gated 17 checks
python tests/materials_check.py # D6 - material list, the CR-005 pattern 17 checks
python tests/materials_check.py # D6 - material list, the CR-005 pattern 20 checks
python tests/mreq_check.py # CR-013 - lightweight request, end to end 19 checks
```

View File

@@ -554,3 +554,43 @@ deliberately deferred.
entry rather than a drive-by.
- **Suggested wave or follow-up:** next housekeeping pass, with the check
widened so it cannot recur.
### BL-026 — No version stamp: "is live current?" cannot be answered from the app
- **Found during:** the 2026-08-21 outage triage (the question that started it)
- **Where:** `Dockerfile` / build, `server/app.py` `/api/health`, admin console
- **What:** the app carries no record of what code it is running. `/api/health`
returns `{"ok": true}` and nothing identifies the deployed commit, so
answering "is the live site on the latest code?" took fingerprinting
(probing for files/routes that only exist after certain merges) in the
middle of an outage. The fix: bake the git SHA into the image at build time
(`ARG GIT_SHA`), return it from `/api/health`
(`{"ok": true, "version": "<sha>"}`), and show it on the admin console's
diagnostics card. Then currency is one glance against `git log -1`.
- **Why not now:** new scope — needs its own item id per the working rules
(D13 is the natural next), and it touches the image build, which deserves a
deploy alongside someone with host access.
- **Suggested wave or follow-up:** next housekeeping pass; ~1 task including a
probe check that /api/health carries a version field.
### BL-027 — Migrations are rehearsed on SQLite only; production is Postgres
- **Found during:** the 2026-08-21 production outage (D6's `material_items`
migration crash-looped the api container)
- **Where:** `DEPLOYMENT.md` (the update/deploy steps), `tests/`
- **What:** the migration chain is verified end-to-end on scratch SQLite, but
production runs Postgres, and the dialects disagree exactly where it hurts:
`server_default=sa.text('1')` on a Boolean passed every SQLite rehearsal and
was refused by Postgres at deploy (DatatypeMismatch), taking the API down
until the table was created by hand. The hotfix (64eac0c) fixed that one
instance and pinned the Boolean-default class in `materials_check`; the
CLASS of dialect drift is still unguarded. Two cheap layers: (1) a runbook
step — render `alembic upgrade --sql` for the postgresql dialect and read it
before restarting (offline, needs no live DB; this render would have shown
`DEFAULT 1` on a boolean); (2) better, a probe that renders every migration
for the postgresql dialect on each run and fails on anything the dialect
rejects or on known-bad patterns.
- **Why not now:** the outage is resolved and the one known instance is fixed
and pinned; the systematic guard is its own small task, not a hotfix rider.
- **Suggested wave or follow-up:** next housekeeping pass, paired with BL-026
(both are "deploys should be boring" work).

View File

@@ -0,0 +1,75 @@
# Decisions — September 2, 2026
One item, and it retires two decided-and-built items rather than amending them.
---
## D15 — Authentication moves to Okta OIDC; D13 and D14 are retired before deployment
- **Amends:** retires `D13` (LDAPS simple bind against `prime.local`) and `D14` (the CLI
authenticates against the domain). Both were decided and reaffirmed August 21 2026,
built across nine tasks (`T10.1``T10.9`), and verified against the live domain. Neither
reached production. Approved by Nick Siegfried.
- **Surface:** `server/auth.py`, `server/app.py` (`login()`), `html/login.html`,
`html/login.js`, `html/auth-guard.js`. `server/ldap_auth.py` does not carry forward —
there is no LDAPS bind in the new design, not even as a fallback.
- **Wave:** 10. The label is reused fresh: the LDAPS work that previously answered to
"wave 10" was built on `feat/ldaps-directory-auth`, which is deleted rather than merged,
and never appeared in `IMPLEMENTATION.md`'s wave table. It carries no claim on the
number.
### What D13/D14 were, for the record
The branch carrying them is deleted, not merged, so their decision record
(`docs/waves/decisions-2026-08-21.md`) no longer exists on any branch. Preserved here so
the reasoning isn't lost along with it:
D13 chose a direct LDAPS simple bind to `ldaps://prime.local:636` as the sign-in
mechanism: no password stored, a successful bind was the authentication, accounts were
provisioned just-in-time from the directory, and roles stayed local. D14 moved the CLI
onto the same bind, removing `create-admin`/`create`. Both were built, tested
(1284/1288 checks, Aug 24), and screenshotted at 390px and 1440px. Neither ever deployed —
`wp.controls.dev` still runs the pre-D13 local-password login as of this decision.
### The decision
Skip LDAPS entirely. Authentication becomes an Okta OIDC authorization-code flow,
replacing local passwords directly — the same full replacement D13 intended, just via
Okta instead of a domain bind. No LDAPS bind exists in this design at any point.
Four things carry forward from D13 unchanged, because they were never LDAPS-specific to
begin with:
1. **No password is stored.** The app never sees a credential of any kind; Okta owns
authentication entirely.
2. **Accounts are provisioned just-in-time.** A first successful Okta sign-in with no
matching local `users` row creates one, at the default role. The matching logic that
was going to key off a directory search instead keys off an OIDC identity claim.
3. **Roles stay local.** Okta, and AD behind it, supplies identity only. This app decides
what an identity may do. Restated because it is the one rule the whole access-control
design depends on — see `BL-029` and the governance discussion that followed it.
4. **Existing accounts keep their roles** on first Okta login, exactly as D13's criterion
4 read for LDAPS.
### Why this, and not LDAPS first and Okta second
`BL-029` (recorded on this branch as `BL-027` before the renumbering forced by main's
independent use of that number) already laid out why OIDC beats the LDAPS bind on three
counts: this app never sees a password, MFA comes from Okta rather than needing to be
built, and the domain-lockout hazard that forced `AUTH_MAX_ATTEMPTS` down to 2 disappears,
because failed attempts land on Okta rather than on a bind this app makes. D13 was decided
before it was known the company already runs an Okta tenant. Once that was confirmed
(security's scoping reply, September 2026), shipping LDAPS first and replacing it with
Okta days later would mean building and deploying the weaker mechanism on purpose. Going
straight to Okta avoids that.
### What still needs answering before this is buildable
Open from the security scoping thread, not yet closed:
- Which OIDC claim carries the AD `sAMAccountName` equivalent (`preferred_username`,
`upn`, or a custom claim) — asked of security, answer pending.
- The exact redirect/callback URI once the hostname situation is reconfirmed
(`https://wp.controls.dev/api/auth/okta/callback` proposed).
- The `Business Technology Group` pilot in Okta, requested for initial testing, with
normal Okta session/MFA behavior rather than a stricter per-app rule.

55
docs/waves/wave-10.md Normal file
View File

@@ -0,0 +1,55 @@
# Wave 10 — Okta OIDC authentication
Fresh wave 10. The label was previously used by the LDAPS work under `D13`/`D14`, built on
`feat/ldaps-directory-auth`; that branch was deleted rather than merged and never appeared
in `IMPLEMENTATION.md`'s wave table, so it carries no claim on the number. See
`docs/waves/decisions-2026-09-02.md` (`D15`) for why LDAPS was retired before deployment
and Okta chosen instead.
Depends only on `main` as it stands after `D15`. Not sequenced behind any other wave.
## Tasks
- **T10.1 — Add the Okta OIDC client.** Authlib as a dependency. Config via env vars
(`OKTA_ISSUER`, `OKTA_CLIENT_ID`, `OKTA_CLIENT_SECRET`, `OKTA_REDIRECT_URI`), same
pattern `AUTH_SECRET_KEY` already uses in `server/auth.py`.
- **T10.2 — Login-redirect and callback routes.** A route that sends the browser to
Okta's authorize endpoint, and a callback route that exchanges the code for tokens and
validates the ID token. Access gating is Okta's job, not this app's: only accounts
assigned to the app integration in Okta can reach it at all, so there is no app-side
required-group or claim check layered on top. This is a deliberate difference from D13,
which had to gate on a required AD group itself because an LDAPS bind alone could not
distinguish an assigned user from any other domain account.
- **T10.3 — Identity matching and JIT provisioning.** Reuses D13's shape
(`_provision_from_directory`-style matching) keyed off an OIDC claim instead of an LDAP
search result. **Open dependency:** which claim carries the AD `sAMAccountName`
equivalent (`preferred_username`, `upn`, or a custom claim) is asked of security and not
yet answered. Build with a configurable claim name and a documented default, not a
hardcoded one, so the answer can drop in without a code change.
- **T10.4 — Remove the local password path entirely.** Drop `password_hash`, remove the
bcrypt-based `login()`, remove the username/password form. Real deletion, matching
`D15`'s "full replacement," not a toggle or a fallback.
- **T10.5 — Frontend: login becomes a redirect, not a form.** `login.html`/`login.js`
change to a "Sign in with Okta" flow. Sign-out lands back on the app's own login page.
- **T10.6 — Deployment docs and env var reference.** `DEPLOYMENT.md`,
`server/.env.example`, `server/README.md` describe the Okta config in place of the LDAP
config they never ended up describing (D13 never shipped, so these still describe the
original local-password system today).
- **T10.7 — Test coverage without a live Okta dependency.** A fake-OIDC-provider test
seam, mirroring `ldap_fake.py`, so the suite runs with no live Okta tenant reachable.
- **T10.8 — Verification.** 390px and 1440px, full suite, done-when checks per task,
matching the rigor D13 was held to.
## Still open
- The OIDC claim mapping (`T10.3`).
- Final confirmation of the redirect/callback URI (`https://wp.controls.dev/api/auth/okta/callback`
proposed, pending security).
- The `Business Technology Group` pilot assignment in Okta.

View File

@@ -105,6 +105,18 @@
say(bits.join(''), problem);
}
// A 500 answers plain text ("Internal Server Error"), and r.json() on that
// throws - which used to land in catch() and read as "could not reach the
// server" while the server was answering fine (found 2026-08-23, the
// production locations import). Read text, parse if it parses, keep status.
function readJson(r) {
return r.text().then(function (t) {
var j = null;
try { j = t ? JSON.parse(t) : null; } catch (e) { /* not JSON: a proxy or 500 page */ }
return { ok: r.ok, status: r.status, body: j };
});
}
function importText(dryRun) {
var text = (el(p + '-paste') || {}).value || '';
if (!text.trim()) { say('Paste some rows or choose a CSV file first.', true); return; }
@@ -114,7 +126,7 @@
method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
body: JSON.stringify({ text: text, dry_run: !!dryRun }),
})
.then(function (r) { return r.json().then(function (j) { return { ok: r.ok, status: r.status, body: j }; }); })
.then(readJson)
.then(function (res) {
if (!res.ok) {
say('⚠ Import refused — ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true);
@@ -142,7 +154,7 @@
method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
body: JSON.stringify(read.payload),
})
.then(function (r) { return r.json().then(function (j) { return { ok: r.ok, status: r.status, body: j }; }); })
.then(readJson)
.then(function (res) {
if (!res.ok) {
setAddError((res.body && res.body.detail) || ('Could not add it (HTTP ' + res.status + ')'));
@@ -161,7 +173,7 @@
method: 'PATCH', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
body: JSON.stringify(patchBody),
})
.then(function (r) { return r.json().then(function (j) { return { ok: r.ok, status: r.status, body: j }; }); })
.then(readJson)
.then(function (res) {
if (!res.ok) {
say('⚠ ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true);

View File

@@ -52,6 +52,13 @@ def run_migrations_online() -> None:
connection=connection,
target_metadata=target_metadata,
compare_type=True,
# Each migration commits on its own. One transaction for the WHOLE
# run meant a crash at step N rolled back steps 1..N-1 while their
# "Running upgrade" lines stayed on screen claiming they ran - the
# 2026-08-21 outage's stamp-to-head repair trusted those lines and
# left production missing two tables (found 2026-08-23 when the
# locations import 500'd on a table that "had been created").
transaction_per_migration=True,
)
with context.begin_transaction():
context.run_migrations()

View File

@@ -2388,6 +2388,24 @@ def parse_location_rows(text: str) -> tuple[list[tuple[int, list[str]]], list[di
rejected.append({"line": i, "text": line,
"reason": "no letters or digits to make a code from"})
continue
# Postgres enforces VARCHAR lengths and refuses NUL/control bytes;
# SQLite shrugs at both - which is how ONE bad CSV line 500'd the whole
# production import (2026-08-23, BL-027's class again) instead of coming
# back as a rejection with its line number. Validate per row, here,
# so every dialect answers the same way: with a reason.
if any(any(ord(ch) < 32 for ch in p) for p in parts):
rejected.append({"line": i, "text": line[:120],
"reason": "contains control characters — re-save the file as plain CSV (UTF-8)"})
continue
long_p = next((p for p in parts if len(p) > 200), None)
if long_p is not None:
rejected.append({"line": i, "text": line[:120],
"reason": "a name is longer than 200 characters (%d)" % len(long_p)})
continue
if any(len(location_slug(p)) > 60 for p in parts):
rejected.append({"line": i, "text": line[:120],
"reason": "a code would be longer than 60 characters"})
continue
rows.append((i, parts))
return rows, rejected
@@ -2454,6 +2472,7 @@ def import_locations(project_id: str, body: LocationImportIn,
require_project_writable(db, user, project_id, "The location list cannot be changed")
rows, rejected = parse_location_rows(body.text)
read_total = len(rows) + len(rejected)
existing = {n.path: n for n in db.scalars(
select(models.LocationNode).where(models.LocationNode.project_id == project_id)
@@ -2468,6 +2487,10 @@ def import_locations(project_id: str, body: LocationImportIn,
for line_no, parts in rows:
segs = [location_slug(p) for p in parts]
full = "/".join(segs)
if len(full) > 200:
rejected.append({"line": line_no, "text": "/".join(parts)[:120],
"reason": "the combined path is longer than 200 characters"})
continue
if full in seen_in_file:
duplicates.append({"line": line_no, "path": full, "names": parts,
"reason": "already on line %d of this import" % seen_in_file[full]})
@@ -2510,7 +2533,7 @@ def import_locations(project_id: str, body: LocationImportIn,
result = {
"project_id": project_id, "dry_run": bool(body.dry_run),
"read": len(rows) + len(rejected),
"read": read_total,
"created": created, "duplicates": duplicates,
"reactivated": reactivated, "rejected": rejected,
}
@@ -3007,6 +3030,18 @@ def parse_material_rows(text: str):
rejected.append({"line": i, "text": raw.strip()[:120],
"reason": "more than three columns - description, unit, code is the whole shape"})
continue
# Same guard as parse_location_rows: reject what Postgres would refuse
# (VARCHAR limits, control bytes) with the line number, never a 500.
if any(any(ord(ch) < 32 for ch in p) for p in parts):
rejected.append({"line": i, "text": raw.strip()[:120],
"reason": "contains control characters - re-save the file as plain CSV (UTF-8)"})
continue
caps = ((300, "description"), (20, "unit"), (80, "code"))
long_col = next((("%s is longer than %d characters (%d)" % (label, cap, len(p)))
for (cap, label), p in zip(caps, parts) if len(p) > cap), None)
if long_col:
rejected.append({"line": i, "text": raw.strip()[:120], "reason": long_col})
continue
rows.append((i, parts))
return rows, rejected

View File

@@ -323,6 +323,13 @@ def run(page, base, tok):
{"text": "Probe Building One,Probe Level 1,Probe Sector B"})
chk("the import reports it as reactivated, not created or duplicate",
len(again["body"]["reactivated"]) == 1 and not again["body"]["created"], again["body"])
# The 2026-08-23 production 500, pinned (locations side): Postgres-refused
# values reject by line, on every dialect, never crash the request.
hz = api(page, "POST", "/api/projects/projA/locations/import",
{"text": "Probe Building One," + "Y" * 220 + ",S1", "dry_run": True})
chk("an over-long name is a line rejection, not a 500",
hz["status"] == 200 and hz["body"]["rejected"]
and "200 characters" in hz["body"]["rejected"][0]["reason"], hz["body"])
same = [n for n in api(page, "GET",
"/api/projects/projA/locations?include_inactive=true")["body"]["nodes"]
if n["path"] == sec["path"]]

View File

@@ -105,6 +105,20 @@ def main():
_, listing = api(base, "/api/projects/projA/materials", root)
chk("...and nothing was written", listing["items"] == [])
# The 2026-08-23 production 500, pinned: what Postgres refuses (VARCHAR
# overflow, control bytes) must come back as a per-line rejection - on
# EVERY dialect - never crash the request.
code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
{"text": "Sample " + "x" * 300 + ",EA", "dry_run": True})
chk("an over-long description is a line rejection, not a 500",
code == 200 and rep["rejected"] and "300 characters" in rep["rejected"][0]["reason"]
and not rep["created"], ascii_(rep))
code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
{"text": "Sample widget\u0000,EA", "dry_run": True})
chk("a control byte is a line rejection, not a 500",
code == 200 and rep["rejected"]
and "control characters" in rep["rejected"][0]["reason"], ascii_(rep))
code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
{"text": text, "dry_run": False})
_, listing = api(base, "/api/projects/projA/materials", root)