docs: tenant tool config 4-tier overlay + Phase 0b security hotfix

- docs/03-tools-system.md § 14: 4-tier overlay architecture, opt-in
  pattern for tool authors, secret vs non-secret split, cache
  invalidation, master-scope guard pattern.
- docs/17-changelog.md: concise Security + Added entries for the
  2026-04-12 hotfix and refactor.
- docs/23-multi-tenant-architecture.md: extend Per-Tenant Overrides
  table with settings column + cross-reference to § 14; add
  master-scope guard row to the Security table.

Phase 9 of tenant tool config refactor. Documents what shipped in
commits b419f352, 933c2e10, 56eb6869, 96e38c59, ed32f6e6, fbbba5e8,
6d7473b5, 1e5e84d5.
This commit is contained in:
viettranx committed 2026-04-12 10:44:05 +07:00
1 parent 1e5e84d55c
commit 714600f2a9
3 files changed
+79 -1

No files matched your search

+59
View File
@@ -698,6 +698,65 @@ The tool registry supports per-session rate limiting via `ToolRateLimiter`. When
---
## 14. Per-Tenant Tool Configuration
Tools expose configuration via a **4-tier overlay** resolved at Execute time. Callers read the merged view through `tools.BuiltinToolSettingsFromCtx(ctx)` — no tool-interface changes, no per-tenant tool instances.
### Resolution order (high → low)
```
Execute(ctx, params) → BuiltinToolSettingsFromCtx(ctx)
1. Per-agent override (agents.builtin_tool_settings)
2. Tenant override (builtin_tool_tenant_configs.settings)
3. Global default (builtin_tools.settings)
4. Hardcoded default (embedded in Go)
```
Merge is tool-name keyed: each tool's config is an independent JSON object; the highest-priority tier with a non-nil value wins (no deep merge). Resolver lives in `internal/tools/context_keys.go`.
### Opt-in for tool authors
Reading `BuiltinToolSettingsFromCtx(ctx)` auto-participates in the overlay:
```go
func (t *MyTool) Execute(ctx context.Context, params MyParams) (*Result, error) {
settings := tools.BuiltinToolSettingsFromCtx(ctx)
cfg := t.defaults
if raw, ok := settings["my_tool"]; ok {
_ = json.Unmarshal(raw, &cfg) // fall back to defaults on error
}
return t.doWork(ctx, cfg, params)
}
```
### Secret vs non-secret split
- **Non-secret** → `builtin_tool_tenant_configs.settings` (tool authors own the schema): provider priorities, `max_results`, `allowed_domains`, UI-tunable params
- **Secret** → `config_secrets` (already tenant-scoped): API keys, tokens
The overlay does **not** validate the split. Tool authors must never put credentials in `settings`. Current adopters: `web_search` (Exa/Tavily/Brave/DuckDuckGo provider chain), `create_image`/`read_image`/`create_audio`/`read_audio` (Gemini/OpenAI/Qwen chain), `web_fetch` (allowlist policy), `knowledge_graph_search` (traversal config).
### Cache invalidation
Changes propagate via `bus.TopicCacheBuiltinTools` with tenant scoping (Phase 1 refactor, commit `d77a3664`). Handler emits `emitCacheInvalidate(toolName, tenantID)` → subscribers wipe cached loop contexts → next agent turn re-resolves.
### Security: master-scope guard pattern
Writes to global tables (e.g. `PUT /v1/tools/builtin/{name}` touching `builtin_tools`) require **master tenant scope**, enforced by `http.requireMasterScope(w, r)` symmetric to `requireTenantAdmin`. Non-master tenant admins must use the `/tenant-config` endpoint. Same predicate (`store.IsMasterScope(ctx)`) guards WS `config.*`. See commit `6d7473b5` (Phase 0b hotfix) + `plans/260412-0726-tenant-tool-config-refactor/phase-0b-tenant-scope-hotfix.md`.
```go
func (h *MyHandler) handleWrite(w http.ResponseWriter, r *http.Request) {
if !requireMasterScope(w, r) { // reject non-master; 403 written
return
}
// ... global mutation
}
```
Tenant-scoped writes use the symmetric `requireTenantAdmin(w, r, tenantStore)` instead.
---
## File Reference
### Core Infrastructure
+18
View File
@@ -32,6 +32,24 @@ All notable changes to GoClaw Gateway are documented here. Format follows [Keep
## [Unreleased]
### Security
#### Tenant-Scope Hotfix (2026-04-12)
3 privilege-escalation vulnerabilities closed, same class as `b419f352` (Phase 1 `config.*` hotfix):
- **CRITICAL** `PUT /v1/tools/builtin/{name}` — non-master admin could corrupt global tool defaults
- **CRITICAL** `POST /v1/packages/install|uninstall` — non-master admin could run `pip`/`npm`/`apk` server-wide
- **HIGH** `POST /v1/api-keys/{id}/revoke` (HTTP + WS) — tenant admin could revoke NULL-tenant system keys
Fix adds shared `store.IsMasterScope(ctx)` predicate + `http.requireMasterScope` guard on all three endpoints. `APIKeyStore.Delete` dropped (YAGNI + dormant same-class vuln). WS router now injects role into ctx. Tests: 17 new unit tests. Audit: `plans/reports/debugger-260412-0922-tenant-scope-audit.md`.
### Added
#### Per-Tenant Tool Configuration — 4-Tier Overlay (2026-04-12)
Tenant admins can override tool configuration without affecting other tenants. Overlay: `per-agent > tenant > global > hardcoded`, resolved at Execute time via `tools.BuiltinToolSettingsFromCtx(ctx)` — no Tool interface changes. See `docs/03-tools-system.md` § 14. Applies to `web_search`, media tools, `web_fetch`, `knowledge_graph_search`. Web UI dialog is tenant-scope aware. Pending: Exa/Tavily provider port (Phase 7 rest), `web_fetch`/`tts` singleton refactor (Phase 8).
### Fixed
#### Feishu/Lark Writer Management Commands — Issue #818 Closed (2026-04-11)
+2 -1
View File
@@ -304,7 +304,7 @@ Tenants can customize their environment without affecting other tenants:
| Feature | Scope | How |
|---------|-------|-----|
| **LLM Providers** | Per-tenant provider configs | Each tenant registers own API keys + models |
| **Builtin Tools** | Enable/disable per tenant | `builtin_tool_tenant_configs` table |
| **Builtin Tools** | Enable/disable + settings override per tenant | `builtin_tool_tenant_configs` (`enabled` + `settings` JSONB). 4-tier overlay (per-agent > tenant > global > hardcoded) resolved at Execute time — see `docs/03-tools-system.md` § 14 |
| **Skills** | Enable/disable per tenant | `skill_tenant_configs` table |
| **MCP Servers** | Per-tenant + per-user credentials | Server-level shared, user-level overrides |
| **MCP Require User Credentials** | Per-server setting | `settings.require_user_credentials` — forces per-user API keys |
@@ -327,6 +327,7 @@ When `require_user_credentials` is enabled, users without personal credentials c
| Missing tenant context | Fail-closed: returns error, never unfiltered data |
| API key storage | Keys hashed with SHA-256 at rest; only prefix shown in UI |
| Tenant impersonation | Tenant resolved from API key binding, not client headers |
| Cross-tenant privilege escalation on global writes | `store.IsMasterScope(ctx)` + `http.requireMasterScope(w, r)` guard every admin-gated write to global tables (`builtin_tools`, package management, config.*). Symmetric `requireTenantAdmin` guards tenant-scoped writes. Predicate shared between HTTP + WS layers. See commits `b419f352` (Phase 1 WS) + `6d7473b5` (Phase 0b HTTP) |
| Privilege escalation | Role derived from key scopes, not client claims |
| Gateway token abuse | Only configured owner IDs get cross-tenant; others are tenant-scoped |
| System config access | Config page restricted to cross-tenant owners only |