From cc64c88c3eca99c3a43bdd8a278dbd9218d8c4bb Mon Sep 17 00:00:00 2001 From: Matt Mabrey Date: Thu, 3 Sep 2026 16:27:13 -0700 Subject: [PATCH] T10.10: audit manage_users.py promote cmd_promote() changed a user's role with no audit trail, unlike the identical change from the web Admin Console (set_user_role() -> log_event(), action "role_changed"). Not a new privilege - anyone with container-exec access already has DB access directly, D16's own trust-tier reasoning - but there was no record of who ran it or what changed. Now writes an AuditLog row matching set_user_role()'s shape, tagged via:cli (mirrors JIT provisioning's via:okta_jit) since a container shell exec carries no signed-in identity to attribute the change to. Verified against a scratch SQLite db: audit row lands correctly, role change persists, the no-such-user refusal still exits 1 clean. --- docs/waves/wave-10.md | 18 ++++++++++++++++++ server/manage_users.py | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/docs/waves/wave-10.md b/docs/waves/wave-10.md index 96223eb..b11c584 100644 --- a/docs/waves/wave-10.md +++ b/docs/waves/wave-10.md @@ -156,6 +156,24 @@ Depends only on `main` as it stands after `D15`. Not sequenced behind any other Wave 10 is complete. The three items in "Still open" below are external (security team / Okta admin), not blocked on any task in this wave. +- **T10.10 — Audit `manage_users.py promote`.** Raised in review after T10.8: + `cmd_promote()` changed a user's role with no audit trail at all, unlike the + identical role change from the web Admin Console (`app.py`'s + `set_user_role()` → `log_event()`, action `"role_changed"`). Not a new + privilege — anyone with Portainer/container-exec access to `wp_api` already + has shell access to the database directly, same trust tier D16 already named + for this command — but there was no record of who ran it or what changed. + + Built: `cmd_promote()` now writes an `AuditLog` row with the same + `action`/`detail` shape `set_user_role()` uses (`{"from": old_role, "to": + role}`), tagged `"via": "cli"` (mirrors JIT provisioning's own `"via": + "okta_jit"` tag) and `actor="cli:manage_users"` — a container shell exec + carries no signed-in identity to attribute the change to a real person, so + it names the tool rather than guessing one. Verified end to end against a + scratch SQLite database: the audit row lands with the exact expected shape, + the role change persists, and the existing "no such user" refusal still + exits 1 with no partial write. + - **T10.9 — Rollback-aware deploy runbook.** Raised after hazard review found `DEPLOY-runbook-2026-08-04.md`'s Rollback section has no case for a migration whose `downgrade()` cannot restore the data it drops — see `D17`. `T10.4`'s diff --git a/server/manage_users.py b/server/manage_users.py index 79a11d2..939ca3e 100644 --- a/server/manage_users.py +++ b/server/manage_users.py @@ -24,6 +24,7 @@ and .env resolve the same way the API does: """ import argparse import sys +import uuid from .db import SessionLocal, Base, engine from . import models, auth @@ -44,7 +45,24 @@ def cmd_promote(args) -> None: f"No user named '{args.username}'. This promotes an existing account, it " f"doesn't create one — they need to sign in through Okta at least once first." ) + old_role = u.role u.role = role + # Audited the same way a role change from the web Admin Console already is + # (server/app.py's set_user_role() -> log_event(), action "role_changed") — + # this command changes the same field and previously left no record of who + # ran it or what it changed (T10.10). "actor" can't name a real person here: + # a container shell exec carries no signed-in identity to attribute it to, + # so it's tagged as the tool itself rather than guessing. "via" mirrors JIT + # provisioning's own tag on user_created events. + db.add(models.AuditLog( + id=f"ev_{uuid.uuid4().hex[:12]}", + actor="cli:manage_users", + action="role_changed", + entity_type="user", + entity_id=u.id, + summary=u.username, + detail={"from": old_role, "to": role, "via": "cli"}, + )) db.commit() print(f"{u.username} is now {auth.ROLE_LABELS.get(role, role)}.")