diff --git a/services/core/IMPLEMENTATION.md b/services/core/IMPLEMENTATION.md index a4637edb7..13d442c73 100644 --- a/services/core/IMPLEMENTATION.md +++ b/services/core/IMPLEMENTATION.md @@ -23,6 +23,7 @@ Shared vocabulary has one owner each, and domains use it rather than copy it. `i Domain owners, each with its PostgreSQL adapter under `internal/persistence/postgres`: - `agents` (`agentpg`): saved Agents, their configuration merge and bounds, and the encrypted model-provider bundle bound to each Agent. +- `files` (`filepg`): source Files. ## Request handling @@ -42,7 +43,7 @@ On the Beta group, the OpenAI-Beta check (exactly one `agents=v1` value) runs be ## Source Files and Artifacts -Source Files are Project resources with a lifecycle independent of copied workspace files. Store immutable source metadata and PostgreSQL large objects in Core's database with the pinned pgx driver. Upload validation, metadata insertion and the bytes commit atomically; deletion removes the metadata and unlinks the object in one transaction. Keep OIDs private and authorize every metadata, content and delete lookup by tenant before opening a body. Stream bounded chunks; never hold an entire upload in memory or use a filename as a filesystem path. A direct download of the `user_data` purpose is rejected after the tenant-scoped metadata lookup, while initialization and workspace copies keep their authorized store read. A read-only repeatable-read transaction preserves an admitted source across concurrent deletion; resolve that snapshot before entering the Environment write path, and a later deletion never undoes a completed workspace copy. Bound request and transaction lifetimes, roll back incomplete bodies and never retry an ambiguous commit automatically. Backups must include PostgreSQL large objects, and a schema rollback must not orphan them. +Source Files are Project resources with a lifecycle independent of copied workspace files. `files` owns their vocabulary, the upload envelope and 512 MiB content bound, the list rules and the create and delete use cases; `filepg` stores immutable source metadata and PostgreSQL large objects in Core's database with the pinned pgx driver. Upload validation, metadata insertion, the bytes and the write audit commit atomically; deletion removes the metadata, unlinks the object and records the audit in one transaction. Keep OIDs private and authorize every metadata, content and delete lookup by tenant before opening a body. Write content through `pgunit`'s large-object writer, which streams bounded chunks and reports the size and SHA-256 digest; never hold an entire upload in memory or use a filename as a filesystem path. A direct download of the `user_data` purpose is rejected after the tenant-scoped metadata lookup, while workspace copies keep their authorized `filepg` read and Session initialization copies read the object inside Session creation's store transaction. A read-only repeatable-read transaction preserves an admitted source across concurrent deletion; resolve that snapshot before entering the Environment write path, and a later deletion never undoes a completed workspace copy. Bound request and transaction lifetimes, roll back incomplete bodies and never retry an ambiguous commit automatically. Backups must include PostgreSQL large objects, and a schema rollback must not orphan them. Session Artifacts are immutable published copies, separate from live workspace files and source Files. The private output exporter reuses the authorized workspace path boundary and streams bounded bytes; publication requires complete capture and confirmed helper and transport success, not merely valid archive syntax. The daemon owns and drains the exporter's stdout pipe separately from child reaping, so pull-transport backpressure cannot consume the process-exit I/O deadline; after helper exit, each pipe read has one second, reset after consumer delays, which rejects inherited pipes that never close. Cancellation closes the owned reader and the dispatch consumer, then waits for the child. Never extract an output archive into Core's filesystem or hold the execution lease through a large transfer. diff --git a/services/core/cmd/server/http_routes_test.go b/services/core/cmd/server/http_routes_test.go index ee574a376..2fd157351 100644 --- a/services/core/cmd/server/http_routes_test.go +++ b/services/core/cmd/server/http_routes_test.go @@ -88,7 +88,7 @@ func daemonComposition(t testing.TB) http.Handler { apiHandler, err := api.NewHandler(api.Dependencies{ Engine: "codex", CoreKeys: admin, InstallationBindings: struct{ api.InstallationBindings }{}, Projects: trapProjects{keys: keys}, Vaults: struct{ api.Vaults }{}, ModelProviders: struct{ api.ModelProviders }{}, - Files: struct{ api.Files }{}, Skills: struct{ api.Skills }{}, EnvironmentTemplates: struct{ api.EnvironmentTemplates }{}, + Files: struct{ api.Files }{}, FilesReader: struct{ api.FilesReader }{}, Skills: struct{ api.Skills }{}, EnvironmentTemplates: struct{ api.EnvironmentTemplates }{}, Agents: struct{ api.Agents }{}, AgentsReader: struct{ api.AgentsReader }{}, Sessions: struct{ api.Sessions }{}, SessionEvents: struct{ api.SessionEvents }{}, SessionHistory: struct{ api.SessionHistory }{}, Subagents: struct{ api.Subagents }{}, Artifacts: struct{ api.Artifacts }{}, SessionAdmin: struct{ api.SessionAdmin }{}, Environments: struct{ api.Environments }{}, ExecutorConnections: struct{ api.ExecutorConnections }{}, diff --git a/services/core/cmd/server/main.go b/services/core/cmd/server/main.go index 986219268..6f40a95fb 100644 --- a/services/core/cmd/server/main.go +++ b/services/core/cmd/server/main.go @@ -36,9 +36,11 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/coremetrics" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/databaseurl" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/execution" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/nativeinstaller" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/agentpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/filepg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtime" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimeenrollment" @@ -299,11 +301,18 @@ func run() error { metricsDone := make(chan struct{}) go func() { defer close(metricsDone); metrics.Run(metricsCtx) }() defer func() { cancelMetrics(); <-metricsDone }() + fileStore := filepg.New(units) + fileService, err := files.NewService(fileStore) + if err != nil { + return err + } deps := api.Dependencies{ Engine: engine, Harnesses: kinds, CoreKeys: keyAdmin, Installation: installation, InstallationBindings: executionStore, - Projects: executionStore, Vaults: executionStore, ModelProviders: executionStore, Files: executionStore, - Skills: executionStore, EnvironmentTemplates: executionStore, Agents: agentService, AgentsReader: agentStore, + Projects: executionStore, Vaults: executionStore, ModelProviders: executionStore, + Skills: executionStore, EnvironmentTemplates: executionStore, + Files: fileService, FilesReader: fileStore, + Agents: agentService, AgentsReader: agentStore, Sessions: executionStore, SessionEvents: executionStore, SessionHistory: executionStore, Subagents: executionStore, Artifacts: executionStore, SessionAdmin: executionStore, Environments: executionStore, ExecutorConnections: executorConnections{store: executionStore, registry: registry}, diff --git a/services/core/internal/api/dependencies.go b/services/core/internal/api/dependencies.go index 97f01b944..9ea033253 100644 --- a/services/core/internal/api/dependencies.go +++ b/services/core/internal/api/dependencies.go @@ -33,6 +33,7 @@ type Dependencies struct { Vaults Vaults ModelProviders ModelProviders Files Files + FilesReader FilesReader Skills Skills EnvironmentTemplates EnvironmentTemplates Agents Agents @@ -111,9 +112,10 @@ func (d Dependencies) validate() error { } if err := required( field{"InstallationBindings", d.InstallationBindings}, field{"Projects", d.Projects}, field{"Vaults", d.Vaults}, - field{"ModelProviders", d.ModelProviders}, field{"Files", d.Files}, field{"Skills", d.Skills}, - field{"EnvironmentTemplates", d.EnvironmentTemplates}, field{"Agents", d.Agents}, field{"AgentsReader", d.AgentsReader}, - field{"Sessions", d.Sessions}, + field{"ModelProviders", d.ModelProviders}, field{"Skills", d.Skills}, + field{"Files", d.Files}, field{"FilesReader", d.FilesReader}, + field{"Agents", d.Agents}, field{"AgentsReader", d.AgentsReader}, + field{"EnvironmentTemplates", d.EnvironmentTemplates}, field{"Sessions", d.Sessions}, field{"SessionEvents", d.SessionEvents}, field{"SessionHistory", d.SessionHistory}, field{"Subagents", d.Subagents}, field{"Artifacts", d.Artifacts}, field{"SessionAdmin", d.SessionAdmin}, field{"Environments", d.Environments}, field{"ExecutorConnections", d.ExecutorConnections}, field{"Admin", d.Admin}, field{"AdminAudit", d.AdminAudit}, field{"WriteAudit", d.WriteAudit}, diff --git a/services/core/internal/api/dependencies_test.go b/services/core/internal/api/dependencies_test.go index b0a6d4b3d..153c9e92b 100644 --- a/services/core/internal/api/dependencies_test.go +++ b/services/core/internal/api/dependencies_test.go @@ -19,6 +19,7 @@ type testFakes struct { vaults *fakeVaults modelProviders *fakeModelProviders files *fakeFiles + filesReader *fakeFilesReader skills *fakeSkills environmentTemplates *fakeEnvironmentTemplates agents *fakeAgents @@ -55,8 +56,10 @@ func testDependencies(t testing.TB) (Dependencies, *testFakes) { t.Helper() f := &testFakes{ projects: &fakeProjects{t: t}, vaults: &fakeVaults{t: t}, modelProviders: &fakeModelProviders{t: t}, - files: &fakeFiles{t: t}, skills: &fakeSkills{t: t}, environmentTemplates: &fakeEnvironmentTemplates{t: t}, - agents: &fakeAgents{t: t}, agentsReader: &fakeAgentsReader{t: t}, sessions: &fakeSessions{t: t}, sessionEvents: &fakeSessionEvents{t: t}, + skills: &fakeSkills{t: t}, environmentTemplates: &fakeEnvironmentTemplates{t: t}, + files: &fakeFiles{t: t}, filesReader: &fakeFilesReader{t: t}, + agents: &fakeAgents{t: t}, agentsReader: &fakeAgentsReader{t: t}, + sessions: &fakeSessions{t: t}, sessionEvents: &fakeSessionEvents{t: t}, sessionHistory: &fakeSessionHistory{t: t}, subagents: &fakeSubagents{t: t}, artifacts: &fakeArtifacts{t: t}, sessionAdmin: &fakeSessionAdmin{t: t}, environments: &fakeEnvironments{t: t}, executorConnections: &fakeExecutorConnections{t: t}, admin: &fakeAdmin{t: t}, adminAudit: &fakeAdminAudit{t: t}, writeAudit: &fakeWriteAudit{t: t}, metrics: &fakeMetrics{t: t}, @@ -66,8 +69,10 @@ func testDependencies(t testing.TB) (Dependencies, *testFakes) { } return Dependencies{ Engine: "codex", CoreKeys: coreKeys(t, "admin"), InstallationBindings: f.installationBindings, - Projects: f.projects, Vaults: f.vaults, ModelProviders: f.modelProviders, Files: f.files, Skills: f.skills, - EnvironmentTemplates: f.environmentTemplates, Agents: f.agents, AgentsReader: f.agentsReader, Sessions: f.sessions, SessionEvents: f.sessionEvents, + Projects: f.projects, Vaults: f.vaults, ModelProviders: f.modelProviders, Skills: f.skills, + Files: f.files, FilesReader: f.filesReader, + Agents: f.agents, AgentsReader: f.agentsReader, + EnvironmentTemplates: f.environmentTemplates, Sessions: f.sessions, SessionEvents: f.sessionEvents, SessionHistory: f.sessionHistory, Subagents: f.subagents, Artifacts: f.artifacts, SessionAdmin: f.sessionAdmin, Environments: f.environments, ExecutorConnections: f.executorConnections, Admin: f.admin, AdminAudit: f.adminAudit, WriteAudit: f.writeAudit, Metrics: f.metrics, RuntimeObservations: f.runtimeObservations, RuntimeHistory: f.runtimeHistory, diff --git a/services/core/internal/api/environment_files_create.go b/services/core/internal/api/environment_files_create.go index 8adebd5a7..1f39433b7 100644 --- a/services/core/internal/api/environment_files_create.go +++ b/services/core/internal/api/environment_files_create.go @@ -17,6 +17,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/echotext" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/execution" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/go-chi/chi/v5" ) @@ -101,9 +102,9 @@ func (h *Handler) createEnvironmentFile(w http.ResponseWriter, r *http.Request) if request.Type == "file_id" { ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) defer cancel() - err = h.Files.ReadSourceFile(ctx, tenantID(r), *request.FileID, func(file store.SourceFile, body io.Reader) error { + err = h.FilesReader.Read(ctx, tenantID(r), *request.FileID, func(file files.File, body io.Reader) error { if file.SizeBytes > proto.WorkspaceWriteMaxBytes { - return store.ErrSourceFileTooLarge + return files.ErrTooLarge } data, err = io.ReadAll(io.LimitReader(body, proto.WorkspaceWriteMaxBytes+1)) if err == nil && int64(len(data)) != file.SizeBytes { @@ -112,7 +113,7 @@ func (h *Handler) createEnvironmentFile(w http.ResponseWriter, r *http.Request) return err }) if err != nil { - writeStoreError(w, r, err) + writeFilesError(w, r, err) return } } diff --git a/services/core/internal/api/environment_files_write_test.go b/services/core/internal/api/environment_files_write_test.go index c470aab21..29beb0b02 100644 --- a/services/core/internal/api/environment_files_write_test.go +++ b/services/core/internal/api/environment_files_write_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/execution" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/google/uuid" ) @@ -47,7 +48,7 @@ func TestEnvironmentFileCreateSourceCopyKeepsDestinationBound(t *testing.T) { h, f := environmentFileCreateHandler(t, sources.wire) data := bytes.Repeat([]byte{9}, 6<<20) sources.tenant, sources.data = f.environment.TenantID, data - sources.file = store.SourceFile{ID: "file-" + uuid.NewString(), SizeBytes: int64(len(data)), CreatedAt: time.Unix(1, 0)} + sources.file = files.File{ID: "file-" + uuid.NewString(), SizeBytes: int64(len(data)), CreatedAt: time.Unix(1, 0)} body := `{"type":"file_id","file_id":"` + sources.file.ID + `","path":"/workspace/copy.bin"}` if w := requestCreateEnvironmentFile(h, f.environment.ID, body, "files-key"); w.Code != 201 || f.writes != 1 || !bytes.Equal(f.data, data) { t.Fatal("6 MiB source copy rejected", w.Code, w.Body) diff --git a/services/core/internal/api/errors.go b/services/core/internal/api/errors.go index 03dda1553..1c832d4fc 100644 --- a/services/core/internal/api/errors.go +++ b/services/core/internal/api/errors.go @@ -73,6 +73,12 @@ func writeInputError(w http.ResponseWriter, r *http.Request, err error) { } } +// writeContentTooLarge reports uploaded or copied content beyond the +// operation's limit. +func writeContentTooLarge(w http.ResponseWriter) { + writeError(w, http.StatusRequestEntityTooLarge, "request_too_large", "File exceeds this operation's content limit.") +} + const unstorableTextMessage = "Request text contains characters this service cannot store or compare, such as U+0000 or invalid UTF-8." // fieldError is a request validation failure reported with the official @@ -185,8 +191,6 @@ func writeStoreError(w http.ResponseWriter, r *http.Request, err error, notFound case errors.Is(err, store.ErrDefaultSkillVersion): writeError(w, http.StatusBadRequest, "invalid_value", "Cannot delete the default skill version.", "version") - case errors.Is(err, store.ErrSourceFileTooLarge): - writeError(w, http.StatusRequestEntityTooLarge, "request_too_large", "File exceeds this operation's content limit.") case errors.Is(err, store.ErrModelProviderRequired): writeError(w, http.StatusBadRequest, "model_provider_required", "This Session was created without a model provider and cannot run. Create a new Session with x_agents_core.model_provider or an Agent that has one saved.") case errors.Is(err, store.ErrHostedEnvironmentFailed): @@ -220,8 +224,8 @@ func writeStoreError(w http.ResponseWriter, r *http.Request, err error, notFound } case errors.Is(err, store.ErrNotFound): code := "not_found_error" - // Files and Skills retain their non-beta error envelope. - if strings.HasPrefix(r.URL.Path, "/v1/files/") || strings.HasPrefix(r.URL.Path, "/v1/skills/") || r.URL.Path == "/v1/files" || r.URL.Path == "/v1/skills" { + // Skills retain their non-beta error envelope. + if strings.HasPrefix(r.URL.Path, "/v1/skills/") || r.URL.Path == "/v1/skills" { code = "" } writeError(w, http.StatusNotFound, code, "Resource not found.", notFoundParam...) diff --git a/services/core/internal/api/errors_files.go b/services/core/internal/api/errors_files.go new file mode 100644 index 000000000..ed21fc37c --- /dev/null +++ b/services/core/internal/api/errors_files.go @@ -0,0 +1,35 @@ +package api + +import ( + "errors" + "net/http" + "strings" + + "github.com/MiniMax-AI/OpenAgentCore/internal/obs/log" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" +) + +// writeFilesError maps a files error to its public response. notFoundParam +// names the parameter a missing File came from. +func writeFilesError(w http.ResponseWriter, r *http.Request, err error, notFoundParam ...string) { + if writeAuditSourceError(w, r, err) || writeTextValueError(w, r, err) || writeCredentialUnavailableError(w, r, err) { + return + } + switch { + case errors.Is(err, files.ErrNotFound): + code := "not_found_error" + // The Files routes keep their non-beta error envelope. + if r.URL.Path == "/v1/files" || strings.HasPrefix(r.URL.Path, "/v1/files/") { + code = "" + } + writeError(w, http.StatusNotFound, code, "Resource not found.", notFoundParam...) + case errors.Is(err, files.ErrTooLarge): + writeContentTooLarge(w) + case errors.Is(err, files.ErrInvalidInput): + writeError(w, http.StatusBadRequest, "invalid_request", "Invalid resource identifier or request limits.") + default: + // Storage failures can include submitted values; do not log the error. + log.Ctx(r.Context()).Error("oac-core persistence operation failed") + writeError(w, http.StatusInternalServerError, "internal_error", "The operation could not be completed.") + } +} diff --git a/services/core/internal/api/errors_test.go b/services/core/internal/api/errors_test.go index 97b571dea..02618526e 100644 --- a/services/core/internal/api/errors_test.go +++ b/services/core/internal/api/errors_test.go @@ -13,6 +13,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/adminaudit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/execution" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/textvalue" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/writeaudit" @@ -27,7 +28,11 @@ func TestResourceNotFoundErrorSurfaces(t *testing.T) { t.Run(path, func(t *testing.T) { response := httptest.NewRecorder() request := httptest.NewRequest(http.MethodGet, path, nil) - writeStoreError(response, request, fmt.Errorf("lookup: %w", store.ErrNotFound)) + if strings.HasPrefix(path, "/v1/files") { + writeFilesError(response, request, fmt.Errorf("lookup: %w", files.ErrNotFound)) + } else { + writeStoreError(response, request, fmt.Errorf("lookup: %w", store.ErrNotFound)) + } var body v1.ErrorResponse if response.Code != http.StatusNotFound || json.Unmarshal(response.Body.Bytes(), &body) != nil { t.Fatalf("response = %d %s", response.Code, response.Body) diff --git a/services/core/internal/api/fakes_test.go b/services/core/internal/api/fakes_test.go index 3170131e0..180b892fc 100644 --- a/services/core/internal/api/fakes_test.go +++ b/services/core/internal/api/fakes_test.go @@ -12,6 +12,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/coremetrics" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/environmentconfig" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/identity" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimehistory" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimeobs" @@ -461,48 +462,52 @@ func (f *fakeExecutorConnections) ExecutorConnected(a0 context.Context, a1 strin return f.executorConnected(a0, a1, a2) } -type fakeFiles struct { - t testing.TB - createSourceFile func(context.Context, string, func(io.Writer) (store.SourceFileUpload, error)) (store.SourceFile, error) - getSourceFile func(context.Context, string, string) (store.SourceFile, error) - listSourceFiles func(context.Context, string, string, int, bool, *string) (store.SourceFilePage, error) - readSourceFile func(context.Context, string, string, func(store.SourceFile, io.Reader) error) error - deleteSourceFile func(context.Context, string, string) error +type fakeFilesReader struct { + t testing.TB + get func(ctx context.Context, tenantID, fileID string) (files.File, error) + list func(ctx context.Context, tenantID string, query files.ListQuery) (files.Page, error) + read func(ctx context.Context, tenantID, fileID string, consume func(files.File, io.Reader) error) error } -func (f *fakeFiles) CreateSourceFile(a0 context.Context, a1 string, a2 func(io.Writer) (store.SourceFileUpload, error)) (store.SourceFile, error) { - if f.createSourceFile == nil { - unexpectedCall(f.t, "CreateSourceFile") +func (f *fakeFilesReader) Get(a0 context.Context, a1 string, a2 string) (files.File, error) { + if f.get == nil { + unexpectedCall(f.t, "Get") } - return f.createSourceFile(a0, a1, a2) + return f.get(a0, a1, a2) } -func (f *fakeFiles) GetSourceFile(a0 context.Context, a1 string, a2 string) (store.SourceFile, error) { - if f.getSourceFile == nil { - unexpectedCall(f.t, "GetSourceFile") +func (f *fakeFilesReader) List(a0 context.Context, a1 string, a2 files.ListQuery) (files.Page, error) { + if f.list == nil { + unexpectedCall(f.t, "List") } - return f.getSourceFile(a0, a1, a2) + return f.list(a0, a1, a2) } -func (f *fakeFiles) ListSourceFiles(a0 context.Context, a1 string, a2 string, a3 int, a4 bool, a5 *string) (store.SourceFilePage, error) { - if f.listSourceFiles == nil { - unexpectedCall(f.t, "ListSourceFiles") +func (f *fakeFilesReader) Read(a0 context.Context, a1 string, a2 string, a3 func(files.File, io.Reader) error) error { + if f.read == nil { + unexpectedCall(f.t, "Read") } - return f.listSourceFiles(a0, a1, a2, a3, a4, a5) + return f.read(a0, a1, a2, a3) +} + +type fakeFiles struct { + t testing.TB + create func(context.Context, files.CreateCommand) (files.File, error) + delete func(context.Context, files.DeleteCommand) error } -func (f *fakeFiles) ReadSourceFile(a0 context.Context, a1 string, a2 string, a3 func(store.SourceFile, io.Reader) error) error { - if f.readSourceFile == nil { - unexpectedCall(f.t, "ReadSourceFile") +func (f *fakeFiles) Create(a0 context.Context, a1 files.CreateCommand) (files.File, error) { + if f.create == nil { + unexpectedCall(f.t, "Create") } - return f.readSourceFile(a0, a1, a2, a3) + return f.create(a0, a1) } -func (f *fakeFiles) DeleteSourceFile(a0 context.Context, a1 string, a2 string) error { - if f.deleteSourceFile == nil { - unexpectedCall(f.t, "DeleteSourceFile") +func (f *fakeFiles) Delete(a0 context.Context, a1 files.DeleteCommand) error { + if f.delete == nil { + unexpectedCall(f.t, "Delete") } - return f.deleteSourceFile(a0, a1, a2) + return f.delete(a0, a1) } type fakeInstallationBindings struct { diff --git a/services/core/internal/api/routing_test.go b/services/core/internal/api/routing_test.go index d369ec35a..f3047473d 100644 --- a/services/core/internal/api/routing_test.go +++ b/services/core/internal/api/routing_test.go @@ -20,8 +20,8 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/obs/log" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/go-chi/chi/v5" "github.com/google/uuid" ) @@ -92,8 +92,8 @@ func routingFixture(t *testing.T) (http.Handler, *chi.Mux, *routingStore) { deps, fakes := testDependencies(trapTB{t}) fakes.projects.resolveProjectAPIKey = keys.ResolveProjectAPIKey fakes.agentsReader.getAgent, fakes.agentsReader.listAgents, fakes.agents.update = s.GetAgent, s.ListAgents, s.Update - fakes.files.getSourceFile = func(context.Context, string, string) (store.SourceFile, error) { - return store.SourceFile{}, store.ErrNotFound + fakes.filesReader.get = func(context.Context, string, string) (files.File, error) { + return files.File{}, files.ErrNotFound } deps.CoreKeys = coreKeys(t, routingAdminKey) deps.Execution, deps.Sandboxes = fakes.execution(), fakes.sandboxes() diff --git a/services/core/internal/api/skills_transfer.go b/services/core/internal/api/skills_transfer.go index 7463d1de7..fbd9f1621 100644 --- a/services/core/internal/api/skills_transfer.go +++ b/services/core/internal/api/skills_transfer.go @@ -52,7 +52,7 @@ func (h *Handler) uploadSkill(w http.ResponseWriter, r *http.Request, version bo if err != nil { var limit *http.MaxBytesError if errors.As(err, &limit) { - writeStoreError(w, r, store.ErrSourceFileTooLarge) + writeContentTooLarge(w) } else { writeStoreError(w, r, store.ErrInvalidInput) } diff --git a/services/core/internal/api/source_files.go b/services/core/internal/api/source_files.go index 77338d275..cd74e7a2d 100644 --- a/services/core/internal/api/source_files.go +++ b/services/core/internal/api/source_files.go @@ -7,17 +7,21 @@ import ( "time" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/go-chi/chi/v5" ) -// Files manages project-owned source Files and streams their bytes. +// Files creates and deletes project-owned source Files. type Files interface { - CreateSourceFile(context.Context, string, func(io.Writer) (store.SourceFileUpload, error)) (store.SourceFile, error) - GetSourceFile(context.Context, string, string) (store.SourceFile, error) - ListSourceFiles(context.Context, string, string, int, bool, *string) (store.SourceFilePage, error) - ReadSourceFile(context.Context, string, string, func(store.SourceFile, io.Reader) error) error - DeleteSourceFile(context.Context, string, string) error + Create(context.Context, files.CreateCommand) (files.File, error) + Delete(context.Context, files.DeleteCommand) error +} + +// FilesReader reads source File metadata and streams their bytes. +type FilesReader interface { + Get(ctx context.Context, tenantID, fileID string) (files.File, error) + List(ctx context.Context, tenantID string, query files.ListQuery) (files.Page, error) + Read(ctx context.Context, tenantID, fileID string, consume func(files.File, io.Reader) error) error } // @Summary Retrieve source file metadata @@ -32,9 +36,9 @@ type Files interface { func (h *Handler) getSourceFile(w http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) defer cancel() - file, err := h.Files.GetSourceFile(ctx, tenantID(r), chi.URLParam(r, "file_id")) + file, err := h.FilesReader.Get(ctx, tenantID(r), chi.URLParam(r, "file_id")) if err != nil { - writeStoreError(w, r, err, "id") + writeFilesError(w, r, err, "id") return } writeJSON(w, http.StatusOK, sourceFileResponse(file)) @@ -53,14 +57,14 @@ func (h *Handler) deleteSourceFile(w http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) defer cancel() id := chi.URLParam(r, "file_id") - if err := h.Files.DeleteSourceFile(ctx, tenantID(r), id); err != nil { - writeStoreError(w, r, err, "id") + if err := h.Files.Delete(ctx, files.DeleteCommand{TenantID: tenantID(r), FileID: id}); err != nil { + writeFilesError(w, r, err, "id") return } writeJSON(w, http.StatusOK, v1.SourceFileDeleted{ID: id, Object: "file", Deleted: true}) } -func sourceFileResponse(file store.SourceFile) v1.SourceFile { +func sourceFileResponse(file files.File) v1.SourceFile { return v1.SourceFile{ID: file.ID, Object: "file", Bytes: file.SizeBytes, CreatedAt: file.CreatedAt.Unix(), Filename: file.Filename, Purpose: file.Purpose, Status: "processed"} diff --git a/services/core/internal/api/source_files_content.go b/services/core/internal/api/source_files_content.go index 194e7486c..5e804e83a 100644 --- a/services/core/internal/api/source_files_content.go +++ b/services/core/internal/api/source_files_content.go @@ -22,9 +22,9 @@ import ( func (h *Handler) sourceFileContent(w http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) defer cancel() - _, err := h.Files.GetSourceFile(ctx, tenantID(r), chi.URLParam(r, "file_id")) + _, err := h.FilesReader.Get(ctx, tenantID(r), chi.URLParam(r, "file_id")) if err != nil { - writeStoreError(w, r, err, "id") + writeFilesError(w, r, err, "id") return } writeError(w, http.StatusBadRequest, "", "Not allowed to download files of purpose: user_data") diff --git a/services/core/internal/api/source_files_errors_test.go b/services/core/internal/api/source_files_errors_test.go index d9d3b8537..eee42750f 100644 --- a/services/core/internal/api/source_files_errors_test.go +++ b/services/core/internal/api/source_files_errors_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "testing" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" ) @@ -23,7 +24,7 @@ func TestSourceFileMissingErrorParameters(t *testing.T) { {http.MethodGet, "/v1/files?after=file-missing&unknown=1", "after"}, } { t.Run(tc.method+tc.path, func(t *testing.T) { - f := &sourceFilesFixture{listErr: fmt.Errorf("wrapped: %w", store.ErrNotFound)} + f := &sourceFilesFixture{listErr: fmt.Errorf("wrapped: %w", files.ErrNotFound)} h, _ := environmentFileCreateHandler(t, f.wire) server := newSourceFileServer(t, h) status, raw := sourceRequest(t, server, tc.method, tc.path, "files-key", "", nil) @@ -49,8 +50,6 @@ func TestStoreErrorOptionalParameterPreservesOtherErrors(t *testing.T) { }{ {"/v1/skills/skill_missing", store.ErrNotFound, 404, nil, nil}, {"/v1/agents/agent_missing", store.ErrNotFound, 404, "not_found_error", nil}, - {"/v1/files/file-missing", store.ErrInvalidInput, 400, "invalid_request", []string{"id"}}, - {"/v1/files/file-missing", fmt.Errorf("database unavailable"), 500, "internal_error", []string{"id"}}, } { w := httptest.NewRecorder() writeStoreError(w, httptest.NewRequest(http.MethodGet, tc.path, nil), tc.err, tc.param...) @@ -63,3 +62,31 @@ func TestStoreErrorOptionalParameterPreservesOtherErrors(t *testing.T) { } } } + +func TestFilesErrorOptionalParameterPreservesOtherErrors(t *testing.T) { + for _, tc := range []struct { + path string + err error + status int + code any + }{ + {"/v1/files/file-missing", files.ErrInvalidInput, 400, "invalid_request"}, + {"/v1/files/file-missing", fmt.Errorf("database unavailable"), 500, "internal_error"}, + {"/v1/files/file-missing", files.ErrTooLarge, 413, "request_too_large"}, + {"/core/v1/projects/project/files/file-missing", files.ErrNotFound, 404, "not_found_error"}, + } { + w := httptest.NewRecorder() + writeFilesError(w, httptest.NewRequest(http.MethodGet, tc.path, nil), tc.err, "id") + var body map[string]map[string]any + if w.Code != tc.status || json.Unmarshal(w.Body.Bytes(), &body) != nil { + t.Fatalf("unexpected error: %d %s", w.Code, w.Body.String()) + } + wantParam := any(nil) + if tc.status == http.StatusNotFound { + wantParam = "id" + } + if body["error"]["code"] != tc.code || body["error"]["param"] != wantParam { + t.Fatalf("unrelated error changed: %s", w.Body.String()) + } + } +} diff --git a/services/core/internal/api/source_files_list.go b/services/core/internal/api/source_files_list.go index 14c2e95a0..ed266eb2d 100644 --- a/services/core/internal/api/source_files_list.go +++ b/services/core/internal/api/source_files_list.go @@ -4,6 +4,7 @@ import ( "net/http" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" ) // @Summary List source files @@ -23,9 +24,9 @@ func (h *Handler) listSourceFiles(w http.ResponseWriter, r *http.Request) { if !ok { return } - page, err := h.Files.ListSourceFiles(r.Context(), tenantID(r), options.after, options.limit, options.ascending, purpose) + page, err := h.FilesReader.List(r.Context(), tenantID(r), files.ListQuery{After: options.after, Limit: options.limit, Ascending: options.ascending, Purpose: purpose}) if err != nil { - writeStoreError(w, r, err, "after") + writeFilesError(w, r, err, "after") return } response := v1.SourceFileList{Object: "list", Data: make([]v1.SourceFile, 0, len(page.Files)), HasMore: page.NextCursor != ""} @@ -41,7 +42,7 @@ func (h *Handler) listSourceFiles(w http.ResponseWriter, r *http.Request) { func readSourceFilePage(w http.ResponseWriter, r *http.Request) (pageOptions, *string, bool) { q := r.URL.Query() - options, ok := readPageQueryLimits(w, r, q, 10000, 10000, false, "purpose") + options, ok := readPageQueryLimits(w, r, q, files.MaxPageSize, files.MaxPageSize, false, "purpose") if !ok { return pageOptions{}, nil, false } @@ -51,7 +52,7 @@ func readSourceFilePage(w http.ResponseWriter, r *http.Request) (pageOptions, *s return options, nil, true } switch values[0] { - case "user_data", "assistants", "batch", "fine-tune", "vision", "evals", "assistants_output", "batch_output", "fine-tune-results": + case files.PurposeUserData, "assistants", "batch", "fine-tune", "vision", "evals", "assistants_output", "batch_output", "fine-tune-results": default: writeError(w, http.StatusBadRequest, "", "Invalid purpose.", "purpose") return pageOptions{}, nil, false diff --git a/services/core/internal/api/source_files_list_test.go b/services/core/internal/api/source_files_list_test.go index f63bae6e0..f36f74aec 100644 --- a/services/core/internal/api/source_files_list_test.go +++ b/services/core/internal/api/source_files_list_test.go @@ -9,7 +9,7 @@ import ( "time" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" ) func TestSourceFileListParametersAndEnvelope(t *testing.T) { @@ -27,8 +27,8 @@ func TestSourceFileListParametersAndEnvelope(t *testing.T) { t.Fatalf("default list changed: %s calls=%d after=%q limit=%d asc=%t purpose=%v", raw, f.listCalls, f.listAfter, f.listLimit, f.listAsc, f.listPurpose) } - file := store.SourceFile{ID: "file-00000000-0000-0000-0000-000000000001", Filename: "one.bin", Purpose: "user_data", SizeBytes: 3, CreatedAt: time.Unix(123, 0)} - f.listPage = store.SourceFilePage{Files: []store.SourceFile{file}, NextCursor: file.ID} + file := files.File{ID: "file-00000000-0000-0000-0000-000000000001", Filename: "one.bin", Purpose: "user_data", SizeBytes: 3, CreatedAt: time.Unix(123, 0)} + f.listPage = files.Page{Files: []files.File{file}, NextCursor: file.ID} status, raw = sourceRequest(t, server, http.MethodGet, "/v1/files?after="+file.ID+"&limit=7&order=asc&purpose=user_data", "files-key", "", nil) var page v1.SourceFileList if status != http.StatusOK || json.Unmarshal(raw, &page) != nil { @@ -90,7 +90,7 @@ func TestSourceFileListIgnoresUnknownKeysAndEmptyPurpose(t *testing.T) { } func TestSourceFileListMapsStorageErrors(t *testing.T) { - f := &sourceFilesFixture{listErr: store.ErrNotFound} + f := &sourceFilesFixture{listErr: files.ErrNotFound} h, _ := environmentFileCreateHandler(t, f.wire) server := newSourceFileServer(t, h) status, _ := sourceRequest(t, server, http.MethodGet, "/v1/files?after=file-missing", "files-key", "", nil) @@ -102,7 +102,7 @@ func TestSourceFileListMapsStorageErrors(t *testing.T) { func TestSourceFileListPurposeValidationBeforeCursorLookup(t *testing.T) { for _, purpose := range []string{"", "user_data", "assistants", "batch", "fine-tune", "vision", "evals", "assistants_output", "batch_output", "fine-tune-results", "unknown", "USER_DATA"} { t.Run("purpose="+purpose, func(t *testing.T) { - f := &sourceFilesFixture{listErr: store.ErrNotFound} + f := &sourceFilesFixture{listErr: files.ErrNotFound} h, _ := environmentFileCreateHandler(t, f.wire) server := newSourceFileServer(t, h) status, raw := sourceRequest(t, server, http.MethodGet, "/v1/files?after=file-missing&purpose="+purpose, "files-key", "", nil) diff --git a/services/core/internal/api/source_files_test.go b/services/core/internal/api/source_files_test.go index 37b69cbfa..068ec1b48 100644 --- a/services/core/internal/api/source_files_test.go +++ b/services/core/internal/api/source_files_test.go @@ -15,17 +15,19 @@ import ( v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/google/uuid" ) +// sourceFilesFixture keeps Files in memory. It is the storage behind a real +// files.Service and the FilesReader, so the handlers run the domain rules. type sourceFilesFixture struct { mu sync.Mutex tenant string - file store.SourceFile + file files.File data []byte reads int - listPage store.SourceFilePage + listPage files.Page listErr error listCalls int listAfter string @@ -34,38 +36,38 @@ type sourceFilesFixture struct { listPurpose *string } -func (f *sourceFilesFixture) CreateSourceFile(_ context.Context, tenant string, upload func(io.Writer) (store.SourceFileUpload, error)) (store.SourceFile, error) { +func (f *sourceFilesFixture) Create(_ context.Context, tenant string, write func(io.Writer) (files.Upload, error)) (files.File, error) { var body bytes.Buffer - input, err := upload(&body) + input, err := write(&body) if err != nil { - return store.SourceFile{}, err + return files.File{}, err } f.mu.Lock() defer f.mu.Unlock() f.tenant, f.data = tenant, body.Bytes() - f.file = store.SourceFile{ID: "file-" + uuid.NewString(), Filename: input.Filename, Purpose: input.Purpose, SizeBytes: int64(body.Len()), CreatedAt: time.Unix(123, 0)} + f.file = files.File{ID: "file-" + uuid.NewString(), Filename: input.Filename, Purpose: input.Purpose, SizeBytes: int64(body.Len()), CreatedAt: time.Unix(123, 0)} return f.file, nil } -func (f *sourceFilesFixture) GetSourceFile(_ context.Context, tenant, id string) (store.SourceFile, error) { +func (f *sourceFilesFixture) Get(_ context.Context, tenant, id string) (files.File, error) { f.mu.Lock() defer f.mu.Unlock() if f.file.ID != id || f.tenant != tenant { - return store.SourceFile{}, store.ErrNotFound + return files.File{}, files.ErrNotFound } return f.file, nil } -func (f *sourceFilesFixture) ListSourceFiles(_ context.Context, tenant, after string, limit int, ascending bool, purpose *string) (store.SourceFilePage, error) { +func (f *sourceFilesFixture) List(_ context.Context, tenant string, query files.ListQuery) (files.Page, error) { f.mu.Lock() defer f.mu.Unlock() - f.tenant, f.listAfter, f.listLimit, f.listAsc, f.listPurpose = tenant, after, limit, ascending, purpose + f.tenant, f.listAfter, f.listLimit, f.listAsc, f.listPurpose = tenant, query.After, query.Limit, query.Ascending, query.Purpose f.listCalls++ return f.listPage, f.listErr } -func (f *sourceFilesFixture) ReadSourceFile(ctx context.Context, tenant, id string, consume func(store.SourceFile, io.Reader) error) error { - file, err := f.GetSourceFile(ctx, tenant, id) +func (f *sourceFilesFixture) Read(ctx context.Context, tenant, id string, consume func(files.File, io.Reader) error) error { + file, err := f.Get(ctx, tenant, id) if err != nil { return err } @@ -76,21 +78,25 @@ func (f *sourceFilesFixture) ReadSourceFile(ctx context.Context, tenant, id stri return consume(file, bytes.NewReader(data)) } -func (f *sourceFilesFixture) DeleteSourceFile(ctx context.Context, tenant, id string) error { - if _, err := f.GetSourceFile(ctx, tenant, id); err != nil { +func (f *sourceFilesFixture) Delete(ctx context.Context, tenant, id string) error { + if _, err := f.Get(ctx, tenant, id); err != nil { return err } f.mu.Lock() defer f.mu.Unlock() - f.file = store.SourceFile{} + f.file = files.File{} f.data = nil return nil } -// wire serves the Files area from f. +// wire serves the Files areas from f. func (f *sourceFilesFixture) wire(_ *Dependencies, fakes *testFakes) { - fakes.files.createSourceFile, fakes.files.getSourceFile, fakes.files.listSourceFiles = f.CreateSourceFile, f.GetSourceFile, f.ListSourceFiles - fakes.files.readSourceFile, fakes.files.deleteSourceFile = f.ReadSourceFile, f.DeleteSourceFile + service, err := files.NewService(f) + if err != nil { + fakes.files.t.Fatal(err) + } + fakes.files.create, fakes.files.delete = service.Create, service.Delete + fakes.filesReader.get, fakes.filesReader.list, fakes.filesReader.read = f.Get, f.List, f.Read } func sourceMultipart(t *testing.T, fields []string, data []byte) ([]byte, string) { @@ -214,7 +220,7 @@ func TestSourceFilesRejectIncompleteOrUnsupportedMultipart(t *testing.T) { } func TestEnvironmentSourceCopyEnforcesScopeUnionAndSize(t *testing.T) { - f := &sourceFilesFixture{file: store.SourceFile{ID: "file-" + uuid.NewString(), SizeBytes: proto.WorkspaceWriteMaxBytes + 1}} + f := &sourceFilesFixture{file: files.File{ID: "file-" + uuid.NewString(), SizeBytes: proto.WorkspaceWriteMaxBytes + 1}} h, env := environmentFileCreateHandler(t, f.wire) f.tenant = env.environment.TenantID body := `{"type":"file_id","file_id":"` + f.file.ID + `","path":"/workspace/source.bin"}` diff --git a/services/core/internal/api/source_files_upload.go b/services/core/internal/api/source_files_upload.go index 97c39b919..b4e4c44a3 100644 --- a/services/core/internal/api/source_files_upload.go +++ b/services/core/internal/api/source_files_upload.go @@ -8,7 +8,7 @@ import ( "net/http" "time" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" ) const sourceTransferTimeout = 5 * time.Minute @@ -33,25 +33,25 @@ func (h *Handler) createSourceFile(w http.ResponseWriter, r *http.Request) { } ctx, cancel := context.WithDeadline(r.Context(), deadline) defer cancel() - r.Body = http.MaxBytesReader(w, r.Body, store.MaxSourceFileBytes+(64<<10)) - file, err := h.Files.CreateSourceFile(ctx, tenantID(r), func(dst io.Writer) (store.SourceFileUpload, error) { + r.Body = http.MaxBytesReader(w, r.Body, files.MaxBytes+(64<<10)) + file, err := h.Files.Create(ctx, files.CreateCommand{TenantID: tenantID(r), Upload: func(dst io.Writer) (files.Upload, error) { return readSourceUpload(r, dst) - }) + }}) if err != nil { - writeStoreError(w, r, err) + writeFilesError(w, r, err) return } writeJSON(w, http.StatusOK, sourceFileResponse(file)) } -func readSourceUpload(r *http.Request, dst io.Writer) (store.SourceFileUpload, error) { - var input store.SourceFileUpload +func readSourceUpload(r *http.Request, dst io.Writer) (files.Upload, error) { + var input files.Upload if r.Header.Get("Content-Encoding") != "" { - return input, store.ErrInvalidInput + return input, files.ErrInvalidInput } multi, err := r.MultipartReader() if err != nil { - return input, store.ErrInvalidInput + return input, files.ErrInvalidInput } seen := make(map[string]bool, 2) buffer := make([]byte, 256<<10) @@ -66,7 +66,7 @@ func readSourceUpload(r *http.Request, dst io.Writer) (store.SourceFileUpload, e kind, attrs, err := mime.ParseMediaType(part.Header.Get("Content-Disposition")) name := attrs["name"] if err != nil || kind != "form-data" || seen[name] || (name != "file" && name != "purpose") || part.Header.Get("Content-Transfer-Encoding") != "" { - return input, store.ErrInvalidInput + return input, files.ErrInvalidInput } seen[name] = true if name == "file" { @@ -76,11 +76,11 @@ func readSourceUpload(r *http.Request, dst io.Writer) (store.SourceFileUpload, e } } else { if _, exists := attrs["filename"]; exists { - return input, store.ErrInvalidInput + return input, files.ErrInvalidInput } value, err := io.ReadAll(io.LimitReader(part, 65)) if err != nil || len(value) > 64 { - return input, store.ErrInvalidInput + return input, files.ErrInvalidInput } input.Purpose = string(value) } @@ -91,16 +91,16 @@ func readSourceUpload(r *http.Request, dst io.Writer) (store.SourceFileUpload, e if _, err := io.CopyBuffer(io.Discard, r.Body, buffer); err != nil { return input, sourceUploadError(err) } - if !seen["file"] || !seen["purpose"] || input.Purpose != "user_data" { - return input, store.ErrInvalidInput + if !seen["file"] || !seen["purpose"] || input.Purpose != files.PurposeUserData { + return input, files.ErrInvalidInput } return input, nil } func sourceUploadError(err error) error { var limit *http.MaxBytesError - if errors.As(err, &limit) || errors.Is(err, store.ErrSourceFileTooLarge) { - return store.ErrSourceFileTooLarge + if errors.As(err, &limit) || errors.Is(err, files.ErrTooLarge) { + return files.ErrTooLarge } - return store.ErrInvalidInput + return files.ErrInvalidInput } diff --git a/services/core/internal/files/doc.go b/services/core/internal/files/doc.go new file mode 100644 index 000000000..8fae28f56 --- /dev/null +++ b/services/core/internal/files/doc.go @@ -0,0 +1,5 @@ +// Package files owns source Files: Project-owned uploaded bytes with immutable +// metadata, independent of Sessions and of the workspace copies made from them. +// It defines their vocabulary, the upload and listing rules, and the create and +// delete use cases, and declares the storage they need. +package files diff --git a/services/core/internal/files/errors.go b/services/core/internal/files/errors.go new file mode 100644 index 000000000..dbd9b126b --- /dev/null +++ b/services/core/internal/files/errors.go @@ -0,0 +1,14 @@ +package files + +import "errors" + +var ( + // ErrNotFound reports a File the tenant does not own, including a malformed + // or foreign ID, so missing and foreign Files are indistinguishable. + ErrNotFound = errors.New("file not found") + // ErrInvalidInput reports an upload envelope, list query or tenant that + // Core does not accept. + ErrInvalidInput = errors.New("invalid file request") + // ErrTooLarge reports content beyond the operation's limit. + ErrTooLarge = errors.New("file exceeds the content limit") +) diff --git a/services/core/internal/files/files.go b/services/core/internal/files/files.go new file mode 100644 index 000000000..01799855b --- /dev/null +++ b/services/core/internal/files/files.go @@ -0,0 +1,93 @@ +package files + +import ( + "fmt" + "strings" + "time" + "unicode/utf8" + + "github.com/google/uuid" +) + +const ( + // MaxBytes bounds the content of one File. + MaxBytes int64 = 512 << 20 + // MaxPageSize bounds one List page. + MaxPageSize = 10000 + // PurposeUserData is the only purpose Core stores. + PurposeUserData = "user_data" + + idPrefix = "file-" +) + +// File is a stored File's immutable metadata. +type File struct { + ID string + Filename string + Purpose string + SizeBytes int64 + CreatedAt time.Time +} + +// Upload is the envelope read alongside an upload's body. +type Upload struct { + Filename string + Purpose string +} + +// Validate accepts the envelopes Core stores: a filename of 1 to 1024 bytes of +// valid UTF-8 without U+0000, and the user_data purpose. +func (u Upload) Validate() error { + if len(u.Filename) < 1 || len(u.Filename) > 1024 || !validText(u.Filename) || u.Purpose != PurposeUserData { + return ErrInvalidInput + } + return nil +} + +// ListQuery selects one page of a tenant's Files, ordered by creation time and +// then ID. +type ListQuery struct { + // After is the ID of the previous page's last File, or empty for the first + // page. + After string + Limit int + Ascending bool + // Purpose, when not nil, keeps only Files with this purpose. + Purpose *string +} + +// Validate accepts a page size of 1 to MaxPageSize and a purpose filter that +// PostgreSQL can compare. +func (q ListQuery) Validate() error { + if q.Limit < 1 || q.Limit > MaxPageSize { + return fmt.Errorf("%w: page size must be 1..%d", ErrInvalidInput, MaxPageSize) + } + if q.Purpose != nil && !validText(*q.Purpose) { + return ErrInvalidInput + } + return nil +} + +// Page is one List page. NextCursor is the ID of its last File when more Files +// follow, and empty otherwise. +type Page struct { + Files []File + NextCursor string +} + +// FormatID returns the ID of the File stored under id. +func FormatID(id uuid.UUID) string { return idPrefix + id.String() } + +// ParseID returns the stored UUID that a File ID names. ok is false for an ID +// Core never assigns: another spelling, another prefix or the nil UUID. +func ParseID(id string) (uuid.UUID, bool) { + parsed, err := uuid.Parse(strings.TrimPrefix(id, idPrefix)) + if err != nil || parsed == uuid.Nil || id != FormatID(parsed) { + return uuid.UUID{}, false + } + return parsed, true +} + +func validText(value string) bool { + return utf8.ValidString(value) && !strings.ContainsRune(value, '\x00') +} diff --git a/services/core/internal/files/files_test.go b/services/core/internal/files/files_test.go new file mode 100644 index 000000000..20649890a --- /dev/null +++ b/services/core/internal/files/files_test.go @@ -0,0 +1,109 @@ +package files + +import ( + "errors" + "strings" + "testing" + + "github.com/google/uuid" +) + +func TestUploadValidate(t *testing.T) { + for name, test := range map[string]struct { + upload Upload + valid bool + }{ + "one byte": {Upload{Filename: "a", Purpose: PurposeUserData}, true}, + "longest filename": {Upload{Filename: strings.Repeat("a", 1024), Purpose: PurposeUserData}, true}, + "empty filename": {Upload{Purpose: PurposeUserData}, false}, + "long filename": {Upload{Filename: strings.Repeat("a", 1025), Purpose: PurposeUserData}, false}, + "invalid UTF-8": {Upload{Filename: "bad\xff", Purpose: PurposeUserData}, false}, + "NUL": {Upload{Filename: "bad\x00", Purpose: PurposeUserData}, false}, + "other purpose": {Upload{Filename: "a", Purpose: "assistants"}, false}, + "missing purpose": {Upload{Filename: "a"}, false}, + "multibyte at 1024": {Upload{Filename: strings.Repeat("é", 512), Purpose: PurposeUserData}, true}, + } { + t.Run(name, func(t *testing.T) { + err := test.upload.Validate() + if test.valid != (err == nil) || err != nil && !errors.Is(err, ErrInvalidInput) { + t.Fatalf("Validate() = %v, want valid %v", err, test.valid) + } + }) + } +} + +func TestListQueryValidate(t *testing.T) { + purpose := func(value string) *string { return &value } + for name, test := range map[string]struct { + query ListQuery + valid bool + }{ + "smallest page": {ListQuery{Limit: 1}, true}, + "largest page": {ListQuery{Limit: MaxPageSize}, true}, + "empty page": {ListQuery{}, false}, + "oversized page": {ListQuery{Limit: MaxPageSize + 1}, false}, + "any storable purpose": {ListQuery{Limit: 1, Purpose: purpose("assistants")}, true}, + "invalid UTF-8 purpose": {ListQuery{Limit: 1, Purpose: purpose("\xff")}, false}, + "NUL purpose": {ListQuery{Limit: 1, Purpose: purpose("user\x00data")}, false}, + } { + t.Run(name, func(t *testing.T) { + err := test.query.Validate() + if test.valid != (err == nil) || err != nil && !errors.Is(err, ErrInvalidInput) { + t.Fatalf("Validate() = %v, want valid %v", err, test.valid) + } + }) + } +} + +func TestParseIDAcceptsOnlyAssignedIDs(t *testing.T) { + id := uuid.New() + if parsed, ok := ParseID(FormatID(id)); !ok || parsed != id { + t.Fatalf("ParseID(FormatID) = %v, %v", parsed, ok) + } + for _, value := range []string{ + "", id.String(), "file_" + id.String(), "file-" + strings.ToUpper(id.String()), + "file-{" + id.String() + "}", "file-" + strings.ReplaceAll(id.String(), "-", ""), + "file-urn:uuid:" + id.String(), FormatID(uuid.Nil), "file-not-a-uuid", + } { + if _, ok := ParseID(value); ok { + t.Errorf("ParseID(%q) accepted an ID Core never assigns", value) + } + } +} + +func TestBoundedWriterRejectsOverflowWithoutWriting(t *testing.T) { + var body strings.Builder + w := &boundedWriter{body: &body, left: 4} + if n, err := w.Write([]byte("abc")); n != 3 || err != nil { + t.Fatal(n, err) + } + if n, err := w.Write([]byte("de")); n != 0 || !errors.Is(err, ErrTooLarge) { + t.Fatal("overflowing write", n, err) + } + if n, err := w.Write([]byte("d")); n != 0 || !errors.Is(err, ErrTooLarge) { + t.Fatal("write after a failure was not rejected", n, err) + } + if body.String() != "abc" { + t.Fatalf("body = %q", body.String()) + } +} + +func TestBoundedWriterKeepsBodyFailures(t *testing.T) { + failure := errors.New("body failed") + w := &boundedWriter{body: shortWriter{err: failure}, left: 10} + if n, err := w.Write([]byte("abc")); n != 1 || !errors.Is(err, failure) { + t.Fatal(n, err) + } + if _, err := w.Write([]byte("d")); !errors.Is(err, failure) { + t.Fatal("body failure was not kept", err) + } + w = &boundedWriter{body: shortWriter{}, left: 10} + if n, err := w.Write([]byte("abc")); n != 1 || err == nil { + t.Fatal("short write without an error was accepted", n, err) + } +} + +// shortWriter accepts one byte of each write and returns err. +type shortWriter struct{ err error } + +func (w shortWriter) Write(p []byte) (int, error) { return min(len(p), 1), w.err } diff --git a/services/core/internal/files/service.go b/services/core/internal/files/service.go new file mode 100644 index 000000000..325f7ec31 --- /dev/null +++ b/services/core/internal/files/service.go @@ -0,0 +1,87 @@ +package files + +import ( + "context" + "errors" + "io" +) + +// Service runs the File use cases that change stored state. +type Service struct { + storage Storage +} + +func NewService(storage Storage) (*Service, error) { + if storage == nil { + return nil, errors.New("files: storage is required") + } + return &Service{storage: storage}, nil +} + +// CreateCommand uploads one File. Upload writes the content to the writer it +// receives and returns the envelope read with it. +type CreateCommand struct { + TenantID string + Upload func(io.Writer) (Upload, error) +} + +// Create stores the File only after its complete content fits MaxBytes and its +// envelope validates. A content write that failed, including one beyond +// MaxBytes, fails the upload even when Upload ignored the write error. +func (s *Service) Create(ctx context.Context, command CreateCommand) (File, error) { + if command.Upload == nil { + return File{}, ErrInvalidInput + } + return s.storage.Create(ctx, command.TenantID, func(body io.Writer) (Upload, error) { + content := &boundedWriter{body: body, left: MaxBytes} + upload, err := command.Upload(content) + if content.err != nil { + return Upload{}, content.err + } + if err != nil { + return Upload{}, err + } + if err := upload.Validate(); err != nil { + return Upload{}, err + } + return upload, nil + }) +} + +// DeleteCommand deletes one File. +type DeleteCommand struct { + TenantID string + FileID string +} + +// Delete removes the File and its content. Workspace copies made from it are +// independent and stay. +func (s *Service) Delete(ctx context.Context, command DeleteCommand) error { + return s.storage.Delete(ctx, command.TenantID, command.FileID) +} + +// boundedWriter passes at most left bytes to body. A write that would exceed +// the bound writes nothing and fails with ErrTooLarge. The first failure is +// kept and returned by every later write. +type boundedWriter struct { + body io.Writer + left int64 + err error +} + +func (w *boundedWriter) Write(p []byte) (int, error) { + if w.err != nil { + return 0, w.err + } + if int64(len(p)) > w.left { + w.err = ErrTooLarge + return 0, w.err + } + n, err := w.body.Write(p) + w.left -= int64(n) + if err == nil && n != len(p) { + err = io.ErrShortWrite + } + w.err = err + return n, err +} diff --git a/services/core/internal/files/service_test.go b/services/core/internal/files/service_test.go new file mode 100644 index 000000000..a3c2a1200 --- /dev/null +++ b/services/core/internal/files/service_test.go @@ -0,0 +1,148 @@ +package files + +import ( + "bytes" + "context" + "errors" + "io" + "testing" +) + +type fakeStorage struct { + t *testing.T + create func(context.Context, string, func(io.Writer) (Upload, error)) (File, error) + delete func(context.Context, string, string) error +} + +func (f fakeStorage) Create(ctx context.Context, tenantID string, write func(io.Writer) (Upload, error)) (File, error) { + if f.create == nil { + f.t.Fatal("unexpected call to Create") + } + return f.create(ctx, tenantID, write) +} + +func (f fakeStorage) Delete(ctx context.Context, tenantID, fileID string) error { + if f.delete == nil { + f.t.Fatal("unexpected call to Delete") + } + return f.delete(ctx, tenantID, fileID) +} + +func newTestService(t *testing.T, storage fakeStorage) *Service { + t.Helper() + storage.t = t + service, err := NewService(storage) + if err != nil { + t.Fatal(err) + } + return service +} + +// writeInto runs the storage callback against body, as the adapter does. +func writeInto(body io.Writer) func(context.Context, string, func(io.Writer) (Upload, error)) (File, error) { + return func(_ context.Context, _ string, write func(io.Writer) (Upload, error)) (File, error) { + upload, err := write(body) + if err != nil { + return File{}, err + } + return File{ID: "file-stored", Filename: upload.Filename, Purpose: upload.Purpose}, nil + } +} + +func TestNewServiceRequiresStorage(t *testing.T) { + if _, err := NewService(nil); err == nil { + t.Fatal("NewService accepted nil storage") + } +} + +func TestCreateStoresValidatedUpload(t *testing.T) { + var body bytes.Buffer + service := newTestService(t, fakeStorage{create: writeInto(&body)}) + file, err := service.Create(t.Context(), CreateCommand{TenantID: "tenant", Upload: func(w io.Writer) (Upload, error) { + _, err := io.WriteString(w, "content") + return Upload{Filename: "a.txt", Purpose: PurposeUserData}, err + }}) + if err != nil || file.Filename != "a.txt" || body.String() != "content" { + t.Fatal(file, err, body.String()) + } +} + +func TestCreateRejectsWithoutStoring(t *testing.T) { + uploadFailure := errors.New("multipart failed") + for name, test := range map[string]struct { + upload func(io.Writer) (Upload, error) + want error + }{ + "missing upload": {nil, ErrInvalidInput}, + "invalid envelope": {func(io.Writer) (Upload, error) { + return Upload{Filename: "a", Purpose: "assistants"}, nil + }, ErrInvalidInput}, + "upload failure": {func(io.Writer) (Upload, error) { return Upload{}, uploadFailure }, uploadFailure}, + "content beyond the limit": {func(w io.Writer) (Upload, error) { + _, err := w.Write(make([]byte, 1)) + if err != nil { + return Upload{}, err + } + _, err = io.CopyN(w, zeroes{}, MaxBytes) + return Upload{}, err + }, ErrTooLarge}, + // An upload that ignores a failed write must not store a partial File. + "ignored write failure": {func(w io.Writer) (Upload, error) { + _, _ = io.CopyN(w, zeroes{}, MaxBytes+1) + return Upload{Filename: "a", Purpose: PurposeUserData}, nil + }, ErrTooLarge}, + // A write failure takes precedence over the upload's own error. + "write failure before upload failure": {func(w io.Writer) (Upload, error) { + _, _ = io.CopyN(w, zeroes{}, MaxBytes+1) + return Upload{}, uploadFailure + }, ErrTooLarge}, + } { + t.Run(name, func(t *testing.T) { + storage := fakeStorage{} + if test.upload != nil { + storage.create = writeInto(io.Discard) + } + _, err := newTestService(t, storage).Create(t.Context(), CreateCommand{TenantID: "tenant", Upload: test.upload}) + if !errors.Is(err, test.want) { + t.Fatalf("Create() = %v, want %v", err, test.want) + } + }) + } +} + +func TestCreateAcceptsContentAtTheLimit(t *testing.T) { + var size int64 + service := newTestService(t, fakeStorage{create: writeInto(countingWriter{&size})}) + _, err := service.Create(t.Context(), CreateCommand{TenantID: "tenant", Upload: func(w io.Writer) (Upload, error) { + _, err := io.CopyN(w, zeroes{}, MaxBytes) + return Upload{Filename: "a", Purpose: PurposeUserData}, err + }}) + if err != nil || size != MaxBytes { + t.Fatal(size, err) + } +} + +func TestDeletePassesStorageOutcome(t *testing.T) { + var deleted string + service := newTestService(t, fakeStorage{delete: func(_ context.Context, tenantID, fileID string) error { + deleted = tenantID + "/" + fileID + return ErrNotFound + }}) + if err := service.Delete(t.Context(), DeleteCommand{TenantID: "tenant", FileID: "file-1"}); !errors.Is(err, ErrNotFound) || deleted != "tenant/file-1" { + t.Fatal(deleted, err) + } +} + +type zeroes struct{} + +func (zeroes) Read(p []byte) (int, error) { + clear(p) + return len(p), nil +} + +type countingWriter struct{ size *int64 } + +func (w countingWriter) Write(p []byte) (int, error) { + *w.size += int64(len(p)) + return len(p), nil +} diff --git a/services/core/internal/files/storage.go b/services/core/internal/files/storage.go new file mode 100644 index 000000000..8b495a3bc --- /dev/null +++ b/services/core/internal/files/storage.go @@ -0,0 +1,29 @@ +package files + +import ( + "context" + "io" +) + +// Storage persists Files. Each method runs in one transaction. +type Storage interface { + // Create allocates the content, calls write with a writer for it, and stores + // the File with the returned envelope only when write returns nil. A + // storage failure while writing takes precedence over write's own error. + Create(ctx context.Context, tenantID string, write func(io.Writer) (Upload, error)) (File, error) + // Delete removes the File and its content, or returns ErrNotFound. + Delete(ctx context.Context, tenantID, fileID string) error +} + +// Reader reads a tenant's Files. +type Reader interface { + // Get returns the File's metadata, or ErrNotFound. + Get(ctx context.Context, tenantID, fileID string) (File, error) + // List returns one page. An After cursor that names no File of the tenant + // returns ErrNotFound. + List(ctx context.Context, tenantID string, query ListQuery) (Page, error) + // Read passes consume the File's metadata and content from one snapshot, so + // a concurrent Delete never truncates a read it admitted. It returns + // consume's error unchanged. + Read(ctx context.Context, tenantID, fileID string, consume func(File, io.Reader) error) error +} diff --git a/services/core/internal/persistence/postgres/filepg/filepg.go b/services/core/internal/persistence/postgres/filepg/filepg.go new file mode 100644 index 000000000..d9da13e9b --- /dev/null +++ b/services/core/internal/persistence/postgres/filepg/filepg.go @@ -0,0 +1,215 @@ +// Package filepg stores Files in PostgreSQL: metadata in source_files and +// content in large objects. +package filepg + +import ( + "context" + "errors" + "io" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/writeaudit" +) + +type Store struct{ pool *pgunit.Pool } + +func New(pool *pgunit.Pool) *Store { return &Store{pool: pool} } + +var ( + _ files.Storage = (*Store)(nil) + _ files.Reader = (*Store)(nil) +) + +// Create writes the content into a large object and stores the File and its +// write audit in the same transaction, so a failed upload leaves nothing. +func (s *Store) Create(ctx context.Context, tenantID string, write func(io.Writer) (files.Upload, error)) (files.File, error) { + tenant, err := parseTenant(tenantID) + if err != nil { + return files.File{}, err + } + var created files.File + err = s.pool.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { + body, err := pgunit.CreateLargeObject(ctx, tx) + if err != nil { + return err + } + upload, err := write(body) + if body.Err() != nil { + return body.Err() + } + if err != nil { + return err + } + content, err := body.Close() + if err != nil { + return err + } + q := sqlc.New(tx) + row, err := q.CreateSourceFile(ctx, sqlc.CreateSourceFileParams{ + ID: pgtype.UUID{Bytes: uuid.New(), Valid: true}, TenantID: tenant, + Filename: upload.Filename, Purpose: upload.Purpose, BodyOid: pgtype.Uint32{Uint32: content.OID, Valid: true}, + SizeBytes: content.Size, Sha256: content.SHA256, + }) + if err != nil { + return err + } + created = fileFromRow(row) + return auditpg.RecordWriteAudit(ctx, q, tenantID, "create", "file", created.ID, "", writeaudit.Resource{Type: "file", ID: created.ID}) + }) + if err != nil { + return files.File{}, err + } + return created, nil +} + +func (s *Store) Get(ctx context.Context, tenantID, fileID string) (files.File, error) { + tenant, id, err := fileKey(tenantID, fileID) + if err != nil { + return files.File{}, err + } + row, err := s.pool.Queries().GetSourceFile(ctx, sqlc.GetSourceFileParams{TenantID: tenant, ID: id}) + if err != nil { + return files.File{}, rowError(err) + } + return fileFromRow(row), nil +} + +// List resolves the cursor and reads the page from one snapshot. +func (s *Store) List(ctx context.Context, tenantID string, query files.ListQuery) (files.Page, error) { + if err := query.Validate(); err != nil { + return files.Page{}, err + } + tenant, err := parseTenant(tenantID) + if err != nil { + return files.Page{}, err + } + params := sqlc.ListSourceFilesParams{ + TenantID: tenant, PageLimit: int32(query.Limit + 1), Ascending: query.Ascending, + AfterID: pgtype.UUID{Valid: true}, + } + if query.Purpose != nil { + params.Purpose = pgtype.Text{String: *query.Purpose, Valid: true} + } + var after pgtype.UUID + if query.After != "" { + id, ok := files.ParseID(query.After) + if !ok { + return files.Page{}, files.ErrNotFound + } + after = pgtype.UUID{Bytes: id, Valid: true} + } + var rows []sqlc.SourceFile + err = s.pool.Snapshot(ctx, func(ctx context.Context, tx pgx.Tx) error { + q := sqlc.New(tx) + if after.Valid { + after, err := q.GetSourceFile(ctx, sqlc.GetSourceFileParams{TenantID: tenant, ID: after}) + if err != nil { + return rowError(err) + } + params.AfterCreated, params.AfterID = after.CreatedAt, after.ID + } + listed, err := q.ListSourceFiles(ctx, params) + rows = listed + return err + }) + if err != nil { + return files.Page{}, err + } + page := files.Page{Files: make([]files.File, 0, min(query.Limit, len(rows)))} + if len(rows) > query.Limit { + page.NextCursor = fileFromRow(rows[query.Limit-1]).ID + rows = rows[:query.Limit] + } + for _, row := range rows { + page.Files = append(page.Files, fileFromRow(row)) + } + return page, nil +} + +// Read streams the content from a snapshot taken with the metadata, so a +// concurrent Delete never truncates it. +func (s *Store) Read(ctx context.Context, tenantID, fileID string, consume func(files.File, io.Reader) error) error { + tenant, id, err := fileKey(tenantID, fileID) + if err != nil { + return err + } + return s.pool.Snapshot(ctx, func(ctx context.Context, tx pgx.Tx) error { + row, err := sqlc.New(tx).GetSourceFile(ctx, sqlc.GetSourceFileParams{TenantID: tenant, ID: id}) + if err != nil { + return rowError(err) + } + objects := tx.LargeObjects() + body, err := objects.Open(ctx, row.BodyOid.Uint32, pgx.LargeObjectModeRead) + if err != nil { + return err + } + if err := consume(fileFromRow(row), body); err != nil { + return err + } + return body.Close() + }) +} + +// Delete removes the row and its large object and records the write audit in +// one transaction. +func (s *Store) Delete(ctx context.Context, tenantID, fileID string) error { + tenant, id, err := fileKey(tenantID, fileID) + if err != nil { + return err + } + return s.pool.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { + q := sqlc.New(tx) + oid, err := q.DeleteSourceFile(ctx, sqlc.DeleteSourceFileParams{TenantID: tenant, ID: id}) + if err != nil { + return rowError(err) + } + objects := tx.LargeObjects() + if err := objects.Unlink(ctx, oid.Uint32); err != nil { + return err + } + return auditpg.RecordWriteAudit(ctx, q, tenantID, "delete", "file", fileID, "") + }) +} + +// rowError translates the missing row that every File lookup acts on. The +// domain validates every stored value first, so no other database outcome is +// one a caller acts on. +func rowError(err error) error { + if errors.Is(err, pgx.ErrNoRows) { + return files.ErrNotFound + } + return err +} + +func parseTenant(tenantID string) (pgtype.UUID, error) { + tenant, err := pgunit.ParseID(tenantID) + if err != nil { + return pgtype.UUID{}, files.ErrInvalidInput + } + return tenant, nil +} + +// fileKey locates a tenant's File. An ID Core never assigns names no File. +func fileKey(tenantID, fileID string) (pgtype.UUID, pgtype.UUID, error) { + tenant, err := parseTenant(tenantID) + if err != nil { + return pgtype.UUID{}, pgtype.UUID{}, err + } + id, ok := files.ParseID(fileID) + if !ok { + return pgtype.UUID{}, pgtype.UUID{}, files.ErrNotFound + } + return tenant, pgtype.UUID{Bytes: id, Valid: true}, nil +} + +func fileFromRow(row sqlc.SourceFile) files.File { + return files.File{ID: files.FormatID(row.ID.Bytes), Filename: row.Filename, Purpose: row.Purpose, + SizeBytes: row.SizeBytes, CreatedAt: row.CreatedAt.Time} +} diff --git a/services/core/internal/persistence/postgres/filepg/filepg_test.go b/services/core/internal/persistence/postgres/filepg/filepg_test.go new file mode 100644 index 000000000..974b6449a --- /dev/null +++ b/services/core/internal/persistence/postgres/filepg/filepg_test.go @@ -0,0 +1,407 @@ +package filepg_test + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "io" + "os" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/adminaudit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/filepg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgtest" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/writeaudit" +) + +// open returns the adapter and the files service over pool. +func open(t *testing.T, pool *pgxpool.Pool) (*filepg.Store, *files.Service) { + t.Helper() + store := filepg.New(pgunit.NewPool(pool)) + service, err := files.NewService(store) + if err != nil { + t.Fatal(err) + } + return store, service +} + +func upload(data []byte) func(io.Writer) (files.Upload, error) { + return func(w io.Writer) (files.Upload, error) { + _, err := w.Write(data) + return files.Upload{Filename: "source.bin", Purpose: files.PurposeUserData}, err + } +} + +func create(t *testing.T, service *files.Service, ctx context.Context, tenant string, data []byte) files.File { + t.Helper() + file, err := service.Create(ctx, files.CreateCommand{TenantID: tenant, Upload: upload(data)}) + if err != nil { + t.Fatal(err) + } + return file +} + +func bodyOID(t *testing.T, pool *pgxpool.Pool, tenant string, file files.File) uint32 { + t.Helper() + var oid uint32 + if err := pool.QueryRow(t.Context(), "SELECT body_oid FROM source_files WHERE tenant_id=$1 AND id=$2", tenant, strings.TrimPrefix(file.ID, "file-")).Scan(&oid); err != nil { + t.Fatal(err) + } + return oid +} + +func objectExists(t *testing.T, pool *pgxpool.Pool, oid uint32) bool { + t.Helper() + var exists bool + if err := pool.QueryRow(t.Context(), "SELECT EXISTS (SELECT 1 FROM pg_largeobject_metadata WHERE oid=$1)", oid).Scan(&exists); err != nil { + t.Fatal(err) + } + return exists +} + +func objectCount(t *testing.T, pool *pgxpool.Pool) int { + t.Helper() + var count int + if err := pool.QueryRow(t.Context(), "SELECT count(*) FROM pg_largeobject_metadata").Scan(&count); err != nil { + t.Fatal(err) + } + return count +} + +func TestFilesPersistScopeAndDelete(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool) + tenant, foreign := uuid.NewString(), uuid.NewString() + for _, data := range [][]byte{{}, {0, 1, 255}, bytes.Repeat([]byte("binary\x00"), 300000)} { + file := create(t, service, t.Context(), tenant, data) + if file.SizeBytes != int64(len(data)) || file.Filename != "source.bin" || file.Purpose != files.PurposeUserData || !strings.HasPrefix(file.ID, "file-") || file.CreatedAt.IsZero() { + t.Fatalf("create: %+v", file) + } + oid := bodyOID(t, pool, tenant, file) + var digest string + if err := pool.QueryRow(t.Context(), "SELECT sha256 FROM source_files WHERE body_oid=$1", oid).Scan(&digest); err != nil || digest != sha256Hex(data) { + t.Fatal("stored digest differs", err) + } + if _, err := store.Get(t.Context(), foreign, file.ID); !errors.Is(err, files.ErrNotFound) { + t.Fatalf("foreign metadata: %v", err) + } + if err := store.Read(t.Context(), foreign, file.ID, func(files.File, io.Reader) error { + t.Fatal("foreign content callback reached") + return nil + }); !errors.Is(err, files.ErrNotFound) { + t.Fatalf("foreign content: %v", err) + } + if err := service.Delete(t.Context(), files.DeleteCommand{TenantID: foreign, FileID: file.ID}); !errors.Is(err, files.ErrNotFound) { + t.Fatalf("foreign delete: %v", err) + } + reopened, _ := open(t, pool) + if got, err := reopened.Get(t.Context(), tenant, file.ID); err != nil || got != file { + t.Fatalf("metadata: %+v %v", got, err) + } + if err := reopened.Read(t.Context(), tenant, file.ID, func(meta files.File, r io.Reader) error { + got, err := io.ReadAll(r) + if meta != file || !bytes.Equal(got, data) { + t.Error("persisted contents differ") + } + return err + }); err != nil { + t.Fatal(err) + } + if err := service.Delete(t.Context(), files.DeleteCommand{TenantID: tenant, FileID: file.ID}); err != nil { + t.Fatal(err) + } + if _, err := store.Get(t.Context(), tenant, file.ID); !errors.Is(err, files.ErrNotFound) { + t.Fatalf("deleted metadata: %v", err) + } + if err := service.Delete(t.Context(), files.DeleteCommand{TenantID: tenant, FileID: file.ID}); !errors.Is(err, files.ErrNotFound) { + t.Fatalf("repeated delete: %v", err) + } + if objectExists(t, pool, oid) { + t.Fatal("deletion orphaned the content") + } + } +} + +func TestFilesRejectInvalidIdentifiers(t *testing.T) { + store, service := open(t, pgtest.Open(t)) + tenant := uuid.NewString() + file := create(t, service, t.Context(), tenant, []byte("x")) + id := strings.TrimPrefix(file.ID, "file-") + for _, missing := range []string{id, "file-" + strings.ToUpper(id), "file-" + uuid.Nil.String(), "invalid", ""} { + if _, err := store.Get(t.Context(), tenant, missing); !errors.Is(err, files.ErrNotFound) { + t.Fatalf("Get(%q) = %v", missing, err) + } + } + for _, invalid := range []string{"", "invalid", uuid.Nil.String()} { + if _, err := store.Get(t.Context(), invalid, file.ID); !errors.Is(err, files.ErrInvalidInput) { + t.Fatalf("tenant %q: %v", invalid, err) + } + if _, err := service.Create(t.Context(), files.CreateCommand{TenantID: invalid, Upload: upload(nil)}); !errors.Is(err, files.ErrInvalidInput) { + t.Fatalf("create in tenant %q: %v", invalid, err) + } + } +} + +func auditContext(ctx context.Context, tenant, request string) context.Context { + return writeaudit.WithSource(ctx, writeaudit.Source{ + KeyID: "static:" + strings.Repeat("a", 64), Name: "file audit fixture", Prefix: "aaaaaaaa", + Kind: "static", TenantID: tenant, RequestID: request, TraceID: "file-audit-trace", + }) +} + +// project registers tenant as a Project, which administrator audit rows +// reference. +func project(t *testing.T, pool *pgxpool.Pool, tenant string) { + t.Helper() + for _, statement := range []string{ + "INSERT INTO execution_project_scopes(tenant_id,organization_id,project_id) VALUES($1,'files',$2)", + "INSERT INTO projects(id,name,tenant_id,subject_kind,subject_id) VALUES($1,'Files fixture',$2,'service_account','project:files')", + } { + if _, err := pool.Exec(t.Context(), statement, tenant, tenant); err != nil { + t.Fatal(err) + } + } +} + +func adminContext(ctx context.Context, tenant, request string) context.Context { + // An inherited public provenance must not turn an administrator operation + // into a user-key operation. + return adminaudit.WithSource(auditContext(ctx, tenant, request), adminaudit.Source{ + CredentialID: "87654321", ActorLabel: "administrator fixture", ProjectID: tenant, RequestID: request, TraceID: "admin-trace", + }) +} + +// Every failed create or delete leaves no row, content or audit behind. The +// database is isolated because it counts every large object and installs +// triggers that fail the final audit insertion. +func TestFilesRollBackFailedWrites(t *testing.T) { + pool := pgtest.OpenIsolated(t, nil) + _, err := pool.Exec(t.Context(), `CREATE FUNCTION reject_file_audit_fixture() RETURNS trigger LANGUAGE plpgsql AS $$ + BEGIN IF NEW.request_id = 'reject-file-audit' THEN RAISE EXCEPTION 'forced audit insertion failure'; END IF; RETURN NEW; END $$; + CREATE TRIGGER reject_file_audit_fixture BEFORE INSERT ON write_audit_operations FOR EACH ROW EXECUTE FUNCTION reject_file_audit_fixture(); + CREATE TRIGGER reject_file_admin_audit_fixture BEFORE INSERT ON admin_audit_log FOR EACH ROW EXECUTE FUNCTION reject_file_audit_fixture()`) + if err != nil { + t.Fatal(err) + } + _, service := open(t, pool) + tenant := uuid.NewString() + project(t, pool, tenant) + snapshot := func() string { + t.Helper() + var rows string + if err := pool.QueryRow(t.Context(), `SELECT (SELECT COALESCE(jsonb_agg(to_jsonb(r) ORDER BY r.id)::text, '[]') FROM source_files r) + || (SELECT count(*) FROM write_audit_operations)::text || (SELECT count(*) FROM write_audit_owners)::text || (SELECT count(*) FROM admin_audit_log)::text`).Scan(&rows); err != nil { + t.Fatal(err) + } + return rows + } + kept := create(t, service, t.Context(), tenant, []byte("kept")) + before, objects := snapshot(), objectCount(t, pool) + uploadFailure := errors.New("interrupted upload") + for name, test := range map[string]struct { + ctx func(context.Context) context.Context + upload func(cancel context.CancelFunc) func(io.Writer) (files.Upload, error) + delete bool + want error + }{ + "interrupted body": {upload: func(context.CancelFunc) func(io.Writer) (files.Upload, error) { + return func(w io.Writer) (files.Upload, error) { + _, _ = w.Write([]byte("not committed")) + return files.Upload{}, uploadFailure + } + }, want: uploadFailure}, + "invalid purpose": {upload: envelope(files.Upload{Filename: "source.bin", Purpose: "batch"}), want: files.ErrInvalidInput}, + "invalid filename": {upload: envelope(files.Upload{Filename: "bad\x00name", Purpose: files.PurposeUserData}), want: files.ErrInvalidInput}, + "cancelled": {upload: func(cancel context.CancelFunc) func(io.Writer) (files.Upload, error) { + return func(w io.Writer) (files.Upload, error) { + _, err := w.Write([]byte("not committed")) + cancel() + return files.Upload{Filename: "source.bin", Purpose: files.PurposeUserData}, err + } + }}, + "invalid audit source": {ctx: func(ctx context.Context) context.Context { + return auditContext(ctx, uuid.NewString(), "foreign-source") + }, upload: envelope(files.Upload{Filename: "source.bin", Purpose: files.PurposeUserData}), want: writeaudit.ErrInvalidSource}, + "create audit failure": {ctx: func(ctx context.Context) context.Context { + return auditContext(ctx, tenant, "reject-file-audit") + }, upload: envelope(files.Upload{Filename: "source.bin", Purpose: files.PurposeUserData})}, + "delete audit failure": {ctx: func(ctx context.Context) context.Context { + return auditContext(ctx, tenant, "reject-file-audit") + }, delete: true}, + "delete administrator audit failure": {ctx: func(ctx context.Context) context.Context { + return adminContext(ctx, tenant, "reject-file-audit") + }, delete: true}, + } { + t.Run(name, func(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + if test.ctx != nil { + ctx = test.ctx(ctx) + } + if test.delete { + err = service.Delete(ctx, files.DeleteCommand{TenantID: tenant, FileID: kept.ID}) + } else { + _, err = service.Create(ctx, files.CreateCommand{TenantID: tenant, Upload: test.upload(cancel)}) + } + if err == nil || test.want != nil && !errors.Is(err, test.want) { + t.Fatalf("got %v, want %v", err, test.want) + } + if snapshot() != before || objectCount(t, pool) != objects { + t.Fatal("failed write left rows, content or audit") + } + }) + } +} + +func envelope(input files.Upload) func(context.CancelFunc) func(io.Writer) (files.Upload, error) { + return func(context.CancelFunc) func(io.Writer) (files.Upload, error) { + return func(w io.Writer) (files.Upload, error) { + _, err := w.Write([]byte("audit-private-token")) + return input, err + } + } +} + +func TestFilesRecordWriteAudit(t *testing.T) { + pool := pgtest.Open(t) + _, service := open(t, pool) + tenant := uuid.NewString() + created := create(t, service, auditContext(t.Context(), tenant, "create-request"), tenant, []byte("audit-private-token")) + if err := service.Delete(auditContext(t.Context(), tenant, "delete-request"), files.DeleteCommand{TenantID: tenant, FileID: created.ID}); err != nil { + t.Fatal(err) + } + unattributed := create(t, service, t.Context(), tenant, []byte("unattributed")) + var operations string + if err := pool.QueryRow(t.Context(), `SELECT string_agg(action || ' ' || resource_type || ' ' || resource_id || ' ' || request_id, ',' ORDER BY request_id) + FROM write_audit_operations WHERE tenant_id=$1`, tenant).Scan(&operations); err != nil { + t.Fatal(err) + } + if want := "create file " + created.ID + " create-request,delete file " + created.ID + " delete-request"; operations != want { + t.Fatalf("operations %q, want %q", operations, want) + } + var owners int + var raw string + if err := pool.QueryRow(t.Context(), `SELECT (SELECT count(*) FROM write_audit_owners WHERE tenant_id=$1 AND resource_type='file' AND resource_id=$2), + (SELECT COALESCE(jsonb_agg(to_jsonb(r))::text, '') FROM write_audit_operations r WHERE tenant_id=$1)`, tenant, created.ID).Scan(&owners, &raw); err != nil { + t.Fatal(err) + } + if owners != 1 || strings.Contains(raw, "audit-private-token") || strings.Contains(raw, unattributed.ID) { + t.Fatal("create ownership or audit content differs", owners) + } +} + +func TestFilesDeleteRecordsAdministratorAudit(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool) + tenant := uuid.NewString() + project(t, pool, tenant) + file := create(t, service, t.Context(), tenant, []byte("audit-private-token")) + oid := bodyOID(t, pool, tenant, file) + if err := service.Delete(adminContext(t.Context(), tenant, "admin-request"), files.DeleteCommand{TenantID: tenant, FileID: file.ID}); err != nil { + t.Fatal(err) + } + var credential, action, kind, id, raw string + if err := pool.QueryRow(t.Context(), `SELECT admin_credential_id,action,resource_type,resource_id,to_jsonb(a)::text FROM admin_audit_log a WHERE tenant_id=$1 AND request_id='admin-request'`, tenant).Scan(&credential, &action, &kind, &id, &raw); err != nil { + t.Fatal(err) + } + if credential != "87654321" || action != "delete" || kind != "file" || id != file.ID || strings.Contains(raw, "audit-private-token") { + t.Fatal("administrator audit identity differs", raw) + } + var operations int + if err := pool.QueryRow(t.Context(), "SELECT count(*) FROM write_audit_operations WHERE tenant_id=$1", tenant).Scan(&operations); err != nil || operations != 0 { + t.Fatal("administrator impersonated public-key provenance", operations, err) + } + if _, err := store.Get(t.Context(), tenant, file.ID); !errors.Is(err, files.ErrNotFound) || objectExists(t, pool, oid) { + t.Fatal("administrator deletion left the File", err) + } +} + +func TestFileReadAdmittedBeforeDeletionCompletes(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool) + tenant := uuid.NewString() + data := bytes.Repeat([]byte("immutable\x00"), 10000) + file := create(t, service, t.Context(), tenant, data) + if err := store.Read(t.Context(), tenant, file.ID, func(_ files.File, r io.Reader) error { + prefix := make([]byte, 1) + if _, err := io.ReadFull(r, prefix); err != nil { + return err + } + _, other := open(t, pool) + if err := other.Delete(t.Context(), files.DeleteCommand{TenantID: tenant, FileID: file.ID}); err != nil { + return err + } + if _, err := store.Get(t.Context(), tenant, file.ID); !errors.Is(err, files.ErrNotFound) { + t.Fatalf("new read after delete: %v", err) + } + rest, err := io.ReadAll(r) + if !bytes.Equal(append(prefix, rest...), data) { + t.Error("deletion damaged admitted read") + } + return err + }); err != nil { + t.Fatal(err) + } +} + +func TestFileReadReturnsConsumerErrors(t *testing.T) { + store, service := open(t, pgtest.Open(t)) + tenant := uuid.NewString() + file := create(t, service, t.Context(), tenant, []byte("content")) + stop := errors.New("consumer stopped") + if err := store.Read(t.Context(), tenant, file.ID, func(files.File, io.Reader) error { return stop }); err != stop { + t.Fatalf("Read() = %v, want the consumer's error", err) + } +} + +func TestFileLargeStream(t *testing.T) { + if os.Getenv("OAC_TEST_SOURCE_FILE_LARGE") != "1" { + t.Skip("opt-in 512 MiB source storage acceptance") + } + store, service := open(t, pgtest.Open(t)) + tenant := uuid.NewString() + chunk := bytes.Repeat([]byte("source\x00binary"), 20000) + want := sha256.New() + file, err := service.Create(t.Context(), files.CreateCommand{TenantID: tenant, Upload: func(w io.Writer) (files.Upload, error) { + for left := files.MaxBytes; left > 0; { + b := chunk[:min(int64(len(chunk)), left)] + if _, err := w.Write(b); err != nil { + return files.Upload{}, err + } + want.Write(b) + left -= int64(len(b)) + } + return files.Upload{Filename: "large.bin", Purpose: files.PurposeUserData}, nil + }}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := service.Delete(context.Background(), files.DeleteCommand{TenantID: tenant, FileID: file.ID}); err != nil { + t.Error(err) + } + }) + got := sha256.New() + if err := store.Read(t.Context(), tenant, file.ID, func(meta files.File, r io.Reader) error { + n, err := io.CopyBuffer(got, r, chunk) + if n != files.MaxBytes || meta.SizeBytes != n { + t.Errorf("size: %d metadata: %d", n, meta.SizeBytes) + } + return err + }); err != nil || !bytes.Equal(got.Sum(nil), want.Sum(nil)) { + t.Fatalf("large stream mismatch: %v", err) + } +} + +func sha256Hex(data []byte) string { + sum := sha256.Sum256(data) + return hex.EncodeToString(sum[:]) +} diff --git a/services/core/internal/persistence/postgres/filepg/list_test.go b/services/core/internal/persistence/postgres/filepg/list_test.go new file mode 100644 index 000000000..767788493 --- /dev/null +++ b/services/core/internal/persistence/postgres/filepg/list_test.go @@ -0,0 +1,127 @@ +package filepg_test + +import ( + "cmp" + "errors" + "reflect" + "slices" + "strings" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/filepg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgtest" +) + +func TestFileListPaginationIsolationAndReconnect(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool) + tenant, other := uuid.NewString(), uuid.NewString() + empty, err := store.List(t.Context(), tenant, files.ListQuery{Limit: files.MaxPageSize}) + if err != nil || empty.Files == nil || len(empty.Files) != 0 || empty.NextCursor != "" { + t.Fatalf("empty page: %+v, %v", empty, err) + } + + all := make([]files.File, 0, 105) + for i := range 105 { + file := create(t, service, t.Context(), tenant, []byte{byte(i)}) + stamp := time.Unix(1700000000+int64(i%2), 0).UTC() + if _, err := pool.Exec(t.Context(), "UPDATE source_files SET created_at=$1 WHERE tenant_id=$2 AND id=$3", stamp, tenant, strings.TrimPrefix(file.ID, "file-")); err != nil { + t.Fatal(err) + } + file, err = store.Get(t.Context(), tenant, file.ID) + if err != nil { + t.Fatal(err) + } + all = append(all, file) + } + slices.SortFunc(all, func(a, b files.File) int { + if c := a.CreatedAt.Compare(b.CreatedAt); c != 0 { + return c + } + return cmp.Compare(a.ID, b.ID) + }) + foreign := create(t, service, t.Context(), other, []byte("foreign")) + + read := func(current *filepg.Store, ascending bool, purpose *string, size int) []files.File { + t.Helper() + actual := []files.File{} + cursor := "" + for { + page, err := current.List(t.Context(), tenant, files.ListQuery{After: cursor, Limit: size, Ascending: ascending, Purpose: purpose}) + if err != nil || len(page.Files) == 0 || len(page.Files) > size { + t.Fatalf("page: %+v, %v", page, err) + } + actual = append(actual, page.Files...) + if len(actual) > len(all) { + t.Fatal("pagination repeated files") + } + if page.NextCursor == "" { + break + } + if page.NextCursor != page.Files[len(page.Files)-1].ID { + t.Fatal("cursor is not the last included file") + } + cursor = page.NextCursor + } + return actual + } + userData, otherPurpose, badPurpose := files.PurposeUserData, "batch", "bad\x00purpose" + for _, ascending := range []bool{true, false} { + want := slices.Clone(all) + if !ascending { + slices.Reverse(want) + } + for _, purpose := range []*string{nil, &userData} { + for _, size := range []int{17, files.MaxPageSize} { + if got := read(store, ascending, purpose, size); !reflect.DeepEqual(got, want) { + t.Fatalf("ordered page mismatch: ascending=%t size=%d", ascending, size) + } + } + } + } + filtered, err := store.List(t.Context(), tenant, files.ListQuery{Limit: files.MaxPageSize, Purpose: &otherPurpose}) + if err != nil || filtered.Files == nil || len(filtered.Files) != 0 || filtered.NextCursor != "" { + t.Fatalf("purpose filter: %+v, %v", filtered, err) + } + for _, cursor := range []string{foreign.ID, "file-" + uuid.NewString(), "invalid"} { + if _, err := store.List(t.Context(), tenant, files.ListQuery{After: cursor, Limit: 20, Ascending: true}); !errors.Is(err, files.ErrNotFound) { + t.Fatalf("foreign/unknown cursor %q: %v", cursor, err) + } + } + for _, tc := range []struct { + tenant string + query files.ListQuery + }{ + {"invalid", files.ListQuery{Limit: 20}}, {tenant, files.ListQuery{}}, {tenant, files.ListQuery{Limit: files.MaxPageSize + 1}}, + {tenant, files.ListQuery{Limit: 20, Purpose: &badPurpose}}, + } { + if _, err := store.List(t.Context(), tc.tenant, tc.query); !errors.Is(err, files.ErrInvalidInput) { + t.Fatalf("invalid query %+v: %v", tc.query, err) + } + } + tail, err := store.List(t.Context(), tenant, files.ListQuery{After: all[len(all)-1].ID, Limit: files.MaxPageSize, Ascending: true}) + if err != nil || tail.Files == nil || len(tail.Files) != 0 || tail.NextCursor != "" { + t.Fatalf("terminal page: %+v, %v", tail, err) + } + foreignPage, err := store.List(t.Context(), other, files.ListQuery{Limit: files.MaxPageSize}) + if err != nil || !reflect.DeepEqual(foreignPage.Files, []files.File{foreign}) || foreignPage.NextCursor != "" { + t.Fatalf("project isolation: %+v, %v", foreignPage, err) + } + deleted := create(t, service, t.Context(), tenant, nil) + if err := service.Delete(t.Context(), files.DeleteCommand{TenantID: tenant, FileID: deleted.ID}); err != nil { + t.Fatal(err) + } + if _, err := store.List(t.Context(), tenant, files.ListQuery{After: deleted.ID, Limit: 20}); !errors.Is(err, files.ErrNotFound) { + t.Fatalf("deleted cursor accepted: %v", err) + } + + pool.Close() + reopened, _ := open(t, pgtest.Open(t)) + if got := read(reopened, true, nil, 17); !reflect.DeepEqual(got, all) { + t.Fatal("listing changed after reconnect") + } +} diff --git a/services/core/internal/persistence/postgres/pgunit/largeobject.go b/services/core/internal/persistence/postgres/pgunit/largeobject.go new file mode 100644 index 000000000..d6611fcb7 --- /dev/null +++ b/services/core/internal/persistence/postgres/pgunit/largeobject.go @@ -0,0 +1,86 @@ +package pgunit + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "hash" + "io" + + "github.com/jackc/pgx/v5" +) + +// largeObjectChunkBytes bounds each large-object write, so one caller write +// never becomes one unbounded protocol message. +const largeObjectChunkBytes = 256 << 10 + +// LargeObject is stored content: its large object's OID, size and hex SHA-256 +// digest. +type LargeObject struct { + OID uint32 + Size int64 + SHA256 string +} + +// LargeObjectWriter streams content into a new large object in bounded chunks +// and records its size and digest. The first failure is kept: every later +// Write returns it, and Err reports it. +type LargeObjectWriter struct { + oid uint32 + body *pgx.LargeObject + hash hash.Hash + size int64 + err error +} + +// CreateLargeObject creates an empty large object in tx and opens it for +// writing. The object belongs to tx: it disappears when tx rolls back. +func CreateLargeObject(ctx context.Context, tx pgx.Tx) (*LargeObjectWriter, error) { + objects := tx.LargeObjects() + oid, err := objects.Create(ctx, 0) + if err != nil { + return nil, err + } + body, err := objects.Open(ctx, oid, pgx.LargeObjectModeWrite) + if err != nil { + return nil, err + } + return &LargeObjectWriter{oid: oid, body: body, hash: sha256.New()}, nil +} + +func (w *LargeObjectWriter) Write(p []byte) (int, error) { + if w.err != nil { + return 0, w.err + } + written := 0 + for len(p) > 0 { + chunk := p[:min(len(p), largeObjectChunkBytes)] + n, err := w.body.Write(chunk) + w.hash.Write(chunk[:n]) + w.size += int64(n) + written += n + if err == nil && n != len(chunk) { + err = io.ErrShortWrite + } + if err != nil { + w.err = err + return written, err + } + p = p[n:] + } + return written, nil +} + +// Err returns the first write failure, or nil. +func (w *LargeObjectWriter) Err() error { return w.err } + +// Close closes the object after the content is complete and describes it. +func (w *LargeObjectWriter) Close() (LargeObject, error) { + if w.err != nil { + return LargeObject{}, w.err + } + if err := w.body.Close(); err != nil { + return LargeObject{}, err + } + return LargeObject{OID: w.oid, Size: w.size, SHA256: hex.EncodeToString(w.hash.Sum(nil))}, nil +} diff --git a/services/core/internal/persistence/postgres/pgunit/largeobject_test.go b/services/core/internal/persistence/postgres/pgunit/largeobject_test.go new file mode 100644 index 000000000..4f93af05e --- /dev/null +++ b/services/core/internal/persistence/postgres/pgunit/largeobject_test.go @@ -0,0 +1,121 @@ +package pgunit + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "io" + "testing" + + "github.com/jackc/pgx/v5" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgtest" +) + +func TestLargeObjectWriterStoresChunkedContent(t *testing.T) { + pool := NewPool(pgtest.Open(t)) + content := bytes.Repeat([]byte("0123456789abcdef"), largeObjectChunkBytes/8+3) + var stored LargeObject + if err := pool.Transaction(t.Context(), func(ctx context.Context, tx pgx.Tx) error { + writer, err := CreateLargeObject(ctx, tx) + if err != nil { + return err + } + if n, err := writer.Write(content); err != nil || n != len(content) { + return errors.Join(err, io.ErrShortWrite) + } + stored, err = writer.Close() + return err + }); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { unlink(t, pool, stored.OID) }) + digest := sha256.Sum256(content) + if stored.Size != int64(len(content)) || stored.SHA256 != hex.EncodeToString(digest[:]) { + t.Fatalf("stored %+v", stored) + } + if err := pool.Snapshot(t.Context(), func(ctx context.Context, tx pgx.Tx) error { + objects := tx.LargeObjects() + body, err := objects.Open(ctx, stored.OID, pgx.LargeObjectModeRead) + if err != nil { + return err + } + read, err := io.ReadAll(body) + if err != nil { + return err + } + if !bytes.Equal(read, content) { + return errors.New("stored content differs") + } + return body.Close() + }); err != nil { + t.Fatal(err) + } +} + +func TestLargeObjectWriterBelongsToItsTransaction(t *testing.T) { + raw := pgtest.Open(t) + pool := NewPool(raw) + rollback := errors.New("roll back") + var oid uint32 + err := pool.Transaction(t.Context(), func(ctx context.Context, tx pgx.Tx) error { + writer, err := CreateLargeObject(ctx, tx) + if err != nil { + return err + } + if _, err := writer.Write([]byte("content")); err != nil { + return err + } + stored, err := writer.Close() + oid = stored.OID + return errors.Join(err, rollback) + }) + if !errors.Is(err, rollback) || oid == 0 { + t.Fatal(oid, err) + } + var exists bool + if err := raw.QueryRow(t.Context(), "SELECT EXISTS (SELECT 1 FROM pg_largeobject_metadata WHERE oid=$1)", oid).Scan(&exists); err != nil || exists { + t.Fatal("rolled-back large object remains", exists, err) + } +} + +func TestLargeObjectWriterKeepsTheFirstFailure(t *testing.T) { + pool := NewPool(pgtest.Open(t)) + failed := errors.New("failed") + err := pool.Transaction(t.Context(), func(ctx context.Context, tx pgx.Tx) error { + writer, err := CreateLargeObject(ctx, tx) + if err != nil { + return err + } + // Closing the object underneath the writer makes the next write fail. + if err := writer.body.Close(); err != nil { + return err + } + if _, err := writer.Write([]byte("content")); err == nil { + return errors.New("write to a closed object succeeded") + } + first := writer.Err() + if _, err := writer.Write([]byte("more")); err != first || first == nil { + return errors.New("later write did not return the first failure") + } + if _, err := writer.Close(); err != first { + return errors.New("Close did not return the first failure") + } + return failed + }) + if !errors.Is(err, failed) { + t.Fatal(err) + } +} + +func unlink(t *testing.T, pool *Pool, oid uint32) { + t.Helper() + if err := pool.Transaction(context.Background(), func(ctx context.Context, tx pgx.Tx) error { + objects := tx.LargeObjects() + return objects.Unlink(ctx, oid) + }); err != nil { + t.Error(err) + } +} diff --git a/services/core/internal/sandbox/providers/configuration_flow_test.go b/services/core/internal/sandbox/providers/configuration_flow_test.go index adb5bd455..1611a17a0 100644 --- a/services/core/internal/sandbox/providers/configuration_flow_test.go +++ b/services/core/internal/sandbox/providers/configuration_flow_test.go @@ -103,8 +103,10 @@ func TestAdditionalConfigurationProviderUsesCommonAPIAndStore(t *testing.T) { // The flow reaches only the store areas and the deployment setup; every // other dependency panics if called. h, err := api.NewHandler(api.Dependencies{ - Engine: "codex", CoreKeys: auth, InstallationBindings: s, Projects: s, Vaults: s, ModelProviders: s, Files: s, Skills: s, - EnvironmentTemplates: s, Agents: struct{ api.Agents }{}, AgentsReader: struct{ api.AgentsReader }{}, Sessions: s, SessionEvents: s, SessionHistory: s, Subagents: s, Artifacts: s, + Engine: "codex", CoreKeys: auth, InstallationBindings: s, Projects: s, Vaults: s, ModelProviders: s, Skills: s, + Files: struct{ api.Files }{}, FilesReader: struct{ api.FilesReader }{}, + Agents: struct{ api.Agents }{}, AgentsReader: struct{ api.AgentsReader }{}, + EnvironmentTemplates: s, Sessions: s, SessionEvents: s, SessionHistory: s, Subagents: s, Artifacts: s, SessionAdmin: s, Environments: s, Admin: s, AdminAudit: struct{ api.AdminAudit }{}, WriteAudit: struct{ api.WriteAudit }{}, ExecutorConnections: struct{ api.ExecutorConnections }{}, Metrics: struct{ api.Metrics }{}, RuntimeObservations: struct{ api.RuntimeObservations }{}, RuntimeHistory: struct{ api.RuntimeHistory }{}, diff --git a/services/core/internal/store/admin_delete_audit_test.go b/services/core/internal/store/admin_delete_audit_test.go index 3ee6c27e2..b68342b40 100644 --- a/services/core/internal/store/admin_delete_audit_test.go +++ b/services/core/internal/store/admin_delete_audit_test.go @@ -105,8 +105,8 @@ func TestAdminDeleteResourceAuditTransactions(t *testing.T) { s := NewWithCredentialCipher(pool, cipher) rejectAdminAuditInsert(t, s) archive := skillArchive(t, "admin-private-archive") - tables := []string{"agents", "agent_model_execution", "environment_templates", "skills", "skill_versions", "source_files", "vaults", "vault_credentials", "sessions", "turns", "environments", "session_artifacts", "admin_audit_log", "write_audit_operations", "write_audit_owners", "pg_largeobject_metadata", "pg_largeobject"} - for _, name := range []string{"template_delete", "skill_delete", "version_delete", "version_delete_last", "file_delete", "vault_delete", "credential_delete", "oauth_delete", "session_delete", "artifact_delete"} { + tables := []string{"agents", "agent_model_execution", "environment_templates", "skills", "skill_versions", "vaults", "vault_credentials", "sessions", "turns", "environments", "session_artifacts", "admin_audit_log", "write_audit_operations", "write_audit_owners", "pg_largeobject_metadata", "pg_largeobject"} + for _, name := range []string{"template_delete", "skill_delete", "version_delete", "version_delete_last", "vault_delete", "credential_delete", "oauth_delete", "session_delete", "artifact_delete"} { t.Run(name, func(t *testing.T) { tenant := uuid.NewString() var mutation resourceAuditMutation @@ -116,9 +116,6 @@ func TestAdminDeleteResourceAuditTransactions(t *testing.T) { tenant, mutation, verifyRestored, removedObjects = prepareAdminHistoryDelete(t, s, name) } else { mutation = prepareResourceAuditMutation(t, s, tenant, name, archive) - if name == "file_delete" { - removedObjects = 1 - } if name == "template_delete" { var id string if err := pool.QueryRow(t.Context(), "SELECT id FROM environment_templates WHERE tenant_id=$1", tenant).Scan(&id); err != nil { @@ -135,7 +132,7 @@ func TestAdminDeleteResourceAuditTransactions(t *testing.T) { if _, err := pool.Exec(t.Context(), "INSERT INTO projects(id,name,tenant_id,subject_kind,subject_id) VALUES($1,'Delete fixture',$1,'service_account',$2)", tenant, "project:"+tenant); err != nil { t.Fatal(err) } - before, objects := adminMutationSnapshot(t, s, tables...), sourceObjectCount(t, pool) + before, objects := adminMutationSnapshot(t, s, tables...), largeObjectCount(t, pool) rejections := adminAuditRejections(t, s) _, err := mutation.run(adminDeleteContext(t.Context(), tenant, rejectedAdminRequest)) requireAdminAuditFailure(t, s, err, rejections) @@ -151,7 +148,7 @@ func TestAdminDeleteResourceAuditTransactions(t *testing.T) { t.Fatal(err) } assertAdminMutationAudit(t, s, tenant, request, "delete", mutation.kind, id) - if sourceObjectCount(t, pool) != objects-removedObjects { + if largeObjectCount(t, pool) != objects-removedObjects { t.Fatal("successful deletion did not unlink exactly its large objects") } assertAdminDeletedResource(t, s, tenant, mutation, id) @@ -219,8 +216,6 @@ func assertAdminDeletedResource(t *testing.T, s *Store, tenant string, mutation t.Fatal("skill version survived deletion", queryErr) } return - case "file": - _, err = s.GetSourceFile(t.Context(), tenant, id) case "vault": _, err = s.GetVault(t.Context(), tenant, id) case "credential": diff --git a/services/core/internal/store/admin_session_archive_test.go b/services/core/internal/store/admin_session_archive_test.go index 1b5d05d6b..10740942c 100644 --- a/services/core/internal/store/admin_session_archive_test.go +++ b/services/core/internal/store/admin_session_archive_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" "github.com/google/uuid" @@ -126,7 +127,8 @@ func TestManagedSessionArchiveRetainsHistoryAndSettledResources(t *testing.T) { s, w, installation := managedArchiveFixture(t) tenant, session := managedArchiveSession(t, s, managerSessionInput(uuid.NewString())) owner := archiveAllocation(t, w, tenant, session, installation) - file, err := s.CreateSourceFile(t.Context(), tenant, uploadSource([]byte("retained source file"))) + sourceFiles, sourceFileReader := testFiles(t, s.pool) + file, err := sourceFiles.Create(t.Context(), files.CreateCommand{TenantID: tenant, Upload: uploadSource([]byte("retained source file"))}) if err != nil { t.Fatal(err) } @@ -189,7 +191,7 @@ func TestManagedSessionArchiveRetainsHistoryAndSettledResources(t *testing.T) { }); err != nil { t.Fatal(err) } - if err := s.ReadSourceFile(t.Context(), tenant, file.ID, func(_ SourceFile, r io.Reader) error { + if err := sourceFileReader.Read(t.Context(), tenant, file.ID, func(_ files.File, r io.Reader) error { got, err := io.ReadAll(r) if string(got) != "retained source file" { t.Error("archive damaged source file bytes") diff --git a/services/core/internal/store/artifact_capture.go b/services/core/internal/store/artifact_capture.go index 1bd336866..06b8e88a5 100644 --- a/services/core/internal/store/artifact_capture.go +++ b/services/core/internal/store/artifact_capture.go @@ -3,7 +3,6 @@ package store import ( "archive/tar" "context" - "encoding/hex" "encoding/json" "errors" "io" @@ -11,6 +10,7 @@ import ( "strings" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" "github.com/google/uuid" "github.com/jackc/pgx/v5" @@ -82,7 +82,6 @@ func (s *Store) StageTurnArtifacts(ctx context.Context, tenantID, sessionID, tur func captureArtifactArchive(ctx context.Context, tx pgx.Tx, input io.Reader) ([]sqlc.StageSessionArtifactParams, error) { archive := tar.NewReader(input) - objects := tx.LargeObjects() rows := make([]sqlc.StageSessionArtifactParams, 0) seen := make(map[string]bool) var total int64 @@ -99,22 +98,18 @@ func captureArtifactArchive(ctx context.Context, tx pgx.Tx, input io.Reader) ([] } seen[header.Name] = true total += header.Size - oid, err := objects.Create(ctx, 0) + writer, err := pgunit.CreateLargeObject(ctx, tx) if err != nil { return nil, err } - body, err := objects.Open(ctx, oid, pgx.LargeObjectModeWrite) - if err != nil { - return nil, err - } - writer := newSourceFileWriter(body) if _, err := io.CopyN(writer, archive, header.Size); err != nil { return nil, err } - if err := body.Close(); err != nil { + body, err := writer.Close() + if err != nil { return nil, err } - rows = append(rows, sqlc.StageSessionArtifactParams{ID: pgtype.UUID{Bytes: uuid.New(), Valid: true}, Path: "/workspace/" + header.Name, SizeBytes: writer.size, BodyOid: pgtype.Uint32{Uint32: oid, Valid: true}, Sha256: hex.EncodeToString(writer.hash.Sum(nil))}) + rows = append(rows, sqlc.StageSessionArtifactParams{ID: pgtype.UUID{Bytes: uuid.New(), Valid: true}, Path: "/workspace/" + header.Name, SizeBytes: body.Size, BodyOid: pgtype.Uint32{Uint32: body.OID, Valid: true}, Sha256: body.SHA256}) } // Require transport EOF after the archive trailer, including confirmed helper exit. padding, err := io.ReadAll(io.LimitReader(input, 32769)) diff --git a/services/core/internal/store/environment_file_write_semantics_public_test.go b/services/core/internal/store/environment_file_write_semantics_public_test.go index 9b74a7537..0de859659 100644 --- a/services/core/internal/store/environment_file_write_semantics_public_test.go +++ b/services/core/internal/store/environment_file_write_semantics_public_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/google/uuid" @@ -65,10 +66,11 @@ func TestEnvironmentFileCreateRejectionsLeaveNoReceiptOrConsumption(t *testing.T } server := httptest.NewServer(handler) defer server.Close() - source, err := h.s.CreateSourceFile(t.Context(), h.tenant, func(out io.Writer) (store.SourceFileUpload, error) { + fileStore, fileService := fixtureFiles(t, h.db) + source, err := fileService.Create(t.Context(), files.CreateCommand{TenantID: h.tenant, Upload: func(out io.Writer) (files.Upload, error) { _, err := out.Write([]byte("src")) - return store.SourceFileUpload{Filename: "source.txt", Purpose: "user_data"}, err - }) + return files.Upload{Filename: "source.txt", Purpose: files.PurposeUserData}, err + }}) if err != nil { t.Fatal(err) } @@ -144,7 +146,7 @@ func TestEnvironmentFileCreateRejectionsLeaveNoReceiptOrConsumption(t *testing.T } } // The rejected copy did not consume its Source File. - if got, err := h.s.GetSourceFile(t.Context(), h.tenant, source.ID); err != nil || got.SizeBytes != 3 { + if got, err := fileStore.Get(t.Context(), h.tenant, source.ID); err != nil || got.SizeBytes != 3 { t.Fatal("source file consumed", got, err) } // The inline bound is checked before any mutation intent or dispatch. diff --git a/services/core/internal/store/initial_files.go b/services/core/internal/store/initial_files.go index 6d4f4e89b..8182d9147 100644 --- a/services/core/internal/store/initial_files.go +++ b/services/core/internal/store/initial_files.go @@ -9,6 +9,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/environmentconfig" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/google/uuid" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" @@ -99,8 +100,8 @@ func (s *Store) resolveEnvironmentTemplate(ctx context.Context, q *sqlc.Queries, return value, files, nil } -func (s *Store) saveInitialFiles(ctx context.Context, q *sqlc.Queries, tx pgx.Tx, tenant string, session pgtype.UUID, files []environmentconfig.InitialFile) ([]byte, error) { - if environmentconfig.ValidateInitialFiles(files) != nil { +func (s *Store) saveInitialFiles(ctx context.Context, q *sqlc.Queries, tx pgx.Tx, tenant string, session pgtype.UUID, initial []environmentconfig.InitialFile) ([]byte, error) { + if environmentconfig.ValidateInitialFiles(initial) != nil { return nil, ErrInvalidInput } tenantID, err := parseID(tenant) @@ -108,33 +109,22 @@ func (s *Store) saveInitialFiles(ctx context.Context, q *sqlc.Queries, tx pgx.Tx return nil, err } tenant = uuid.UUID(tenantID.Bytes).String() - metadata := environmentconfig.InitialFilesMetadata(files) - for i, f := range files { + metadata := environmentconfig.InitialFilesMetadata(initial) + for i, f := range initial { body := f.Data if f.Type == "file_id" { - sourceTenant, sourceID, err := sourceFileIDs(tenant, f.FileID) - if err != nil { - return nil, err + sourceID, ok := files.ParseID(f.FileID) + if !ok { + return nil, ErrNotFound } - source, err := q.LockInitialSourceFile(ctx, sqlc.LockInitialSourceFileParams{TenantID: sourceTenant, ID: sourceID}) + source, err := q.LockInitialSourceFile(ctx, sqlc.LockInitialSourceFileParams{TenantID: tenantID, ID: pgtype.UUID{Bytes: sourceID, Valid: true}}) if errors.Is(err, pgx.ErrNoRows) { return nil, ErrNotFound } if err != nil { return nil, err } - err = consumeSourceFile(ctx, tx, source, func(source SourceFile, reader io.Reader) error { - if source.SizeBytes > environmentconfig.MaxInitialFileBytes { - return ErrInvalidInput - } - var err error - body, err = io.ReadAll(io.LimitReader(reader, environmentconfig.MaxInitialFileBytes+1)) - if err == nil && (len(body) > environmentconfig.MaxInitialFileBytes || int64(len(body)) != source.SizeBytes) { - return ErrInvalidInput - } - return err - }) - if err != nil { + if body, err = readInitialSourceFile(ctx, tx, source); err != nil { return nil, err } } @@ -154,6 +144,28 @@ func (s *Store) saveInitialFiles(ctx context.Context, q *sqlc.Queries, tx pgx.Tx return json.Marshal(metadata) } +// readInitialSourceFile copies a File's content for a new Session. Session +// creation reads source_files through store's SQL until Sessions move out of +// store. +func readInitialSourceFile(ctx context.Context, tx pgx.Tx, source sqlc.SourceFile) ([]byte, error) { + if source.SizeBytes > environmentconfig.MaxInitialFileBytes { + return nil, ErrInvalidInput + } + objects := tx.LargeObjects() + reader, err := objects.Open(ctx, source.BodyOid.Uint32, pgx.LargeObjectModeRead) + if err != nil { + return nil, err + } + body, err := io.ReadAll(io.LimitReader(reader, environmentconfig.MaxInitialFileBytes+1)) + if err != nil { + return nil, err + } + if len(body) > environmentconfig.MaxInitialFileBytes || int64(len(body)) != source.SizeBytes { + return nil, ErrInvalidInput + } + return body, reader.Close() +} + // ReadInitialEnvironmentFile decrypts only the next frozen file, bounding memory per installation. func (s *Store) ReadInitialEnvironmentFile(ctx context.Context, tenant, session string, position int) (environmentconfig.InitialFileMetadata, []byte, error) { lookup, err := deviceLookup(tenant, session) diff --git a/services/core/internal/store/initial_files_test.go b/services/core/internal/store/initial_files_test.go index fc6a9c6a6..2b3cad0e8 100644 --- a/services/core/internal/store/initial_files_test.go +++ b/services/core/internal/store/initial_files_test.go @@ -10,9 +10,31 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/environmentconfig" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/filepg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" ) +// testFiles returns the files domain over pool for tests that need a File. +func testFiles(t *testing.T, pool *pgxpool.Pool) (*files.Service, *filepg.Store) { + t.Helper() + storage := filepg.New(pgunit.NewPool(pool)) + service, err := files.NewService(storage) + if err != nil { + t.Fatal(err) + } + return service, storage +} + +func uploadSource(data []byte) func(io.Writer) (files.Upload, error) { + return func(w io.Writer) (files.Upload, error) { + _, err := w.Write(data) + return files.Upload{Filename: "source.bin", Purpose: files.PurposeUserData}, err + } +} + func TestInitialFilesFrozenEncryptedIsolatedAndRetryable(t *testing.T) { _, pool := testStore(t) cipher, err := credentialcrypto.New(bytes.Repeat([]byte{7}, 32)) @@ -22,15 +44,13 @@ func TestInitialFilesFrozenEncryptedIsolatedAndRetryable(t *testing.T) { s := NewWithCredentialCipher(pool, cipher) tenant, foreign := uuid.NewString(), uuid.NewString() canary := []byte("private-initial-file-canary\x00\xff") - upload, err := s.CreateSourceFile(t.Context(), tenant, func(w io.Writer) (SourceFileUpload, error) { - _, err := w.Write(canary) - return SourceFileUpload{Filename: "source.bin", Purpose: "user_data"}, err - }) + sourceFiles, _ := testFiles(t, pool) + upload, err := sourceFiles.Create(t.Context(), files.CreateCommand{TenantID: tenant, Upload: uploadSource(canary)}) if err != nil { t.Fatal(err) } - files := []environmentconfig.InitialFile{{Type: "inline", Path: "/workspace/a/data", Data: canary}, {Type: "file_id", Path: "/workspace/b", FileID: upload.ID}} - template, err := s.CreateEnvironmentTemplate(t.Context(), tenant, EnvironmentTemplateInput{SetFiles: true, Files: files}) + initial := []environmentconfig.InitialFile{{Type: "inline", Path: "/workspace/a/data", Data: canary}, {Type: "file_id", Path: "/workspace/b", FileID: upload.ID}} + template, err := s.CreateEnvironmentTemplate(t.Context(), tenant, EnvironmentTemplateInput{SetFiles: true, Files: initial}) if err != nil { t.Fatal(err) } @@ -41,7 +61,7 @@ func TestInitialFilesFrozenEncryptedIsolatedAndRetryable(t *testing.T) { if _, _, err := s.ResolveEnvironmentTemplate(t.Context(), foreign, template.ID); !errors.Is(err, ErrNotFound) { t.Fatal("foreign template resolved", err) } - if _, err := s.UpdateEnvironmentTemplate(t.Context(), strings.ToUpper(tenant), strings.ToUpper(template.ID), EnvironmentTemplateInput{SetFiles: true, Files: files}); err != nil { + if _, err := s.UpdateEnvironmentTemplate(t.Context(), strings.ToUpper(tenant), strings.ToUpper(template.ID), EnvironmentTemplateInput{SetFiles: true, Files: initial}); err != nil { t.Fatal("noncanonical update", err) } if _, _, err := s.ResolveEnvironmentTemplate(t.Context(), strings.ToUpper(tenant), strings.ToUpper(template.ID)); err != nil { @@ -65,14 +85,14 @@ func TestInitialFilesFrozenEncryptedIsolatedAndRetryable(t *testing.T) { if _, err := s.DeleteEnvironmentTemplate(t.Context(), tenant, template.ID); err != nil { t.Fatal(err) } - if err := s.DeleteSourceFile(t.Context(), tenant, upload.ID); err != nil { + if err := sourceFiles.Delete(t.Context(), files.DeleteCommand{TenantID: tenant, FileID: upload.ID}); err != nil { t.Fatal(err) } retry, err := s.CreateSession(t.Context(), tenant, input) if err != nil || retry.ID != session.ID { t.Fatal("retry re-resolved deleted resources", err) } - for position := range files { + for position := range initial { metadata, body, err := s.ReadInitialEnvironmentFile(t.Context(), strings.ToUpper(tenant), strings.ToUpper(session.ID), position) if err != nil || !bytes.Equal(body, canary) || metadata.ID == "" { t.Fatal("frozen initial content", err) @@ -86,7 +106,7 @@ func TestInitialFilesFrozenEncryptedIsolatedAndRetryable(t *testing.T) { } } changed := input - changed.InitialFiles = append([]environmentconfig.InitialFile(nil), files...) + changed.InitialFiles = append([]environmentconfig.InitialFile(nil), initial...) changed.InitialFiles[0].Data = []byte("changed") if _, err := s.CreateSession(t.Context(), tenant, changed); !errors.Is(err, ErrIdempotencyConflict) { t.Fatal("changed bytes retried", err) diff --git a/services/core/internal/store/public_handler_fixture_test.go b/services/core/internal/store/public_handler_fixture_test.go index 8fcf324ee..b3da7c695 100644 --- a/services/core/internal/store/public_handler_fixture_test.go +++ b/services/core/internal/store/public_handler_fixture_test.go @@ -11,8 +11,10 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/api" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/coremetrics" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/execution" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/files" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/agentpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/filepg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimehistory" @@ -27,10 +29,10 @@ const testExecutorURL = "wss://core.example/api/v1/agent-daemon/ws" // publicHandler serves s through api.NewHandler. s backs every area the Store // implements, and db is the database and credential key that built s; the -// audit reads come from db. keys authenticate as Project keys and "admin" as -// the Core key. Metrics, Runtime observation and history, and executor -// connections are strict stand-ins. Execution and Sandboxes stay disabled -// unless configure sets them. +// audit reads, Agents and Files come from db. keys authenticate as Project +// keys and "admin" as the Core key. Metrics, Runtime observation and history, +// and executor connections are strict stand-ins. Execution and Sandboxes stay +// disabled unless configure sets them. func publicHandler(t testing.TB, s *store.Store, db fixtureDB, keys fixtureKeyResolver, engine string, configure ...func(*api.Dependencies)) (http.Handler, error) { t.Helper() admin, err := api.NewDeploymentAuthenticator([]string{runtimedevice.HashCredential("admin")}) @@ -40,10 +42,13 @@ func publicHandler(t testing.TB, s *store.Store, db fixtureDB, keys fixtureKeyRe strict := strictStandIn{t} audit := auditpg.New(pgunit.NewPool(db.pool)) agentStore, agentService := fixtureAgents(t, db) + fileStore, fileService := fixtureFiles(t, db) deps := api.Dependencies{ Engine: engine, CoreKeys: admin, InstallationBindings: s, - Projects: fixtureProjects{Store: s, keys: keys}, Vaults: s, ModelProviders: s, Files: s, Skills: s, - EnvironmentTemplates: s, Agents: agentService, AgentsReader: agentStore, Sessions: s, SessionEvents: s, SessionHistory: s, Subagents: s, + Projects: fixtureProjects{Store: s, keys: keys}, Vaults: s, ModelProviders: s, Skills: s, + Files: fileService, FilesReader: fileStore, + Agents: agentService, AgentsReader: agentStore, + EnvironmentTemplates: s, Sessions: s, SessionEvents: s, SessionHistory: s, Subagents: s, Artifacts: s, SessionAdmin: s, Environments: s, Admin: s, AdminAudit: audit, WriteAudit: audit, ExecutorConnections: strict, Metrics: strict, RuntimeObservations: strict, RuntimeHistory: strict, } @@ -64,6 +69,17 @@ func fixtureAgents(t testing.TB, db fixtureDB) (*agentpg.Store, *agents.Service) return agentStore, agentService } +// fixtureFiles builds the File adapter and service on db. +func fixtureFiles(t testing.TB, db fixtureDB) (*filepg.Store, *files.Service) { + t.Helper() + fileStore := filepg.New(pgunit.NewPool(db.pool)) + fileService, err := files.NewService(fileStore) + if err != nil { + t.Fatal(err) + } + return fileStore, fileService +} + // fixtureProjects serves Projects from the Store and resolves Project keys from // the test's fixture keys. type fixtureProjects struct { diff --git a/services/core/internal/store/session_artifacts_test.go b/services/core/internal/store/session_artifacts_test.go index 4146706d8..908e136b7 100644 --- a/services/core/internal/store/session_artifacts_test.go +++ b/services/core/internal/store/session_artifacts_test.go @@ -17,8 +17,18 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" ) +func largeObjectCount(t *testing.T, pool *pgxpool.Pool) int { + t.Helper() + var count int + if err := pool.QueryRow(t.Context(), "SELECT count(*) FROM pg_largeobject_metadata").Scan(&count); err != nil { + t.Fatal(err) + } + return count +} + func artifactArchive(t *testing.T, files map[string][]byte) []byte { t.Helper() var data bytes.Buffer @@ -62,7 +72,7 @@ func TestSessionArtifactsPublishVersionScopeAndLifetime(t *testing.T) { func testSessionArtifactsPublishVersionScopeAndLifetime(t *testing.T, kind string) { s, pool := testStore(t) tenant, session, environment, turn := artifactTurn(t, s, kind) - before := sourceObjectCount(t, pool) + before := largeObjectCount(t, pool) data := bytes.Repeat([]byte("immutable\x00"), 100000) archive := artifactArchive(t, map[string][]byte{"outputs/a.bin": data, "outputs/nested/empty": {}}) if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, bytes.NewReader(archive)); err != nil { @@ -168,7 +178,7 @@ func testSessionArtifactsPublishVersionScopeAndLifetime(t *testing.T, kind strin if err := s.DeleteSession(t.Context(), tenant, session); err != nil { t.Fatal(err) } - if count := sourceObjectCount(t, pool); count != before { + if count := largeObjectCount(t, pool); count != before { t.Fatalf("objects leaked: %d -> %d", before, count) } } @@ -186,7 +196,7 @@ func TestSessionArtifactsRejectIncompleteAndUnownedCapture(t *testing.T) { func testSessionArtifactsRejectIncompleteAndUnownedCapture(t *testing.T, kind string) { s, pool := testStore(t) tenant, session, environment, turn := artifactTurn(t, s, kind) - before := sourceObjectCount(t, pool) + before := largeObjectCount(t, pool) valid := artifactArchive(t, map[string][]byte{"outputs/a": []byte("data")}) for name, body := range map[string]io.Reader{ "transport-failure-after-valid-tar": io.MultiReader(bytes.NewReader(valid), artifactReadError{}), @@ -199,7 +209,7 @@ func testSessionArtifactsRejectIncompleteAndUnownedCapture(t *testing.T, kind st if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, body); err == nil { t.Fatal("invalid capture accepted") } - if count := sourceObjectCount(t, pool); count != before { + if count := largeObjectCount(t, pool); count != before { t.Fatalf("rollback leaked objects: %d -> %d", before, count) } }) @@ -216,19 +226,19 @@ func TestSessionArtifactsDiscardTerminalPrivateCapture(t *testing.T) { t.Run(status, func(t *testing.T) { s, pool := testStore(t) tenant, session, environment, turn := artifactTurn(t, s, "openai_hosted") - before := sourceObjectCount(t, pool) + before := largeObjectCount(t, pool) body := artifactArchive(t, map[string][]byte{"outputs/a": []byte("private")}) if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, bytes.NewReader(body)); err != nil { t.Fatal(err) } transition(t, s, tenant, session, turn, sessions.TurnInProgress, status) - if count := sourceObjectCount(t, pool); count != before { + if count := largeObjectCount(t, pool); count != before { t.Fatalf("terminal capture leaked objects: %d -> %d", before, count) } if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, bytes.NewReader(body)); !errors.Is(err, ErrTurnConflict) { t.Fatalf("late capture accepted: %v", err) } - if count := sourceObjectCount(t, pool); count != before { + if count := largeObjectCount(t, pool); count != before { t.Fatalf("late capture leaked objects: %d -> %d", before, count) } }) @@ -240,7 +250,7 @@ func TestSessionArtifactTransferDoesNotBlockDeletionOrCancellation(t *testing.T) t.Run(operation, func(t *testing.T) { s, pool := testStore(t) tenant, session, environment, turn := artifactTurn(t, s, "openai_hosted") - before := sourceObjectCount(t, pool) + before := largeObjectCount(t, pool) reader, writer := io.Pipe() defer reader.Close() defer writer.Close() @@ -271,7 +281,7 @@ func TestSessionArtifactTransferDoesNotBlockDeletionOrCancellation(t *testing.T) if err := <-result; !errors.Is(err, want) { t.Fatalf("late publication after %s: %v", operation, err) } - if count := sourceObjectCount(t, pool); count != before { + if count := largeObjectCount(t, pool); count != before { t.Fatalf("late capture leaked objects: %d -> %d", before, count) } }) @@ -341,7 +351,7 @@ func publishedPaths(published map[string]SessionArtifact) []string { func TestSessionArtifactsRepublishOnlyNewChangedOrDeletedPaths(t *testing.T) { s, pool := testStore(t) tenant, session, environment, first := artifactTurn(t, s, "openai_hosted") - before := sourceObjectCount(t, pool) + before := largeObjectCount(t, pool) turnNumber := 1 run := func(files map[string]string, want ...string) map[string]SessionArtifact { t.Helper() @@ -378,7 +388,7 @@ func TestSessionArtifactsRepublishOnlyNewChangedOrDeletedPaths(t *testing.T) { if err := pool.QueryRow(t.Context(), "SELECT count(*) FROM session_artifacts WHERE session_id = $1", session).Scan(&rows); err != nil { t.Fatal(err) } - if count := sourceObjectCount(t, pool); count != before+rows { + if count := largeObjectCount(t, pool); count != before+rows { t.Fatalf("unpublished captures kept private objects: %d objects for %d Artifacts", count-before, rows) } } @@ -451,7 +461,7 @@ func TestSessionArtifactsRepublishOnlyNewChangedOrDeletedPaths(t *testing.T) { t.Fatal(err) } } - if count := sourceObjectCount(t, pool); count != before { + if count := largeObjectCount(t, pool); count != before { t.Fatalf("objects leaked: %d -> %d", before, count) } } @@ -505,7 +515,7 @@ func TestSessionArtifactsNewestVersionFollowsTurnOrder(t *testing.T) { func TestSessionArtifactsCompletionWaitsForConcurrentDeletion(t *testing.T) { s, pool := testStore(t) tenant, session, environment, first := artifactTurn(t, s, "openai_hosted") - before := sourceObjectCount(t, pool) + before := largeObjectCount(t, pool) stageArtifactOutputs(t, s, tenant, session, environment, first, map[string]string{"a.txt": "alpha"}) transition(t, s, tenant, session, first, sessions.TurnInProgress, sessions.TurnCompleted) newest := publishedByTurn(t, s, tenant, session, first)["a.txt"] @@ -575,7 +585,7 @@ func TestSessionArtifactsCompletionWaitsForConcurrentDeletion(t *testing.T) { if _, err := s.GetSessionArtifact(t.Context(), tenant, session, newest.ID); !errors.Is(err, ErrNotFound) { t.Fatalf("deleted Artifact remains: %v", err) } - if count := sourceObjectCount(t, pool); count != before+1 { + if count := largeObjectCount(t, pool); count != before+1 { t.Fatalf("private objects: %d -> %d, want one published Artifact", before, count) } } diff --git a/services/core/internal/store/source_file_writer.go b/services/core/internal/store/source_file_writer.go deleted file mode 100644 index c75f63553..000000000 --- a/services/core/internal/store/source_file_writer.go +++ /dev/null @@ -1,47 +0,0 @@ -package store - -import ( - "crypto/sha256" - "hash" - "io" -) - -const sourceFileChunkBytes = 256 << 10 - -type sourceFileWriter struct { - body io.Writer - hash hash.Hash - size int64 - err error -} - -func newSourceFileWriter(body io.Writer) *sourceFileWriter { - return &sourceFileWriter{body: body, hash: sha256.New()} -} - -func (w *sourceFileWriter) Write(p []byte) (int, error) { - if w.err != nil { - return 0, w.err - } - if int64(len(p)) > MaxSourceFileBytes-w.size { - w.err = ErrSourceFileTooLarge - return 0, w.err - } - written := 0 - for len(p) > 0 { - chunk := p[:min(len(p), sourceFileChunkBytes)] - n, err := w.body.Write(chunk) - w.hash.Write(chunk[:n]) - w.size += int64(n) - written += n - if err == nil && n != len(chunk) { - err = io.ErrShortWrite - } - if err != nil { - w.err = err - return written, err - } - p = p[n:] - } - return written, nil -} diff --git a/services/core/internal/store/source_files.go b/services/core/internal/store/source_files.go deleted file mode 100644 index 517eb86b7..000000000 --- a/services/core/internal/store/source_files.go +++ /dev/null @@ -1,225 +0,0 @@ -package store - -import ( - "context" - "encoding/hex" - "errors" - "fmt" - "io" - "strings" - "time" - "unicode/utf8" - - "github.com/google/uuid" - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgtype" - - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/writeaudit" -) - -const MaxSourceFileBytes int64 = 512 << 20 - -var ErrSourceFileTooLarge = errors.New("source file exceeds storage limit") - -type SourceFile struct { - ID string - Filename string - Purpose string - SizeBytes int64 - CreatedAt time.Time -} - -type SourceFileUpload struct { - Filename string - Purpose string -} - -type SourceFilePage struct { - Files []SourceFile - NextCursor string -} - -// CreateSourceFile commits only after the complete upload envelope has validated. -func (s *Store) CreateSourceFile(ctx context.Context, tenantID string, upload func(io.Writer) (SourceFileUpload, error)) (SourceFile, error) { - tenant, err := parseID(tenantID) - if err != nil || upload == nil { - return SourceFile{}, ErrInvalidInput - } - var created SourceFile - err = s.pooled.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { - objects := tx.LargeObjects() - oid, err := objects.Create(ctx, 0) - if err != nil { - return err - } - body, err := objects.Open(ctx, oid, pgx.LargeObjectModeWrite) - if err != nil { - return err - } - writer := newSourceFileWriter(body) - input, err := upload(writer) - if writer.err != nil { - return writer.err - } - if err != nil { - return err - } - if !validSourceFilename(input.Filename) || input.Purpose != "user_data" { - return ErrInvalidInput - } - if err := body.Close(); err != nil { - return err - } - row, err := s.queries.WithTx(tx).CreateSourceFile(ctx, sqlc.CreateSourceFileParams{ - ID: pgtype.UUID{Bytes: uuid.New(), Valid: true}, TenantID: tenant, - Filename: input.Filename, Purpose: input.Purpose, BodyOid: pgtype.Uint32{Uint32: oid, Valid: true}, - SizeBytes: writer.size, Sha256: hex.EncodeToString(writer.hash.Sum(nil)), - }) - if err != nil { - return fmt.Errorf("create source file: %w", err) - } - resource := sourceFileFromRow(row) - if err := auditpg.RecordWriteAudit(ctx, s.queries.WithTx(tx), tenantID, "create", "file", resource.ID, "", writeaudit.Resource{Type: "file", ID: resource.ID}); err != nil { - return err - } - created = resource - return nil - }) - if err != nil { - return SourceFile{}, err - } - return created, nil -} - -func (s *Store) GetSourceFile(ctx context.Context, tenantID, fileID string) (SourceFile, error) { - tenant, id, err := sourceFileIDs(tenantID, fileID) - if err != nil { - return SourceFile{}, err - } - row, err := s.queries.GetSourceFile(ctx, sqlc.GetSourceFileParams{TenantID: tenant, ID: id}) - if errors.Is(err, pgx.ErrNoRows) { - return SourceFile{}, ErrNotFound - } - if err != nil { - return SourceFile{}, err - } - return sourceFileFromRow(row), nil -} - -func (s *Store) ListSourceFiles(ctx context.Context, tenantID, cursor string, limit int, ascending bool, purpose *string) (SourceFilePage, error) { - tenant, err := parseID(tenantID) - if err != nil { - return SourceFilePage{}, err - } - if limit < 1 || limit > 10000 { - return SourceFilePage{}, fmt.Errorf("%w: internal page size must be 1..10000", ErrInvalidInput) - } - params := sqlc.ListSourceFilesParams{ - TenantID: tenant, PageLimit: int32(limit + 1), Ascending: ascending, - AfterID: pgtype.UUID{Valid: true}, - } - if purpose != nil { - if !utf8.ValidString(*purpose) || strings.ContainsRune(*purpose, '\x00') { - return SourceFilePage{}, ErrInvalidInput - } - params.Purpose = pgtype.Text{String: *purpose, Valid: true} - } - if cursor != "" { - after, err := s.GetSourceFile(ctx, tenantID, cursor) - if err != nil { - return SourceFilePage{}, err - } - params.AfterCreated = pgtype.Timestamptz{Time: after.CreatedAt, Valid: true} - _, params.AfterID, _ = sourceFileIDs(tenantID, after.ID) - } - rows, err := s.queries.ListSourceFiles(ctx, params) - if err != nil { - return SourceFilePage{}, fmt.Errorf("list source files: %w", err) - } - page := SourceFilePage{Files: make([]SourceFile, 0, min(limit, len(rows)))} - if len(rows) > limit { - page.NextCursor = sourceFileFromRow(rows[limit-1]).ID - rows = rows[:limit] - } - for _, row := range rows { - page.Files = append(page.Files, sourceFileFromRow(row)) - } - return page, nil -} - -// ReadSourceFile retains an authorized immutable snapshot during concurrent deletion. -func (s *Store) ReadSourceFile(ctx context.Context, tenantID, fileID string, consume func(SourceFile, io.Reader) error) error { - tenant, id, err := sourceFileIDs(tenantID, fileID) - if err != nil { - return err - } - if consume == nil { - return ErrInvalidInput - } - return s.pooled.Snapshot(ctx, func(ctx context.Context, tx pgx.Tx) error { - row, err := s.queries.WithTx(tx).GetSourceFile(ctx, sqlc.GetSourceFileParams{TenantID: tenant, ID: id}) - if errors.Is(err, pgx.ErrNoRows) { - return ErrNotFound - } - if err != nil { - return err - } - return consumeSourceFile(ctx, tx, row, consume) - }) -} - -func consumeSourceFile(ctx context.Context, tx pgx.Tx, row sqlc.SourceFile, consume func(SourceFile, io.Reader) error) error { - objects := tx.LargeObjects() - body, err := objects.Open(ctx, row.BodyOid.Uint32, pgx.LargeObjectModeRead) - if err != nil { - return err - } - if err := consume(sourceFileFromRow(row), body); err != nil { - return err - } - return body.Close() -} - -func (s *Store) DeleteSourceFile(ctx context.Context, tenantID, fileID string) error { - tenant, id, err := sourceFileIDs(tenantID, fileID) - if err != nil { - return err - } - return s.pooled.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { - oid, err := s.queries.WithTx(tx).DeleteSourceFile(ctx, sqlc.DeleteSourceFileParams{TenantID: tenant, ID: id}) - if errors.Is(err, pgx.ErrNoRows) { - return ErrNotFound - } - if err != nil { - return err - } - objects := tx.LargeObjects() - if err := objects.Unlink(ctx, oid.Uint32); err != nil { - return err - } - return auditpg.RecordWriteAudit(ctx, s.queries.WithTx(tx), tenantID, "delete", "file", fileID, "") - }) -} - -func sourceFileIDs(tenantID, fileID string) (pgtype.UUID, pgtype.UUID, error) { - tenant, err := parseID(tenantID) - if err != nil { - return tenant, pgtype.UUID{}, err - } - id, err := uuid.Parse(strings.TrimPrefix(fileID, "file-")) - if err != nil || id == uuid.Nil || fileID != "file-"+id.String() { - return tenant, pgtype.UUID{}, ErrNotFound - } - return tenant, pgtype.UUID{Bytes: id, Valid: true}, nil -} - -func validSourceFilename(name string) bool { - return len(name) >= 1 && len(name) <= 1024 && utf8.ValidString(name) && !strings.ContainsRune(name, '\x00') -} - -func sourceFileFromRow(row sqlc.SourceFile) SourceFile { - return SourceFile{ID: "file-" + uuid.UUID(row.ID.Bytes).String(), Filename: row.Filename, - Purpose: row.Purpose, SizeBytes: row.SizeBytes, CreatedAt: row.CreatedAt.Time} -} diff --git a/services/core/internal/store/source_files_list_test.go b/services/core/internal/store/source_files_list_test.go deleted file mode 100644 index 6d8d5e965..000000000 --- a/services/core/internal/store/source_files_list_test.go +++ /dev/null @@ -1,128 +0,0 @@ -package store - -import ( - "cmp" - "errors" - "reflect" - "slices" - "strings" - "testing" - "time" - - "github.com/google/uuid" -) - -func TestSourceFileListPaginationIsolationAndReconnect(t *testing.T) { - s, pool := testStore(t) - tenant, other := uuid.NewString(), uuid.NewString() - empty, err := s.ListSourceFiles(t.Context(), tenant, "", 10000, false, nil) - if err != nil || empty.Files == nil || len(empty.Files) != 0 || empty.NextCursor != "" { - t.Fatalf("empty page: %+v, %v", empty, err) - } - - all := make([]SourceFile, 0, 105) - for i := range 105 { - file, err := s.CreateSourceFile(t.Context(), tenant, uploadSource([]byte{byte(i)})) - if err != nil { - t.Fatal(err) - } - stamp := time.Unix(1700000000+int64(i%2), 0).UTC() - if _, err := pool.Exec(t.Context(), "UPDATE source_files SET created_at=$1 WHERE tenant_id=$2 AND id=$3", stamp, tenant, strings.TrimPrefix(file.ID, "file-")); err != nil { - t.Fatal(err) - } - file, err = s.GetSourceFile(t.Context(), tenant, file.ID) - if err != nil { - t.Fatal(err) - } - all = append(all, file) - } - slices.SortFunc(all, func(a, b SourceFile) int { - if c := a.CreatedAt.Compare(b.CreatedAt); c != 0 { - return c - } - return cmp.Compare(a.ID, b.ID) - }) - foreign, err := s.CreateSourceFile(t.Context(), other, uploadSource([]byte("foreign"))) - if err != nil { - t.Fatal(err) - } - - read := func(current *Store, ascending bool, purpose *string, size int) []SourceFile { - t.Helper() - actual := []SourceFile{} - cursor := "" - for { - page, err := current.ListSourceFiles(t.Context(), tenant, cursor, size, ascending, purpose) - if err != nil || len(page.Files) == 0 || len(page.Files) > size { - t.Fatalf("page: %+v, %v", page, err) - } - actual = append(actual, page.Files...) - if len(actual) > len(all) { - t.Fatal("pagination repeated files") - } - if page.NextCursor == "" { - break - } - if page.NextCursor != page.Files[len(page.Files)-1].ID { - t.Fatal("cursor is not the last included file") - } - cursor = page.NextCursor - } - return actual - } - userData, otherPurpose := "user_data", "batch" - for _, ascending := range []bool{true, false} { - want := slices.Clone(all) - if !ascending { - slices.Reverse(want) - } - for _, purpose := range []*string{nil, &userData} { - for _, size := range []int{17, 10000} { - if got := read(s, ascending, purpose, size); !reflect.DeepEqual(got, want) { - t.Fatalf("ordered page mismatch: ascending=%t size=%d", ascending, size) - } - } - } - } - filtered, err := s.ListSourceFiles(t.Context(), tenant, "", 10000, false, &otherPurpose) - if err != nil || filtered.Files == nil || len(filtered.Files) != 0 || filtered.NextCursor != "" { - t.Fatalf("purpose filter: %+v, %v", filtered, err) - } - for _, cursor := range []string{foreign.ID, "file-" + uuid.NewString(), "invalid"} { - if _, err := s.ListSourceFiles(t.Context(), tenant, cursor, 20, true, nil); !errors.Is(err, ErrNotFound) { - t.Fatalf("foreign/unknown cursor %q: %v", cursor, err) - } - } - for _, tc := range []struct { - tenant string - limit int - purpose *string - }{{"invalid", 20, nil}, {tenant, 0, nil}, {tenant, 10001, nil}, {tenant, 20, sourceFilePurposePtr("bad\x00purpose")}} { - if _, err := s.ListSourceFiles(t.Context(), tc.tenant, "", tc.limit, true, tc.purpose); !errors.Is(err, ErrInvalidInput) { - t.Fatalf("invalid store query: %v", err) - } - } - tail, err := s.ListSourceFiles(t.Context(), tenant, all[len(all)-1].ID, 10000, true, nil) - if err != nil || tail.Files == nil || len(tail.Files) != 0 || tail.NextCursor != "" { - t.Fatalf("terminal page: %+v, %v", tail, err) - } - foreignPage, err := s.ListSourceFiles(t.Context(), other, "", 10000, false, nil) - if err != nil || !reflect.DeepEqual(foreignPage.Files, []SourceFile{foreign}) || foreignPage.NextCursor != "" { - t.Fatalf("project isolation: %+v, %v", foreignPage, err) - } - deleted, err := s.CreateSourceFile(t.Context(), tenant, uploadSource(nil)) - if err != nil || s.DeleteSourceFile(t.Context(), tenant, deleted.ID) != nil { - t.Fatal(err) - } - if _, err := s.ListSourceFiles(t.Context(), tenant, deleted.ID, 20, false, nil); !errors.Is(err, ErrNotFound) { - t.Fatalf("deleted cursor accepted: %v", err) - } - - pool.Close() - reopened, _ := testStore(t) - if got := read(reopened, true, nil, 17); !reflect.DeepEqual(got, all) { - t.Fatal("listing changed after reconnect") - } -} - -func sourceFilePurposePtr(value string) *string { return &value } diff --git a/services/core/internal/store/source_files_test.go b/services/core/internal/store/source_files_test.go deleted file mode 100644 index 9772645cf..000000000 --- a/services/core/internal/store/source_files_test.go +++ /dev/null @@ -1,191 +0,0 @@ -package store - -import ( - "bytes" - "context" - "crypto/sha256" - "errors" - "io" - "os" - "strings" - "testing" - - "github.com/google/uuid" - "github.com/jackc/pgx/v5/pgxpool" -) - -func sourceObjectCount(t *testing.T, pool *pgxpool.Pool) int { - t.Helper() - var count int - if err := pool.QueryRow(t.Context(), "SELECT count(*) FROM pg_largeobject_metadata").Scan(&count); err != nil { - t.Fatal(err) - } - return count -} - -func uploadSource(data []byte) func(io.Writer) (SourceFileUpload, error) { - return func(w io.Writer) (SourceFileUpload, error) { - _, err := w.Write(data) - return SourceFileUpload{Filename: "source.bin", Purpose: "user_data"}, err - } -} - -func TestSourceFilesPersistScopeAndDelete(t *testing.T) { - s, pool := testStore(t) - tenant, foreign := uuid.NewString(), uuid.NewString() - before := sourceObjectCount(t, pool) - for _, data := range [][]byte{{}, {0, 1, 255}, bytes.Repeat([]byte("binary\x00"), 300000)} { - file, err := s.CreateSourceFile(t.Context(), tenant, uploadSource(data)) - if err != nil || file.SizeBytes != int64(len(data)) || file.Filename != "source.bin" || !strings.HasPrefix(file.ID, "file-") || file.CreatedAt.IsZero() { - t.Fatalf("create: %+v %v", file, err) - } - if _, err := s.GetSourceFile(t.Context(), foreign, file.ID); !errors.Is(err, ErrNotFound) { - t.Fatalf("foreign metadata: %v", err) - } - if err := s.ReadSourceFile(t.Context(), foreign, file.ID, func(SourceFile, io.Reader) error { - t.Fatal("foreign content callback reached") - return nil - }); !errors.Is(err, ErrNotFound) { - t.Fatalf("foreign content: %v", err) - } - if err := s.DeleteSourceFile(t.Context(), foreign, file.ID); !errors.Is(err, ErrNotFound) { - t.Fatalf("foreign delete: %v", err) - } - reopened := New(pool) - if got, err := reopened.GetSourceFile(t.Context(), tenant, file.ID); err != nil || got != file { - t.Fatalf("metadata: %+v %v", got, err) - } - if err := reopened.ReadSourceFile(t.Context(), tenant, file.ID, func(meta SourceFile, r io.Reader) error { - got, err := io.ReadAll(r) - if meta != file || !bytes.Equal(got, data) { - t.Error("persisted contents differ") - } - return err - }); err != nil { - t.Fatal(err) - } - if err := s.DeleteSourceFile(t.Context(), tenant, file.ID); err != nil { - t.Fatal(err) - } - if _, err := s.GetSourceFile(t.Context(), tenant, file.ID); !errors.Is(err, ErrNotFound) { - t.Fatalf("deleted metadata: %v", err) - } - if err := s.DeleteSourceFile(t.Context(), tenant, file.ID); !errors.Is(err, ErrNotFound) { - t.Fatalf("repeated delete: %v", err) - } - } - if got := sourceObjectCount(t, pool); got != before { - t.Fatalf("orphaned objects: %d -> %d", before, got) - } -} - -func TestSourceFilesRollbackInvalidOrInterruptedUpload(t *testing.T) { - s, pool := testStore(t) - tenant := uuid.NewString() - before := sourceObjectCount(t, pool) - for _, reason := range []string{"body", "purpose", "filename", "cancel", "ignored-write-error"} { - t.Run(reason, func(t *testing.T) { - ctx, cancel := context.WithCancel(t.Context()) - defer cancel() - _, err := s.CreateSourceFile(ctx, tenant, func(w io.Writer) (SourceFileUpload, error) { - if _, err := w.Write([]byte("not committed")); err != nil { - return SourceFileUpload{}, err - } - input := SourceFileUpload{Filename: "source.bin", Purpose: "user_data"} - switch reason { - case "body": - return input, io.ErrUnexpectedEOF - case "purpose": - input.Purpose = "not-supported" - case "filename": - input.Filename = "bad\x00name" - case "cancel": - cancel() - case "ignored-write-error": - bounded := w.(*sourceFileWriter) - bounded.size = MaxSourceFileBytes - _, _ = w.Write([]byte("over limit")) - } - return input, nil - }) - if err == nil { - t.Fatal("invalid upload committed") - } - if got := sourceObjectCount(t, pool); got != before { - t.Fatalf("rollback orphan: %d -> %d", before, got) - } - }) - } - var count int - if err := pool.QueryRow(t.Context(), "SELECT count(*) FROM source_files WHERE tenant_id=$1", tenant).Scan(&count); err != nil || count != 0 { - t.Fatalf("failed upload left metadata: %d %v", count, err) - } -} - -func TestSourceFileReadAdmittedBeforeDeletionCompletes(t *testing.T) { - s, pool := testStore(t) - tenant := uuid.NewString() - data := bytes.Repeat([]byte("immutable\x00"), 10000) - file, err := s.CreateSourceFile(t.Context(), tenant, uploadSource(data)) - if err != nil { - t.Fatal(err) - } - if err := s.ReadSourceFile(t.Context(), tenant, file.ID, func(_ SourceFile, r io.Reader) error { - prefix := make([]byte, 1) - if _, err := io.ReadFull(r, prefix); err != nil { - return err - } - if err := New(pool).DeleteSourceFile(t.Context(), tenant, file.ID); err != nil { - return err - } - if _, err := s.GetSourceFile(t.Context(), tenant, file.ID); !errors.Is(err, ErrNotFound) { - t.Fatalf("new read after delete: %v", err) - } - rest, err := io.ReadAll(r) - if !bytes.Equal(append(prefix, rest...), data) { - t.Error("deletion damaged admitted read") - } - return err - }); err != nil { - t.Fatal(err) - } -} - -func TestSourceFileLargeStream(t *testing.T) { - if os.Getenv("OAC_TEST_SOURCE_FILE_LARGE") != "1" { - t.Skip("opt-in 512 MiB source storage acceptance") - } - s, _ := testStore(t) - tenant := uuid.NewString() - chunk := bytes.Repeat([]byte("source\x00binary"), 20000) - want := sha256.New() - file, err := s.CreateSourceFile(t.Context(), tenant, func(w io.Writer) (SourceFileUpload, error) { - for left := MaxSourceFileBytes; left > 0; { - b := chunk[:min(int64(len(chunk)), left)] - if _, err := w.Write(b); err != nil { - return SourceFileUpload{}, err - } - want.Write(b) - left -= int64(len(b)) - } - return SourceFileUpload{Filename: "large.bin", Purpose: "user_data"}, nil - }) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { - if err := s.DeleteSourceFile(context.Background(), tenant, file.ID); err != nil { - t.Error(err) - } - }) - got := sha256.New() - if err := s.ReadSourceFile(t.Context(), tenant, file.ID, func(meta SourceFile, r io.Reader) error { - n, err := io.CopyBuffer(got, r, chunk) - if n != MaxSourceFileBytes || meta.SizeBytes != n { - t.Errorf("size: %d metadata: %d", n, meta.SizeBytes) - } - return err - }); err != nil || !bytes.Equal(got.Sum(nil), want.Sum(nil)) { - t.Fatalf("large stream mismatch: %v", err) - } -} diff --git a/services/core/internal/store/write_audit_resources_test.go b/services/core/internal/store/write_audit_resources_test.go index 05f7adbda..434bccbcb 100644 --- a/services/core/internal/store/write_audit_resources_test.go +++ b/services/core/internal/store/write_audit_resources_test.go @@ -29,7 +29,7 @@ func resourceAuditContext(ctx context.Context, tenant, request string) context.C // A database trigger fails the final audit insertion after each real business // mutation. Comparing complete tenant rows proves rollback of secret ciphertext, -// version counters, timestamps, cascades, and the source-file large object. +// version counters, timestamps, and cascades. func TestWriteAuditStandaloneResourceTransactions(t *testing.T) { _, pool := newManagedTestStore(t) cipher, err := credentialcrypto.New(bytes.Repeat([]byte{91}, 32)) @@ -48,7 +48,7 @@ func TestWriteAuditStandaloneResourceTransactions(t *testing.T) { for _, name := range []string{ "template_create", "template_update", "template_delete", "skill_create", "skill_upload_version", "skill_update_default", "skill_delete", "version_delete", "version_delete_last", - "file_create", "file_delete", "vault_create", "vault_delete", "credential_create", "credential_update", "credential_delete", + "vault_create", "vault_delete", "credential_create", "credential_update", "credential_delete", "oauth_create", "oauth_update", "oauth_delete", } { t.Run(name, func(t *testing.T) { @@ -56,7 +56,7 @@ func TestWriteAuditStandaloneResourceTransactions(t *testing.T) { mutation := prepareResourceAuditMutation(t, s, tenant, name, archive) snapshot := func() map[string]string { result := make(map[string]string) - for _, table := range []string{"agents", "environment_templates", "skills", "skill_versions", "source_files", "vaults", "vault_credentials", "write_audit_operations", "write_audit_owners"} { + for _, table := range []string{"agents", "environment_templates", "skills", "skill_versions", "vaults", "vault_credentials", "write_audit_operations", "write_audit_owners"} { query := "SELECT COALESCE(jsonb_agg(to_jsonb(r) ORDER BY to_jsonb(r)::text)::text, '[]') FROM " + pgx.Identifier{table}.Sanitize() + " r WHERE tenant_id=$1" if table == "vault_credentials" { query = "SELECT COALESCE(jsonb_agg(to_jsonb(r) ORDER BY r.id)::text, '[]') FROM vault_credentials r JOIN vaults v ON v.id=r.vault_id WHERE v.tenant_id=$1" @@ -69,11 +69,11 @@ func TestWriteAuditStandaloneResourceTransactions(t *testing.T) { } return result } - before, objects := snapshot(), sourceObjectCount(t, pool) + before := snapshot() if _, err := mutation.run(resourceAuditContext(ctx, tenant, "reject-resource-audit")); err == nil { t.Fatal("audit failure was accepted") } - if !reflect.DeepEqual(before, snapshot()) || objects != sourceObjectCount(t, pool) { + if !reflect.DeepEqual(before, snapshot()) { t.Fatal("audit failure left business or audit changes") } request := uuid.NewString() @@ -170,17 +170,6 @@ func prepareResourceAuditMutation(t *testing.T, s *Store, tenant, name string, a return v.ID, e }} } - if strings.HasPrefix(name, "file_") { - if name == "file_create" { - return resourceAuditMutation{action: "create", kind: "file", owners: 1, run: func(ctx context.Context) (string, error) { - v, e := s.CreateSourceFile(ctx, tenant, uploadSource([]byte("audit-private-token"))) - return v.ID, e - }} - } - v, err := s.CreateSourceFile(ctx, tenant, uploadSource([]byte("audit-private-token"))) - must(err) - return resourceAuditMutation{action: "delete", kind: "file", run: func(ctx context.Context) (string, error) { return v.ID, s.DeleteSourceFile(ctx, tenant, v.ID) }} - } if name == "vault_create" { return resourceAuditMutation{action: "create", kind: "vault", owners: 1, run: func(ctx context.Context) (string, error) { v, e := s.CreateVault(ctx, tenant, CreateVaultInput{})