From c4435d1415eda80e402c52b5ea865cf680be1997 Mon Sep 17 00:00:00 2001 From: weichen Date: Tue, 13 Oct 2026 09:46:20 +0000 Subject: [PATCH] Protect shared project metadata credentials and managed settings Why: Legacy project metadata can contain credentials outside the settings scope table. Sharing raw projection settings exposed them, and generic project PATCH bypassed the system-managed path guard already enforced by project settings endpoints. Evidence: SELF member Host regression failed before the fix. The fixed 4f72aaf server baseline overlay reproduced HTTP metadata leakage and managed PATCH returning 200. SELF shared Host-to-HTTP, immutable mask, dotted/schema/fallback, public mutation and atomic managed-write regressions passed; uncached full suite, isolated PG Store suite and static build passed. Independent delta replay remains a gate. Impact: Registered lifecycle metadata returns detached credential-masked views while preserving service/configuration keys and persisted data. Generic project PATCH shares the existing managed validator before rename or settings mutation; allowed metadata key shapes and raw owner-only management lookups remain available. Refs: Dev project-member-access plan r1 Task 1 and R5; approved hard secret boundary; 2026-10-13 shared project metadata review follow-up --- internal/app/project_lifecycle.go | 3 + internal/app/project_lifecycle_test.go | 86 ++++++++++++++++++++++ internal/server/project_lifecycle.go | 9 +++ internal/server/project_lifecycle_test.go | 82 +++++++++++++++++++++ internal/server/server.go | 14 +++- internal/settings/project_metadata.go | 86 ++++++++++++++++++++++ internal/settings/project_metadata_test.go | 53 +++++++++++++ internal/settings/settings.go | 6 +- pkg/extension/projects.go | 3 +- 9 files changed, 335 insertions(+), 7 deletions(-) create mode 100644 internal/settings/project_metadata.go create mode 100644 internal/settings/project_metadata_test.go diff --git a/internal/app/project_lifecycle.go b/internal/app/project_lifecycle.go index 883d5c2..78ccf95 100644 --- a/internal/app/project_lifecycle.go +++ b/internal/app/project_lifecycle.go @@ -98,6 +98,9 @@ func (h inprocHost) ProjectMetadata(id string) (core.Project, error) { if err != nil { return core.Project{}, err } + if h.caps.projects != nil && h.caps.projects.active() { + p.Settings = h.caps.set.MaskedProjectMetadata(p.Settings) + } if p.Owner == h.owner { return p, nil } diff --git a/internal/app/project_lifecycle_test.go b/internal/app/project_lifecycle_test.go index ad261c0..1650ff9 100644 --- a/internal/app/project_lifecycle_test.go +++ b/internal/app/project_lifecycle_test.go @@ -1,11 +1,16 @@ package app import ( + "agent-system-backend/internal/server" + "agent-system-backend/internal/settings" "agent-system-backend/internal/store" "agent-system-backend/pkg/extension" core "agent-system-core" + "bytes" "context" + "encoding/json" "errors" + "net/http/httptest" "testing" ) @@ -75,3 +80,84 @@ func TestProjectHostMetadataDoesNotGrantMutation(t *testing.T) { t.Fatalf("V2 denial ignored=%v", err) } } + +func TestProjectHostMetadataMasksSharedSecretsWithoutMutation(t *testing.T) { + st, err := store.OpenSQLite(t.TempDir() + "/t.db") + if err != nil { + t.Fatal(err) + } + defer st.Close() + p, err := st.CreateProject("alice", "Shared") + if err != nil { + t.Fatal(err) + } + raw := map[string]any{"models": map[string]any{"chat": map[string]any{"apiKey": "fixture-nested-secret"}}, "models.embedding.apiKey": "fixture-flat-secret", "legacy.token": "fixture-legacy-secret", "plugins.private": map[string]any{"credential": "fixture-schema-secret"}, "display": map[string]any{"caption": "Shared configuration"}, "rdm.projectId": "canonical", "rdm.state": "ready"} + if err := st.UpdateProjectSettings("alice", p.ID, raw); err != nil { + t.Fatal(err) + } + svc := settings.NewService(st) + svc.SetExtraSchema([]core.SettingDesc{{Key: "plugins.private.credential", Secret: true}}) + slot := &projectLifecycleSlot{} + auth := &projectAuthz{} + caps := hostcaps{st: st, set: svc, projects: slot, authz: auth} + provider := completeLifecycle() + provider.List = func(_ context.Context, h extension.Host) ([]core.Project, error) { + project, err := h.(extension.ProjectHost).ProjectMetadata(p.ID) + return []core.Project{project}, err + } + if err := (pluginRegistry{pluginID: "rdm", caps: caps}).RegisterProjectLifecycle(provider); err != nil { + t.Fatal(err) + } + auth.setV2("rdm", func(q extension.ResourceAuthzQuery) (bool, error) { + return q.Actor == "bob" && q.ResourceType == "project" && q.ResourceID == p.ID, nil + }) + for _, actor := range []string{"bob", "alice"} { + h := inprocHost{owner: actor, caps: caps} + view, err := h.ProjectMetadata(p.ID) + if err != nil { + t.Fatal(err) + } + encoded, _ := json.Marshal(view.Settings) + for _, value := range []string{"fixture-nested-secret", "fixture-flat-secret", "fixture-legacy-secret", "fixture-schema-secret"} { + if bytes.Contains(encoded, []byte(value)) { + t.Fatalf("%s metadata exposed a fixture credential", actor) + } + } + if view.Settings["rdm.projectId"] != "canonical" || view.Settings["rdm.state"] != "ready" || view.Settings["display"].(map[string]any)["caption"] != "Shared configuration" { + t.Fatal("allowed service/configuration metadata changed") + } + } + // Exercise the real registry -> actor-scoped Host -> public HTTP list chain. + srv := server.New(st, nil, nil, nil, svc, nil) + srv.Auth = server.NewAuthenticator(st, true) + slot.wire(srv, caps) + key, err := st.CreateAPIKey("bob", "test") + if err != nil { + t.Fatal(err) + } + request := httptest.NewRequest("GET", "/api/projects", nil) + request.Header.Set("Authorization", "Bearer "+key) + response := httptest.NewRecorder() + srv.Handler().ServeHTTP(response, request) + if response.Code != 200 { + t.Fatalf("member list=%d", response.Code) + } + for _, value := range []string{"fixture-nested-secret", "fixture-flat-secret", "fixture-legacy-secret", "fixture-schema-secret"} { + if bytes.Contains(response.Body.Bytes(), []byte(value)) { + t.Fatal("actor-scoped lifecycle list exposed a fixture credential") + } + } + if !bytes.Contains(response.Body.Bytes(), []byte(`"rdm.projectId":"canonical"`)) { + t.Fatal("actor-scoped list lost canonical metadata") + } + + owned, err := (inprocHost{owner: "alice", caps: caps}).OwnedProject(p.ID) + if err != nil { + t.Fatal(err) + } + before, _ := json.Marshal(raw) + after, _ := json.Marshal(owned.Settings) + if !bytes.Equal(before, after) { + t.Fatal("metadata read mutated persisted owner settings") + } +} diff --git a/internal/server/project_lifecycle.go b/internal/server/project_lifecycle.go index eb4d747..ed2b29c 100644 --- a/internal/server/project_lifecycle.go +++ b/internal/server/project_lifecycle.go @@ -71,3 +71,12 @@ func writeProjectJSON(w http.ResponseWriter, p any, err error) { } writeJSON(w, http.StatusOK, p) } + +// publicProjectMetadata also masks provider-returned metadata, since providers +// may obtain raw owner-management rows before producing a shared public view. +func (s *Server) publicProjectMetadata(p core.Project) core.Project { + if s.projectLifecycle != nil { + p.Settings = s.set.MaskedProjectMetadata(p.Settings) + } + return p +} diff --git a/internal/server/project_lifecycle_test.go b/internal/server/project_lifecycle_test.go index 61ea6dc..d4dc059 100644 --- a/internal/server/project_lifecycle_test.go +++ b/internal/server/project_lifecycle_test.go @@ -9,6 +9,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "testing" @@ -197,3 +198,84 @@ func TestProjectLifecycleSettingsRetainServiceMetadata(t *testing.T) { t.Fatalf("settings lost service metadata: %d %s", w.Code, w.Body.String()) } } + +func TestProjectLifecyclePublicMetadataMasksSecrets(t *testing.T) { + s, st := newAuthTestServer(t) + alice, _ := st.CreateAPIKey("alice", "test") + bob, _ := st.CreateAPIKey("bob", "test") + p, _ := st.CreateProject("alice", "Shared") + raw := map[string]any{"models.chat.apiKey": "fixture-key", "custom": map[string]any{"token": "fixture-token"}, "custom.api_key": "fixture-flat-token", "plugin.private.credential": "fixture-schema-token", "display": map[string]any{"caption": "Shared"}, "rdm.projectId": "canonical", "rdm.state": "ready"} + if err := st.UpdateProjectSettings("alice", p.ID, raw); err != nil { + t.Fatal(err) + } + s.set.SetExtraSchema([]core.SettingDesc{{Key: "plugin.private.credential", Secret: true}}) + read := func() (core.Project, error) { return st.GetProject("alice", p.ID) } + s.SetProjectLifecycle(extension.ProjectLifecycle{List: func(context.Context, extension.Host) ([]core.Project, error) { + current, err := read() + return []core.Project{current}, err + }, Create: func(context.Context, extension.Host, string) (core.Project, error) { return read() }, Rename: func(context.Context, extension.Host, string, string) (core.Project, error) { return read() }}, nil) + for _, q := range []struct{ key, method, path, body string }{{bob, "GET", "/api/projects", ""}, {alice, "GET", "/api/projects", ""}, {alice, "POST", "/api/projects", `{"name":"New"}`}, {alice, "PATCH", "/api/projects/" + p.ID, `{"name":"Renamed"}`}, {alice, "PATCH", "/api/projects/" + p.ID, `{"settings":{"display":{"caption":"Updated"}}}`}} { + w := lifecycleRequest(t, s, q.key, q.method, q.path, q.body) + if w.Code != 200 { + t.Fatalf("public metadata %s=%d", q.method, w.Code) + } + for _, secret := range []string{"fixture-key", "fixture-token", "fixture-flat-token", "fixture-schema-token"} { + if bytes.Contains(w.Body.Bytes(), []byte(secret)) { + t.Fatalf("%s %s leaked a fixture credential", q.method, q.path) + } + } + if !bytes.Contains(w.Body.Bytes(), []byte(`"rdm.projectId":"canonical"`)) || !bytes.Contains(w.Body.Bytes(), []byte(`"rdm.state":"ready"`)) { + t.Fatal("service metadata lost") + } + } + owned, err := read() + if err != nil { + t.Fatal(err) + } + if owned.Settings["models.chat.apiKey"] != "fixture-key" || owned.Settings["custom"].(map[string]any)["token"] != "fixture-token" { + t.Fatal("public metadata masking changed stored credentials") + } +} + +func TestGenericProjectPatchRejectsManagedSettingsAtomically(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", "Original") + s.set.SetExtraSchema([]core.SettingDesc{{Key: "plugins.private.managed", Scope: core.SettingScopeSystem}}) + 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 + }, Rename: func(_ context.Context, _ extension.Host, id, name string) (core.Project, error) { + if err := st.RenameProject("alice", id, name); err != nil { + return core.Project{}, err + } + return st.GetProject("alice", id) + }}, nil) + } + for _, body := range []string{`{"name":"Changed","settings":{"models":{"chat":{"apiKey":"fixture-key"}},"display":{"caption":"Forbidden partial write"}}}`, `{"settings":{"models.chat.apiKey":"fixture-key"}}`, `{"settings":{"models":"smash"}}`, `{"settings":{"plugins.private.managed":"override"}}`} { + w := lifecycleRequest(t, s, key, "PATCH", "/api/projects/"+p.ID, body) + if w.Code != 400 { + t.Fatalf("managed patch=%d", w.Code) + } + current, err := st.GetProject("alice", p.ID) + if err != nil { + t.Fatal(err) + } + if current.Name != "Original" || len(current.Settings) != 0 { + t.Fatal("rejected managed patch applied a partial mutation") + } + } + w := lifecycleRequest(t, s, key, "PATCH", "/api/projects/"+p.ID, `{"settings":{"custom.caption":"Allowed"}}`) + if w.Code != 200 { + t.Fatalf("allowed patch=%d", w.Code) + } + current, err := st.GetProject("alice", p.ID) + if err != nil || current.Settings["custom.caption"] != "Allowed" { + t.Fatal("allowed metadata shape changed") + } + }) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index c36f113..f932783 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -627,7 +627,7 @@ func (s *Server) listProjects(w http.ResponseWriter, r *http.Request) { } out := make([]projectView, 0, len(ps)) for _, p := range ps { - out = append(out, projectView{Project: p, Shared: p.Owner != owner}) + out = append(out, projectView{Project: s.publicProjectMetadata(p), Shared: p.Owner != owner}) } writeJSON(w, http.StatusOK, out) return @@ -674,7 +674,7 @@ func (s *Server) createProject(w http.ResponseWriter, r *http.Request) { return } p, err := s.projectLifecycle.Create(r.Context(), s.lifecycleHost(s.owner(r)), strings.TrimSpace(req.Name)) - writeProjectJSON(w, p, err) + writeProjectJSON(w, s.publicProjectMetadata(p), err) return } p, err := s.st.CreateProject(s.owner(r), strings.TrimSpace(req.Name)) @@ -691,6 +691,14 @@ func (s *Server) patchProject(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusBadRequest) return } + if req.Settings != nil { + values, err := s.set.PrepareProjectMetadataWrite(req.Settings) + if err != nil { + s.writeSettingsMerge(w, func() error { return err }) + return + } + req.Settings = values + } if s.projectLifecycle != nil { var p core.Project var err error @@ -732,7 +740,7 @@ func (s *Server) patchProject(w http.ResponseWriter, r *http.Request) { } } } - writeProjectJSON(w, p, err) + writeProjectJSON(w, s.publicProjectMetadata(p), err) return } if req.Name != nil { diff --git a/internal/settings/project_metadata.go b/internal/settings/project_metadata.go new file mode 100644 index 0000000..56359ab --- /dev/null +++ b/internal/settings/project_metadata.go @@ -0,0 +1,86 @@ +package settings + +import ( + core "agent-system-core" + "strings" +) + +// MaskedProjectMetadata returns a detached credential-safe view without changing +// stored values or dotted service/configuration key shapes. The builtin and +// plugin secret schema plus the existing secret-leaf fallback cover legacy data. +func (s *Service) MaskedProjectMetadata(values map[string]any) map[string]any { + if values == nil { + return map[string]any{} + } + schema := Schema() + if s != nil { + schema = append(schema, s.extraSchema...) + } + secrets := map[string]bool{} + for _, d := range schema { + if d.Type == core.SettingSecret || d.Secret { + secrets[d.Key] = true + } + } + return maskedProjectValue(values, "", secrets).(map[string]any) +} + +func maskedProjectValue(value any, path string, secrets map[string]bool) any { + leaf := path + if i := strings.LastIndexByte(leaf, '.'); i >= 0 { + leaf = leaf[i+1:] + } + if secrets[path] || isSecretKey(leaf) { + if value == nil { + return nil + } + if text, ok := value.(string); ok && text == "" { + return text + } + return MaskPlaceholder + } + switch v := value.(type) { + case map[string]any: + if v == nil { + return v + } + out := make(map[string]any, len(v)) + for key, item := range v { + child := key + if path != "" { + child = path + "." + key + } + out[key] = maskedProjectValue(item, child, secrets) + } + return out + case []any: + if v == nil { + return v + } + out := make([]any, len(v)) + for i, item := range v { + out[i] = maskedProjectValue(item, path, secrets) + } + return out + default: + return value + } +} + +// PrepareProjectMetadataWrite applies the same managed-path and mask-placeholder +// 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) + if err := s.checkManagedWrite(normalizeKeys(incoming)); err != nil { + return nil, err + } + return incoming, nil +} + +func (s *Service) checkManagedWrite(incoming map[string]any) error { + if hits := core.TouchesManagedSettings(incoming, s.ManagedSettingPaths()); len(hits) > 0 { + return &ErrManagedPath{Paths: hits} + } + return nil +} diff --git a/internal/settings/project_metadata_test.go b/internal/settings/project_metadata_test.go new file mode 100644 index 0000000..12fa9f3 --- /dev/null +++ b/internal/settings/project_metadata_test.go @@ -0,0 +1,53 @@ +package settings + +import ( + core "agent-system-core" + "bytes" + "encoding/json" + "testing" +) + +func TestMaskedProjectMetadataCopiesSecretViews(t *testing.T) { + s := NewService(nil) + s.SetExtraSchema([]core.SettingDesc{{Key: "plugins.private.credential", Secret: true}}) + raw := map[string]any{"models.chat.apiKey": "fixture-known", "legacy.token": "fixture-flat", "plugins.private": map[string]any{"credential": "fixture-schema"}, "nested": []any{map[string]any{"api_key": "fixture-array"}}, "opaque.secret": map[string]any{"value": "fixture-opaque"}, "custom.token": []any{"fixture-list"}, "custom.caption": "Allowed", "rdm.projectId": "canonical", "rdm.state": "ready"} + before, _ := json.Marshal(raw) + view := s.MaskedProjectMetadata(raw) + after, _ := json.Marshal(raw) + if string(before) != string(after) { + t.Fatal("read-only metadata view mutated its source") + } + encoded, _ := json.Marshal(view) + for _, secret := range []string{"fixture-known", "fixture-flat", "fixture-schema", "fixture-array", "fixture-opaque", "fixture-list"} { + if bytes.Contains(encoded, []byte(secret)) { + t.Fatal("metadata view exposed a fixture secret") + } + } + if view["custom.caption"] != "Allowed" || view["rdm.projectId"] != "canonical" || view["rdm.state"] != "ready" { + t.Fatal("nonsecret metadata shape/value changed") + } + if view["legacy.token"] != MaskPlaceholder || view["opaque.secret"] != MaskPlaceholder || view["custom.token"] != MaskPlaceholder { + t.Fatal("secret fallback did not mask complete dotted values") + } +} + +func TestPrepareProjectMetadataWriteUsesManagedGuard(t *testing.T) { + s := NewService(nil) + s.SetExtraSchema([]core.SettingDesc{{Key: "plugins.private.managed", Scope: core.SettingScopeSystem}}) + for _, raw := range []map[string]any{{"models.chat.apiKey": "fixture"}, {"models": map[string]any{"chat": map[string]any{"apiKey": "fixture"}}}, {"models": "smash"}, {"plugins.private.managed": "override"}} { + if _, err := s.PrepareProjectMetadataWrite(raw); err == nil { + t.Fatal("managed metadata write accepted") + } + } + raw := map[string]any{"custom.caption": "Allowed", "custom.token": MaskPlaceholder, "models.chat.apiKey": MaskPlaceholder} + view, err := s.PrepareProjectMetadataWrite(raw) + if err != nil { + t.Fatal(err) + } + if len(view) != 1 || view["custom.caption"] != "Allowed" { + t.Fatal("metadata writes must preserve allowed dotted keys and ignore mask placeholders") + } + if raw["custom.token"] != MaskPlaceholder { + t.Fatal("write preparation mutated its input") + } +} diff --git a/internal/settings/settings.go b/internal/settings/settings.go index 05dfd0a..c07074c 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -167,7 +167,7 @@ func (s *Service) SetExtraSchema(descs []core.SettingDesc) { s.extraSchema = des // ManagedSettingPaths 返回全部受管键路径（内置 schema + 插件声明）。 func (s *Service) ManagedSettingPaths() []string { paths := core.ManagedPaths(Schema()) - if len(s.extraSchema) > 0 { + if s != nil && len(s.extraSchema) > 0 { paths = append(paths, core.ManagedPaths(s.extraSchema)...) } return paths @@ -238,8 +238,8 @@ func (s *Service) MergeSystem(values map[string]any) error { func (s *Service) mergeScope(owner, scope string, values map[string]any) error { incoming := prepareWrite(values) if scope != ScopeSystem { - if hits := core.TouchesManagedSettings(incoming, s.ManagedSettingPaths()); len(hits) > 0 { - return &ErrManagedPath{Paths: hits} + if err := s.checkManagedWrite(incoming); err != nil { + return err } } if err := s.st.UpdateSettingsScope(owner, scope, func(cur map[string]any) (map[string]any, error) { diff --git a/pkg/extension/projects.go b/pkg/extension/projects.go index 259e4dd..d0f5042 100644 --- a/pkg/extension/projects.go +++ b/pkg/extension/projects.go @@ -6,7 +6,8 @@ import ( ) // ProjectHost is an optional actor-scoped host projection capability. Metadata -// can be shared by an authorizer; owned lookups and mutations remain owner-only. +// can be shared by an authorizer; registered-lifecycle metadata masks credentials. +// OwnedProject remains an owner-only management lookup; mutations remain owner-only. type ProjectHost interface { Owner() string OwnedProjects() ([]core.Project, error)