d0bd3ec3d9
executus CI / test (push) Has been cancelled
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>
114 lines
4.6 KiB
Go
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)
|
|
}
|