diff --git a/docs/waves/backlog.md b/docs/waves/backlog.md index 2e1aaba..df784c5 100644 --- a/docs/waves/backlog.md +++ b/docs/waves/backlog.md @@ -590,3 +590,36 @@ deliberately deferred. `CLAUDE.md` forbids. Recording it is not the same as reopening it. - **Suggested wave or follow-up:** its own item and its own decision, with Nick and whoever administers the Okta tenant. Not a widening of D13. + +### BL-028 — `assets_check` fails on any machine that has `MICRON_DB_URL` set + +- **Found during:** `T10.7` (D13), running the full suite +- **Where:** `tests/assets_check.py`, the "no `MICRON_DB_URL`" case +- **What:** the check asserts `/api/assets` answers `configured:false` when the catalog + is not configured, but `start_server` passes the ambient environment through. On a + developer machine whose `.env` sets `MICRON_DB_URL` — which is the normal state for + anyone who has ever used the asset picker — the API is genuinely configured, returns + real Micron tags, and three checks fail. Nothing is wrong with the app; the test's + premise is violated by the environment it runs in. +- **Fix:** pop `MICRON_DB_URL` from the env for that server, exactly as `start_server` + now pops `LDAP_REQUIRED_GROUP` for the same reason (`T10.7`). +- **Why not now:** it is not this wave's defect and the fix belongs with whoever owns + the asset picker's tests. Recorded so the failure is not mistaken for D13 fallout. +- **Suggested wave or follow-up:** next housekeeping pass. + +### BL-029 — `generalinfo_check` flags a pre-existing `rgba()` in the creator stylesheet + +- **Found during:** `T10.7` (D13), running the full suite +- **Where:** `html/wp-creation-styles.css:929` — `box-shadow:0 8px 24px rgba(20,30,50,.18)` +- **What:** `generalinfo_check`'s token-rule check reports "no colour literal was added + to the creator's stylesheet" and fails on `rgba(`. The literal predates this wave — + last touched by `8efe624` (F6) — and `git diff main...HEAD` shows the file untouched + by the LDAPS branch. +- **The real question is which is wrong.** `C4`'s recorded exception allows rgba + **alphas** as opacity recipes, which is arguably what a shadow is; if so the check is + too strict and should match a colour literal rather than the `rgba(` token. If not, + the shadow needs a token. Either way it is a one-line change plus a decision, and + the decision is not this wave's to make. +- **Why not now:** drive-by fixes to the token system are what `CLAUDE.md` forbids, and + this one needs the C4 exception interpreted rather than guessed. +- **Suggested wave or follow-up:** next housekeeping pass, with `C4` re-read first. diff --git a/docs/waves/wave-10.md b/docs/waves/wave-10.md index cae8949..1c4aec5 100644 --- a/docs/waves/wave-10.md +++ b/docs/waves/wave-10.md @@ -188,38 +188,48 @@ creating it is exactly the kind of event that record exists for. --- -### T10.5 — The required group is configurable +### T10.5 — CLOSED, NOT BUILT (Aug 24 2026): the required group stays an env var - **Items:** `D13` (3) -- **Depends on:** T10.1 -- **Blocks:** T10.8 -- **Surface:** `server/` + `html/` -- **Files:** `server/notify.py` (`DEFAULTS`), `server/app.py` (`SettingsIn`, `/api/settings`), - `html/admin.js` +- **Status:** **won't build.** `LDAP_REQUIRED_GROUP` in the environment is the answer. -**Problem:** The group has to be settable without a redeploy, but it gates login — a typo -in a text box locks the whole company out, including whoever typed it. +**Do not build this later by reading the original task and assuming it was skipped.** +It was proposed, examined and rejected on purpose, and the reasoning is below. -**Do:** Add `ldap_required_group` to `notify.DEFAULTS` and `SettingsIn`, and a field in the -Admin console's settings card next to the notification block. `LDAP_REQUIRED_GROUP` from the -environment is the initial value and the fallback when the setting is empty. +**What it was going to be:** the required group moved out of the environment into an +Admin console setting, with a validate-on-save guard that resolved the group in the +directory and confirmed the saving admin was a member. -**Validate on save, not on login.** Before persisting, resolve the group in the directory and -confirm the saving admin is themselves a member. Refuse the save with a specific message if -either check fails. That single guard is what stops the lockout scenario, so it is not -optional. Do not add it to `notify.PUBLIC_KEYS` — an unauthenticated caller must not be able -to read the group name off the login page. +**Why it is not being built:** -**Done when:** +1. **The console requirement was invented here, not asked for.** D13 criterion 3 says + *"An AD group is configured"* — not "configurable from the console". The env var + satisfies the criterion as written. +2. **The validate-on-save guard existed only to defend against a risk the console + itself introduced.** A feature whose complexity exists to defend against itself is + usually the wrong feature. +3. **The lockout scenario it defended against is already handled.** A group that does + not resolve raises `LookupError` in `member_of`, which `verify()` maps to + `GROUP_NOT_FOUND`, which `is_config_problem` classifies as ours — so `login()` + answers **503**, not 401, and the log says *"required group 'X' does not resolve in + DC=prime,DC=local — refusing the sign-in. This is a configuration fault, not a bad + password."* A genuine non-member still gets 401. The two are already distinguishable + in both the log and the response. +4. **A redeploy is deliberate and reviewable; a text box is not.** The group is set + once and effectively never changes — it is not SMTP configuration. +5. **The console version creates a circular failure.** Fixing a lockout would require + the console the lockout prevents you from reaching. Editing the env var does not. -- [ ] the field appears in the Admin console and persists across a restart -- [ ] saving a group that does not exist is refused, with a message naming the group -- [ ] saving a group the current admin is not in is refused -- [ ] an empty setting falls back to `LDAP_REQUIRED_GROUP`, and empty-with-no-env means no group gate (documented, logged at startup) -- [ ] the group name is absent from `/api/settings` for a non-admin and from any pre-login response -- [ ] the field is reachable by keyboard, labelled, and announces its save state through the existing `aria-live` region (`C1`) +**What is genuinely lost, and accepted:** ---- +- **Discoverability.** An app admin cannot see which group is required without + Portainer or shell access. A read-only line in the Admin console diagnostics would + give the useful half without the dangerous half; it was offered and declined on + Aug 24 as not needed. +- **Deploy-time validation.** Nothing confirms the group resolves until the first + sign-in attempt. This is not fixable: resolving a group needs an authenticated + search, anonymous bind is disabled on this estate, and there is no service account + by design. The first-attempt 503 is the earliest possible detection. ### T10.6 — Strip the password UI