From 714600f2a998ebaa6a0f34eb1ec90e9fba3849d0 Mon Sep 17 00:00:00 2001 From: viettranx Date: Sun, 12 Apr 2026 10:44:05 +0700 Subject: [PATCH] docs: tenant tool config 4-tier overlay + Phase 0b security hotfix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- docs/03-tools-system.md | 59 ++++++++++++++++++++++++++++ docs/17-changelog.md | 18 +++++++++ docs/23-multi-tenant-architecture.md | 3 +- 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/docs/03-tools-system.md b/docs/03-tools-system.md index ef4600f1..106f58ea 100644 --- a/docs/03-tools-system.md +++ b/docs/03-tools-system.md @@ -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 diff --git a/docs/17-changelog.md b/docs/17-changelog.md index f56b8fca..6dc80bed 100644 --- a/docs/17-changelog.md +++ b/docs/17-changelog.md @@ -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) diff --git a/docs/23-multi-tenant-architecture.md b/docs/23-multi-tenant-architecture.md index 0e8a51b3..f5e33a28 100644 --- a/docs/23-multi-tenant-architecture.md +++ b/docs/23-multi-tenant-architecture.md @@ -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 |