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>

tools/classify.go

@@ -25,6 +25,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strconv"
 	"strings"
 
 	"gitea.stevedudenhoeffer.com/steve/executus/llmmeta"
@@ -111,7 +112,7 @@ func NewClassify(helper *llmmeta.Helper, cfg ClassifyConfig, budget SearchBudget
 			}
 
 			if len(text) > classifyMaxInputBytes {
-				text = text[:classifyMaxInputBytes]
+				text = truncateUTF8(text, classifyMaxInputBytes)
 			}
 
 			// Per-run budget gate.
@@ -255,10 +256,14 @@ func coerceClassifyScore(raw any) (float64, bool) {
 	case int64:
 		return float64(v), true
 	case string:
-		s := strings.TrimSuffix(strings.TrimSpace(v), "%")
-		var f float64
-		if _, err := fmt.Sscanf(s, "%f", &f); err == nil {
-			if strings.HasSuffix(strings.TrimSpace(v), "%") {
+		trimmed := strings.TrimSpace(v)
+		hasPct := strings.HasSuffix(trimmed, "%")
+		s := strings.TrimSuffix(trimmed, "%")
+		// strconv.ParseFloat (unlike fmt.Sscanf %f) rejects trailing garbage,
+		// so "50extra" / "0.5x" are refused instead of silently parsed as 50/0.5.
+		f, err := strconv.ParseFloat(strings.TrimSpace(s), 64)
+		if err == nil {
+			if hasPct {
 				f = f / 100.0
 			}
 			return f, true
@@ -292,7 +297,10 @@ func selectClassifyLabels(scores map[string]float64, categories []string, multiL
 			bestCat = c
 		}
 	}
-	if bestCat == "" {
+	// No category fit: an all-zero score set must not yield a false-positive
+	// top-1 (the first category trivially beats the -1.0 sentinel). Returning
+	// no label keeps "nothing matched" distinguishable from "category A won".
+	if bestCat == "" || bestScore <= 0 {
 		return nil
 	}
 	return []string{bestCat}

tools/create_file_url.go

@@ -124,14 +124,37 @@ func NewCreateFileURL(minter FileTokenMinter, fileStorage FileStorage, baseURL s
 				}
 				return "", fmt.Errorf("create_file_url: %w", err)
 			}
-			if meta.SkillID != inv.SkillID && !descendantFileGrant(ctx, fileStorage, inv, meta.SkillID) {
+			grantedViaDescendant := false
+			if meta.SkillID != inv.SkillID {
+				if !descendantFileGrant(ctx, fileStorage, inv, meta.SkillID) {
 					return "", fmt.Errorf("create_file_url: file_id %q does not belong to this skill (cross-skill refs rejected)", args.FileID)
 				}
+				grantedViaDescendant = true
+			}
+			// Scope gate — this is a PUBLICATION primitive (it mints an
+			// unauthenticated link), so it must enforce the same per-user/per-run
+			// scope isolation the read tools do: a same-skill caller must not be
+			// able to publish a file scoped to another user/run. Skipped only for
+			// the descendant-grant case (the worker's file scope is the worker's
+			// run, not the caller's).
+			if !grantedViaDescendant {
+				if err := ValidateScope(inv, meta.Scope, inv.CallerIsAdmin); err != nil {
+					return "", fmt.Errorf("create_file_url: %w", err)
+				}
+			}
 
-			// Resolve expiry.
+			// Resolve expiry. Clamp the caller's seconds BEFORE the multiply so a
+			// huge value can't overflow int64 nanoseconds into a negative
+			// duration that slips under the max-expiry cap (minting an
+			// already-expired token).
 			expiry := DefaultFileURLExpiry
 			if args.ExpiresInSeconds > 0 {
-				expiry = time.Duration(args.ExpiresInSeconds) * time.Second
+				maxSecs := int(MaxFileURLExpiry / time.Second)
+				secs := args.ExpiresInSeconds
+				if secs > maxSecs {
+					secs = maxSecs
+				}
+				expiry = time.Duration(secs) * time.Second
 			}
 			if expiry > MaxFileURLExpiry {
 				expiry = MaxFileURLExpiry
@@ -178,7 +201,10 @@ func NewCreateFileURL(minter FileTokenMinter, fileStorage FileStorage, baseURL s
 			if maxViews != nil {
 				res.MaxViews = *maxViews
 			}
-			b, _ := json.Marshal(res)
+			b, err := json.Marshal(res)
+			if err != nil {
+				return "", fmt.Errorf("create_file_url: marshal: %w", err)
+			}
 			return string(b), nil
 		},
 	)

tools/extract_entities.go

@@ -121,7 +121,7 @@ func NewExtractEntities(helper *llmmeta.Helper, cfg ExtractEntitiesConfig, budge
 			}
 
 			if len(text) > extractEntitiesMaxInputBytes {
-				text = text[:extractEntitiesMaxInputBytes]
+				text = truncateUTF8(text, extractEntitiesMaxInputBytes)
 			}
 
 			// Per-run budget gate.

tools/file_delete.go

@@ -49,12 +49,21 @@ func NewFileDelete(storage FileStorage) tool.Tool {
 				}
 				return "", fmt.Errorf("file_delete: %w", err)
 			}
+			// Honor the descendant grant like the read tools do, so a parent
+			// orchestrator can clean up a worker's artifacts (gadfly flagged the
+			// asymmetry: delete previously rejected cross-skill outright).
+			grantedViaDescendant := false
 			if meta.SkillID != inv.SkillID {
+				if !descendantFileGrant(ctx, storage, inv, meta.SkillID) {
 					return "", fmt.Errorf("file_delete: file does not belong to this skill")
 				}
-			if err := ValidateScope(inv, meta.Scope, false); err != nil {
+				grantedViaDescendant = true
+			}
+			if !grantedViaDescendant {
+				if err := ValidateScope(inv, meta.Scope, inv.CallerIsAdmin); err != nil {
 					return "", fmt.Errorf("file_delete: %w", err)
 				}
+			}
 
 			if err := storage.FileDelete(ctx, args.FileID); err != nil {
 				if errors.Is(err, ErrFileNotFound) {

tools/file_get.go

@@ -82,7 +82,7 @@ func NewFileGet(storage FileStorage) tool.Tool {
 			// row gates access (e.g. user:bob's file is unreadable by
 			// alice). The descendant grant stands in for it — the file's
 			// scope is the WORKER's run, never the caller's.
-			if err := ValidateScope(inv, meta.Scope, false); err != nil && !grantedViaDescendant {
+			if err := ValidateScope(inv, meta.Scope, inv.CallerIsAdmin); err != nil && !grantedViaDescendant {
 				return "", fmt.Errorf("file_get: %w", err)
 			}
 

tools/file_get_metadata.go

@@ -70,7 +70,7 @@ func NewFileGetMetadata(storage FileStorage) tool.Tool {
 				grantedViaDescendant = true
 			}
 			if !grantedViaDescendant {
-				if err := ValidateScope(inv, meta.Scope, false); err != nil {
+				if err := ValidateScope(inv, meta.Scope, inv.CallerIsAdmin); err != nil {
 					return "", fmt.Errorf("file_get_metadata: %w", err)
 				}
 			}

tools/file_get_text.go

@@ -77,7 +77,7 @@ func NewFileGetText(storage FileStorage) tool.Tool {
 				grantedViaDescendant = true
 			}
 			if !grantedViaDescendant {
-				if err := ValidateScope(inv, meta.Scope, false); err != nil {
+				if err := ValidateScope(inv, meta.Scope, inv.CallerIsAdmin); err != nil {
 					return "", fmt.Errorf("file_get_text: %w", err)
 				}
 			}

tools/file_list.go

@@ -41,7 +41,7 @@ func NewFileList(storage FileStorage) tool.Tool {
 			if storage == nil {
 				return "", fmt.Errorf("file_list: not configured")
 			}
-			if err := ValidateScope(inv, args.Scope, false); err != nil {
+			if err := ValidateScope(inv, args.Scope, inv.CallerIsAdmin); err != nil {
 				return "", fmt.Errorf("file_list: %w", err)
 			}
 			// root_run is a KV-only scope (v1) — see file_save's guard.

tools/file_save.go

@@ -43,7 +43,7 @@ import (
 	"gitea.stevedudenhoeffer.com/steve/executus/tool"
 )
 
-const defaultFileMaxBytes = 10 * 1024 * 1024 // 10 MiB
+const defaultFileMaxBytes = 16 * 1024 * 1024 // 10 MiB
 
 type fileSaveArgs struct {
 	Scope         string `json:"scope" description:"Storage scope: 'skill' (shared across all callers of this skill), 'user:<your_id>' (per-caller), or 'run:<run_id>' (this run's scratchpad)."`
@@ -84,7 +84,7 @@ func NewFileSave(storage FileStorage, quota QuotaProvider, maxFileBytes int) too
 			if storage == nil {
 				return "", fmt.Errorf("file_save: not configured")
 			}
-			if err := ValidateScope(inv, args.Scope, false); err != nil {
+			if err := ValidateScope(inv, args.Scope, inv.CallerIsAdmin); err != nil {
 				return "", fmt.Errorf("file_save: %w", err)
 			}
 			// root_run is a KV-only scope (v1): file storage partitions

tools/file_search.go

@@ -98,7 +98,7 @@ func NewFileSearch(searcher FileSearcher) tool.Tool {
 			}
 			scope := args.Scope
 			if scope != "" {
-				if err := ValidateScope(inv, scope, false); err != nil {
+				if err := ValidateScope(inv, scope, inv.CallerIsAdmin); err != nil {
 					return "", fmt.Errorf("file_search: %w", err)
 				}
 			}

tools/kv_delete.go

@@ -33,7 +33,7 @@ func NewKVDelete(storage KVStorage) tool.Tool {
 			if storage == nil {
 				return "", fmt.Errorf("kv_delete: not configured")
 			}
-			if err := ValidateScope(inv, args.Scope, false); err != nil {
+			if err := ValidateScope(inv, args.Scope, inv.CallerIsAdmin); err != nil {
 				return "", fmt.Errorf("kv_delete: %w", err)
 			}
 			if args.Key == "" {

tools/kv_get.go

@@ -43,7 +43,7 @@ func NewKVGet(storage KVStorage) tool.Tool {
 			if storage == nil {
 				return "", fmt.Errorf("kv_get: not configured")
 			}
-			if err := ValidateScope(inv, args.Scope, false); err != nil {
+			if err := ValidateScope(inv, args.Scope, inv.CallerIsAdmin); err != nil {
 				return "", fmt.Errorf("kv_get: %w", err)
 			}
 			if args.Key == "" {

tools/kv_list.go

@@ -49,7 +49,7 @@ func NewKVList(storage KVStorage) tool.Tool {
 			if storage == nil {
 				return "", fmt.Errorf("kv_list: not configured")
 			}
-			if err := ValidateScope(inv, args.Scope, false); err != nil {
+			if err := ValidateScope(inv, args.Scope, inv.CallerIsAdmin); err != nil {
 				return "", fmt.Errorf("kv_list: %w", err)
 			}
 

tools/kv_set.go

@@ -59,7 +59,7 @@ func NewKVSet(storage KVStorage, quota QuotaProvider, maxValueBytes int) tool.To
 			if storage == nil {
 				return "", fmt.Errorf("kv_set: not configured")
 			}
-			if err := ValidateScope(inv, args.Scope, false); err != nil {
+			if err := ValidateScope(inv, args.Scope, inv.CallerIsAdmin); err != nil {
 				return "", fmt.Errorf("kv_set: %w", err)
 			}
 			if args.Key == "" {

tools/scope_validate.go

@@ -18,7 +18,7 @@
 // vocabulary evolves and one place for the test matrix.
 //
 // Why the isAdmin parameter: the v4 Invocation does NOT carry an
-// admin flag — production tools always pass isAdmin=false. The
+// 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.

tools/summarize.go

@@ -198,7 +198,7 @@ func loadSummarizeInput(ctx context.Context, inv tool.Invocation, args summarize
 	if meta.SkillID != inv.SkillID {
 		return "", false, fmt.Errorf("summarize: file does not belong to this skill")
 	}
-	if err := ValidateScope(inv, meta.Scope, false); err != nil {
+	if err := ValidateScope(inv, meta.Scope, inv.CallerIsAdmin); err != nil {
 		return "", false, fmt.Errorf("summarize: %w", err)
 	}
 	return capInput(string(content))
@@ -210,7 +210,7 @@ func capInput(text string) (string, bool, error) {
 	if len(text) <= summarizeMaxInputBytes {
 		return text, false, nil
 	}
-	return text[:summarizeMaxInputBytes], true, nil
+	return truncateUTF8(text, summarizeMaxInputBytes), true, nil
 }
 
 // buildSummarizePrompt composes the user message handed to the LLM.

tools/think.go

@@ -16,7 +16,6 @@ package tools
 
 import (
 	"context"
-	"fmt"
 	"strings"
 
 	"gitea.stevedudenhoeffer.com/steve/executus/tool"
@@ -69,4 +68,3 @@ func NewThink() tool.Tool {
 // switch back to json.Marshal — but until then, the literal is the
 // idiom that matches the tool's "do nothing" intent.
 var _ = thinkResponse{} // declared so vet doesn't flag the unused struct
-var _ = fmt.Errorf

tools/tools.go

@@ -55,15 +55,22 @@ func RegisterMeta(reg tool.Registry, d MetaDeps) error {
 	if d.Helper == nil {
 		return errors.New("tools: MetaDeps.Helper is required for the meta tools")
 	}
-	if d.Budget == nil {
-		d.Budget = NewInMemorySearchBudget(nil)
-	}
 	if d.MaxPerRun <= 0 {
 		d.MaxPerRun = 10
 	}
 	if d.MaxWords <= 0 {
 		d.MaxWords = 200
 	}
+	if d.Budget == nil {
+		// Build the default budget WITH the configured per-run cap so
+		// MetaDeps.MaxPerRun is honored — an empty caps map would fall back to
+		// the budget's hardcoded default and silently ignore MaxPerRun.
+		d.Budget = NewInMemorySearchBudget(map[string]int{
+			"classify":         d.MaxPerRun,
+			"extract_entities": d.MaxPerRun,
+			"summarize":        d.MaxPerRun,
+		})
+	}
 	cfg := fixedMetaConfig{maxPerRun: d.MaxPerRun, maxWords: d.MaxWords}
 	return registerAll(reg,
 		NewClassify(d.Helper, cfg, d.Budget),

tools/truncate.go

@@ -0,0 +1,18 @@
+package tools
+
+import "unicode/utf8"
+
+// truncateUTF8 returns s truncated to at most maxBytes, backing off to the last
+// complete UTF-8 rune boundary so a multibyte rune (CJK, emoji, …) is never
+// split — a byte-boundary cut would hand the LLM invalid UTF-8 / replacement
+// chars. Used by the meta tools' input caps.
+func truncateUTF8(s string, maxBytes int) string {
+	if len(s) <= maxBytes {
+		return s
+	}
+	s = s[:maxBytes]
+	for len(s) > 0 && !utf8.ValidString(s) {
+		s = s[:len(s)-1]
+	}
+	return s
+}