fix(controlplane): restrict organization updates to admins of the target org (CP-N1) (#3321)

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>

Author: migmartri

app/controlplane/internal/service/organization.go

@@ -82,6 +82,19 @@ func (s *OrganizationService) Update(ctx context.Context, req *pb.OrganizationSe
 		return nil, err
 	}
 
+	currentOrg, err := requireCurrentOrg(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	// The authorization middleware evaluates the caller's role against the organization
+	// selected in the request headers, so the update has to target that same organization.
+	// Honoring an arbitrary name here would let an admin of one organization change the
+	// settings of another one they merely belong to.
+	if req.Name != currentOrg.Name {
+		return nil, errors.Forbidden("forbidden", "the organization to update must be the currently selected one")
+	}
+
 	// we want to differentiate between setting the value to empty or not setting it at all
 	// to do that we will use a nil slice to represent not setting it at all
 	var policiesAllowedHostnames []string
@@ -102,7 +115,7 @@ func (s *OrganizationService) Update(ctx context.Context, req *pb.OrganizationSe
 		apiTokenMaxDaysInactive = &days
 	}
 
-	org, err := s.orgUC.Update(ctx, currentUser.ID, req.Name, &biz.OrganizationUpdateOpts{
+	org, err := s.orgUC.Update(ctx, currentUser.ID, currentOrg.Name, &biz.OrganizationUpdateOpts{
 		BlockOnPolicyViolation:              req.BlockOnPolicyViolation,
 		PoliciesAllowedHostnames:            policiesAllowedHostnames,
 		PreventImplicitWorkflowCreation:     req.PreventImplicitWorkflowCreation,

app/controlplane/internal/service/organization_test.go

@@ -0,0 +1,72 @@
+//
+// Copyright 2026 The Chainloop Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package service
+
+import (
+	"context"
+	"testing"
+
+	pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
+	"github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/entities"
+	"github.com/go-kratos/kratos/v2/errors"
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestUpdateIsPinnedToCurrentOrg is a regression test for CP-N1. The authz
+// middleware evaluates the caller's role against the organization selected in
+// the request headers, so Update must refuse to operate on any other
+// organization. Otherwise an admin of org A could target org B by naming it in
+// the request body.
+func TestUpdateIsPinnedToCurrentOrg(t *testing.T) {
+	// A nil use case is deliberate: a request that reaches the biz layer means
+	// the guard did not run, and the test fails loudly instead of silently
+	// passing.
+	svc := NewOrganizationService(nil, nil)
+
+	ctxWithOrg := func(orgName string) context.Context {
+		ctx := entities.WithCurrentUser(context.Background(), &entities.User{ID: uuid.NewString(), Email: "user@test.com"})
+		return entities.WithCurrentOrg(ctx, &entities.Org{ID: uuid.NewString(), Name: orgName})
+	}
+
+	testCases := []struct {
+		name       string
+		currentOrg string
+		reqName    string
+	}{
+		{name: "different organization", currentOrg: "my-org", reqName: "victim-org"},
+		{name: "empty name", currentOrg: "my-org", reqName: ""},
+		{name: "case variation", currentOrg: "my-org", reqName: "My-Org"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := svc.Update(ctxWithOrg(tc.currentOrg), &pb.OrganizationServiceUpdateRequest{
+				Name:                   tc.reqName,
+				BlockOnPolicyViolation: toPtrBool(false),
+			})
+
+			require.Error(t, err)
+			assert.Nil(t, got)
+			assert.True(t, errors.IsForbidden(err), "want forbidden, got %v", err)
+		})
+	}
+}
+
+func toPtrBool(b bool) *bool {
+	return &b
+}

app/controlplane/pkg/biz/apitoken_stale_revoker_integration_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz"
 	"github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/testhelpers"
 	"github.com/google/uuid"
@@ -195,8 +196,8 @@ func (s *staleRevokerTestSuite) createOrgWithThreshold(ctx context.Context, days
 	org, err := s.Organization.CreateWithRandomName(ctx)
 	require.NoError(s.T(), err)
 
-	// Need a membership so Update works
-	_, err = s.Membership.Create(ctx, org.ID, s.user.ID, biz.WithCurrentMembership())
+	// Need an admin membership so Update works
+	_, err = s.Membership.Create(ctx, org.ID, s.user.ID, biz.WithMembershipRole(authz.RoleOwner), biz.WithCurrentMembership())
 	require.NoError(s.T(), err)
 
 	org, err = s.Organization.Update(ctx, s.user.ID, org.Name, &biz.OrganizationUpdateOpts{

app/controlplane/pkg/biz/organization.go

@@ -245,6 +245,14 @@ func (uc *OrganizationUseCase) Update(ctx context.Context, userID, orgName strin
 		return nil, NewErrNotFound("membership")
 	}
 
+	// These settings are organization-wide security controls, so they require an
+	// admin membership in the organization being updated. Authorizing against this
+	// membership, and not against the caller's current role, is what keeps a user
+	// from tampering with another organization they happen to belong to.
+	if !membership.Role.IsAdmin() {
+		return nil, NewErrUnauthorizedStr("only organization admins can update the organization settings")
+	}
+
 	orgUUID, err := uuid.Parse(membership.Org.ID)
 	if err != nil {
 		return nil, NewErrInvalidUUID(err)

app/controlplane/pkg/biz/organization_integration_test.go

@@ -17,6 +17,7 @@ package biz_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
@@ -196,6 +197,93 @@ func (s *OrgIntegrationTestSuite) TestUpdate() {
 	})
 }
 
+// TestUpdateRequiresAdminMembership verifies that changing organization-wide
+// security settings requires an admin/owner membership in the organization
+// being updated. Holding any membership is not enough: the settings gated here
+// (policy enforcement, allowed policy hostnames, runner env-var capture) are
+// security controls for the whole org.
+func (s *OrgIntegrationTestSuite) TestUpdateRequiresAdminMembership() {
+	ctx := context.Background()
+
+	testCases := []struct {
+		name    string
+		role    authz.Role
+		allowed bool
+	}{
+		{name: "owner can update", role: authz.RoleOwner, allowed: true},
+		{name: "admin can update", role: authz.RoleAdmin, allowed: true},
+		{name: "viewer cannot update", role: authz.RoleViewer},
+		{name: "member cannot update", role: authz.RoleOrgMember},
+		{name: "contributor cannot update", role: authz.RoleOrgContributor},
+	}
+
+	for _, tc := range testCases {
+		s.Run(tc.name, func() {
+			org, err := s.Organization.CreateWithRandomName(ctx)
+			require.NoError(s.T(), err)
+
+			user, err := s.User.UpsertByEmail(ctx, fmt.Sprintf("%s@test.com", uuid.NewString()), nil)
+			require.NoError(s.T(), err)
+
+			_, err = s.Membership.Create(ctx, org.ID, user.ID, biz.WithMembershipRole(tc.role))
+			require.NoError(s.T(), err)
+
+			got, err := s.Organization.Update(ctx, user.ID, org.Name, &biz.OrganizationUpdateOpts{
+				BlockOnPolicyViolation: toPtrBool(false),
+			})
+
+			if tc.allowed {
+				s.NoError(err)
+				s.False(got.BlockOnPolicyViolation)
+				return
+			}
+
+			s.Error(err)
+			s.True(biz.IsErrUnauthorized(err), "want unauthorized, got %v", err)
+			s.Nil(got)
+		})
+	}
+}
+
+// TestUpdateCrossOrgTampering is a regression test for CP-N1: a user who is an
+// owner of their own organization but only a viewer of a victim organization
+// must not be able to change the victim's security settings. The authz
+// middleware evaluates the caller's role against the organization in the
+// request header, so the biz layer has to authorize against the organization
+// actually being updated.
+func (s *OrgIntegrationTestSuite) TestUpdateCrossOrgTampering() {
+	ctx := context.Background()
+
+	victimOrg, err := s.Organization.CreateWithRandomName(ctx)
+	require.NoError(s.T(), err)
+	attackerOrg, err := s.Organization.CreateWithRandomName(ctx)
+	require.NoError(s.T(), err)
+
+	attacker, err := s.User.UpsertByEmail(ctx, "attacker@test.com", nil)
+	require.NoError(s.T(), err)
+
+	// Owner of their own org, which is what gets them past the authz middleware
+	_, err = s.Membership.Create(ctx, attackerOrg.ID, attacker.ID, biz.WithMembershipRole(authz.RoleOwner), biz.WithCurrentMembership())
+	require.NoError(s.T(), err)
+	// ...but only a viewer of the victim org
+	_, err = s.Membership.Create(ctx, victimOrg.ID, attacker.ID, biz.WithMembershipRole(authz.RoleViewer))
+	require.NoError(s.T(), err)
+
+	got, err := s.Organization.Update(ctx, attacker.ID, victimOrg.Name, &biz.OrganizationUpdateOpts{
+		BlockOnPolicyViolation:   toPtrBool(false),
+		PoliciesAllowedHostnames: []string{"evil.example.com"},
+		SkipRunnerEnvVars:        toPtrBool(false),
+	})
+	s.Error(err)
+	s.True(biz.IsErrUnauthorized(err), "want unauthorized, got %v", err)
+	s.Nil(got)
+
+	// The victim org keeps its settings
+	victim, err := s.Organization.FindByName(ctx, victimOrg.Name)
+	s.NoError(err)
+	s.Empty(victim.PoliciesAllowedHostnames)
+}
+
 // We are doing an integration test here because there are some database constraints
 // and delete cascades that we want to validate that they work too
 func (s *OrgIntegrationTestSuite) TestDeleteOrg() {
@@ -288,7 +376,7 @@ func (s *OrgIntegrationTestSuite) SetupTest() {
 
 	s.user, err = s.User.UpsertByEmail(ctx, "foo@test.com", nil)
 	assert.NoError(err)
-	_, err = s.Membership.Create(ctx, s.org.ID, s.user.ID, biz.WithCurrentMembership())
+	_, err = s.Membership.Create(ctx, s.org.ID, s.user.ID, biz.WithMembershipRole(authz.RoleOwner), biz.WithCurrentMembership())
 	assert.NoError(err)
 
 	// Integration