coder
diff --git a/‎cli/sharing.go‎
Lines changed: 8 additions & 7 deletions b/‎cli/sharing.go‎
Lines changed: 8 additions & 7 deletions
diff --git a/‎cli/sharing_test.go‎
Lines changed: 42 additions & 0 deletions b/‎cli/sharing_test.go‎
Lines changed: 42 additions & 0 deletions
diff --git a/‎coderd/workspaces.go‎
Lines changed: 32 additions & 26 deletions b/‎coderd/workspaces.go‎
Lines changed: 32 additions & 26 deletions
diff --git a/‎enterprise/cli/sharing_test.go‎
Lines changed: 5 additions & 2 deletions b/‎enterprise/cli/sharing_test.go‎
Lines changed: 5 additions & 2 deletions
diff --git a/‎enterprise/coderd/workspaces_test.go‎
Lines changed: 63 additions & 0 deletions b/‎enterprise/coderd/workspaces_test.go‎
Lines changed: 63 additions & 0 deletions
@@ -312,13 +312,14 @@ func workspaceACLToTable(ctx context.Context, acl *codersdk.WorkspaceACL) (strin
 			continue
 		}
 
-		for _, user := range group.Members {
-			outputRows = append(outputRows, workspaceShareRow{
-				User:  user.Username,
-				Group: group.Name,
-				Role:  group.Role,
-			})
-		}
+		// The ACL endpoint intentionally omits the group's member roster to
+		// avoid leaking member PII, so we display one row per group rather
+		// than one row per member.
+		outputRows = append(outputRows, workspaceShareRow{
+			User:  defaultGroupDisplay,
+			Group: group.Name,
+			Role:  group.Role,
+		})
 	}
 	out, err := formatter.Format(ctx, outputRows)
 	if err != nil {
 
@@ -205,6 +205,48 @@ func TestSharingStatus(t *testing.T) {
 		}
 		assert.True(t, found, "expected to find username %s with role %s in the output: %s", toShareWithUser.Username, codersdk.WorkspaceRoleUse, out.String())
 	})
+
+	t.Run("ListSharedGroups", func(t *testing.T) {
+		t.Parallel()
+
+		var (
+			client, db                           = coderdtest.NewWithDatabase(t, nil)
+			orgOwner                             = coderdtest.CreateFirstUser(t, client)
+			workspaceOwnerClient, workspaceOwner = coderdtest.CreateAnotherUser(t, client, orgOwner.OrganizationID, rbac.ScopedRoleOrgAuditor(orgOwner.OrganizationID))
+			workspace                            = dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
+				OwnerID:        workspaceOwner.ID,
+				OrganizationID: orgOwner.OrganizationID,
+			}).Do().Workspace
+			ctx = testutil.Context(t, testutil.WaitMedium)
+		)
+
+		// The Everyone group always exists for an organization and shares the
+		// organization's ID. The workspace ACL endpoint no longer returns the
+		// group's member roster, so the CLI must still list the group itself.
+		err := client.UpdateWorkspaceACL(ctx, workspace.ID, codersdk.UpdateWorkspaceACL{
+			GroupRoles: map[string]codersdk.WorkspaceRole{
+				orgOwner.OrganizationID.String(): codersdk.WorkspaceRoleUse,
+			},
+		})
+		require.NoError(t, err)
+
+		inv, root := clitest.New(t, "sharing", "status", workspace.Name)
+		clitest.SetupConfig(t, workspaceOwnerClient, root)
+
+		out := new(bytes.Buffer)
+		inv.Stdout = out
+		err = inv.WithContext(ctx).Run()
+		require.NoError(t, err)
+
+		found := false
+		for _, line := range strings.Split(out.String(), "\n") {
+			if strings.Contains(line, database.EveryoneGroup) && strings.Contains(line, string(codersdk.WorkspaceRoleUse)) {
+				found = true
+				break
+			}
+		}
+		assert.True(t, found, "expected to find group %s with role %s in the output: %s", database.EveryoneGroup, codersdk.WorkspaceRoleUse, out.String())
+	})
 }
 
 func TestSharingRemove(t *testing.T) {
 
@@ -2372,13 +2372,13 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// This is largely based on the template ACL implementation, and is far from
-	// ideal. Usually, when we use the System context it's because we need to
-	// run some query that won't actually be exposed to the user. That is not
-	// the case here. This data goes directly to an unauthorized user. We are
-	// just straight up breaking security promises.
-	//
-	// TODO: This needs to be fixed before GA. Currently in beta.
+	// Callers are authorized to read this workspace, not necessarily the
+	// users and groups on its ACL. We deliberately use the System context to
+	// look up that data, but only return minimal identity information that is
+	// safe to expose to anyone who can read the ACL: MinimalUser for ACL users
+	// (no email or other PII) and group identity plus a member count for ACL
+	// groups (no member roster). This mirrors the chat ACL and template
+	// available-ACL endpoints.
 
 	// Fetch all of the users and their organization memberships
 	userIDs := make([]uuid.UUID, 0, len(workspaceACL.Users))
@@ -2390,7 +2390,8 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
 		}
 		userIDs = append(userIDs, id)
 	}
-	// For context see https://github.com/coder/coder/pull/19375
+	// ACL users are returned as MinimalUser, which contains no PII, so it is
+	// safe to fetch them under the System context.
 	// nolint:gocritic
 	dbUsers, err := api.Database.GetUsersByIDs(dbauthz.AsSystemRestricted(ctx), userIDs)
 	if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
@@ -2422,7 +2423,8 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
 	// before making the DB call.
 	dbGroups := make([]database.GetGroupsRow, 0)
 	if len(groupIDs) > 0 {
-		// For context see https://github.com/coder/coder/pull/19375
+		// Group identity must be visible to anyone who can read the ACL so
+		// that owners and shared users can see and manage entries.
 		// nolint:gocritic
 		dbGroups, err = api.Database.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{GroupIds: groupIDs})
 		if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
@@ -2431,26 +2433,30 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// Fetch member counts for all groups in a single query to avoid an N+1
+	// lookup. We intentionally do not populate the per-group member rosters:
+	// callers authorized to read the ACL are not necessarily authorized to
+	// read group membership, and the roster includes member PII. Only the
+	// total member count is returned (see Group.TotalMemberCount).
+	// nolint:gocritic
+	countRows, err := api.Database.GetGroupMembersCountByGroupIDs(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersCountByGroupIDsParams{
+		GroupIds:      groupIDs,
+		IncludeSystem: false,
+	})
+	if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
+		httpapi.InternalServerError(rw, err)
+		return
+	}
+	countByGroup := make(map[uuid.UUID]int64, len(countRows))
+	for _, row := range countRows {
+		countByGroup[row.GroupID] = row.MemberCount
+	}
+
 	groups := make([]codersdk.WorkspaceGroup, 0, len(dbGroups))
 	for _, it := range dbGroups {
-		var members []database.GroupMember
-		// For context see https://github.com/coder/coder/pull/19375
-		// nolint:gocritic
-		members, err = api.Database.GetGroupMembersByGroupID(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersByGroupIDParams{
-			GroupID:       it.Group.ID,
-			IncludeSystem: false,
-		})
-		if err != nil {
-			httpapi.InternalServerError(rw, err)
-			return
-		}
 		groups = append(groups, codersdk.WorkspaceGroup{
-			Group: db2sdk.Group(database.GetGroupsRow{
-				Group:                   it.Group,
-				OrganizationName:        it.OrganizationName,
-				OrganizationDisplayName: it.OrganizationDisplayName,
-			}, members, len(members)),
-			Role: convertToWorkspaceRole(workspaceACL.Groups[it.Group.ID.String()].Permissions),
+			Group: db2sdk.Group(it, nil, int(countByGroup[it.Group.ID])),
+			Role:  convertToWorkspaceRole(workspaceACL.Groups[it.Group.ID.String()].Permissions),
 		})
 	}
 
 
@@ -216,14 +216,17 @@ func TestSharingStatus(t *testing.T) {
 		err = inv.WithContext(ctx).Run()
 		require.NoError(t, err)
 
+		// The ACL endpoint omits group member rosters to avoid leaking member
+		// PII, so the output lists the group itself rather than its members.
 		found := false
 		for _, line := range strings.Split(out.String(), "\n") {
-			if strings.Contains(line, orgMember.Username) && strings.Contains(line, string(codersdk.WorkspaceRoleUse)) && strings.Contains(line, group.Name) {
+			if strings.Contains(line, group.Name) && strings.Contains(line, string(codersdk.WorkspaceRoleUse)) {
 				found = true
 				break
 			}
 		}
-		assert.True(t, found, "expected to find username %s with role %s in the output: %s", orgMember.Username, codersdk.WorkspaceRoleUse, out.String())
+		assert.True(t, found, "expected to find group %s with role %s in the output: %s", group.Name, codersdk.WorkspaceRoleUse, out.String())
+		assert.NotContains(t, out.String(), orgMember.Username, "group member roster must not be exposed in sharing status output")
 	})
 }
 
 
@@ -4477,6 +4477,69 @@ func TestUpdateWorkspaceACL(t *testing.T) {
 		require.Equal(t, workspaceACL.Groups[0].Role, codersdk.WorkspaceRoleAdmin)
 	})
 
+	// A user who has merely been shared a workspace must not be able to
+	// enumerate the full roster and PII of groups on that workspace's ACL.
+	// The endpoint returns the group identity and total member count only.
+	t.Run("GroupMembersNotReturned", func(t *testing.T) {
+		t.Parallel()
+
+		dv := coderdtest.DeploymentValues(t)
+
+		adminClient, adminUser := coderdenttest.New(t, &coderdenttest.Options{
+			Options: &coderdtest.Options{
+				IncludeProvisionerDaemon: true,
+				DeploymentValues:         dv,
+			},
+			LicenseOptions: &coderdenttest.LicenseOptions{
+				Features: license.Features{
+					codersdk.FeatureTemplateRBAC: 1,
+				},
+			},
+		})
+		orgID := adminUser.OrganizationID
+		client, _ := coderdtest.CreateAnotherUser(t, adminClient, orgID)
+		sharedClient, sharedUser := coderdtest.CreateAnotherUser(t, adminClient, orgID)
+		_, member := coderdtest.CreateAnotherUser(t, adminClient, orgID)
+		group := coderdtest.CreateGroup(t, adminClient, orgID, "bloob", member)
+
+		tv := coderdtest.CreateTemplateVersion(t, adminClient, orgID, nil)
+		coderdtest.AwaitTemplateVersionJobCompleted(t, adminClient, tv.ID)
+		template := coderdtest.CreateTemplate(t, adminClient, orgID, tv.ID)
+
+		ws := coderdtest.CreateWorkspace(t, client, template.ID)
+		coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
+
+		ctx := testutil.Context(t, testutil.WaitMedium)
+		err := client.UpdateWorkspaceACL(ctx, ws.ID, codersdk.UpdateWorkspaceACL{
+			UserRoles: map[string]codersdk.WorkspaceRole{
+				sharedUser.ID.String(): codersdk.WorkspaceRoleUse,
+			},
+			GroupRoles: map[string]codersdk.WorkspaceRole{
+				group.ID.String(): codersdk.WorkspaceRoleUse,
+			},
+		})
+		require.NoError(t, err)
+
+		// The low-privilege shared user can read the ACL, but must not see
+		// the group's member roster (which would expose member emails and
+		// other PII). Only the total member count is returned.
+		workspaceACL, err := sharedClient.WorkspaceACL(ctx, ws.ID)
+		require.NoError(t, err)
+		require.Len(t, workspaceACL.Groups, 1)
+		require.Equal(t, group.ID, workspaceACL.Groups[0].ID)
+		require.Equal(t, codersdk.WorkspaceRoleUse, workspaceACL.Groups[0].Role)
+		require.Equal(t, 1, workspaceACL.Groups[0].TotalMemberCount)
+		require.Empty(t, workspaceACL.Groups[0].Members)
+
+		// The workspace owner sees the same count-only shape; the roster is
+		// omitted for all callers.
+		workspaceACL, err = client.WorkspaceACL(ctx, ws.ID)
+		require.NoError(t, err)
+		require.Len(t, workspaceACL.Groups, 1)
+		require.Equal(t, 1, workspaceACL.Groups[0].TotalMemberCount)
+		require.Empty(t, workspaceACL.Groups[0].Members)
+	})
+
 	t.Run("UnknownIDs", func(t *testing.T) {
 		t.Parallel()
Original file line number	Diff line number	Diff line change
`@@ -216,14 +216,17 @@ func TestSharingStatus(t *testing.T) {`
`216`	`216`	`err = inv.WithContext(ctx).Run()`
`217`	`217`	`require.NoError(t, err)`
`218`	`218`
	`219`	`+ // The ACL endpoint omits group member rosters to avoid leaking member`
	`220`	`+ // PII, so the output lists the group itself rather than its members.`
`219`	`221`	`found := false`
`220`	`222`	`for _, line := range strings.Split(out.String(), "\n") {`
`221`		`- if strings.Contains(line, orgMember.Username) && strings.Contains(line, string(codersdk.WorkspaceRoleUse)) && strings.Contains(line, group.Name) {`
	`223`	`+ if strings.Contains(line, group.Name) && strings.Contains(line, string(codersdk.WorkspaceRoleUse)) {`
`222`	`224`	`found = true`
`223`	`225`	`break`
`224`	`226`	`}`
`225`	`227`	`}`
`226`		`- assert.True(t, found, "expected to find username %s with role %s in the output: %s", orgMember.Username, codersdk.WorkspaceRoleUse, out.String())`
	`228`	`+ assert.True(t, found, "expected to find group %s with role %s in the output: %s", group.Name, codersdk.WorkspaceRoleUse, out.String())`
	`229`	`+ assert.NotContains(t, out.String(), orgMember.Username, "group member roster must not be exposed in sharing status output")`
`227`	`230`	`})`
`228`	`231`	`}`
`229`	`232`