Files
executus/tools/scope_validate.go
T
steve ee6e9ef9f8
executus CI / test (pull_request) Successful in 59s
fix: address verified gadfly P3 review (3-cloud fleet)
All 3 cloud models converged on a real access-control bug; fixed it + the
other genuine findings (the false-positives were dropped):

Security (HIGH — all 3 models):
- create_file_url skipped ValidateScope: a same-skill caller could mint a
  PUBLIC url for a file scoped to another user/run. Now runs ValidateScope
  (admin-aware), skipped only for the descendant-grant case — mirroring the
  read tools.

Other real fixes:
- ValidateScope hard-coded `false` at every call site (admin branch dead) ->
  pass inv.CallerIsAdmin (the executor sets it via the host AdminPolicy; still
  false/fail-closed when no admin). Stale "no admin flag" comment corrected.
- create_file_url: ExpiresInSeconds clamped BEFORE the *time.Second multiply
  (huge values overflowed to a negative duration that slipped under the cap,
  minting already-expired tokens); swallowed json.Marshal error now returned.
- RegisterMeta: build the default budget WITH the configured MaxPerRun (was
  NewInMemorySearchBudget(nil) -> hardcoded 10, ignoring MetaDeps.MaxPerRun).
- classify: all-zero scores no longer return a false-positive top-1 winner;
  coerceClassifyScore uses strconv.ParseFloat (rejects trailing garbage like
  "50extra" that fmt.Sscanf silently accepted).
- file_delete: honor the descendant grant (parent can clean up a worker's
  artifacts) — was the lone cross-skill-reject-outright file tool.
- meta tools: input caps truncate at a UTF-8 rune boundary (truncateUTF8), not
  mid-rune.
- think: removed the dead `var _ = fmt.Errorf` import-keeper; file_save default
  aligned to 16 MiB (matched RegisterStore).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-26 22:31:59 -04:00

114 lines
4.6 KiB
Go

// scope_validate.go centralises the storage-scope authorisation check
// shared by every v4 KV and file tool. It enforces:
//
// - "skill" — always allowed (the skill's shared, cross-caller area).
// - "user:<callerID>" — allowed if it matches inv.CallerID (or admin).
// - "user:<other>" — allowed only for admin callers.
// - "run:<runID>" — allowed if it matches inv.RunID (or admin).
// - "run:<other>" — allowed only for admin callers.
// - "root_run:<id>" — allowed if it matches inv.RootRunID (or admin):
// the dispatch tree's SHARED scratchpad, readable
// and writable by every run under one root
// (parallel sibling workers coordinate here).
// - any other shape — rejected with a descriptive error.
//
// Why a single helper (vs inline checks in each tool): the parsing rules
// must match exactly across kv_get/set/list/delete and file_save/get/
// list/delete. Centralising them means one place to fix when the
// vocabulary evolves and one place for the test matrix.
//
// Why the isAdmin parameter: the v4 Invocation does NOT carry an
// admin flag — the executor sets inv.CallerIsAdmin via the host AdminPolicy; tools pass it through
// parameter exists for tests (which exercise the admin paths) and for a
// future Invocation extension that adds an admin signal without
// breaking this helper's signature.
package tools
import (
"fmt"
"strings"
"gitea.stevedudenhoeffer.com/steve/executus/tool"
)
// ValidateScope rejects scope strings the caller is not authorised to
// access. See file-level doc for the exact ruleset.
//
// Why isAdmin is parameterised: tests pass true to verify admin paths;
// production tools currently always pass false because Invocation
// doesn't carry admin status. The gate is "you can access your own
// scope only" until a future extension threads an admin signal through
// the executor.
func ValidateScope(inv tool.Invocation, scope string, isAdmin bool) error {
if scope == "skill" {
return nil
}
if rest, ok := strings.CutPrefix(scope, "user:"); ok {
if rest == "" {
return fmt.Errorf("scope: empty user id after 'user:'")
}
if rest == inv.CallerID {
return nil
}
if isAdmin {
return nil
}
return fmt.Errorf("scope %q: cannot access another user's storage", scope)
}
if rest, ok := strings.CutPrefix(scope, "root_run:"); ok {
if rest == "" {
return fmt.Errorf("scope: empty run id after 'root_run:'")
}
// The dispatch tree's shared scratchpad. Every run in one tree
// carries the same RootRunID (stamped by both executors from the
// dispatchguard chain), so siblings spawned in parallel — even
// ephemeral workers with distinct agent IDs — validate against
// the same scope string. Storage-side, root_run scopes live in
// the shared RootRunKVPartition; this check is the isolation
// boundary between trees.
if rest == inv.RootRunID && inv.RootRunID != "" {
return nil
}
if isAdmin {
return nil
}
return fmt.Errorf("scope %q: cannot access another dispatch tree's storage", scope)
}
if rest, ok := strings.CutPrefix(scope, "run:"); ok {
if rest == "" {
return fmt.Errorf("scope: empty run id after 'run:'")
}
if rest == inv.RunID {
return nil
}
// V10: when this run is a reply continuation, the agent may
// access the PARENT run's scope. The parent's run-scope KV is
// the natural carrier for "ask user a question, save state,
// resume on reply" — without this access, every continuation
// would have to re-derive state from parent_output alone.
// Note: the parent's run-scope is subject to the v4
// auto-purge (24h after parent finished). Long-delayed replies
// will see an empty scope.
if inv.Continuation != nil && rest == inv.Continuation.ParentRunID {
return nil
}
// V14: when this run is invoked via skill_invoke /
// skill_invoke_parallel from a parent skill, the agent may
// access the PARENT run's scope. This is the natural carrier
// for the "scout fans out, parent reads consolidated state"
// pattern that deepresearch uses — research-scout writes its
// touched-URL list under run:<parent_run_id> and the parent
// reads it back during the investigate phase. Without this
// access, every parent/child handoff would have to be
// serialised through tool-result strings.
if inv.ParentRunID != "" && rest == inv.ParentRunID {
return nil
}
if isAdmin {
return nil
}
return fmt.Errorf("scope %q: cannot access another run's storage", scope)
}
return fmt.Errorf("scope %q: unknown shape; expected 'skill', 'user:<id>', 'run:<id>', or 'root_run:<id>'", scope)
}