diff --git a/README.md b/README.md index 3cf04d5..aec93db 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ bot) — mort and gadfly are the first two consumers (heavy and light). See tool registry, majordomo's agent loop, context compaction, run-bounding, and step/audit instrumentation into one `Run(ctx, RunnableAgent, inv) Result`, with every host concern behind a nil-safe `run.Ports` (Audit/Budget/Critic/ - Checkpointer/PaletteSource/Delivery). See `examples/minimal`. + Checkpointer/PaletteSource/Delivery/InputFiles). See `examples/minimal`. - `model/` — config-driven tier resolution + failover over majordomo, with pluggable `UsageSink`/`TraceSink` and `GenerateWith[T]` structured output. - `tool/` — the tool registry + 3-stage permission model + SSRF guard. diff --git a/run/executor.go b/run/executor.go index c5bd97f..ef29daf 100644 --- a/run/executor.go +++ b/run/executor.go @@ -318,6 +318,12 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio } ag := agent.New(model, e.systemPrompt(ra), opts...) + // Stage non-image input attachments (audio/PDF/binary) into the host file + // store and fold an [ATTACHED FILES] descriptor into the prompt so the agent + // can reach them by file_id. No-op when Ports.InputFiles is nil or there are + // no files. Done after the model/toolbox build but before the loop, so the + // descriptor rides the very first user turn. + input = e.stageInputFiles(runCtx, inv.RunID, ra.ID, inv.InputFiles, input) // One WithSteer drains BOTH the session mailbox (a tool's AttachImages) and // the critic's nudges before each step. steer := func() []llm.Message { return append(mailbox.drain(), critic.drainSteer()...) } diff --git a/run/input_files.go b/run/input_files.go new file mode 100644 index 0000000..932edfd --- /dev/null +++ b/run/input_files.go @@ -0,0 +1,179 @@ +package run + +import ( + "context" + "fmt" + "log/slog" + "path" + "strings" + "unicode" + + "gitea.stevedudenhoeffer.com/steve/executus/tool" +) + +// maxInputFileBytes is a defense-in-depth cap at the staging boundary. A host's +// extraction path may already cap downloads, but stageInputFiles is the trust +// boundary for the InputFiles seam: a call site or bug that populates InputFiles +// directly must not write an unbounded blob to the host file store. +const maxInputFileBytes = 50_000_000 + +// maxInputFiles bounds how many attachments a single run stages, independent of +// the per-file byte cap — defense-in-depth against a flood of tiny files. +const maxInputFiles = 32 + +// stageInputFiles persists each non-image input attachment into the host file +// store (Ports.InputFiles) under run scope and appends a descriptor block to the +// prompt so the agent knows the file_ids it can pass to a worker tool. The bytes +// are NOT inlined into the model context — the LLM can't read raw audio/binary — +// so the agent reaches them via a file_id-aware tool (e.g. code_exec files_in, +// which writes the file to /workspace/). +// +// Best-effort: a nil stager, no files, or a per-file save error degrades to +// "skip that file" — the run still proceeds. Returns the (possibly augmented) +// prompt. +func (e *Executor) stageInputFiles(ctx context.Context, runID, agentID string, files []tool.InputFile, prompt string) string { + if e.cfg.Ports.InputFiles == nil || len(files) == 0 { + return prompt + } + // Count cap: bound how many attachments one run can stage, independent of the + // per-file byte cap (defense-in-depth against a flood of tiny files). + if len(files) > maxInputFiles { + slog.Warn("run: too many input files, truncating", + "agent", agentID, "run_id", runID, "count", len(files), "cap", maxInputFiles) + files = files[:maxInputFiles] + } + + type stagedFile struct { + name, mime, fileID string + size int + } + var staged []stagedFile + seenNames := make(map[string]int, len(files)) + for _, f := range files { + if len(f.Data) == 0 { + slog.Warn("run: skipping empty input file", + "agent", agentID, "run_id", runID, "name", f.Name) + continue + } + if len(f.Data) > maxInputFileBytes { + slog.Warn("run: skipping oversized input file", + "agent", agentID, "run_id", runID, "name", f.Name, + "size", len(f.Data), "cap", maxInputFileBytes) + continue + } + // Reduce the untrusted filename to a safe base name BEFORE staging or + // inlining: strips ../ and absolute-path components (so it can't escape + // the host store or /workspace/) and drops control chars/newlines + // (so a crafted name can't inject text into the descriptor block below). + // Then disambiguate colliding base names so two attachments don't both map + // to /workspace/ (the second would clobber the first). + name := uniqueName(sanitizeName(f.Name), seenNames) + // Sanitize the mime ONCE and pass the clean value to both the host store + // and the descriptor (don't hand the raw value to StageInputFile). + mime := sanitizeField(f.MimeType) + fileID, err := e.cfg.Ports.InputFiles.StageInputFile(ctx, runID, agentID, name, mime, f.Data) + if err != nil { + slog.Warn("run: failed to stage input file", + "agent", agentID, "run_id", runID, "name", name, "error", err) + continue + } + if fileID == "" { + slog.Warn("run: stager returned empty file_id, skipping", + "agent", agentID, "run_id", runID, "name", name) + continue + } + // fileID is host-generated, but sanitize it too before inlining — the + // descriptor must never carry control chars no matter the stager impl. + staged = append(staged, stagedFile{name: name, mime: mime, fileID: sanitizeField(fileID), size: len(f.Data)}) + } + if len(staged) == 0 { + return prompt + } + + var b strings.Builder + b.WriteString("[ATTACHED FILES]\n") + b.WriteString("The user attached the following file(s). Their contents are NOT included in this prompt and you cannot read them directly. ") + b.WriteString("To work with one, call the code_exec tool with a files_in entry — e.g. ") + b.WriteString(`files_in: [{"name": "", "file_id": ""}]`) + b.WriteString(" — which writes it to /workspace/ inside the Python sandbox. You may also pass a file_id to any other tool that accepts one.\n") + for _, s := range staged { + fmt.Fprintf(&b, "- %s (%s, %s) → file_id: %s\n", s.name, s.mime, humanizeBytes(s.size), s.fileID) + } + + if strings.TrimSpace(prompt) == "" { + return b.String() + } + return prompt + "\n\n" + b.String() +} + +// sanitizeName reduces an untrusted attachment filename to a safe base name. It +// drops control characters / newlines (which would otherwise let a crafted name +// inject text into the [ATTACHED FILES] descriptor) and strips every directory +// component — defeating ../ traversal, nested dirs, and absolute / drive paths +// both in the host file store and at /workspace/. Returns "attachment" +// when nothing usable remains (empty, ".", ".."). +func sanitizeName(name string) string { + name = sanitizeField(name) + // Normalize backslashes so a Windows-style path also reduces to its base. + base := path.Base(strings.ReplaceAll(name, `\`, "/")) + base = strings.TrimSpace(base) + if base == "" || base == "." || base == ".." { + return "attachment" + } + return base +} + +// sanitizeField strips characters that could let a value inlined verbatim into +// the prompt descriptor break out of its line or visually mislead: control +// characters (IsControl covers newlines/tabs) AND Unicode format characters +// (category Cf — e.g. the bidi overrides U+202A–U+202E, which can reorder how +// the descriptor renders). +func sanitizeField(s string) string { + return strings.Map(func(r rune) rune { + if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) { + return -1 + } + return r + }, s) +} + +// uniqueName returns name unchanged the first time it's seen, then name-2, +// name-3, … (suffix inserted before the extension) on repeats, recording each +// result in seen so later collisions keep counting up. +func uniqueName(name string, seen map[string]int) string { + if seen[name] == 0 { + seen[name]++ + return name + } + ext := path.Ext(name) + base := strings.TrimSuffix(name, ext) + for { + seen[name]++ + candidate := fmt.Sprintf("%s-%d%s", base, seen[name], ext) + if seen[candidate] == 0 { + seen[candidate]++ + return candidate + } + } +} + +// humanizeBytes renders a byte count as a short human-readable string (e.g. +// "2.1 MB") for the attached-files descriptor block. +func humanizeBytes(n int) string { + if n < 0 { + n = 0 + } + const unit = 1024 + if n < unit { + return fmt.Sprintf("%d B", n) + } + const prefixes = "KMGTPE" + div, exp := int64(unit), 0 + // Clamp exp to the last prefix so an absurd size (≥1024^7) can't index past + // "KMGTPE" and panic — a no-panic guarantee independent of the per-file cap. + for v := int64(n) / unit; v >= unit && exp < len(prefixes)-1; v /= unit { + div *= unit + exp++ + } + return fmt.Sprintf("%.1f %cB", float64(n)/float64(div), prefixes[exp]) +} diff --git a/run/input_files_test.go b/run/input_files_test.go new file mode 100644 index 0000000..681dcf7 --- /dev/null +++ b/run/input_files_test.go @@ -0,0 +1,243 @@ +package run + +import ( + "context" + "errors" + "strings" + "testing" + + "gitea.stevedudenhoeffer.com/steve/majordomo/llm" + + "gitea.stevedudenhoeffer.com/steve/executus/tool" +) + +// stagerFunc is a test InputFileStager: it records each staged file and returns +// a deterministic file_id ("file_"), or an error if err is set. +type stagerFunc struct { + staged []stagedRec + err error +} + +type stagedRec struct { + runID, agentID, name, mime string + size int +} + +func (s *stagerFunc) StageInputFile(_ context.Context, runID, agentID, name, mime string, content []byte) (string, error) { + if s.err != nil { + return "", s.err + } + s.staged = append(s.staged, stagedRec{runID, agentID, name, mime, len(content)}) + return "file_" + name, nil +} + +func newStagerExecutor(s InputFileStager) *Executor { + return New(Config{ + Registry: tool.NewRegistry(), + Models: func(ctx context.Context, _ string) (context.Context, llm.Model, error) { return ctx, nil, nil }, + Ports: Ports{InputFiles: s}, + }) +} + +// TestStageInputFiles: files are staged via the port and an [ATTACHED FILES] +// descriptor (with each file_id) is appended to the prompt. +func TestStageInputFiles(t *testing.T) { + st := &stagerFunc{} + ex := newStagerExecutor(st) + out := ex.stageInputFiles(context.Background(), "run-1", "agent-1", + []tool.InputFile{{Name: "clip.mp3", MimeType: "audio/mpeg", Data: []byte("abcd")}}, + "transcribe this") + + if len(st.staged) != 1 || st.staged[0].name != "clip.mp3" { + t.Fatalf("staged = %+v, want one clip.mp3", st.staged) + } + if st.staged[0].runID != "run-1" || st.staged[0].agentID != "agent-1" { + t.Errorf("stager got runID/agentID = %q/%q, want run-1/agent-1", st.staged[0].runID, st.staged[0].agentID) + } + for _, want := range []string{"transcribe this", "[ATTACHED FILES]", "clip.mp3", "file_clip.mp3", "audio/mpeg"} { + if !strings.Contains(out, want) { + t.Errorf("output missing %q:\n%s", want, out) + } + } +} + +// TestStageInputFilesNoStager: a nil port leaves the prompt untouched and never +// drops the run. +func TestStageInputFilesNoStager(t *testing.T) { + ex := newStagerExecutor(nil) // Ports.InputFiles == nil + out := ex.stageInputFiles(context.Background(), "r", "a", + []tool.InputFile{{Name: "x.bin", Data: []byte("z")}}, "prompt") + if out != "prompt" { + t.Errorf("nil stager changed the prompt: %q", out) + } +} + +// TestStageInputFilesNoFiles: no attachments leaves the prompt untouched. +func TestStageInputFilesNoFiles(t *testing.T) { + ex := newStagerExecutor(&stagerFunc{}) + out := ex.stageInputFiles(context.Background(), "r", "a", nil, "prompt") + if out != "prompt" { + t.Errorf("no files changed the prompt: %q", out) + } +} + +// TestStageInputFilesDedup: colliding base names are disambiguated so they don't +// clobber each other at /workspace/. +func TestStageInputFilesDedup(t *testing.T) { + st := &stagerFunc{} + ex := newStagerExecutor(st) + out := ex.stageInputFiles(context.Background(), "r", "a", []tool.InputFile{ + {Name: "a.wav", MimeType: "audio/wav", Data: []byte("1")}, + {Name: "a.wav", MimeType: "audio/wav", Data: []byte("2")}, + }, "go") + if len(st.staged) != 2 { + t.Fatalf("staged %d files, want 2", len(st.staged)) + } + if st.staged[0].name != "a.wav" || st.staged[1].name != "a-2.wav" { + t.Errorf("dedup names = %q, %q; want a.wav, a-2.wav", st.staged[0].name, st.staged[1].name) + } + if !strings.Contains(out, "a-2.wav") { + t.Errorf("output missing disambiguated name:\n%s", out) + } +} + +// TestStageInputFilesSkipsBad: empty + oversized files are skipped; a save error +// drops only that file. With nothing staged, the prompt is unchanged. +func TestStageInputFilesSkipsBad(t *testing.T) { + // Empty data → skipped; with no good files the prompt is returned as-is. + ex := newStagerExecutor(&stagerFunc{}) + if out := ex.stageInputFiles(context.Background(), "r", "a", + []tool.InputFile{{Name: "empty.bin", Data: nil}}, "p"); out != "p" { + t.Errorf("empty file should be skipped, got %q", out) + } + // A stager error → that file is dropped; nothing staged → prompt unchanged. + exErr := newStagerExecutor(&stagerFunc{err: errors.New("disk full")}) + if out := exErr.stageInputFiles(context.Background(), "r", "a", + []tool.InputFile{{Name: "x.bin", Data: []byte("z")}}, "p"); out != "p" { + t.Errorf("save error should drop the file and leave the prompt, got %q", out) + } +} + +// TestStageInputFilesOversize: a file past the byte cap is skipped (prompt +// unchanged), exercising the size guard directly. +func TestStageInputFilesOversize(t *testing.T) { + st := &stagerFunc{} + ex := newStagerExecutor(st) + big := make([]byte, maxInputFileBytes+1) + out := ex.stageInputFiles(context.Background(), "r", "a", + []tool.InputFile{{Name: "huge.bin", Data: big}}, "p") + if out != "p" || len(st.staged) != 0 { + t.Errorf("oversized file should be skipped: out=%q staged=%d", out, len(st.staged)) + } +} + +// TestStageInputFilesCountCap: more than maxInputFiles attachments are truncated +// to the cap. +func TestStageInputFilesCountCap(t *testing.T) { + st := &stagerFunc{} + ex := newStagerExecutor(st) + files := make([]tool.InputFile, maxInputFiles+5) + for i := range files { + files[i] = tool.InputFile{Name: "f.bin", Data: []byte("x")} + } + ex.stageInputFiles(context.Background(), "r", "a", files, "p") + if len(st.staged) != maxInputFiles { + t.Errorf("count cap: staged %d, want %d", len(st.staged), maxInputFiles) + } +} + +// TestSanitizeName: traversal + absolute + control-char filenames are reduced to +// a safe base name (no path separators, no newlines), with a fallback. +func TestSanitizeName(t *testing.T) { + cases := map[string]string{ + "../../etc/passwd": "passwd", + "/etc/cron.d/x": "x", + `..\..\windows\sys`: "sys", + "clip.mp3": "clip.mp3", + "": "attachment", + "..": "attachment", + ".": "attachment", + "evil\n- injected": "evil- injected", + "a/b/c.wav": "c.wav", + } + for in, want := range cases { + if got := sanitizeName(in); got != want { + t.Errorf("sanitizeName(%q) = %q, want %q", in, got, want) + } + // A sanitized name must never carry a path separator or newline. + got := sanitizeName(in) + if strings.ContainsAny(got, "/\\\n\r") { + t.Errorf("sanitizeName(%q) = %q still contains a separator/newline", in, got) + } + } +} + +// TestStageInputFilesSanitizesTraversal: a traversal filename is staged AND +// described under its safe base name only. +func TestStageInputFilesSanitizesTraversal(t *testing.T) { + st := &stagerFunc{} + ex := newStagerExecutor(st) + out := ex.stageInputFiles(context.Background(), "r", "a", + []tool.InputFile{{Name: "../../../etc/passwd", MimeType: "text/plain", Data: []byte("x")}}, "go") + if len(st.staged) != 1 || st.staged[0].name != "passwd" { + t.Fatalf("staged name = %+v, want passwd", st.staged) + } + if strings.Contains(out, "..") || strings.Contains(out, "/etc/") { + t.Errorf("descriptor leaked the traversal path:\n%s", out) + } +} + +// TestSanitizeFieldStripsBidiAndControl: control chars AND Unicode format/bidi +// overrides are removed from inlined values. +func TestSanitizeFieldStripsBidiAndControl(t *testing.T) { + in := "audio/‮mpg\n; rm -rf" // bidi override + newline + got := sanitizeField(in) + if strings.ContainsAny(got, "\n\r\t") || strings.ContainsRune(got, '‮') { + t.Errorf("sanitizeField left control/bidi chars: %q", got) + } +} + +// TestStageInputFilesSanitizesMime: a mime with a control char is cleaned in BOTH +// the staged value and the descriptor. +func TestStageInputFilesSanitizesMime(t *testing.T) { + st := &stagerFunc{} + ex := newStagerExecutor(st) + out := ex.stageInputFiles(context.Background(), "r", "a", + []tool.InputFile{{Name: "c.wav", MimeType: "audio/wav\ninjected", Data: []byte("x")}}, "go") + if len(st.staged) != 1 || strings.ContainsAny(st.staged[0].mime, "\n\r") { + t.Errorf("mime not sanitized before staging: %+v", st.staged) + } + if strings.Contains(out, "\ninjected") { + t.Errorf("descriptor carried an unsanitized mime newline:\n%s", out) + } +} + +// TestStageInputFilesEmptyFileID: a stager returning an empty file_id drops the +// file (no blank file_id in the descriptor). +func TestStageInputFilesEmptyFileID(t *testing.T) { + ex := newStagerExecutor(emptyIDStager{}) + out := ex.stageInputFiles(context.Background(), "r", "a", + []tool.InputFile{{Name: "x.bin", Data: []byte("z")}}, "p") + if out != "p" { + t.Errorf("empty file_id should drop the file, got %q", out) + } +} + +type emptyIDStager struct{} + +func (emptyIDStager) StageInputFile(context.Context, string, string, string, string, []byte) (string, error) { + return "", nil +} + +// TestHumanizeBytesNoPanic: an absurd size clamps to the last prefix instead of +// indexing past "KMGTPE". +func TestHumanizeBytesNoPanic(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("humanizeBytes panicked: %v", r) + } + }() + for _, n := range []int{0, 512, 2048, 5_000_000, 1 << 62} { + _ = humanizeBytes(n) + } +} diff --git a/run/ports.go b/run/ports.go index 75d8322..c130dcf 100644 --- a/run/ports.go +++ b/run/ports.go @@ -42,6 +42,20 @@ type Ports struct { // Delivery is where the run's output + artifacts go. nil = the caller // reads the Result in-process (the light-host default). Delivery deliver.Delivery + // InputFiles persists non-image input attachments (audio, PDF, binary) + // carried on Invocation.InputFiles into a host file store under run scope, + // returning file_ids the agent can hand to a worker tool. nil = input files + // are silently ignored (the run still proceeds, text-only). The bytes are + // never inlined into the model context — the LLM can't read raw audio/binary. + InputFiles InputFileStager +} + +// InputFileStager persists a single non-image input attachment into a host file +// store under run scope and returns a file_id the run can reference. It is the +// seam mort's skill FileStorage (and any host blob store) implements so the +// kernel can stage Invocation.InputFiles without importing a storage layer. +type InputFileStager interface { + StageInputFile(ctx context.Context, runID, agentID, name, mime string, content []byte) (fileID string, err error) } // RunInfo describes a run at start time — the attribution a recorder/critic diff --git a/tool/registry.go b/tool/registry.go index 7528f3f..008d20d 100644 --- a/tool/registry.go +++ b/tool/registry.go @@ -154,9 +154,10 @@ type ContinuationContext struct { // InputFile is a non-image file the user supplied with a run (audio, // etc.). The executor stages it into the file store under run scope and -// surfaces its file_id to the agent. Name is a safe base name (no path -// separators) suitable for /workspace/; MimeType is the resolved -// content type; Data is the raw bytes. +// surfaces its file_id to the agent. Name may be an untrusted attachment +// filename — the executor reduces it to a safe base name (stripping path +// separators + control chars) before staging or exposing it as +// /workspace/; MimeType is the resolved content type; Data is the raw bytes. type InputFile struct { Name string MimeType string