Admit private member uploads and preserve masked metadata edits
Why: Nested unchanged credential masks left empty parents that looked like managed subtree overwrites. Shared members also hit an owner-only upload guard, while upload ingestion cards could disclose private filenames and parser metadata. Evidence: SELF independent nested-mask probe, HTTP nested/mixed writeback, member upload, project card privacy and authorization-change regressions failed before each fix and passed after it. Affected settings/server suite passed; exact final-object build and independent replay remain delivery gates owned by the task coordinator. Impact: Prune only parents emptied by ignored masks; explicit empty/real managed writes stay rejected. Registered uploads check live work and trusted project existence, keep originals owned by the actor, and recheck before persistence. Suppress only upload project cards when no private stream exists; explicit structure cards and unregistered upload behavior remain available. Refs: Dev project-member-access plan r1 Task 1/R5; independent nested mask P2; 2026-10-13 composed member-upload and private project-stream acceptance findings
This commit is contained in:
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user