diff --git a/agentteams-controller/api/v1beta1/types.go b/agentteams-controller/api/v1beta1/types.go index 4f4313848..8eb4b6850 100644 --- a/agentteams-controller/api/v1beta1/types.go +++ b/agentteams-controller/api/v1beta1/types.go @@ -43,6 +43,10 @@ const LabelWorkerEdgeUUID = "agentteams.io/worker-edge-uuid" // re-issues credentials and updates the annotation to match. const AnnotationEdgeAppliedUUID = "agentteams.io/edge-applied-uuid" +// AnnotationHumanManagedGroupAllowExtra records Human Matrix IDs added to a +// Worker's groupAllowExtra by the Human controller. +const AnnotationHumanManagedGroupAllowExtra = "agentteams.io/human-group-allow-extra" + // AccessEntry declares one cloud-permission grant under a logical // service. v1 supported services: "object-storage", "ai-gateway", "ai-registry", "schedulerx3". // diff --git a/agentteams-controller/internal/controller/human_controller.go b/agentteams-controller/internal/controller/human_controller.go index e708ca7f0..f9ca27307 100644 --- a/agentteams-controller/internal/controller/human_controller.go +++ b/agentteams-controller/internal/controller/human_controller.go @@ -121,6 +121,9 @@ func (r *HumanReconciler) reconcileHumanNormal(ctx context.Context, s *humanScop return reconcile.Result{RequeueAfter: reconcileInterval}, err } r.reconcileHumanRooms(ctx, s) + if err := r.reconcileHumanWorkerAllowlists(ctx, s); err != nil { + return reconcile.Result{RequeueAfter: reconcileInterval}, err + } return reconcile.Result{RequeueAfter: reconcileInterval}, nil } diff --git a/agentteams-controller/internal/controller/human_controller_test.go b/agentteams-controller/internal/controller/human_controller_test.go index b85f3020d..29f73b9b0 100644 --- a/agentteams-controller/internal/controller/human_controller_test.go +++ b/agentteams-controller/internal/controller/human_controller_test.go @@ -95,6 +95,170 @@ func newReadyTeam(name, roomID string) *v1beta1.Team { return tm } +func TestHumanReconciler_SyncsAccessibleWorkerAllowlist(t *testing.T) { + worker := newReadyWorker("w1", "!room-w1:localhost") + human := newHuman("alice", v1beta1.HumanSpec{ + AccessibleWorkers: []string{"w1"}, + }) + human.Status.MatrixUserID = "@alice:localhost" + human.Status.InitialPassword = "stored-pw" + human.Status.Rooms = []string{"!room-w1:localhost"} + human.Status.Phase = "Active" + human.Finalizers = []string{finalizerName} + + rig := newHumanRig(t, human, worker) + if _, _, err := rig.reconcile("alice"); err != nil { + t.Fatalf("reconcile: %v", err) + } + + var out v1beta1.Worker + if err := rig.client.Get(context.Background(), + types.NamespacedName{Name: "w1", Namespace: "default"}, &out); err != nil { + t.Fatalf("get worker: %v", err) + } + if out.Spec.ChannelPolicy == nil || + len(out.Spec.ChannelPolicy.GroupAllowExtra) != 1 || + out.Spec.ChannelPolicy.GroupAllowExtra[0] != "@alice:localhost" { + t.Fatalf("groupAllowExtra=%v, want [@alice:localhost]", out.Spec.ChannelPolicy) + } + if got := out.Annotations["agentteams.io/human-group-allow-extra"]; got != `{"alice":"@alice:localhost"}` { + t.Fatalf("managed allowlist annotation=%q, want alice ownership", got) + } +} + +func TestHumanReconciler_PreservesManualWorkerAllowlistEntry(t *testing.T) { + worker := newReadyWorker("w1", "!room-w1:localhost") + worker.Spec.ChannelPolicy = &v1beta1.ChannelPolicySpec{ + GroupAllowExtra: []string{"@alice:localhost"}, + } + human := activeHumanWithWorker("alice", "w1", "@alice:localhost") + rig := newHumanRig(t, human, worker) + + if _, _, err := rig.reconcile("alice"); err != nil { + t.Fatalf("initial reconcile: %v", err) + } + updateHumanWorkers(t, rig, "alice", nil) + if _, _, err := rig.reconcile("alice"); err != nil { + t.Fatalf("revoke reconcile: %v", err) + } + + out := getHumanTestWorker(t, rig, "w1") + if got := out.Spec.ChannelPolicy.GroupAllowExtra; len(got) != 1 || got[0] != "@alice:localhost" { + t.Fatalf("groupAllowExtra=%v, want manual entry preserved", got) + } + if got := out.Annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra]; got != "" { + t.Fatalf("manual entry must not be controller-owned, annotation=%q", got) + } +} + +func TestHumanReconciler_MultipleHumansShareWorkerAndRevokeOne(t *testing.T) { + worker := newReadyWorker("w1", "!room-w1:localhost") + alice := activeHumanWithWorker("alice", "w1", "@alice:localhost") + bob := activeHumanWithWorker("bob", "w1", "@bob:localhost") + rig := newHumanRig(t, alice, bob, worker) + + if _, _, err := rig.reconcile("alice"); err != nil { + t.Fatalf("reconcile alice: %v", err) + } + if _, _, err := rig.reconcile("bob"); err != nil { + t.Fatalf("reconcile bob: %v", err) + } + updateHumanWorkers(t, rig, "alice", nil) + if _, _, err := rig.reconcile("alice"); err != nil { + t.Fatalf("revoke alice: %v", err) + } + + out := getHumanTestWorker(t, rig, "w1") + if got := out.Spec.ChannelPolicy.GroupAllowExtra; len(got) != 1 || got[0] != "@bob:localhost" { + t.Fatalf("groupAllowExtra=%v, want only bob", got) + } + if got := out.Annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra]; got != `{"bob":"@bob:localhost"}` { + t.Fatalf("managed allowlist annotation=%q, want only bob ownership", got) + } +} + +func TestHumanReconciler_DeleteRemovesOnlyManagedWorkerAllowlistEntry(t *testing.T) { + now := metav1.Now() + human := activeHumanWithWorker("alice", "w1", "@alice:localhost") + human.DeletionTimestamp = &now + worker := newReadyWorker("w1", "!room-w1:localhost") + worker.Annotations = map[string]string{ + v1beta1.AnnotationHumanManagedGroupAllowExtra: `{"alice":"@alice:localhost"}`, + } + worker.Spec.ChannelPolicy = &v1beta1.ChannelPolicySpec{ + GroupAllowExtra: []string{"@alice:localhost", "@manual:localhost"}, + } + rig := newHumanRig(t, human, worker) + + if _, _, err := rig.reconcile("alice"); err != nil { + t.Fatalf("delete reconcile: %v", err) + } + + out := getHumanTestWorker(t, rig, "w1") + if got := out.Spec.ChannelPolicy.GroupAllowExtra; len(got) != 1 || got[0] != "@manual:localhost" { + t.Fatalf("groupAllowExtra=%v, want only manual entry", got) + } + if got := out.Annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra]; got != "" { + t.Fatalf("managed allowlist annotation=%q, want removed", got) + } +} + +func TestHumanReconciler_WorkerAllowlistIsIdempotent(t *testing.T) { + worker := newReadyWorker("w1", "!room-w1:localhost") + human := activeHumanWithWorker("alice", "w1", "@alice:localhost") + rig := newHumanRig(t, human, worker) + + if _, _, err := rig.reconcile("alice"); err != nil { + t.Fatalf("first reconcile: %v", err) + } + first := getHumanTestWorker(t, rig, "w1") + if _, _, err := rig.reconcile("alice"); err != nil { + t.Fatalf("second reconcile: %v", err) + } + second := getHumanTestWorker(t, rig, "w1") + + if second.ResourceVersion != first.ResourceVersion { + t.Fatalf("Worker resourceVersion changed on idempotent reconcile: %q -> %q", + first.ResourceVersion, second.ResourceVersion) + } + if got := second.Spec.ChannelPolicy.GroupAllowExtra; len(got) != 1 || got[0] != "@alice:localhost" { + t.Fatalf("groupAllowExtra=%v, want one alice entry", got) + } +} + +func activeHumanWithWorker(name, workerName, matrixUserID string) *v1beta1.Human { + human := newHuman(name, v1beta1.HumanSpec{AccessibleWorkers: []string{workerName}}) + human.Status.MatrixUserID = matrixUserID + human.Status.InitialPassword = "stored-pw" + human.Status.Rooms = []string{"!room-" + workerName + ":localhost"} + human.Status.Phase = "Active" + human.Finalizers = []string{finalizerName} + return human +} + +func updateHumanWorkers(t *testing.T, rig *humanTestRig, name string, workers []string) { + t.Helper() + var human v1beta1.Human + key := types.NamespacedName{Name: name, Namespace: "default"} + if err := rig.client.Get(context.Background(), key, &human); err != nil { + t.Fatalf("get Human %s: %v", name, err) + } + human.Spec.AccessibleWorkers = workers + if err := rig.client.Update(context.Background(), &human); err != nil { + t.Fatalf("update Human %s: %v", name, err) + } +} + +func getHumanTestWorker(t *testing.T, rig *humanTestRig, name string) *v1beta1.Worker { + t.Helper() + var worker v1beta1.Worker + if err := rig.client.Get(context.Background(), + types.NamespacedName{Name: name, Namespace: "default"}, &worker); err != nil { + t.Fatalf("get Worker %s: %v", name, err) + } + return &worker +} + // TestHumanReconciler_Create_HappyPath covers the first-time provisioning // path: a brand-new Human CR, one ready Worker, one ready Team. Asserts // that a **single** reconcile converges — EnsureHumanUser is called diff --git a/agentteams-controller/internal/controller/human_reconcile_allowlist.go b/agentteams-controller/internal/controller/human_reconcile_allowlist.go new file mode 100644 index 000000000..11efc77d0 --- /dev/null +++ b/agentteams-controller/internal/controller/human_reconcile_allowlist.go @@ -0,0 +1,138 @@ +package controller + +import ( + "context" + "encoding/json" + "fmt" + + v1beta1 "github.com/agentscope-ai/AgentTeams/agentteams-controller/api/v1beta1" + kerrors "k8s.io/apimachinery/pkg/util/errors" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func (r *HumanReconciler) reconcileHumanWorkerAllowlists(ctx context.Context, s *humanScope) error { + desired := make(map[string]struct{}, len(s.human.Spec.AccessibleWorkers)) + for _, workerName := range s.human.Spec.AccessibleWorkers { + desired[workerName] = struct{}{} + } + return r.syncHumanWorkerAllowlists(ctx, s.human, s.identity.MatrixUserID, desired) +} + +func (r *HumanReconciler) cleanupHumanWorkerAllowlists(ctx context.Context, s *humanScope) error { + userID := s.human.Status.MatrixUserID + if userID == "" { + userID = s.identity.MatrixUserID + } + return r.syncHumanWorkerAllowlists(ctx, s.human, userID, nil) +} + +func (r *HumanReconciler) syncHumanWorkerAllowlists( + ctx context.Context, + human *v1beta1.Human, + userID string, + desired map[string]struct{}, +) error { + var workers v1beta1.WorkerList + if err := r.List(ctx, &workers, client.InNamespace(human.Namespace)); err != nil { + return fmt.Errorf("list workers for human allowlist: %w", err) + } + + var errs []error + for i := range workers.Items { + worker := &workers.Items[i] + _, shouldAllow := desired[worker.Name] + if err := r.syncHumanWorkerAllowlist(ctx, worker, human.Name, userID, shouldAllow); err != nil { + errs = append(errs, err) + } + } + return kerrors.NewAggregate(errs) +} + +func (r *HumanReconciler) syncHumanWorkerAllowlist( + ctx context.Context, + worker *v1beta1.Worker, + humanName string, + userID string, + shouldAllow bool, +) error { + managed, err := managedHumanAllowlist(worker.Annotations) + if err != nil { + return fmt.Errorf("parse managed Human allowlist for Worker %s: %w", worker.Name, err) + } + + managedID, isManaged := managed[humanName] + if !shouldAllow && !isManaged { + return nil + } + + base := worker.DeepCopy() + changed := false + if shouldAllow { + if worker.Spec.ChannelPolicy == nil { + worker.Spec.ChannelPolicy = &v1beta1.ChannelPolicySpec{} + } + allowlist := worker.Spec.ChannelPolicy.GroupAllowExtra + if !containsString(allowlist, userID) { + if isManaged && managedID != userID { + worker.Spec.ChannelPolicy.GroupAllowExtra = removeString(allowlist, managedID) + } + worker.Spec.ChannelPolicy.GroupAllowExtra = append(worker.Spec.ChannelPolicy.GroupAllowExtra, userID) + managed[humanName] = userID + changed = true + } else if isManaged && managedID != userID { + worker.Spec.ChannelPolicy.GroupAllowExtra = removeString(allowlist, managedID) + delete(managed, humanName) + changed = true + } + } else { + if worker.Spec.ChannelPolicy != nil { + worker.Spec.ChannelPolicy.GroupAllowExtra = removeString( + worker.Spec.ChannelPolicy.GroupAllowExtra, + managedID, + ) + } + delete(managed, humanName) + changed = true + } + + if !changed { + return nil + } + if err := setManagedHumanAllowlist(worker, managed); err != nil { + return err + } + if err := r.Patch(ctx, worker, client.MergeFrom(base)); err != nil { + return fmt.Errorf("patch Worker %s Human allowlist: %w", worker.Name, err) + } + return nil +} + +func managedHumanAllowlist(annotations map[string]string) (map[string]string, error) { + managed := map[string]string{} + if annotations == nil || annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra] == "" { + return managed, nil + } + if err := json.Unmarshal( + []byte(annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra]), + &managed, + ); err != nil { + return nil, err + } + return managed, nil +} + +func setManagedHumanAllowlist(worker *v1beta1.Worker, managed map[string]string) error { + if worker.Annotations == nil { + worker.Annotations = map[string]string{} + } + if len(managed) == 0 { + delete(worker.Annotations, v1beta1.AnnotationHumanManagedGroupAllowExtra) + return nil + } + value, err := json.Marshal(managed) + if err != nil { + return fmt.Errorf("marshal managed Human allowlist: %w", err) + } + worker.Annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra] = string(value) + return nil +} diff --git a/agentteams-controller/internal/controller/human_reconcile_delete.go b/agentteams-controller/internal/controller/human_reconcile_delete.go index d546e8045..04d42f6c3 100644 --- a/agentteams-controller/internal/controller/human_reconcile_delete.go +++ b/agentteams-controller/internal/controller/human_reconcile_delete.go @@ -15,10 +15,10 @@ import ( // human to /leave (password may be stale), so we rely on the Tuwunel // admin bot's force-leave-room command instead. // -// Every external call here is non-fatal: a transient Matrix or OSS -// failure must not wedge finalizer removal, and the homeserver's -// delete_rooms_after_leave / forget_forced_upon_leave flags provide a -// safety net if any force-leave never lands. +// Matrix room cleanup is non-fatal: the homeserver's delete_rooms_after_leave / +// forget_forced_upon_leave flags provide a safety net if a force-leave never +// lands. Worker allowlist cleanup must succeed before finalizer removal, +// otherwise the deleted Human could retain access indefinitely. func (r *HumanReconciler) reconcileHumanDelete(ctx context.Context, s *humanScope) (reconcile.Result, error) { logger := log.FromContext(ctx) h := s.human @@ -35,6 +35,10 @@ func (r *HumanReconciler) reconcileHumanDelete(ctx context.Context, s *humanScop } } + if err := r.cleanupHumanWorkerAllowlists(ctx, s); err != nil { + return reconcile.Result{RequeueAfter: reconcileInterval}, err + } + if s.identity.Source != nil { if err := s.identity.Source.EnsureDeactivated(ctx, &h.Spec, &h.Status); err != nil { return reconcile.Result{RequeueAfter: reconcileInterval}, err diff --git a/changelog/current.md b/changelog/current.md index 6436a6d7d..cceafecc1 100644 --- a/changelog/current.md +++ b/changelog/current.md @@ -6,6 +6,7 @@ Record image-affecting changes to `manager/`, `worker/`, `copaw/`, `hermes/`, `o **Bug Fixes** +- **Human Worker access allowlist**: Synchronize each Human's accessible Workers with runtime group allowlists while preserving manually managed entries. - **Team Worker room boundary convergence**: Remove Manager again after standalone Worker infrastructure reconciliation restores regular Team Worker personal-room membership. ([b5b0add](https://github.com/agentscope-ai/AgentTeams/commit/b5b0add)) - **Team Worker reference enforcement**: Keep referenced Worker CRs protected during direct deletion and reject Team API members whose required role is empty. ([d96f1ed](https://github.com/agentscope-ai/AgentTeams/commit/d96f1ed)) - **Team Worker room membership**: Force Manager out of regular Team Worker personal rooms when equal Matrix power levels prevent a normal kick. ([43545c2](https://github.com/agentscope-ai/AgentTeams/commit/43545c2))