Phase 0b of tenant tool config refactor. Closes 3 privilege-escalation
vulnerabilities in the same bug class as commit b419f352 (Phase 1
config.* hotfix):
- CRITICAL: PUT /v1/tools/builtin/{name} — non-master tenant admin
could overwrite global builtin_tools.settings, corrupting tool
defaults for every tenant.
- CRITICAL: POST /v1/packages/install|uninstall — non-master tenant
admin could run pip/npm/apk server-wide. Supply-chain vector.
- HIGH: POST /v1/api-keys/{id}/revoke (HTTP + WS) — tenant admin
could revoke NULL-tenant system keys because store SQL matches
(tenant_id = \$N OR tenant_id IS NULL).
Implementation:
- Export store.IsMasterScope as the single predicate; rewire Phase 1
config.* middleware to delegate (no behaviour change).
- Add http.requireMasterScope helper symmetric to requireTenantAdmin.
- Guard handleUpdate (builtin_tools) and handleInstall/handleUninstall
(packages) with master-scope check before any mutation or shell exec.
- Fix api_keys.Revoke at the handler layer: fetch key via new
APIKeyStore.Get, verify key.TenantID matches caller tenant for
non-owner callers. Applies to both HTTP and WS paths.
- Harden WS router to inject role into ctx so store.IsOwnerRole works
from WS handlers (closes a latent drift between the HTTP and WS
layers that broke the initial WS api_keys.revoke fix).
- Drop unused APIKeyStore.Delete (YAGNI + removes dormant vuln with
the same tenant_id IS NULL arm).
- Emit security.tenant_scope_violation and security.api_key_revoke_
forbidden slog events on every rejection for future SIEM alerting.
- New MsgMasterScopeRequired i18n key + en/vi/zh catalogs.
Tests cover the guard predicate, all 3 HTTP endpoints, and the full
WS api_keys.revoke matrix (cross-tenant deny, system-key deny for
tenant admins, system owner bypass, own-tenant happy path). 14 other
admin-gated write endpoints verified safe by static audit.
Tenant config changes for builtin tools and skills silently failed to
invalidate cached agent Loops, leaving tenants stuck on stale tool/skill
sets until the 10-minute TTL expired or an unrelated event wiped the
cache. Master-level skill CRUD had the same gap in the opposite
direction.
- Add CacheInvalidatePayload.TenantID so events can scope to one tenant
- Add Router.InvalidateTenant(tenantID) with prefix match on
"tenantID:agentKey" cache keys; uuid.Nil is a no-op
- Rework emitCacheInvalidate helpers in builtin_tools + skills handlers
to carry tenant scope; add defense-in-depth uuid.Nil guards in four
tenant-config handlers
- Update TopicCacheBuiltinTools + TopicCacheSkills subscribers to branch
on payload.TenantID (tenant event wipes that tenant, global event
keeps the existing InvalidateAll path)
- Wire emitCacheInvalidate into master skill CRUD paths (update, delete,
toggle, upload, install-deps, rescan-deps, import) that previously
only called BumpVersion
- Document the system-owner bypass in requireTenantAdmin so handlers
keep guarding uuid.Nil themselves
Prevent cross-tenant config manipulation by verifying caller has
owner/admin role within the target tenant before allowing set/delete
operations. Also adds MaxBytesReader, audit trail, and fixes middleware
consistency between skills and builtin_tools endpoints.
viettranx