diff --git a/internal/server/project_lifecycle_test.go b/internal/server/project_lifecycle_test.go index d4dc059..47ea686 100644 --- a/internal/server/project_lifecycle_test.go +++ b/internal/server/project_lifecycle_test.go @@ -1,6 +1,7 @@ package server import ( + "agent-system-backend/internal/cardsvc" "agent-system-backend/internal/cmdbus" "agent-system-backend/internal/surfacesvc" "agent-system-backend/pkg/extension" @@ -10,6 +11,7 @@ import ( "encoding/json" "errors" "fmt" + "mime/multipart" "net/http" "net/http/httptest" "testing" @@ -279,3 +281,223 @@ func TestGenericProjectPatchRejectsManagedSettingsAtomically(t *testing.T) { }) } } + +func TestGenericProjectPatchAcceptsNestedMaskedWriteback(t *testing.T) { + for _, registered := range []bool{false, true} { + t.Run(fmt.Sprint(registered), func(t *testing.T) { + s, st := newAuthTestServer(t) + key, _ := st.CreateAPIKey("alice", "test") + p, _ := st.CreateProject("alice", "Project") + if err := st.UpdateProjectSettings("alice", p.ID, map[string]any{"models": map[string]any{"chat": map[string]any{"apiKey": "fixture-persisted-key"}}, "display": map[string]any{"caption": "Original"}}); err != nil { + t.Fatal(err) + } + if registered { + s.SetProjectLifecycle(extension.ProjectLifecycle{List: func(context.Context, extension.Host) ([]core.Project, error) { + current, err := st.GetProject("alice", p.ID) + return []core.Project{current}, err + }}, nil) + } + for _, tc := range []struct{ body, caption string }{ + {`{"settings":{"models":{"chat":{"apiKey":"••••••"}},"display":{"caption":"Nested updated"}}}`, "Nested updated"}, + {`{"settings":{"models.chat":{"apiKey":"••••••"},"display":{"caption":"Mixed updated"}}}`, "Mixed updated"}, + {`{"settings":{"models":{"chat.apiKey":"••••••"},"display":{"caption":"Mixed leaf updated"}}}`, "Mixed leaf updated"}, + } { + w := lifecycleRequest(t, s, key, "PATCH", "/api/projects/"+p.ID, tc.body) + if w.Code != 200 { + t.Fatalf("masked writeback rejected legal edit: %d", w.Code) + } + current, err := st.GetProject("alice", p.ID) + if err != nil { + t.Fatal(err) + } + if current.Settings["models"].(map[string]any)["chat"].(map[string]any)["apiKey"] != "fixture-persisted-key" || current.Settings["display"].(map[string]any)["caption"] != tc.caption { + t.Fatalf("masked writeback secret preserved=%v caption=%q expected=%q", current.Settings["models"].(map[string]any)["chat"].(map[string]any)["apiKey"] == "fixture-persisted-key", current.Settings["display"].(map[string]any)["caption"], tc.caption) + } + } + for _, body := range []string{`{"settings":{"models":{}}}`, `{"settings":{"models":{"chat":{}}}}`, `{"settings":{"models.chat":{}}}`, `{"settings":{"models":{"chat":{"apiKey":"••••••","model":"Overwrite"}}}}`} { + if w := lifecycleRequest(t, s, key, "PATCH", "/api/projects/"+p.ID, body); w.Code != 400 { + t.Fatalf("explicit empty/real managed write accepted: %d", w.Code) + } + } + }) + } +} + +func lifecycleUpload(t *testing.T, s *Server, key, projectID, name, content string) *httptest.ResponseRecorder { + t.Helper() + var body bytes.Buffer + writer := multipart.NewWriter(&body) + file, err := writer.CreateFormFile("file", name) + if err != nil { + t.Fatal(err) + } + if _, err := file.Write([]byte(content)); err != nil { + t.Fatal(err) + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + req := httptest.NewRequest("POST", "/api/projects/"+projectID+"/uploads", &body) + req.Header.Set("Authorization", "Bearer "+key) + req.Header.Set("Content-Type", writer.FormDataContentType()) + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, req) + return rec +} + +func TestProjectLifecycleMemberUploadStaysPrivate(t *testing.T) { + s, st := newAuthTestServer(t) + alice, _ := st.CreateAPIKey("alice", "test") + bob, _ := st.CreateAPIKey("bob", "test") + mallory, _ := st.CreateAPIKey("mallory", "test") + p, _ := st.CreateProject("alice", "Shared") + ready := true + s.SetProjectLifecycle(extension.ProjectLifecycle{Work: func(_ context.Context, actor string, id core.ID) (bool, error) { + return ready && id == p.ID && (actor == "alice" || actor == "bob"), nil + }}, nil) + s.SetResourceAuthorizerV2(func(q extension.ResourceAuthzQuery) (bool, error) { + if q.ResourceType == "artifact" { + return q.Actor == q.Owner, nil + } + return q.ResourceType == "project" && q.ProjectID == p.ID && (q.Actor == "alice" || q.Actor == "bob"), nil + }) + s.DisableUploadIndexing = true + w := lifecycleUpload(t, s, bob, p.ID, "member-private.txt", "QA_MEMBER_PRIVATE_ORIGINAL") + if w.Code != 200 { + t.Fatalf("member upload=%d", w.Code) + } + var info core.UploadInfo + if err := json.Unmarshal(w.Body.Bytes(), &info); err != nil { + t.Fatal(err) + } + artifact, err := st.GetArtifact("bob", info.ArtifactID) + if err != nil || artifact.ProjectID != p.ID || artifact.Owner != "bob" { + t.Fatalf("upload ownership/target incorrect: %+v %v", artifact, err) + } + if w := lifecycleRequest(t, s, bob, "GET", "/api/artifacts/"+info.ArtifactID+"/content", ""); w.Code != 200 || w.Body.String() != "QA_MEMBER_PRIVATE_ORIGINAL" { + t.Fatalf("owner original read=%d", w.Code) + } + for _, key := range []string{alice, mallory} { + for _, suffix := range []string{"", "/content"} { + if w := lifecycleRequest(t, s, key, "GET", "/api/artifacts/"+info.ArtifactID+suffix, ""); w.Code != 404 { + t.Fatalf("other actor read private original=%d", w.Code) + } + } + } + if w := lifecycleUpload(t, s, mallory, p.ID, "forbidden.txt", "DENIED"); w.Code != 404 { + t.Fatalf("nonmember upload=%d", w.Code) + } + ready = false + if w := lifecycleUpload(t, s, bob, p.ID, "revoked.txt", "DENIED"); w.Code != 404 { + t.Fatalf("revoked upload=%d", w.Code) + } + artifacts, err := st.ListProjectArtifactsAll(p.ID) + if err != nil || len(artifacts) != 1 { + t.Fatalf("denied upload wrote artifacts: count=%d error=%v", len(artifacts), err) + } +} + +func TestProjectLifecycleUploadSuppressesPrivateProjectCards(t *testing.T) { + s, st := newAuthTestServer(t) + alice, _ := st.CreateAPIKey("alice", "test") + bob, _ := st.CreateAPIKey("bob", "test") + p, _ := st.CreateProject("alice", "Shared") + s.SetProjectLifecycle(extension.ProjectLifecycle{Work: func(_ context.Context, actor string, id core.ID) (bool, error) { + return id == p.ID && (actor == "alice" || actor == "bob"), nil + }}, nil) + s.SetResourceAuthorizerV2(func(q extension.ResourceAuthzQuery) (bool, error) { + return q.ResourceType == "project" && q.ProjectID == p.ID && (q.Actor == "alice" || q.Actor == "bob"), nil + }) + s.Cards = cardsvc.New(st, s.mgr.EmitCardEvent) + s.DisableUploadIndexing = true + if _, err := s.Cards.Create("alice", core.ActivityCard{ProjectID: p.ID, Title: "Shared structure", Kind: core.CardKindProgress}, ""); err != nil { + t.Fatal(err) + } + // The owner path already admitted upload before the member authorization fix. + if w := lifecycleUpload(t, s, alice, p.ID, "owner-private-filename.txt", "PRIVATE ORIGINAL"); w.Code != 200 { + t.Fatalf("owner upload=%d", w.Code) + } + if w := lifecycleUpload(t, s, bob, p.ID, "member-private-filename.txt", "MEMBER PRIVATE ORIGINAL"); w.Code != 200 { + t.Fatalf("member upload=%d", w.Code) + } + for _, owner := range []string{"alice", "bob"} { + cards, err := st.ListProjectCards(owner, p.ID) + if err != nil { + t.Fatal(err) + } + expected := 0 + if owner == "alice" { + expected = 1 + } + if len(cards) != expected { + t.Fatal("private upload added project card history") + } + for _, card := range cards { + if card.Title != "Shared structure" { + t.Fatal("private filename/parser card entered project history") + } + } + } + events, err := st.EventsAfter(core.ProjectChannelKey(p.ID), 0, 100) + if err != nil { + t.Fatal(err) + } + if len(events) != 1 { + t.Fatal("private upload added project events") + } + for _, event := range events { + if bytes.Contains(event.Payload, []byte("owner-private-filename.txt")) || bytes.Contains(event.Payload, []byte("member-private-filename.txt")) { + t.Fatal("private upload metadata entered persisted project stream") + } + } + request := httptest.NewRequest("GET", "/api/events?scope=project&projectId="+p.ID, nil) + request.Header.Set("Authorization", "Bearer "+bob) + ctx, cancel := context.WithTimeout(request.Context(), 20*time.Millisecond) + defer cancel() + request = request.WithContext(ctx) + response := httptest.NewRecorder() + s.Handler().ServeHTTP(response, request) + if response.Code != 200 || !bytes.Contains(response.Body.Bytes(), []byte("Shared structure")) { + t.Fatalf("member structural stream did not replay: %d", response.Code) + } + if bytes.Contains(response.Body.Bytes(), []byte("owner-private-filename.txt")) || bytes.Contains(response.Body.Bytes(), []byte("member-private-filename.txt")) { + t.Fatal("another member's SSE exposed private upload metadata") + } +} + +func TestUnregisteredUploadKeepsLegacyCardsAndValidation(t *testing.T) { + s, st := newAuthTestServer(t) + key, _ := st.CreateAPIKey("alice", "test") + p, _ := st.CreateProject("alice", "Private") + s.Cards = cardsvc.New(st, s.mgr.EmitCardEvent) + s.DisableUploadIndexing = true + if w := lifecycleUpload(t, s, key, p.ID, "legacy-upload.txt", "Original"); w.Code != 200 { + t.Fatalf("unregistered owner upload=%d", w.Code) + } + cards, err := st.ListProjectCards("alice", p.ID) + if err != nil || len(cards) != 1 || cards[0].Title != "legacy-upload.txt" { + t.Fatalf("unregistered ingestion card changed: count=%d error=%v", len(cards), err) + } + if w := lifecycleRequest(t, s, key, "POST", "/api/projects/missing/uploads", "not multipart"); w.Code != 400 { + t.Fatalf("legacy missing-file validation changed: %d", w.Code) + } +} + +func TestProjectLifecycleUploadRechecksBeforePersistence(t *testing.T) { + s, st := newAuthTestServer(t) + key, _ := st.CreateAPIKey("bob", "test") + p, _ := st.CreateProject("alice", "Shared") + ready := true + s.SetProjectLifecycle(extension.ProjectLifecycle{Work: func(context.Context, string, core.ID) (bool, error) { + allowed := ready + ready = false + return allowed, nil + }}, nil) + if w := lifecycleUpload(t, s, key, p.ID, "revoked-during-upload.txt", "PRIVATE"); w.Code != 404 { + t.Fatalf("upload persisted after authorization changed: %d", w.Code) + } + artifacts, err := st.ListProjectArtifactsAll(p.ID) + if err != nil || len(artifacts) != 0 { + t.Fatal("revoked upload created an artifact") + } +} diff --git a/internal/server/server.go b/internal/server/server.go index f932783..426070e 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1413,6 +1413,17 @@ func (s *Server) cardDismiss(w http.ResponseWriter, r *http.Request) { func (s *Server) upload(w http.ResponseWriter, r *http.Request) { projectID := r.PathValue("id") + owner := s.owner(r) + if s.projectLifecycle != nil { + if err := s.projectWork(r.Context(), owner, projectID); err != nil { + writeProjectErr(w, err) + return + } + if _, err := s.st.ProjectMetadata(projectID); err != nil { + writeStoreErr(w, err) + return + } + } r.ParseMultipartForm(64 << 20) file, header, err := r.FormFile("file") if err != nil { @@ -1425,10 +1436,18 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusBadRequest) return } - owner := s.owner(r) - if _, err := s.st.GetProject(owner, projectID); err != nil { - writeStoreErr(w, err) - return + if s.projectLifecycle == nil { + if _, err := s.st.GetProject(owner, projectID); err != nil { + writeStoreErr(w, err) + return + } + } + if s.projectLifecycle != nil { + // Request-body reading can outlast the membership/ready decision. + if err := s.projectWork(r.Context(), owner, projectID); err != nil { + writeProjectErr(w, err) + return + } } mime := header.Header.Get("Content-Type") art := core.Artifact{ProjectID: projectID, Owner: owner, Name: header.Filename, Kind: kindOf(header.Filename), MIME: mime} @@ -1443,11 +1462,14 @@ func (s *Server) upload(w http.ResponseWriter, r *http.Request) { Status: core.CardActive, Tone: "info", Body: []core.UINode{{Type: core.UIProgress, Progress: &core.UIProgressSpec{Label: "解析中", Value: 0.3}}}, } - if s.Cards != nil { + // This endpoint has no private conversation stream. Registered uploads keep + // original filename/parser metadata out of shared project cards and events. + showIngestCard := s.Cards != nil && s.projectLifecycle == nil + if showIngestCard { ingestCard, _ = s.Cards.Create(owner, ingestCard, "") } doc := ingest.Parse(art.Name, mime, content) - if s.Cards != nil { + if showIngestCard { ingestCard.Status = core.CardDone ingestCard.Tone = "ok" body := []core.UINode{ diff --git a/internal/settings/project_metadata.go b/internal/settings/project_metadata.go index 56359ab..2eb0035 100644 --- a/internal/settings/project_metadata.go +++ b/internal/settings/project_metadata.go @@ -71,13 +71,35 @@ func maskedProjectValue(value any, path string, secrets map[string]bool) any { // rules as project settings, while preserving the metadata store's key shapes. // Project metadata lives in projects.settings, not the settings scope table. func (s *Service) PrepareProjectMetadataWrite(values map[string]any) (map[string]any, error) { - incoming := dropMaskPlaceholders(values) + incoming := dropProjectMaskPlaceholders(values) if err := s.checkManagedWrite(normalizeKeys(incoming)); err != nil { return nil, err } return incoming, nil } +// dropProjectMaskPlaceholders prunes a parent only when a previously nonempty +// object became empty through mask removal. Explicit empty objects still express +// an overwrite and must reach the managed-path guard unchanged. +func dropProjectMaskPlaceholders(values map[string]any) map[string]any { + out := make(map[string]any, len(values)) + for key, value := range values { + if child, ok := value.(map[string]any); ok { + prepared := dropProjectMaskPlaceholders(child) + if len(child) > 0 && len(prepared) == 0 { + continue + } + out[key] = prepared + continue + } + if text, ok := value.(string); ok && text == MaskPlaceholder { + continue + } + out[key] = value + } + return out +} + func (s *Service) checkManagedWrite(incoming map[string]any) error { if hits := core.TouchesManagedSettings(incoming, s.ManagedSettingPaths()); len(hits) > 0 { return &ErrManagedPath{Paths: hits} diff --git a/internal/settings/project_metadata_test.go b/internal/settings/project_metadata_test.go index 12fa9f3..443eb83 100644 --- a/internal/settings/project_metadata_test.go +++ b/internal/settings/project_metadata_test.go @@ -51,3 +51,36 @@ func TestPrepareProjectMetadataWriteUsesManagedGuard(t *testing.T) { t.Fatal("write preparation mutated its input") } } + +func TestPrepareProjectMetadataWritePrunesOnlyMaskedParents(t *testing.T) { + s := NewService(nil) + for _, raw := range []map[string]any{ + {"models": map[string]any{"chat": map[string]any{"apiKey": MaskPlaceholder}}, "display": map[string]any{"caption": "Updated"}}, + {"models.chat": map[string]any{"apiKey": MaskPlaceholder}, "display.caption": "Updated"}, + {"models": map[string]any{"chat.apiKey": MaskPlaceholder}, "display.caption": "Updated"}, + } { + before, _ := json.Marshal(raw) + got, err := s.PrepareProjectMetadataWrite(raw) + if err != nil { + t.Fatalf("unchanged masked credential rejected legal metadata edit: %v", err) + } + if len(got) != 1 { + t.Fatal("mask-only parent remained in prepared metadata") + } + after, _ := json.Marshal(raw) + if !bytes.Equal(before, after) { + t.Fatal("mask preparation changed its input") + } + } + for _, raw := range []map[string]any{ + {"models": map[string]any{}}, + {"models": map[string]any{"chat": map[string]any{}}}, + {"models.chat": map[string]any{}}, + {"models": map[string]any{"chat": map[string]any{"apiKey": MaskPlaceholder, "model": "Real overwrite"}}}, + {"models.chat": map[string]any{"apiKey": MaskPlaceholder}, "models": map[string]any{"chat": map[string]any{}}}, + } { + if _, err := s.PrepareProjectMetadataWrite(raw); err == nil { + t.Fatal("explicit empty or real managed overwrite accepted") + } + } +}