From eeda94006cdfc46746c83268e763e0a7f8f32868 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 02:50:45 +0800 Subject: [PATCH 1/8] fix(skills): keep resource metadata aligned with default version --- .../agents-api/internal/db/queries/skills.sql | 7 +- .../store/skill_default_metadata_test.go | 105 ++++++++++++++++++ .../internal/store/skill_versions.go | 2 +- services/agents-api/internal/store/skills.go | 5 +- 4 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 services/agents-api/internal/store/skill_default_metadata_test.go diff --git a/services/agents-api/internal/db/queries/skills.sql b/services/agents-api/internal/db/queries/skills.sql index b5dcd37c1..8f0121bff 100644 --- a/services/agents-api/internal/db/queries/skills.sql +++ b/services/agents-api/internal/db/queries/skills.sql @@ -23,11 +23,14 @@ ORDER BY LIMIT sqlc.arg(page_limit); -- name: SetDefaultSkillVersion :one -UPDATE skills SET default_version = $3 WHERE tenant_id = $1 AND id = $2 RETURNING *; +UPDATE skills SET default_version = $3, name = $4, description = $5 +WHERE tenant_id = $1 AND id = $2 RETURNING *; -- name: AdvanceSkillVersion :exec UPDATE skills SET latest_version = next_version, next_version = next_version + 1, - default_version = CASE WHEN sqlc.arg(make_default)::boolean THEN next_version ELSE default_version END + default_version = CASE WHEN sqlc.arg(make_default)::boolean THEN next_version ELSE default_version END, + name = CASE WHEN sqlc.arg(make_default)::boolean THEN sqlc.arg(name)::text ELSE name END, + description = CASE WHEN sqlc.arg(make_default)::boolean THEN sqlc.arg(description)::text ELSE description END WHERE tenant_id = sqlc.arg(tenant_id) AND id = sqlc.arg(id); -- name: DeleteSkill :one diff --git a/services/agents-api/internal/store/skill_default_metadata_test.go b/services/agents-api/internal/store/skill_default_metadata_test.go new file mode 100644 index 000000000..86efffa18 --- /dev/null +++ b/services/agents-api/internal/store/skill_default_metadata_test.go @@ -0,0 +1,105 @@ +package store + +import ( + "archive/zip" + "bytes" + "errors" + "fmt" + "strconv" + "testing" + + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/credentialcrypto" + "github.com/google/uuid" +) + +func skillMetadataArchive(t *testing.T, name, description string) []byte { + t.Helper() + var buffer bytes.Buffer + writer := zip.NewWriter(&buffer) + file, err := writer.CreateHeader(&zip.FileHeader{Name: name + "/SKILL.md", Method: zip.Store}) + if err != nil { + t.Fatal(err) + } + if _, err = fmt.Fprintf(file, "---\nname: %s\ndescription: %s\n---\nPrivate marker for %s.\n", name, description, name); err != nil { + t.Fatal(err) + } + if err = writer.Close(); err != nil { + t.Fatal(err) + } + return buffer.Bytes() +} + +func TestSkillMetadataTracksDefaultVersion(t *testing.T) { + _, pool := testStore(t) + cipher, err := credentialcrypto.New(bytes.Repeat([]byte{74}, 32)) + if err != nil { + t.Fatal(err) + } + s := NewWithCredentialCipher(pool, cipher) + metadataOnly := New(pool) + tenant, foreign := uuid.NewString(), uuid.NewString() + names := []string{"first-proof", "second-proof", "third-proof"} + descriptions := []string{"First immutable version.", "Second immutable version.", "Third immutable version."} + archives := make([][]byte, len(names)) + for i := range names { + archives[i] = skillMetadataArchive(t, names[i], descriptions[i]) + } + created, err := s.CreateSkill(t.Context(), tenant, archives[0]) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = s.DeleteSkill(t.Context(), tenant, created.ID) }) + assertMetadata := func(value Skill, version, latest int64) { + t.Helper() + if value.ID != created.ID || !value.CreatedAt.Equal(created.CreatedAt) || value.DefaultVersion != version || value.LatestVersion != latest || value.Name != names[version-1] || value.Description != descriptions[version-1] { + t.Fatalf("metadata does not track default %d/latest %d: %+v", version, latest, value) + } + } + assertStored := func(version, latest int64) { + t.Helper() + value, err := metadataOnly.GetSkill(t.Context(), tenant, created.ID) + if err != nil { + t.Fatal("metadata read without content key", err) + } + assertMetadata(value, version, latest) + page, err := metadataOnly.ListSkills(t.Context(), tenant, "", 10, true) + if err != nil || len(page.Skills) != 1 { + t.Fatal("metadata list without content key", page, err) + } + assertMetadata(page.Skills[0], version, latest) + selected, body, err := s.ReadDefaultSkillVersion(t.Context(), tenant, created.ID) + if err != nil || selected.Version != version || selected.Name != names[version-1] || selected.Description != descriptions[version-1] || !bytes.Equal(body, archives[version-1]) { + t.Fatal("default content differs from public metadata", selected, err) + } + } + assertMetadata(created, 1, 1) + if _, err = s.CreateSkillVersion(t.Context(), tenant, created.ID, archives[1], false); err != nil { + t.Fatal(err) + } + assertStored(1, 2) + for _, version := range []int64{2, 1} { + updated, err := metadataOnly.UpdateSkillDefault(t.Context(), tenant, created.ID, strconv.FormatInt(version, 10)) + if err != nil { + t.Fatal("default update without content key", err) + } + assertMetadata(updated, version, 2) + assertStored(version, 2) + } + if _, err = metadataOnly.UpdateSkillDefault(t.Context(), foreign, created.ID, "2"); !errors.Is(err, ErrNotFound) { + t.Fatal("foreign default update", err) + } + if _, err = metadataOnly.UpdateSkillDefault(t.Context(), tenant, created.ID, "999"); !errors.Is(err, ErrNotFound) { + t.Fatal("missing default update", err) + } + assertStored(1, 2) + if _, err = s.CreateSkillVersion(t.Context(), tenant, created.ID, archives[2], true); err != nil { + t.Fatal(err) + } + assertStored(3, 3) + for i := range archives { + version, body, err := s.ReadSkillVersion(t.Context(), tenant, created.ID, strconv.Itoa(i+1)) + if err != nil || version.Name != names[i] || version.Description != descriptions[i] || !bytes.Equal(body, archives[i]) { + t.Fatal("default changes modified immutable version", i+1, version, err) + } + } +} diff --git a/services/agents-api/internal/store/skill_versions.go b/services/agents-api/internal/store/skill_versions.go index a8ee780ca..61adc5d41 100644 --- a/services/agents-api/internal/store/skill_versions.go +++ b/services/agents-api/internal/store/skill_versions.go @@ -35,7 +35,7 @@ func (s *Store) CreateSkillVersion(ctx context.Context, tenantID, skillID string if err != nil { return err } - return q.AdvanceSkillVersion(ctx, sqlc.AdvanceSkillVersionParams{TenantID: tenant, ID: id, MakeDefault: makeDefault}) + return q.AdvanceSkillVersion(ctx, sqlc.AdvanceSkillVersionParams{TenantID: tenant, ID: id, MakeDefault: makeDefault, Name: metadata.Name, Description: metadata.Description}) }) if errors.Is(err, pgx.ErrNoRows) { err = ErrNotFound diff --git a/services/agents-api/internal/store/skills.go b/services/agents-api/internal/store/skills.go index db6dd8806..d91316729 100644 --- a/services/agents-api/internal/store/skills.go +++ b/services/agents-api/internal/store/skills.go @@ -86,10 +86,11 @@ func (s *Store) UpdateSkillDefault(ctx context.Context, tenantID, skillID, versi if _, err := q.LockSkill(ctx, sqlc.LockSkillParams{TenantID: tenant, ID: id}); err != nil { return err } - if _, err := q.GetSkillVersion(ctx, sqlc.GetSkillVersionParams{TenantID: tenant, SkillID: id, Version: number}); err != nil { + version, err := q.GetSkillVersion(ctx, sqlc.GetSkillVersionParams{TenantID: tenant, SkillID: id, Version: number}) + if err != nil { return err } - row, err := q.SetDefaultSkillVersion(ctx, sqlc.SetDefaultSkillVersionParams{TenantID: tenant, ID: id, DefaultVersion: number}) + row, err := q.SetDefaultSkillVersion(ctx, sqlc.SetDefaultSkillVersionParams{TenantID: tenant, ID: id, DefaultVersion: number, Name: version.Name, Description: version.Description}) result = skillFromRow(row) return err }) From 698e8d2232327c331226f930d99c7e7f994fe466 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 02:52:14 +0800 Subject: [PATCH 2/8] test: qualify Files and Skills resource semantics --- .../file_resource_semantics_public_test.go | 63 ++++++ .../tests/official_file_resource_semantics.py | 206 ++++++++++++++++++ 2 files changed, 269 insertions(+) create mode 100644 services/agents-api/internal/store/file_resource_semantics_public_test.go create mode 100644 services/agents-api/tests/official_file_resource_semantics.py diff --git a/services/agents-api/internal/store/file_resource_semantics_public_test.go b/services/agents-api/internal/store/file_resource_semantics_public_test.go new file mode 100644 index 000000000..2cedda497 --- /dev/null +++ b/services/agents-api/internal/store/file_resource_semantics_public_test.go @@ -0,0 +1,63 @@ +package store_test + +import ( + "bytes" + "context" + "encoding/json" + "net/http/httptest" + "os" + "os/exec" + "testing" + "time" + + "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/device" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/api" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/credentialcrypto" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" + "github.com/google/uuid" +) + +func TestFileResourceSemanticsOfficialClientPostgres(t *testing.T) { + python := os.Getenv("PARSAR_OFFICIAL_SDK_PYTHON") + if python == "" { + t.Skip("pinned official Python SDK required") + } + _, pool := store.NewTestStore(t) + cipher, err := credentialcrypto.New(bytes.Repeat([]byte{53}, 32)) + if err != nil { + t.Fatal(err) + } + token, foreign := uuid.NewString(), uuid.NewString() + auth, err := api.NewAuthenticator([]api.APIKey{ + {OrganizationID: "test-org", ProjectID: uuid.NewString(), SubjectKind: "service_account", SubjectID: "resources-owner", TokenSHA256: device.HashCredential(token), TenantID: uuid.NewString()}, + {OrganizationID: "test-org", ProjectID: uuid.NewString(), SubjectKind: "service_account", SubjectID: "resources-foreign", TokenSHA256: device.HashCredential(foreign), TenantID: uuid.NewString()}, + }) + if err != nil { + t.Fatal(err) + } + newServer := func() *httptest.Server { + t.Helper() + s := store.NewWithCredentialCipher(pool, cipher) + h, err := api.NewHandler(s, auth, "codex", api.WithSourceFiles(s), api.WithSkills(s)) + if err != nil { + t.Fatal(err) + } + server := httptest.NewServer(h) + t.Cleanup(server.Close) + return server + } + server, recovered := newServer(), newServer() + settings, err := json.Marshal(map[string]string{"base": server.URL, "recovered": recovered.URL, "token": token, "foreign": foreign}) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(t.Context(), 2*time.Minute) + defer cancel() + command := exec.CommandContext(ctx, python, "../../tests/official_file_resource_semantics.py") + command.Stdin = bytes.NewReader(settings) + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("official Files and Skills resource semantics: %v %s", err, output) + } + t.Log(string(output)) +} diff --git a/services/agents-api/tests/official_file_resource_semantics.py b/services/agents-api/tests/official_file_resource_semantics.py new file mode 100644 index 000000000..fc0cdc29f --- /dev/null +++ b/services/agents-api/tests/official_file_resource_semantics.py @@ -0,0 +1,206 @@ +"""Qualified Files and Skills semantics over real Core HTTP and PostgreSQL.""" + +import importlib.metadata +import io +import json +import secrets +import sys +import zipfile +from contextlib import ExitStack +from pathlib import Path + +import httpx2 +from openai import BadRequestError, DefaultHttpxClient, OpenAI + + +def assert_error(response, status, param, code=None): + assert response.status_code == status, (response.status_code, response.text) + body = response.json()["error"] + assert set(body) == {"type", "code", "param", "message"}, body + assert (body["type"], body["code"], body["param"]) == ("invalid_request_error", code, param), body + assert isinstance(body["message"], str) and body["message"], body + return body + + +def check_files(raw, owner, outsider, recovered, settings, cleanup): + marker = "private-resource-content-" + secrets.token_hex(16) + uploaded = owner.files.with_raw_response.create( + file=("resource-semantics.txt", marker.encode()), purpose="user_data", + ) + file = uploaded.parse() + cleanup.callback(owner.files.delete, file.id) + metadata = uploaded.http_response.json() + assert uploaded.status_code == 200 + assert metadata["status"] == "processed" + assert metadata["expires_at"] is None and metadata["status_details"] is None + assert file.bytes == len(marker.encode()) and file.filename == "resource-semantics.txt" + assert file.purpose == "user_data" and file.object == "file" + assert owner.files.retrieve(file.id).to_dict() == file.to_dict() + assert recovered.files.retrieve(file.id).to_dict() == file.to_dict() + path = settings["base"] + "/v1/files/" + file.id + "/content" + headers = {"Authorization": "Bearer " + settings["token"]} + denied = raw.get(path, headers=headers) + body = assert_error(denied, 400, None) + assert body["message"] == "Not allowed to download files of purpose: user_data" + assert marker not in denied.text + try: + owner.files.content(file.id) + except BadRequestError as error: + assert error.status_code == 400 and error.body == body + else: + raise AssertionError("SDK downloaded a user_data File") + foreign = raw.get(path, headers={"Authorization": "Bearer " + settings["foreign"]}) + missing = raw.get(settings["base"] + "/v1/files/file-" + secrets.token_hex(24) + "/content", headers=headers) + assert_error(foreign, 404, "id") + assert_error(missing, 404, "id") + assert foreign.json() == missing.json() + assert all(value not in foreign.text for value in (file.id, file.filename, marker, "user_data")) + assert list(outsider.files.list()) == [] + assert owner.files.retrieve(file.id).to_dict() == file.to_dict() + for purpose in (None, "", "user_data", "assistants", "batch", "fine-tune", "vision", "evals", + "assistants_output", "batch_output", "fine-tune-results"): + query = {"after": "file-" + secrets.token_hex(24)} + if purpose is not None: + query["purpose"] = purpose + assert_error(raw.get(settings["base"] + "/v1/files", headers=headers, params=query), 404, "after") + for purpose in ("invalid-" + secrets.token_hex(12), "USER_DATA"): + for key, cursor in ((settings["token"], "file-" + secrets.token_hex(24)), (settings["foreign"], file.id)): + assert_error(raw.get(settings["base"] + "/v1/files", + headers={"Authorization": "Bearer " + key}, + params={"purpose": purpose, "after": cursor}), 400, "purpose") + try: + owner.files.list(purpose=purpose, after="file-" + secrets.token_hex(24)) + except BadRequestError as error: + assert error.status_code == 400 and error.code is None and error.param == "purpose" + else: + raise AssertionError("SDK accepted an invalid Files purpose") + # Positive filtering is a Core regression; official evidence qualified only + # purpose validation before a missing cursor, not positive list contents. + assert file.id in [item.id for item in owner.files.list(purpose="user_data")] + assert list(outsider.files.list(purpose="user_data")) == [] + assert owner.files.retrieve(file.id).to_dict() == file.to_dict() + + +def skill_bundle(name, description, marker): + manifest = ("---\nname: " + name + "\ndescription: " + description + "\n---\n" + marker).encode() + output = io.BytesIO() + with zipfile.ZipFile(output, "w", zipfile.ZIP_DEFLATED) as archive: + archive.writestr("proof/SKILL.md", manifest) + return output.getvalue(), manifest + + +def check_skills(raw, owner, outsider, recovered, settings, cleanup): + base = settings["base"] + "/v1/skills" + headers = {"Authorization": "Bearer " + settings["token"]} + foreign = {"Authorization": "Bearer " + settings["foreign"]} + marker = "private-skill-resource-" + secrets.token_hex(16) + bundles = [skill_bundle("proof-" + label, "Describe " + label + ".", marker + label) + for label in ("one", "two", "three")] + created = owner.skills.with_raw_response.create(files=[("proof/SKILL.md", bundles[0][1], "text/markdown")]) + skill = created.parse() + cleanup.callback(owner.skills.delete, skill.id) + assert created.status_code == 200 and skill.default_version == skill.latest_version == "1" + first = owner.skills.versions.retrieve(skill_id=skill.id, version="1") + first_content = owner.skills.versions.content.retrieve(skill_id=skill.id, version="1").read() + second = owner.skills.versions.with_raw_response.create( + skill_id=skill.id, files=[("proof/SKILL.md", bundles[1][1], "text/markdown")], default=False, + ) + assert second.status_code == 200 and second.parse().version == "2" + second_content = owner.skills.versions.content.retrieve(skill_id=skill.id, version="2").read() + + def check_default(version, label, content, latest="2"): + current = owner.skills.retrieve(skill.id) + assert (current.default_version, current.latest_version, current.name, current.description) == ( + version, latest, "proof-" + label, "Describe " + label + ".", + ) + assert recovered.skills.retrieve(skill.id).to_dict() == current.to_dict() + assert next(item for item in owner.skills.list() if item.id == skill.id).to_dict() == current.to_dict() + response = raw.get(base + "/" + skill.id, headers=headers) + assert response.status_code == 200 and response.json() == current.to_dict() + assert owner.skills.content.retrieve(skill.id).read() == content + assert recovered.skills.content.retrieve(skill.id).read() == content + return current.to_dict() + + check_default("1", "one", first_content) + updated = owner.skills.update(skill.id, default_version="2") + assert updated.name == "proof-two" and updated.description == "Describe two." + check_default("2", "two", second_content) + updated = owner.skills.update(skill.id, default_version="1") + assert updated.name == "proof-one" and updated.description == "Describe one." + before = check_default("1", "one", first_content) + versions = [version.to_dict() for version in owner.skills.versions.list(skill.id)] + denied = raw.delete(base + "/" + skill.id + "/versions/1", headers=headers) + body = assert_error(denied, 400, "version", "invalid_value") + try: + owner.skills.versions.delete(skill_id=skill.id, version="1") + except BadRequestError as error: + assert error.status_code == 400 and error.body == body + else: + raise AssertionError("SDK deleted a Skill default version") + for method, suffix, kwargs in ( + ("POST", "", {"json": {"default_version": "2"}}), + ("DELETE", "", {}), + ("DELETE", "/versions/2", {}), + ("POST", "/versions", {"files": {"files": ("proof.zip", bundles[2][0], "application/zip")}}), + ): + response = raw.request(method, base + "/" + skill.id + suffix, headers=foreign, **kwargs) + assert response.status_code == 404, response.text + assert marker not in response.text + assert check_default("1", "one", first_content) == before + assert [version.to_dict() for version in owner.skills.versions.list(skill.id)] == versions + assert list(outsider.skills.list()) == [] + malformed = raw.post(base + "/" + skill.id + "/versions", headers=headers, + files=[("files", ("proof.zip", bundles[2][0])), ("unknown", (None, "reject"))]) + assert malformed.status_code == 400 + assert check_default("1", "one", first_content) == before + assert [version.to_dict() for version in owner.skills.versions.list(skill.id)] == versions + assert owner.skills.versions.retrieve(skill_id=skill.id, version="1").to_dict() == first.to_dict() + assert owner.skills.versions.content.retrieve(skill_id=skill.id, version="2").read() == second_content + deleted = owner.skills.versions.delete(skill_id=skill.id, version="2") + assert deleted.deleted and deleted.id == second.parse().id and deleted.version == "2" + check_default("1", "one", first_content, latest="1") + # The official deleted-version visibility window is unqualified; do not + # assert an immediate version GET/list result after successful deletion. + another = owner.skills.create(files=[("proof/SKILL.md", bundles[0][1], "text/markdown")]) + cleanup.callback(owner.skills.delete, another.id) + original = owner.skills.versions.content.retrieve(skill_id=another.id, version="1").read() + third = owner.skills.versions.create( + skill_id=another.id, files=[("proof/SKILL.md", bundles[2][1], "text/markdown")], default=True, + ) + current = owner.skills.retrieve(another.id) + assert (current.name, current.description, current.default_version, current.latest_version) == ( + "proof-three", "Describe three.", third.version, third.version, + ) + assert recovered.skills.retrieve(another.id).to_dict() == current.to_dict() + assert next(item for item in owner.skills.list() if item.id == another.id).to_dict() == current.to_dict() + content = owner.skills.content.retrieve(another.id).read() + assert content == owner.skills.versions.content.retrieve(skill_id=another.id, version=third.version).read() + with zipfile.ZipFile(io.BytesIO(content)) as archive: + assert archive.read("proof/SKILL.md") == bundles[2][1] + assert owner.skills.versions.content.retrieve(skill_id=another.id, version="1").read() == original + + +def main(): + settings = json.load(sys.stdin) + pin = json.loads((Path(__file__).resolve().parents[3] / "contracts/agents-api/upstream.json").read_text()) + distribution = importlib.metadata.distribution("openai") + assert distribution.version == pin["sdk_version"] == "3.13.0" + assert json.loads(distribution.read_text("direct_url.json"))["vcs_info"]["commit_id"] == pin["commit"] + with ExitStack() as cleanup: + raw = cleanup.enter_context(httpx2.Client(trust_env=False, timeout=20)) + clients = [cleanup.enter_context(OpenAI( + api_key=key, base_url=base + "/v1", max_retries=0, + _strict_response_validation=True, http_client=DefaultHttpxClient(trust_env=False), + )) for key, base in ( + (settings["token"], settings["base"]), + (settings["foreign"], settings["base"]), + (settings["token"], settings["recovered"]), + )] + check_files(raw, *clients, settings, cleanup) + check_skills(raw, *clients, settings, cleanup) + print(json.dumps({"result": "passed", "sdk": distribution.version, + "tenant_isolation": True, "postgres": True, "native_model": False})) + + +if __name__ == "__main__": + main() From 15726205a9e42fcb0db6680110c5b7f50692f077 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 02:54:29 +0800 Subject: [PATCH 3/8] Align source Files purpose validation and content policy --- .../internal/api/source_files_content.go | 19 ++++++------ .../internal/api/source_files_list.go | 8 ++++- .../internal/api/source_files_list_test.go | 25 ++++++++++++++++ .../internal/api/source_files_test.go | 23 +++++++++------ .../tests/official_source_file_errors.py | 4 ++- .../tests/official_source_file_list.py | 11 +++++++ .../agents-api/tests/official_source_files.py | 29 ++++++++++++++----- 7 files changed, 91 insertions(+), 28 deletions(-) diff --git a/services/agents-api/internal/api/source_files_content.go b/services/agents-api/internal/api/source_files_content.go index 327a86511..d65237fd0 100644 --- a/services/agents-api/internal/api/source_files_content.go +++ b/services/agents-api/internal/api/source_files_content.go @@ -8,28 +8,29 @@ import ( "strconv" "time" - "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" "github.com/go-chi/chi/v5" ) // @Summary Download source file bytes -// @Description Streams an authorized immutable source snapshot. Already-admitted reads may finish after deletion; later reads reject. No Beta header is required. Range requests and exact hosted headers/error behavior are not implemented or verified. +// @Description Resolves project-owned File metadata before enforcing download policy. Public download of user_data Files returns 400; missing and foreign Files return the same 404. Internal initial-file and workspace copies remain available. No Beta header is required. // @Tags Files -// @Produce octet-stream +// @Produce json // @Security BearerAuth // @Param file_id path string true "Source file ID" -// @Success 200 {file} binary // @Failure 400,401,404,500,503 {object} v1.ErrorResponse // @Router /files/{file_id}/content [get] func (h *Handler) sourceFileContent(w http.ResponseWriter, r *http.Request) { if !h.sourceFilesReady(w, r) { return } - serveStoredContent(w, r, func(ctx context.Context, consume func(string, int64, io.Reader) error) error { - return h.sourceFiles.ReadSourceFile(ctx, tenantID(r), chi.URLParam(r, "file_id"), func(file store.SourceFile, body io.Reader) error { - return consume(file.Filename, file.SizeBytes, body) - }) - }, "id") + ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) + defer cancel() + _, err := h.sourceFiles.GetSourceFile(ctx, tenantID(r), chi.URLParam(r, "file_id")) + if err != nil { + writeStoreError(w, r, err, "id") + return + } + writeError(w, http.StatusBadRequest, "", "Not allowed to download files of purpose: user_data") } func serveStoredContent(w http.ResponseWriter, r *http.Request, read func(context.Context, func(string, int64, io.Reader) error) error, notFoundParam ...string) { diff --git a/services/agents-api/internal/api/source_files_list.go b/services/agents-api/internal/api/source_files_list.go index 298c20da2..68c8fddb9 100644 --- a/services/agents-api/internal/api/source_files_list.go +++ b/services/agents-api/internal/api/source_files_list.go @@ -7,7 +7,7 @@ import ( ) // @Summary List source files -// @Description Lists project-owned Files without reading their bodies. Supports the pinned after, limit, order and purpose query surface. The limit defaults to 10000 and must be 1–10000. Equal creation times use ID ordering. Current storage contains only user_data; exact hosted default order, invalid-cursor errors and concurrent-page behavior remain unverified. No Beta header is required. +// @Description Lists project-owned Files without reading their bodies. The limit defaults to 10000 and must be 1–10000. Equal creation times use ID ordering. Purpose validation precedes cursor lookup; current storage contains only user_data. An explicit empty purpose retains the local exact-filter behavior. Hosted positive filtering, default order and concurrent-page behavior remain unverified. No Beta header is required. // @Tags Files // @Produce json // @Security BearerAuth @@ -52,5 +52,11 @@ func readSourceFilePage(w http.ResponseWriter, r *http.Request) (pageOptions, *s if !present { return options, nil, true } + switch values[0] { + case "", "user_data", "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 + } return options, &values[0], true } diff --git a/services/agents-api/internal/api/source_files_list_test.go b/services/agents-api/internal/api/source_files_list_test.go index f665fc958..61e65ac25 100644 --- a/services/agents-api/internal/api/source_files_list_test.go +++ b/services/agents-api/internal/api/source_files_list_test.go @@ -67,6 +67,31 @@ 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} + h, _ := environmentFileCreateHandler(t, WithSourceFiles(f)) + server := newSourceFileServer(t, h) + status, raw := sourceRequest(t, server, http.MethodGet, "/v1/files?after=file-missing&purpose="+purpose, "files-key", "", nil) + wantStatus, wantParam, wantCalls := http.StatusNotFound, "after", 1 + if purpose == "unknown" || purpose == "USER_DATA" { + wantStatus, wantParam, wantCalls = http.StatusBadRequest, "purpose", 0 + } + var body map[string]map[string]any + if status != wantStatus || json.Unmarshal(raw, &body) != nil || f.listCalls != wantCalls { + t.Fatalf("purpose validation: %d %s calls=%d", status, raw, f.listCalls) + } + if e := body["error"]; e["type"] != "invalid_request_error" || e["code"] != nil || e["param"] != wantParam { + t.Fatalf("purpose error projection: %s", raw) + } + if wantCalls == 1 && (f.listPurpose == nil || *f.listPurpose != purpose) { + t.Fatalf("purpose filter changed: %v", f.listPurpose) + } + }) + } +} + func newSourceFileServer(t *testing.T, handler http.Handler) *httptest.Server { t.Helper() server := httptest.NewServer(handler) diff --git a/services/agents-api/internal/api/source_files_test.go b/services/agents-api/internal/api/source_files_test.go index f6555a49a..d1230b92e 100644 --- a/services/agents-api/internal/api/source_files_test.go +++ b/services/agents-api/internal/api/source_files_test.go @@ -153,8 +153,13 @@ func TestSourceFilesPublicLifecycleAndEnvironmentCopy(t *testing.T) { t.Fatalf("foreign source: %d", status) } } - if status, got := sourceRequest(t, server, "GET", "/v1/files/"+file.ID+"/content", "files-key", "", nil); status != 200 || !bytes.Equal(got, data) { - t.Fatalf("content: %d %v", status, got) + status, raw = sourceRequest(t, server, "GET", "/v1/files/"+file.ID+"/content", "files-key", "", nil) + var denied map[string]map[string]any + if status != http.StatusBadRequest || json.Unmarshal(raw, &denied) != nil || f.reads != 0 { + t.Fatalf("content policy read source bytes: %d %s reads=%d", status, raw, f.reads) + } + if e := denied["error"]; e["type"] != "invalid_request_error" || e["code"] != nil || e["param"] != nil || e["message"] != "Not allowed to download files of purpose: user_data" { + t.Fatalf("content error projection: %s", raw) } copyBody := `{"type":"file_id","file_id":"` + file.ID + `","path":"/workspace/source.bin"}` if status, _ := sourceRequest(t, server, "POST", "/v1/agents/environments/"+env.environment.ID+"/files", "files-key", "application/json", []byte(copyBody)); status != 400 { @@ -221,17 +226,17 @@ func TestEnvironmentSourceCopyEnforcesScopeUnionAndSize(t *testing.T) { } } -func TestSourceContentDoesNotCompleteTruncatedResponse(t *testing.T) { - f := &sourceFilesFixture{file: store.SourceFile{ID: "file-" + uuid.NewString(), Filename: "source.bin", SizeBytes: 3}, data: []byte("x")} - h, env := environmentFileCreateHandler(t, WithSourceFiles(f)) - f.tenant = env.environment.TenantID - server := httptest.NewServer(h) +func TestStoredContentDoesNotCompleteTruncatedResponse(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + serveStoredContent(w, r, func(_ context.Context, consume func(string, int64, io.Reader) error) error { + return consume("source.bin", 3, strings.NewReader("x")) + }) + })) defer server.Close() - r, err := http.NewRequestWithContext(t.Context(), "GET", server.URL+"/v1/files/"+f.file.ID+"/content", nil) + r, err := http.NewRequestWithContext(t.Context(), "GET", server.URL, nil) if err != nil { t.Fatal(err) } - r.Header.Set("Authorization", "Bearer files-key") resp, err := server.Client().Do(r) if err != nil { return diff --git a/services/agents-api/tests/official_source_file_errors.py b/services/agents-api/tests/official_source_file_errors.py index 08a7b198b..96ba4b5cc 100644 --- a/services/agents-api/tests/official_source_file_errors.py +++ b/services/agents-api/tests/official_source_file_errors.py @@ -10,6 +10,8 @@ import httpx2 from openai import DefaultHttpxClient, NotFoundError, OpenAI +from official_source_files import verify_source_download_denied + def main(): base, token, foreign = sys.argv[1:] @@ -65,7 +67,7 @@ def invoke(client, file_id): assert unauthorized.status_code == 401 assert all(value not in unauthorized.text for value in (file.id, file.filename, secret)) assert owner.files.retrieve(file.id).to_dict() == before - assert owner.files.content(file.id).read() == secret.encode() + verify_source_download_denied(owner, raw, file.id) assert list(outsider.files.list()) == [] tail = owner.files.list(after=file.id) assert tail.data == [] and tail.has_more is False diff --git a/services/agents-api/tests/official_source_file_list.py b/services/agents-api/tests/official_source_file_list.py index 6393dccc7..240a175e9 100644 --- a/services/agents-api/tests/official_source_file_list.py +++ b/services/agents-api/tests/official_source_file_list.py @@ -62,6 +62,17 @@ def sdk_page(values, has_more, query, **request): response = raw.get(endpoint + "?" + query, headers=headers) assert response.status_code == 400 assert response.json()["error"]["type"] == "invalid_request_error" + for purpose in ("unknown", "USER_DATA"): + response = raw.get(endpoint, headers=headers, params={"purpose": purpose, "after": "file-missing"}) + assert response.status_code == 400 + error = response.json()["error"] + assert (error["type"], error["code"], error["param"]) == ("invalid_request_error", None, "purpose") + try: + files.list(purpose=purpose, after="file-missing") + except BadRequestError as denied: + assert denied.status_code == 400 and denied.body == error + else: + raise AssertionError("SDK accepted invalid list purpose") response = raw.get(endpoint, headers=headers, params={"after": "not-a-file"}) assert response.status_code == 404 assert response.json()["error"]["code"] is None diff --git a/services/agents-api/tests/official_source_files.py b/services/agents-api/tests/official_source_files.py index 3c6084a92..a5d38ed57 100644 --- a/services/agents-api/tests/official_source_files.py +++ b/services/agents-api/tests/official_source_files.py @@ -4,6 +4,22 @@ from pathlib import Path import tempfile +from openai import BadRequestError + + +def verify_source_download_denied(client, http, source_id): + endpoint = str(client.base_url).rstrip("/") + "/files/" + source_id + "/content" + response = http.get(endpoint, headers={"Authorization": "Bearer " + client.api_key}) + assert response.status_code == 400 + error = response.json()["error"] + assert (error["type"], error["code"], error["param"]) == ("invalid_request_error", None, None) + try: + client.files.content(source_id) + except BadRequestError as denied: + assert denied.status_code == 400 and denied.body == error + else: + raise AssertionError("SDK allowed public user_data content download") + def verify_source_files(client, foreign, http, environment, directory, cases): base = str(client.base_url).rstrip("/") @@ -25,7 +41,7 @@ def verify_source_files(client, foreign, http, environment, directory, cases): assert source["purpose"] == "user_data" and source["status"] == "processed" assert source["expires_at"] is None and source["status_details"] is None assert client.files.retrieve(source_id).to_dict() == source - assert client.files.content(source_id).read() == content + verify_source_download_denied(client, http, source_id) for method, suffix in (("GET", ""), ("GET", "/content"), ("DELETE", "")): response = http.request(method, endpoint + suffix, headers=other) assert response.status_code == 404 and "error" in response.json(), "Foreign source access succeeded" @@ -64,17 +80,14 @@ def verify_source_files(client, foreign, http, environment, directory, cases): large = client.files.create(file=("512MiB.bin", body), purpose="user_data") try: assert large.bytes == 512 << 20 - actual, size = hashlib.sha256(), 0 - with client.files.with_streaming_response.content(large.id) as response: - for chunk in response.iter_bytes(chunk_size=256 << 10): - size += len(chunk) - actual.update(chunk) - assert size == large.bytes and actual.digest() == digest.digest(), "Large source stream differs" + verify_source_download_denied(client, http, large.id) response = http.post(base + "/agents/environments/" + environment + "/files", headers={**headers, "OpenAI-Beta": "agents=v1"}, json={"type": "file_id", "file_id": large.id, "path": directory + "/too-large.bin"}) assert response.status_code == 413, "Destination size bound bypassed" finally: client.files.delete(large.id) - return copies, receipts, {"source_limit_bytes": 512 << 20, "large_source_sha256": digest.hexdigest(), + # This digest identifies the upload fixture; public download is denied and proves no byte round-trip. + return copies, receipts, {"source_limit_bytes": 512 << 20, "large_source_fixture_sha256": digest.hexdigest(), + "public_source_download_denied": True, "deleted_sources_unavailable": True, "foreign_source_rejected": True} From 3cb9a66f6a72b2abbd27029d550f31bbd7bbd292 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 23 Sep 2026 02:54:44 +0800 Subject: [PATCH 4/8] Align resource errors and Core Web with file policy --- CONTRIBUTING.md | 10 ++- apps/web/e2e/fixture-core.mjs | 13 +--- apps/web/src/App.tsx | 1 - .../features/system/SourceFilesPanel.test.tsx | 3 +- .../src/features/system/SourceFilesPanel.tsx | 46 ++---------- contracts/agents-api/README.md | 2 +- contracts/agents-api/environment-files.md | 5 +- contracts/agents-api/environment-templates.md | 12 +-- .../agents-api/file-resource-semantics.md | 73 +++++++++++++++++++ contracts/agents-api/openapi.yaml | 25 +++---- contracts/agents-api/operation-evidence.md | 19 ++--- contracts/agents-api/source-files.md | 23 ++++-- services/agents-api/internal/api/errors.go | 2 +- .../agents-api/internal/db/sqlc/skills.sql.go | 29 ++++++-- 14 files changed, 166 insertions(+), 97 deletions(-) create mode 100644 contracts/agents-api/file-resource-semantics.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 79d8cc75e..90a743cf7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -375,7 +375,9 @@ tools. This requirement does not add execution prerequisites to Files reads. Skills and their immutable versions are Core-owned tenant resources, independent of Sessions and native Skill installations. Serialize version allocation and pointer -mutations under the owning Skill row; preserve unique version identities across +mutations under the owning Skill row. Keep top-level name and description aligned +with the default version in the same transaction, including default-changing uploads; +nondefault uploads preserve that metadata. Preserve unique version identities across concurrent uploads and deletion. Metadata reads never load or decrypt bundle bytes. Encrypt bundle contents with a tenant, Skill and version binding using the existing service cipher. Deleting a Skill reclaims its versions without affecting already @@ -784,7 +786,11 @@ metadata insertion and bytes commit atomically; deletion removes metadata and unlinks the object in one transaction. Keep OIDs private and authorize every metadata/content/delete lookup by tenant before opening a body. Stream bounded chunks; never hold an entire general Files upload in memory or use filenames as -filesystem paths. A read-only repeatable-read transaction preserves an admitted +filesystem paths. Public source download admission is separate from internal byte +consumption: reject direct downloads of the supported `user_data` purpose after +tenant-scoped metadata lookup; initialization and workspace copies keep their +authorized Store read. Core Web must not offer that unavailable download action. +A read-only repeatable-read transaction preserves an admitted source snapshot across concurrent deletion. Resolve that snapshot before entering the existing Environment write path; deleting a source does not undo a completed workspace copy. Bound request/transaction lifetimes, roll back incomplete bodies, diff --git a/apps/web/e2e/fixture-core.mjs b/apps/web/e2e/fixture-core.mjs index 97845a260..00a142d7b 100644 --- a/apps/web/e2e/fixture-core.mjs +++ b/apps/web/e2e/fixture-core.mjs @@ -843,15 +843,10 @@ const server = http.createServer(async (request, response) => { if (request.headers["openai-beta"] != null) return sendError(response, 400, "Source Files do not accept the Agents beta header in this fixture."); const source = state.sourceFiles.get(id); if (!source) return sendError(response, 404, "Fixture Source File not found."); - response.writeHead(200, { - "content-type": "application/octet-stream", - "content-disposition": `attachment; filename="${source.metadata.filename}"`, - "content-length": source.data.length, - "cache-control": "no-store", - "x-content-type-options": "nosniff", - }); - response.end(source.data); - return; + return sendJson(response, { error: { + message: "Not allowed to download files of purpose: user_data", + type: "invalid_request_error", code: null, param: null, + } }, 400); } const sourceFileMatch = url.pathname.match(/^\/v1\/files\/([^/]+)$/); diff --git a/apps/web/src/App.tsx b/apps/web/src/App.tsx index 41a29971a..0bf723930 100644 --- a/apps/web/src/App.tsx +++ b/apps/web/src/App.tsx @@ -335,7 +335,6 @@ export function App() { const sourceFilesOperations = useMemo(() => ({ uploadSourceFile: (input, options) => core.uploadSourceFile(input, options), retrieveSourceFile: (fileId, options) => core.retrieveSourceFile(fileId, options), - downloadSourceFile: (fileId, options) => core.downloadSourceFile(fileId, options), deleteSourceFile: (fileId, options) => core.deleteSourceFile(fileId, options), retrieveEnvironment: (environmentId, options) => core.retrieveEnvironment(environmentId, options), createEnvironmentFile: (environmentId, input, options) => core.createEnvironmentFile(environmentId, input, options), diff --git a/apps/web/src/features/system/SourceFilesPanel.test.tsx b/apps/web/src/features/system/SourceFilesPanel.test.tsx index a0e96d59d..c3c1492f6 100644 --- a/apps/web/src/features/system/SourceFilesPanel.test.tsx +++ b/apps/web/src/features/system/SourceFilesPanel.test.tsx @@ -7,7 +7,6 @@ function operations(): SourceFilesOperations { return { uploadSourceFile: vi.fn(), retrieveSourceFile: vi.fn(), - downloadSourceFile: vi.fn(), deleteSourceFile: vi.fn(), retrieveEnvironment: vi.fn(), createEnvironmentFile: vi.fn(), @@ -38,6 +37,8 @@ describe("SourceFilesPanel", () => { expect(html).toContain("512 MiB source · 50 MiB destination"); expect(html).toContain("Upload Source File"); expect(html).toContain("Operate by Core ID"); + expect(html).toContain("Uploaded Source Files cannot be downloaded directly"); + expect(html).not.toContain(">Download"); expect(html).toContain("Core has no Source Files list API"); expect(html).toContain("Copy to hosted Workspace"); expect(html).toContain("Check Environment"); diff --git a/apps/web/src/features/system/SourceFilesPanel.tsx b/apps/web/src/features/system/SourceFilesPanel.tsx index ad77d532a..f7524125f 100644 --- a/apps/web/src/features/system/SourceFilesPanel.tsx +++ b/apps/web/src/features/system/SourceFilesPanel.tsx @@ -1,4 +1,4 @@ -import { Download, FileUp, FolderInput, RefreshCw, Search, Trash2 } from "lucide-react"; +import { FileUp, FolderInput, RefreshCw, Search, Trash2 } from "lucide-react"; import { useEffect, useRef, useState, type ChangeEvent } from "react"; import { @@ -16,14 +16,13 @@ import "./SourceFilesPanel.css"; export interface SourceFilesOperations { uploadSourceFile: AgentCore["uploadSourceFile"]; retrieveSourceFile: AgentCore["retrieveSourceFile"]; - downloadSourceFile: AgentCore["downloadSourceFile"]; deleteSourceFile: AgentCore["deleteSourceFile"]; retrieveEnvironment: AgentCore["retrieveEnvironment"]; createEnvironmentFile: AgentCore["createEnvironmentFile"]; listEnvironmentFiles: AgentCore["listEnvironmentFiles"]; } -type Operation = "upload" | "retrieve" | "download" | "delete" | "environment" | "copy" | null; +type Operation = "upload" | "retrieve" | "delete" | "environment" | "copy" | null; type NoticeTone = "info" | "success" | "error" | "warning"; interface Notice { @@ -76,11 +75,6 @@ function parentDirectory(path: string): string { return path.slice(0, path.lastIndexOf("/")) || "/workspace"; } -function safeDownloadFilename(metadata: SourceFile | null, fileId: string): string { - if (metadata?.id !== fileId) return "source-file.bin"; - return metadata.filename.replace(/[\\/\0\r\n]/g, "_") || "source-file.bin"; -} - function knownRejected(error: unknown): boolean { return error instanceof AgentCoreError && error.status >= 400 && error.status < 500; } @@ -197,36 +191,6 @@ export function SourceFilesPanel({ } }; - const download = async () => { - if (!currentId) return; - const { controller, request } = start("download"); - try { - const content = await operations.downloadSourceFile(currentId, { signal: controller.signal }); - if (!finish(request, controller)) return; - const bytes = content.data.buffer.slice( - content.data.byteOffset, - content.data.byteOffset + content.data.byteLength, - ) as ArrayBuffer; - const url = URL.createObjectURL(new Blob([bytes], { type: content.content_type })); - try { - const anchor = document.createElement("a"); - anchor.href = url; - anchor.download = safeDownloadFilename(metadata, currentId); - anchor.rel = "noopener"; - document.body.appendChild(anchor); - anchor.click(); - anchor.remove(); - } finally { - URL.revokeObjectURL(url); - } - setNotice({ tone: "success", text: `Downloaded ${formatFileSize(content.bytes)} after validating the complete binary response.` }); - } catch (error) { - if (!current(request, controller) || isAbort(error)) return; - finish(request, controller); - setNotice({ tone: "error", text: safeFailure(error, "download") }); - } - }; - const deleteFile = async () => { if (!currentId) return; const id = currentId; @@ -361,7 +325,7 @@ export function SourceFilesPanel({
{environmentFilesEnabled ? "512 MiB source · 50 MiB destination" : "512 MiB source"}
@@ -399,9 +363,9 @@ export function SourceFilesPanel({
-
+

Uploaded Source Files cannot be downloaded directly. Copy them to a Workspace for execution.

Core has no Source Files list API. Refreshing or reopening this page requires the ID again.

@@ -463,7 +427,7 @@ export function SourceFilesPanel({ /> {!destinationValid ?

Use one canonical absolute file path beneath /workspace/; parent traversal and trailing slashes are rejected.

: null} - {metadata && metadata.bytes > maxDestinationBytes ?

This Source File is retained and downloadable, but it exceeds the 50 MiB destination-copy limit.

: null} + {metadata && metadata.bytes > maxDestinationBytes ?

This Source File is retained, but it exceeds the 50 MiB destination-copy limit.

: null}