diff --git a/cmd/serve_test.go b/cmd/serve_test.go index 8777e112..8ca21f56 100644 --- a/cmd/serve_test.go +++ b/cmd/serve_test.go @@ -1246,6 +1246,18 @@ func TestParseResponsesInput_FileUploadSavesToDisk(t *testing.T) { if strings.Contains(msg.Parts[0].Text, dataHome) || strings.Contains(msg.Parts[0].Text, entries[0].Name()) { t.Fatalf("parts[0].text leaks upload path: %q", msg.Parts[0].Text) } + + stored := session.NewMessage("sess_upload", msg, 0) + partsJSON, err := stored.PartsJSONForStorage(true) + if err != nil { + t.Fatalf("PartsJSONForStorage: %v", err) + } + if strings.Contains(partsJSON, b64) { + t.Fatalf("storage parts json retained file base64 despite strip option: %s", partsJSON) + } + if !strings.Contains(partsJSON, msg.Parts[0].FilePath) || !strings.Contains(partsJSON, "doc.pdf") || !strings.Contains(partsJSON, "application/pdf") { + t.Fatalf("storage parts json lost file path or metadata: %s", partsJSON) + } } func TestParseResponsesInput_TextFileUploadEmbedsFallback(t *testing.T) { diff --git a/internal/llm/responses_api.go b/internal/llm/responses_api.go index d2061b66..94f0821d 100644 --- a/internal/llm/responses_api.go +++ b/internal/llm/responses_api.go @@ -4,11 +4,15 @@ import ( "bufio" "bytes" "context" + "encoding/base64" "encoding/json" "errors" "fmt" "io" + "mime" "net/http" + "os" + "path/filepath" "strings" "sync" "time" @@ -407,20 +411,20 @@ func buildResponsesMessageItems(role string, parts []Part, policy *FileUploadPol }) } case PartFile: - if part.FileData != nil && part.FileData.Base64 != "" && responseNativeFileAllowed(part.FileData, policy) { + if fileData, ok := responseNativeFileData(part, policy); ok { flushText() - filename := strings.TrimSpace(part.FileData.Filename) + filename := strings.TrimSpace(fileData.Filename) if filename == "" { filename = "upload" } - mediaType := NormalizeMediaType(part.FileData.MediaType) + mediaType := NormalizeMediaType(fileData.MediaType) if mediaType == "" { mediaType = "application/octet-stream" } fileParts := []ResponsesContentPart{{ Type: "input_file", Filename: filename, - FileData: fmt.Sprintf("data:%s;base64,%s", mediaType, part.FileData.Base64), + FileData: fmt.Sprintf("data:%s;base64,%s", mediaType, fileData.Base64), }} items = append(items, ResponsesInputItem{ Type: "message", @@ -463,6 +467,55 @@ func effectiveResponsesFilePolicy(policy *FileUploadPolicy) FileUploadPolicy { return *policy } +func responseNativeFileData(part Part, policy *FileUploadPolicy) (*ToolFileData, bool) { + if part.FileData == nil { + return nil, false + } + fileData := *part.FileData + if strings.TrimSpace(fileData.Base64) == "" { + var ok bool + fileData, ok = hydrateResponseFileDataFromPath(part, fileData, policy) + if !ok { + return nil, false + } + } + if !responseNativeFileAllowed(&fileData, policy) { + return nil, false + } + return &fileData, true +} + +func hydrateResponseFileDataFromPath(part Part, fileData ToolFileData, policy *FileUploadPolicy) (ToolFileData, bool) { + path := strings.TrimSpace(part.FilePath) + if path == "" || !isTermLLMUploadPath(path) { + return fileData, false + } + info, err := os.Stat(path) + if err != nil || info.IsDir() || info.Size() <= 0 || info.Size() > defaultFileUploadMaxBytes { + return fileData, false + } + fileData.SizeBytes = info.Size() + if strings.TrimSpace(fileData.MediaType) == "" { + fileData.MediaType = strings.TrimSpace(mime.TypeByExtension(strings.ToLower(filepath.Ext(path)))) + } + if strings.TrimSpace(fileData.MediaType) == "" { + fileData.MediaType = "application/octet-stream" + } + if strings.TrimSpace(fileData.Filename) == "" { + fileData.Filename = filepath.Base(path) + } + active := effectiveResponsesFilePolicy(policy) + if !active.AllowsNative(fileData.MediaType, fileData.SizeBytes) { + return fileData, false + } + data, err := os.ReadFile(path) + if err != nil || len(data) == 0 || int64(len(data)) > defaultFileUploadMaxBytes { + return fileData, false + } + fileData.Base64 = base64.StdEncoding.EncodeToString(data) + return fileData, true +} + func responseNativeFileAllowed(file *ToolFileData, policy *FileUploadPolicy) bool { if file == nil || strings.TrimSpace(file.Base64) == "" { return false diff --git a/internal/llm/responses_api_test.go b/internal/llm/responses_api_test.go index 72da9fe0..4f491cda 100644 --- a/internal/llm/responses_api_test.go +++ b/internal/llm/responses_api_test.go @@ -9,6 +9,8 @@ import ( "fmt" "io" "net/http" + "os" + "path/filepath" "strings" "testing" "time" @@ -145,6 +147,47 @@ func TestBuildResponsesInput_FilePolicyAllowsNativePDF(t *testing.T) { } } +func TestBuildResponsesInput_FilePolicyHydratesPathOnlyNativePDF(t *testing.T) { + dataHome := t.TempDir() + t.Setenv("XDG_DATA_HOME", dataHome) + uploadsDir := filepath.Join(dataHome, "term-llm", "uploads") + if err := os.MkdirAll(uploadsDir, 0o700); err != nil { + t.Fatalf("mkdir uploads: %v", err) + } + filePath := filepath.Join(uploadsDir, "doc.pdf") + if err := os.WriteFile(filePath, []byte("file-bytes"), 0o600); err != nil { + t.Fatalf("write upload: %v", err) + } + messages := []Message{{Role: RoleUser, Parts: []Part{{ + Type: PartFile, + Text: "[User uploaded file: doc.pdf — saved locally]", + FileData: &ToolFileData{ + MediaType: "application/pdf", + Filename: "doc.pdf", + SizeBytes: int64(len("file-bytes")), + }, + FilePath: filePath, + }}}} + + input := BuildResponsesInput(messages) + if len(input) != 1 { + t.Fatalf("len(input) = %d, want 1", len(input)) + } + parts, ok := input[0].Content.([]ResponsesContentPart) + if !ok || len(parts) != 1 { + t.Fatalf("content = %#v, want one content part", input[0].Content) + } + if parts[0].Type != "input_file" || parts[0].Filename != "doc.pdf" { + t.Fatalf("file part = %#v", parts[0]) + } + if parts[0].FileData != "data:application/pdf;base64,ZmlsZS1ieXRlcw==" { + t.Fatalf("file_data = %q", parts[0].FileData) + } + if strings.Contains(fmt.Sprint(input[0].Content), filePath) { + t.Fatalf("native file content leaked local path: %#v", input[0].Content) + } +} + func TestBuildResponsesInput_FilePolicyFallsBackToTextWhenNativeDisabled(t *testing.T) { policy := DefaultPortableTextFileUploadPolicy() messages := []Message{{Role: RoleUser, Parts: []Part{{ diff --git a/internal/session/file_upload_test.go b/internal/session/file_upload_test.go index 945e8898..d7e270cb 100644 --- a/internal/session/file_upload_test.go +++ b/internal/session/file_upload_test.go @@ -116,6 +116,59 @@ func TestMessageStripsImageBase64WhenStorageOptionEnabled(t *testing.T) { } } +func TestMessageStripsUploadedFileBase64WhenStorageOptionEnabled(t *testing.T) { + dataHome := t.TempDir() + t.Setenv("XDG_DATA_HOME", dataHome) + uploadPath := filepath.Join(dataHome, "term-llm", "uploads", "doc.pdf") + if err := os.MkdirAll(filepath.Dir(uploadPath), 0o700); err != nil { + t.Fatalf("mkdir uploads: %v", err) + } + if err := os.WriteFile(uploadPath, []byte("file-bytes"), 0o600); err != nil { + t.Fatalf("write upload: %v", err) + } + const b64 = "ZmlsZS1ieXRlcw==" + msg := NewMessage("sess_test", llm.Message{Role: llm.RoleUser, Parts: []llm.Part{{ + Type: llm.PartFile, + Text: "[User uploaded file: doc.pdf — saved locally]\n\n", + FileData: &llm.ToolFileData{ + MediaType: "application/pdf", + Base64: b64, + Filename: "doc.pdf", + SizeBytes: int64(len("file-bytes")), + }, + FilePath: uploadPath, + }}}, 0) + + if msg.Parts[0].FileData == nil || msg.Parts[0].FileData.Base64 != b64 { + t.Fatal("in-memory message should preserve file base64 for the active request") + } + partsJSON, err := msg.PartsJSONForStorage(true) + if err != nil { + t.Fatalf("PartsJSONForStorage: %v", err) + } + if strings.Contains(partsJSON, b64) { + t.Fatalf("storage parts json retained file base64 despite strip option: %s", partsJSON) + } + + var roundTrip Message + if err := roundTrip.SetPartsFromJSON(partsJSON); err != nil { + t.Fatalf("SetPartsFromJSON: %v", err) + } + part := roundTrip.Parts[0] + if part.FilePath != uploadPath { + t.Fatalf("FilePath = %q, want preserved path", part.FilePath) + } + if part.FileData == nil { + t.Fatal("FileData is nil, want file metadata preserved") + } + if part.FileData.Base64 != "" { + t.Fatalf("FileData.Base64 = %q, want stripped", part.FileData.Base64) + } + if part.FileData.MediaType != "application/pdf" || part.FileData.Filename != "doc.pdf" || part.FileData.SizeBytes != int64(len("file-bytes")) { + t.Fatalf("FileData metadata = %+v, want media type/filename/size preserved", part.FileData) + } +} + func TestSQLiteStoreStripImageBase64Config(t *testing.T) { ctx := context.Background() dataHome := t.TempDir() diff --git a/internal/session/types.go b/internal/session/types.go index 9024213e..5388dd46 100644 --- a/internal/session/types.go +++ b/internal/session/types.go @@ -285,6 +285,17 @@ func partsForStorage(parts []llm.Part) []llm.Part { out = append(out, copyPart) continue } + if part.Type == llm.PartFile && isSessionUploadPath(part.FilePath) && part.FileData != nil && strings.TrimSpace(part.FileData.Base64) != "" { + if out == nil { + out = append([]llm.Part(nil), parts[:i]...) + } + copyPart := part + fileData := *part.FileData + fileData.Base64 = "" + copyPart.FileData = &fileData + out = append(out, copyPart) + continue + } if out != nil { out = append(out, part) }