Compare commits

...

6 Commits

Author SHA1 Message Date
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
7 changed files with 155 additions and 5 deletions

View File

@@ -292,7 +292,7 @@ Wave 8 adds these:
```bash ```bash
python tests/kitting_check.py # CR-009/010/012 - statuses, owner, delivery 26 checks 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/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 python tests/mreq_check.py # CR-013 - lightweight request, end to end 19 checks
``` ```

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.

View File

@@ -105,6 +105,18 @@
say(bits.join(''), problem); 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) { function importText(dryRun) {
var text = (el(p + '-paste') || {}).value || ''; var text = (el(p + '-paste') || {}).value || '';
if (!text.trim()) { say('Paste some rows or choose a CSV file first.', true); return; } 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' }, method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
body: JSON.stringify({ text: text, dry_run: !!dryRun }), 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) { .then(function (res) {
if (!res.ok) { if (!res.ok) {
say('⚠ Import refused — ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true); 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' }, method: 'POST', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
body: JSON.stringify(read.payload), 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) { .then(function (res) {
if (!res.ok) { if (!res.ok) {
setAddError((res.body && res.body.detail) || ('Could not add it (HTTP ' + res.status + ')')); 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' }, method: 'PATCH', headers: { 'Content-Type': 'application/json', 'Accept': 'application/json' },
body: JSON.stringify(patchBody), 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) { .then(function (res) {
if (!res.ok) { if (!res.ok) {
say('⚠ ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true); say('⚠ ' + esc((res.body && res.body.detail) || ('HTTP ' + res.status)), true);

View File

@@ -52,6 +52,13 @@ def run_migrations_online() -> None:
connection=connection, connection=connection,
target_metadata=target_metadata, target_metadata=target_metadata,
compare_type=True, 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(): with context.begin_transaction():
context.run_migrations() 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, rejected.append({"line": i, "text": line,
"reason": "no letters or digits to make a code from"}) "reason": "no letters or digits to make a code from"})
continue 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)) rows.append((i, parts))
return rows, rejected 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") require_project_writable(db, user, project_id, "The location list cannot be changed")
rows, rejected = parse_location_rows(body.text) rows, rejected = parse_location_rows(body.text)
read_total = len(rows) + len(rejected)
existing = {n.path: n for n in db.scalars( existing = {n.path: n for n in db.scalars(
select(models.LocationNode).where(models.LocationNode.project_id == project_id) 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: for line_no, parts in rows:
segs = [location_slug(p) for p in parts] segs = [location_slug(p) for p in parts]
full = "/".join(segs) 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: if full in seen_in_file:
duplicates.append({"line": line_no, "path": full, "names": parts, duplicates.append({"line": line_no, "path": full, "names": parts,
"reason": "already on line %d of this import" % seen_in_file[full]}) "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 = { result = {
"project_id": project_id, "dry_run": bool(body.dry_run), "project_id": project_id, "dry_run": bool(body.dry_run),
"read": len(rows) + len(rejected), "read": read_total,
"created": created, "duplicates": duplicates, "created": created, "duplicates": duplicates,
"reactivated": reactivated, "rejected": rejected, "reactivated": reactivated, "rejected": rejected,
} }
@@ -3007,6 +3030,18 @@ def parse_material_rows(text: str):
rejected.append({"line": i, "text": raw.strip()[:120], rejected.append({"line": i, "text": raw.strip()[:120],
"reason": "more than three columns - description, unit, code is the whole shape"}) "reason": "more than three columns - description, unit, code is the whole shape"})
continue 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)) rows.append((i, parts))
return rows, rejected return rows, rejected

View File

@@ -323,6 +323,13 @@ def run(page, base, tok):
{"text": "Probe Building One,Probe Level 1,Probe Sector B"}) {"text": "Probe Building One,Probe Level 1,Probe Sector B"})
chk("the import reports it as reactivated, not created or duplicate", chk("the import reports it as reactivated, not created or duplicate",
len(again["body"]["reactivated"]) == 1 and not again["body"]["created"], again["body"]) 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", same = [n for n in api(page, "GET",
"/api/projects/projA/locations?include_inactive=true")["body"]["nodes"] "/api/projects/projA/locations?include_inactive=true")["body"]["nodes"]
if n["path"] == sec["path"]] if n["path"] == sec["path"]]

View File

@@ -105,6 +105,20 @@ def main():
_, listing = api(base, "/api/projects/projA/materials", root) _, listing = api(base, "/api/projects/projA/materials", root)
chk("...and nothing was written", listing["items"] == []) 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", code, rep = api(base, "/api/projects/projA/materials/import", root, "POST",
{"text": text, "dry_run": False}) {"text": text, "dry_run": False})
_, listing = api(base, "/api/projects/projA/materials", root) _, listing = api(base, "/api/projects/projA/materials", root)