#1127 Harden permissions from security review of #1126 Pull request opened by javuto Harden permissions: ignore legacy

""

admin rows, authorize query/carve targets by UUID, audit user creation Follow-up to #1126 from its security review. Problem • Shadow admins still worked. Older releases could create AdminLevel permission rows on environment

""

. #1126 stopped new ones, but existing rows still satisfied the `AdminLevel`/`NoEnvironment` gate, so such a non-admin user passed every global-admin check (user add/edit with

admin:true

, settings, service config, and so on). • Name-keyed rows bypassed revocation. Older

user add

stored environment names as given. Query and carve creation checked permissions against the raw request string, then resolved it by name or UUID to pick target nodes. A leftover name-keyed row kept access to an environment after UUID-keyed access was revoked. • User creation audit was thin. It recorded only

add user X

, not what was granted. Change • Global gates (

pkg/users

).

CheckPermissions

and

CheckPermissionsContext

now decide any non-

UserLevel

check on

NoEnvironment

by the user's global Admin flag alone, skipping the row lookup. Legacy

""

rows are inert with no migration.

UserLevel

on

""

, real environments, and "admin row implies every level" are unchanged. Every caller of the gate was checked: all use it as a global super-admin gate, and none relies on an environment-level admin row passing it. • Query and carve creation. New

authorizeTargetEnvs

resolves each requested environment (name or UUID) before the check, authorizes against

env.UUID

, returns 404 for an unknown environment, and passes the resolved UUIDs on to targeting. No other handler had this pattern. • Targeting (

pkg/handlers

).

CreateQueryCarve

tries an exact UUID match first, so an authorized UUID cannot re-resolve to an environment that happens to be named like it. • Audit. User creation now records admin, service, resolved environment UUIDs and the four access levels, for example

add user carol admin=false service=true envs=<uuids> user=true query=true carve=false admin_access=false

. No password. The response is unchanged. • Docs.

ARCHITECTURE.md

now says

""

rows and name-keyed rows no longer grant anything, and adds a cleanup

DELETE

after the read-only detection queries. Security / operational impact • Users who relied on a

""

admin row lose all global-admin access, and Query/Carve on

""

. This is intended. Before upgrading, run the detection queries in

ARCHITECTURE.md

and decide who should be a real admin. • Users with only name-keyed rows lose access to those environments for queries and carves. Grant them by UUID. • An unknown environment on query/carve creation is now 404 (was 403). An all-whitespace entry is now 404; an exact

""

is unchanged. • Environment target records for name-based requests now store the UUID. They are display-only. • CLI callers passing a name may log one extra "record not found" from the UUID-first lookup. Validation •

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

,

go vet ./...

,

gofmt

,

golangci-lint

,

make openapi-check

. • Each fix has a test seen failing first: •

""

rows refused by both check functions, and a non-admin with one gets 403 on user add and edit-to-admin; • name-keyed query/carve refused (it returned 200/201 before), UUID grant works by name and by UUID, unknown environment gives 404, and a UUID is not re-resolved to a same-named environment; • exact audit line, with no password. • One existing test changed:

TestAPISelfDemoteRefusedForNonGlobalAdminActor

relied on a

""

row passing the gate. It still gets 403, but the message is now "no access" instead of the self-demote text. Not in this PR (from the review) • Partial user creation and audit on failure. • Environment delete leaving permission rows that revive on UUID reuse. • A name equal to another environment's UUID at create time. • No lockout guard on admin demotion via

user edit

. •

""

guards in

SetEnvLevel

and

CreatePermission

. jmpsec/osctrl