From 2ef88f2a739232ee969793da431e264d6509a497 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sun, 28 Jun 2026 13:02:55 -0400 Subject: [PATCH 1/3] =?UTF-8?q?feat(run):=20InputFileStager=20seam=20?= =?UTF-8?q?=E2=80=94=20stage=20non-image=20attachments=20into=20the=20prom?= =?UTF-8?q?pt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit executus's tool.Invocation already carried InputFiles (audio/PDF/binary), but the executor never staged them — only Images were folded into the run. This adds the host seam mort's chat/chatbot surfaces need for audio-input parity with agentexec. - run.Ports gains InputFiles InputFileStager (nil-safe; nil = input files silently ignored, run still proceeds text-only). The interface mirrors mort's skill FileStorage: StageInputFile(ctx, runID, agentID, name, mime, content) → file_id. - run/input_files.go (ported from mort agentexec/input_files.go): stageInputFiles persists each file under run scope and appends an [ATTACHED FILES] descriptor block to the prompt so the agent can reach them by file_id (e.g. code_exec files_in → /workspace/). Bytes are NEVER inlined into model context. Best-effort: empty/oversized(>50MB)/save-error files are skipped; colliding base names are disambiguated (name-2, name-3) so they don't clobber at /workspace/. - Executor.Run calls it after the model/toolbox build, before the loop, so the descriptor rides the first user turn (alongside the existing Images folding). Tests: stages + builds the block; nil stager / no files leave the prompt intact; dedup; empty/save-error skipping. Full suite green. Co-Authored-By: Claude Opus 4.8 (1M context) --- run/executor.go | 6 ++ run/input_files.go | 120 ++++++++++++++++++++++++++++++++++++++++ run/input_files_test.go | 119 +++++++++++++++++++++++++++++++++++++++ run/ports.go | 14 +++++ 4 files changed, 259 insertions(+) create mode 100644 run/input_files.go create mode 100644 run/input_files_test.go 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..f358517 --- /dev/null +++ b/run/input_files.go @@ -0,0 +1,120 @@ +package run + +import ( + "context" + "fmt" + "log/slog" + "path" + "strings" + + "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 + +// 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 + } + + type staged struct { + name, mime, fileID string + size int + } + var ok []staged + 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 + } + // Disambiguate colliding base names so two attachments with the same + // name don't both map to /workspace/ (the second would clobber the + // first when the agent writes them via code_exec). + name := uniqueName(f.Name, seenNames) + fileID, err := e.cfg.Ports.InputFiles.StageInputFile(ctx, runID, agentID, name, f.MimeType, f.Data) + if err != nil { + slog.Warn("run: failed to stage input file", + "agent", agentID, "run_id", runID, "name", name, "error", err) + continue + } + ok = append(ok, staged{name: name, mime: f.MimeType, fileID: fileID, size: len(f.Data)}) + } + if len(ok) == 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 ok { + 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() +} + +// 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) + } + div, exp := int64(unit), 0 + for v := int64(n) / unit; v >= unit; v /= unit { + div *= unit + exp++ + } + return fmt.Sprintf("%.1f %cB", float64(n)/float64(div), "KMGTPE"[exp]) +} diff --git a/run/input_files_test.go b/run/input_files_test.go new file mode 100644 index 0000000..0b0be0d --- /dev/null +++ b/run/input_files_test.go @@ -0,0 +1,119 @@ +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) + } +} 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 -- 2.52.0 From 1e65f4b6e5a797be24aa4f1d38d2f042fb9a0e91 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sun, 28 Jun 2026 13:29:45 -0400 Subject: [PATCH 2/3] =?UTF-8?q?fix(run):=20sanitize=20input-file=20names?= =?UTF-8?q?=20=E2=80=94=20path-traversal=20+=20prompt-injection=20hardenin?= =?UTF-8?q?g=20(gadfly=20#18)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The full swarm (5-6 models) flagged that stageInputFiles passed the untrusted attachment filename straight to StageInputFile and inlined it into the [ATTACHED FILES]/`/workspace/` descriptor with no sanitization — a path the byte-cap already treats as a trust boundary. A name like ../../etc/passwd or an absolute/drive path could escape the host store or the sandbox workspace, and newlines in the name/mime could inject text into the prompt block. - sanitizeName: strips control chars/newlines, then reduces to a base name (path.Base after backslash-normalization) so ../, nested dirs, and absolute / drive paths all collapse to their last element; "attachment" fallback for empty/"."/"..". Applied BEFORE staging AND inlining. - sanitizeField: strips control chars from MimeType (also inlined verbatim). - maxInputFiles (32) count cap — defense-in-depth vs a flood of tiny files, independent of the per-file byte cap. Tests: sanitizeName table (traversal/absolute/backslash/control/fallback, + no-separator invariant); traversal staged+described under the base name only; oversize skip; count-cap truncation. Full suite green (-race). Co-Authored-By: Claude Opus 4.8 (1M context) --- run/input_files.go | 62 ++++++++++++++++++++++++++++++------ run/input_files_test.go | 69 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 9 deletions(-) diff --git a/run/input_files.go b/run/input_files.go index f358517..bad3815 100644 --- a/run/input_files.go +++ b/run/input_files.go @@ -6,6 +6,7 @@ import ( "log/slog" "path" "strings" + "unicode" "gitea.stevedudenhoeffer.com/steve/executus/tool" ) @@ -16,6 +17,10 @@ import ( // 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 @@ -30,12 +35,19 @@ func (e *Executor) stageInputFiles(ctx context.Context, runID, agentID string, f 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 staged struct { + type stagedFile struct { name, mime, fileID string size int } - var ok []staged + var staged []stagedFile seenNames := make(map[string]int, len(files)) for _, f := range files { if len(f.Data) == 0 { @@ -49,19 +61,22 @@ func (e *Executor) stageInputFiles(ctx context.Context, runID, agentID string, f "size", len(f.Data), "cap", maxInputFileBytes) continue } - // Disambiguate colliding base names so two attachments with the same - // name don't both map to /workspace/ (the second would clobber the - // first when the agent writes them via code_exec). - name := uniqueName(f.Name, seenNames) + // 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) fileID, err := e.cfg.Ports.InputFiles.StageInputFile(ctx, runID, agentID, name, f.MimeType, f.Data) if err != nil { slog.Warn("run: failed to stage input file", "agent", agentID, "run_id", runID, "name", name, "error", err) continue } - ok = append(ok, staged{name: name, mime: f.MimeType, fileID: fileID, size: len(f.Data)}) + staged = append(staged, stagedFile{name: name, mime: sanitizeField(f.MimeType), fileID: fileID, size: len(f.Data)}) } - if len(ok) == 0 { + if len(staged) == 0 { return prompt } @@ -71,7 +86,7 @@ func (e *Executor) stageInputFiles(ctx context.Context, runID, agentID string, f 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 ok { + for _, s := range staged { fmt.Fprintf(&b, "- %s (%s, %s) → file_id: %s\n", s.name, s.mime, humanizeBytes(s.size), s.fileID) } @@ -81,6 +96,35 @@ func (e *Executor) stageInputFiles(ctx context.Context, runID, agentID string, f 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 control characters (incl. newlines/tabs) from a value +// that gets inlined verbatim into the prompt descriptor, so it can't break out +// of its line or inject instructions. +func sanitizeField(s string) string { + return strings.Map(func(r rune) rune { + if r == '\n' || r == '\r' || r == '\t' || unicode.IsControl(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. diff --git a/run/input_files_test.go b/run/input_files_test.go index 0b0be0d..77777e4 100644 --- a/run/input_files_test.go +++ b/run/input_files_test.go @@ -117,3 +117,72 @@ func TestStageInputFilesSkipsBad(t *testing.T) { 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) + } +} -- 2.52.0 From df4033f42e566997090b67b13a2a2235410b5c9f Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sun, 28 Jun 2026 14:08:57 -0400 Subject: [PATCH 3/3] fix(run): harden input-file staging per gadfly #18 validation pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-pass findings on the security fix: - Mime sanitized ONCE and passed to BOTH StageInputFile and the descriptor (was passing raw f.MimeType to the host store while only the descriptor sanitized) — 3 models. - sanitizeField now also strips Unicode format chars (category Cf, incl. the bidi overrides U+202A–U+202E that can reorder how the descriptor renders); IsControl already covers \n\r\t so the explicit checks are dropped. - fileID is sanitized before inlining + an empty file_id drops the file (defense vs a misbehaving stager). - humanizeBytes clamps the prefix index so an absurd size (≥1024^6) can't index past "KMGTPE" and panic — a no-panic guarantee independent of the per-file cap. - Docs sync: README Ports list gains InputFiles; tool.InputFile.Name doc now says the executor reduces an untrusted name to a safe base name (was claiming the field is already safe). Tests: bidi/control stripping; mime sanitized in staged value + descriptor; empty file_id drop; humanizeBytes no-panic across sizes up to 1<<62. Suite green (-race). Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 2 +- run/input_files.go | 31 +++++++++++++++++------ run/input_files_test.go | 55 +++++++++++++++++++++++++++++++++++++++++ tool/registry.go | 7 +++--- 4 files changed, 83 insertions(+), 12 deletions(-) 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/input_files.go b/run/input_files.go index bad3815..932edfd 100644 --- a/run/input_files.go +++ b/run/input_files.go @@ -68,13 +68,23 @@ func (e *Executor) stageInputFiles(ctx context.Context, runID, agentID string, f // 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) - fileID, err := e.cfg.Ports.InputFiles.StageInputFile(ctx, runID, agentID, name, f.MimeType, f.Data) + // 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 } - staged = append(staged, stagedFile{name: name, mime: sanitizeField(f.MimeType), fileID: fileID, size: len(f.Data)}) + 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 @@ -113,12 +123,14 @@ func sanitizeName(name string) string { return base } -// sanitizeField strips control characters (incl. newlines/tabs) from a value -// that gets inlined verbatim into the prompt descriptor, so it can't break out -// of its line or inject instructions. +// 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 r == '\n' || r == '\r' || r == '\t' || unicode.IsControl(r) { + if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) { return -1 } return r @@ -155,10 +167,13 @@ func humanizeBytes(n int) string { if n < unit { return fmt.Sprintf("%d B", n) } + const prefixes = "KMGTPE" div, exp := int64(unit), 0 - for v := int64(n) / unit; v >= unit; v /= unit { + // 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), "KMGTPE"[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 index 77777e4..681dcf7 100644 --- a/run/input_files_test.go +++ b/run/input_files_test.go @@ -186,3 +186,58 @@ func TestStageInputFilesSanitizesTraversal(t *testing.T) { 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/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 -- 2.52.0