#1128 Close remaining review gaps: Follow-up to #1126 and #1127 Pull request opened by javuto Close remaining review gaps: last-admin guard, atomic user create, permission cleanup on env delete, UUID-like env names Follow-up to #1126 and #1127 from the same security review. Problem •

user edit

with

not_admin

had no lockout guard, so the last super-admin (or the actor) could be demoted although the permissions endpoints refuse it. CLI DB

user delete

and the shell DB store could delete the last super-admin or yourself. • Creating a user wrote the user, then its permissions. A failure in between left a user with partial permissions, no audit entry, and "already exists" on retry. • Deleting an environment left its permission rows. Recreating an environment with the same client-supplied UUID silently revived the old grants, including admin. • An environment name can look like a UUID, and

Get

matches name or UUID, so a name equal to another environment's UUID could make lookups resolve to the wrong environment. •

SetEnvLevel

and

CreatePermission

accepted environment

""

. Change • Demotion and delete guard. API user edit with

not_admin

now applies

IsSelfDemotion

then

CheckLockout

(403 self-demote, 409 last super-admin) before any write, so a refused request applies none of its edits. CLI DB

user edit --non-admin

,

user delete

, and the shell DB store use the existing

dbLockoutGuard

. API

remove

already had the protection. • Atomic create. New

UserManager.CreateWithPermissions

creates the user and its permission rows in one transaction. The API handler and CLI DB

user add

use it, and compute the admin environment list before any write. • Environment delete.

EnvManager.Delete

now hard-deletes that environment's

user_permissions

rows in the same transaction. All deletion paths go through it: API, CLI DB, and both shell stores. The raw table name is used because

pkg/users

imports

pkg/environments

. • Environment names. New

NewEnvNameFilter

rejects names that parse as a UUID, on create and rename in the API and on CLI DB

env add

. The existing name filter is unchanged so legacy environments can still be deleted. API

environments/actions

create now uses the name-or-UUID

Exists

, so a client UUID equal to an existing environment's name returns 409. • Blank environment.

CreatePermission

and

SetEnvLevel

refuse

""

. •

ARCHITECTURE.md

updated. Security / operational impact • Closes last-admin lockout via user edit and CLI delete. • No half-created users. • Behaviour changes: •

not_admin

on yourself returns 403, and on the last super-admin 409. •

user edit --non-admin

and

user delete

in CLI DB mode refuse self and the last super-admin. The error says "demote", even for delete. • A failed API user add now reports "error creating user" (500). • Environment delete removes its permission rows. • UUID-like environment names are refused on create and rename. • Existing environments with UUID-like names are not migrated or flagged. • Legacy permission rows keyed by environment name are not removed on delete. • A race between two admins demoting each other at the same moment can still reach zero admins. The existing permissions endpoints have the same check-then-write gap. Not fixed here. Validation •

go test ./cmd/api/... ./cmd/cli/... ./pkg/... -count=1

,

go vet ./...

,

gofmt

,

golangci-lint

,

make openapi-check

(annotations already list 400/403/409). • Each item has a test seen failing first (not the new atomic-create package method, which was written with its test), including: self-demote 200 instead of 403, CLI last-admin demote without error, user left behind after a failed permission insert, 4 permission rows left after environment delete, UUID-shaped name accepted with 201. • Delete cleanup is tested through CLI DB, CLI API, and both shell stores, including recreation with the same UUID granting nothing. • Three existing tests that seeded

""

rows through

CreatePermission

now insert them directly. Their intent is unchanged. Known gaps, not in this PR • Shell API store

EditUserField(admin=false)

sends

Admin:false

without

NotAdmin

, so demotion through the API-mode shell does nothing. • CLI DB

env add

has no lowercase or character-class check, unlike the API. Only the UUID check was added. jmpsec/osctrl