fix(security): gate raw inspect; add systemic RBAC coverage

[malickyeu]

Summary

You asked whether the RBAC work was actually confirmed by tests, given that the
security review found a hole the tests missed. Fair challenge — so I went after the
class of gap rather than adding more of the same, and it immediately found a
High-severity pre-existing vulnerability.

GET /api/inspect/{kind} was ungated. It returns the raw Docker inspect
payload — for a container that includes Config.Env, i.e. database passwords and
API keys. Any signed-in account with no sections granted at all, even
read-only, could read the environment of any container on any host via
?host=N. Proven end to end before fixing.

Now gated by the section owning the kind (container→containers, image→images,
volume→volumes, network→networks), with an unknown kind failing closed onto
containers.

Type of change

• Bug fix
• New feature
• Docs only
• Refactor / chore

Checklist

• go test -short ./... and go vet ./... pass
• gofmt gate is clean (gofmt -l $(git ls-files '*.go') after staging)
• Frontend type-checks — N/A (no UI change)
• Rebuilt and committed web/dist — N/A (nothing under web/src changed)
• Added/updated tests for the change
• Updated docs/ and added a CHANGELOG.md entry for user-facing changes —
  CHANGELOG Security entry; no docs change needed (the gating matches what
  docs/users.md already documents)

Notes for reviewers

Read TestRBACEveryAPIRouteHasASectionDecision first. It's the reason the bug
was found and the reason the class won't recur. sectionForPath falls through to
"" (ungated) for any prefix it doesn't recognise — so a route added under a new
prefix is open to every signed-in user, with no error, no warning, and a green
build. The test walks the real chi router and fails unless every /api route
either maps to a section or sits on an explicit allowlist with a stated reason.
Adding an endpoint now forces a decision.

It flagged six routes. Five are genuinely shell-level reads and are allowlisted
with reasons (/api/version, /api/system/df, /api/stats/overview,
/api/stats/ports, /api/metrics/history — counts, series and aggregates, no
config and no env). inspect was not, and is fixed.

No UI impact. The Inspect dialog has exactly four call sites —
ContainerDetail, Images, Volumes, Networks — each on a page that already requires
the matching section, so gating it matches how it is actually used.

The other coverage added targets combinations the per-feature tests
structurally could not reach, because each of those tests checks a rule it already
knows about:

• __admin is not grantable — via a role or the per-user list. If it ever
  became grantable, holding it would mean user management, settings and LDAP.
• A read-only ROLE blocks the privileged GETs (exec, pull, push, scan)
  that isWriteRequest classifies as writes. The earlier tests only exercised the
  account-level read-only flag, so this exact combination — per-section write bit
  vs. GET-shaped write — was untested.
• Role grants don't leak between users — cheap to get wrong with a mis-keyed
  join and invisible in single-user tests.
• A matrix over all 13 sections × read/write × granted/not, rather than spot
  checks, so a section behaving differently from the rest shows up.
• The gate fails closed when grants can't be computed, forced by closing the
  store underneath it.

What I'd still call uncovered, so it isn't implied otherwise: there is no test
driving a real authenticated HTTP session through the permissions middleware
end to end — these assert the routing and the gate separately. That's a bigger
harness (login + 2FA) and belongs with the RBAC UI work, where a browser session
exists anyway.

[Copilot]

Pull request overview

This PR tightens RBAC around the raw Docker inspect endpoint and adds systemic test coverage intended to prevent future “new route accidentally ungated” regressions across the /api surface.

Changes:

• Gate GET /api/inspect/{kind} by the owning RBAC section (container/images/volumes/networks) with unknown kinds failing closed.
• Add systemic RBAC coverage tests, including adversarial pentests for grantability, GET-shaped writes, grant isolation, and fail-closed behavior.
• Document the security fix in CHANGELOG.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
internal/api/rbac_coverage_test.go	Adds router-walk RBAC decision coverage plus RBAC pentests/matrix tests (currently contains a test setup bug and some misleading allowlist entries).
internal/api/access_middleware.go	Adds inspect-kind → section mapping so /api/inspect/{kind} is no longer implicitly ungated.
CHANGELOG.md	Adds a Security entry describing the raw inspect gating change and its impact.

Comments suppressed due to low confidence (1)

internal/api/rbac_coverage_test.go:75

• srv := &Server{cfg: config.Config{}} leaves srv.mw nil, but Handler() unconditionally calls r.Use(s.mw.RequireSession), so this test will panic before it can walk routes. Initialize srv.mw (a minimal token manager is fine since the test never serves requests).

func TestRBACEveryAPIRouteHasASectionDecision(t *testing.T) {
	srv := &Server{cfg: config.Config{}}
	h, ok := srv.Handler().(chi.Routes)
	if !ok {
		t.Fatal("the root handler is not a chi.Routes; cannot enumerate routes")
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/api/rbac_coverage_test.go:59

• ungatedRoutes is documented as "the /api paths deliberately reachable". The entries for /api/metrics, /api/notifications, /api/oauth/*, /api/.well-known/*, /api/mcp(/*) do not exist in the router, and /api/update/restart is section-gated as __admin (not actually ungated). Keeping these in the allowlist weakens this systemic coverage test and could mask a future regression in sectionForPath for the admin update routes.

	"/api/metrics":         "separate metrics-token auth",
	"/api/notifications":   "own in-app feed",
	"/api/oauth/*":         "MCP OAuth authorization server (own flow)",
	"/api/.well-known/*":   "OAuth/protected-resource metadata",
	"/api/mcp":             "bearer-authenticated MCP transport (own principal)",