diff --git a/.github/actions/pack-cypress-deps/action.yml b/.github/actions/pack-cypress-deps/action.yml new file mode 100644 index 000000000000..cdd970f0d052 --- /dev/null +++ b/.github/actions/pack-cypress-deps/action.yml @@ -0,0 +1,40 @@ +name: "Pack Cypress deps artifact" +description: > + Tar the Cypress node_modules and binary into a run-scoped artifact for the + workers in e2e-tests-cypress-template.yml. The payload must stay in sync with + unpack-cypress-deps. + +inputs: + fips: + description: > + Whether this is a FIPS-edition run. e2e-tests-ci.yml invokes the cypress + template twice per workflow run — once per edition — and artifact names are + run-scoped, so the two invocations would otherwise collide. Must match the + unpack step. + required: false + default: "false" + +runs: + using: "composite" + steps: + # -C so the archives hold paths relative to their extraction root, letting + # unpack extract straight into the workspace and HOME. + - name: ci/pack-cypress-deps + shell: bash + run: | + set -euo pipefail + tar -czf "${RUNNER_TEMP}/cypress-node-modules.tgz" -C "${GITHUB_WORKSPACE}" e2e-tests/cypress/node_modules + tar -czf "${RUNNER_TEMP}/cypress-binary.tgz" -C "${HOME}" .cache/Cypress + - name: ci/upload-cypress-deps + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: e2e-cypress-deps${{ inputs.fips == 'true' && '-fips' || '' }} + path: | + ${{ runner.temp }}/cypress-node-modules.tgz + ${{ runner.temp }}/cypress-binary.tgz + # Only ever consumed by workers in the same run. + retention-days: 1 + if-no-files-found: error + # The tarballs are already gzipped; re-compressing them in the zip + # wrapper costs CPU for no gain. + compression-level: 0 diff --git a/.github/actions/unpack-cypress-deps/action.yml b/.github/actions/unpack-cypress-deps/action.yml new file mode 100644 index 000000000000..b52958a9f2fa --- /dev/null +++ b/.github/actions/unpack-cypress-deps/action.yml @@ -0,0 +1,27 @@ +name: "Unpack Cypress deps artifact" +description: > + Download and extract the run-scoped artifact built by pack-cypress-deps, + restoring the Cypress node_modules and binary into place. + +inputs: + fips: + description: > + Whether this is a FIPS-edition run. Must match the pack step so the correct + artifact is downloaded. + required: false + default: "false" + +runs: + using: "composite" + steps: + - name: ci/download-cypress-deps + uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3 # v8.0.0 + with: + name: e2e-cypress-deps${{ inputs.fips == 'true' && '-fips' || '' }} + path: ${{ runner.temp }} + - name: ci/unpack-cypress-deps + shell: bash + run: | + set -euo pipefail + tar -xzf "${RUNNER_TEMP}/cypress-node-modules.tgz" -C "${GITHUB_WORKSPACE}" + tar -xzf "${RUNNER_TEMP}/cypress-binary.tgz" -C "${HOME}" diff --git a/.github/workflows/e2e-tests-cypress-template.yml b/.github/workflows/e2e-tests-cypress-template.yml index a7da8b7bc7ca..67f3d6f0a852 100644 --- a/.github/workflows/e2e-tests-cypress-template.yml +++ b/.github/workflows/e2e-tests-cypress-template.yml @@ -198,13 +198,12 @@ jobs: echo "workers=$(jq -nc --argjson n "${INPUT_WORKERS}" '[range(1; $n+1)]')" >> $GITHUB_OUTPUT echo "start_time=$(date +%s)" >> $GITHUB_OUTPUT - # Install webapp node_modules once via the shared webapp-setup action, then - # workers restore the same stable cache. The node_modules cache is keyed only - # on webapp/package-lock.json and is shared with webapp-ci.yml jobs. + # Build the Cypress deps once and hand them to the workers as a run-scoped + # artifact. prep-deps: name: prep-deps runs-on: ubuntu-24.04 - timeout-minutes: 10 + timeout-minutes: 15 permissions: contents: read steps: @@ -214,23 +213,23 @@ jobs: persist-credentials: false ref: ${{ inputs.commit_sha }} fetch-depth: 1 + # Provides the @mattermost/eslint-plugin target the cypress install + # symlinks to. - name: ci/setup-webapp-node-modules uses: ./.github/actions/webapp-setup - - name: ci/cache-cypress-deps - # node_modules + the cypress binary (downloaded to ~/.cache/Cypress by - # cypress's postinstall, not into node_modules). Both must be cached; - # otherwise workers see "cypress npm package installed but binary missing". - id: cache-cypress - uses: actions/cache@cdf6c1fa76f9f475f3d7449005a359c84ca0f306 # v5.0.3 - with: - path: | - e2e-tests/cypress/node_modules - ~/.cache/Cypress - key: e2e-cypress-deps-${{ runner.os }}-${{ hashFiles('e2e-tests/cypress/package-lock.json') }} + # Read-only restore of the registry cache warmed daily from master, so the + # install below refetches as little as possible. + - name: ci/restore-npm-cache + uses: ./.github/actions/restore-e2e-npm-cache + # The postinstall downloads the Cypress binary to ~/.cache/Cypress rather + # than into node_modules, which is why the artifact carries both. - name: ci/install-cypress-deps - if: steps.cache-cypress.outputs.cache-hit != 'true' working-directory: e2e-tests/cypress run: npm ci + - name: ci/pack-cypress-deps + uses: ./.github/actions/pack-cypress-deps + with: + fips: ${{ inputs.server_edition == 'fips' }} # Register the Test System IO run AFTER prep-deps so workers reach # dispatch-run within Test System IO's inactivity window. @@ -317,14 +316,12 @@ jobs: uses: ./.github/actions/webapp-setup with: read-only: "true" - - name: ci/restore-cypress-deps - uses: actions/cache/restore@cdf6c1fa76f9f475f3d7449005a359c84ca0f306 # v5.0.3 + # prep-deps built these; it is an ancestor of this job via dispatch-begin, + # so the artifact is always present by now. + - name: ci/reuse-cypress-deps + uses: ./.github/actions/unpack-cypress-deps with: - path: | - e2e-tests/cypress/node_modules - ~/.cache/Cypress - key: e2e-cypress-deps-${{ runner.os }}-${{ hashFiles('e2e-tests/cypress/package-lock.json') }} - fail-on-cache-miss: true + fips: ${{ inputs.server_edition == 'fips' }} - name: ci/cloud-init working-directory: e2e-tests run: make cloud-init diff --git a/server/channels/api4/access_control_test.go b/server/channels/api4/access_control_test.go index ec28c4d05baa..8a65b8966212 100644 --- a/server/channels/api4/access_control_test.go +++ b/server/channels/api4/access_control_test.go @@ -16,6 +16,15 @@ import ( "github.com/stretchr/testify/require" ) +// allowSelfInclusion stubs QueryUsersForExpression so checkSelfInclusion +// succeeds for userID. Maybe() so tests whose expressions skip the check +// ("true"/empty) still pass AssertExpectations. +func allowSelfInclusion(mockACS *mocks.AccessControlServiceInterface, userID string) { + mockACS.On("QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything). + Return([]*model.User{{Id: userID}}, int64(1), nil). + Maybe() +} + // maskingOffTestConfig disables attribute-value masking for policy-endpoint // tests that do not cover masking. ABAC and other ABAC sub-flags default on. func maskingOffTestConfig(cfg *model.Config) { @@ -146,6 +155,7 @@ func TestCreateAccessControlPolicy(t *testing.T) { th.App.Srv().Channels().AccessControl = mockAccessControlService notFound := model.NewAppError("GetPolicy", "app.access_control.not_found.app_error", nil, "", http.StatusNotFound) mockAccessControlService.On("GetPolicy", mock.AnythingOfType("*request.Context"), privateChannel.Id).Return(nil, notFound) + allowSelfInclusion(mockAccessControlService, channelAdmin.Id) mockAccessControlService.On("SavePolicy", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.AccessControlPolicy")).Return(channelPolicy, nil).Times(1) th.App.UpdateConfig(func(cfg *model.Config) { @@ -270,6 +280,7 @@ func TestCreateAccessControlPolicy(t *testing.T) { // Set up mock expectations notFound := model.NewAppError("GetPolicy", "app.access_control.not_found.app_error", nil, "", http.StatusNotFound) mockAccessControlService.On("GetPolicy", mock.AnythingOfType("*request.Context"), ch.Id).Return(nil, notFound) + allowSelfInclusion(mockAccessControlService, th.BasicUser.Id) mockAccessControlService.On("SavePolicy", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.AccessControlPolicy")).Return(samplePolicy, nil).Times(1) // Set the mock on the app @@ -598,7 +609,9 @@ func TestCreateAccessControlPolicyPreservesSystemManagedFields(t *testing.T) { _, _, err := client.Login(context.Background(), channelAdmin.Email, channelAdmin.Password) require.NoError(t, err) - return privateChannel, client, enableABAC() + mockACS := enableABAC() + allowSelfInclusion(mockACS, channelAdmin.Id) + return privateChannel, client, mockACS } // Logs th.Client in as a team admin and stubs the per-rule self-inclusion check so the request @@ -2027,6 +2040,7 @@ func setupTeamAdminABAC(t *testing.T, th *TestHelper) *mocks.AccessControlServic }) th.AddPermissionToRole(t, model.PermissionManageTeamAccessRules.Id, model.TeamAdminRoleId) + allowSelfInclusion(mockACS, th.TeamAdminUser.Id) return mockACS } diff --git a/server/channels/app/access_control.go b/server/channels/app/access_control.go index 2c5795ed934c..fa63b9069639 100644 --- a/server/channels/app/access_control.go +++ b/server/channels/app/access_control.go @@ -156,70 +156,18 @@ func (a *App) CreateOrUpdateAccessControlPolicy(rctx request.CTX, policy *model. } } - // Defense in depth: a team admin must remain within their own team policy's - // rules. The api4 handler enforces this for the request path, but guard here - // so any internal caller saving a team policy is held to the same invariant - // regardless of the masking flag. System admins and sessionless internal - // callers may intentionally set rules they don't match, mirroring the - // masking self-inclusion exemption below. - if policy.Type == model.AccessControlPolicyTypeTeam { - if session := rctx.Session(); session != nil && session.UserId != "" && !a.HasPermissionTo(rctx, session.UserId, model.PermissionManageSystem) { - for _, rule := range policy.Rules { - if appErr := a.ValidateTeamAdminSelfInclusion(rctx, session.UserId, rule.Expression); appErr != nil { - return nil, appErr - } - } - } + callerID := "" + if session := rctx.Session(); session != nil { + callerID = session.UserId } - - // ABAC is gated at route registration; only check masking here. Masking is - // attribute-based: edits are allowed with masked values present as long as - // the caller doesn't drop a condition holding values they couldn't see. - if a.Config().FeatureFlags.AttributeValueMasking { - session := rctx.Session() - if session == nil { - return nil, model.NewAppError("CreateOrUpdateAccessControlPolicy", "api.context.session_expired.app_error", nil, "session required for masking validation", http.StatusUnauthorized) - } - callerID := session.UserId - - resolver, appErr := newMaskingResolver(a, rctx, callerID) - if appErr != nil { - return nil, model.NewAppError("CreateOrUpdateAccessControlPolicy", "app.pap.save_policy.resolver_error", nil, "", http.StatusInternalServerError).Wrap(appErr) - } - - // Validate submitted values BEFORE merge: only the values the caller - // actually submitted should be checked against their holdings. Running - // validation after merge would reject the re-injected hidden values - // (e.g. Bravo, Charlie) that the caller legitimately cannot see. - appErr = a.validatePolicyExpressionValues(rctx, policy, resolver) - if appErr != nil { - return nil, appErr - } - - // Merge hidden values back in and block deletion of masked conditions. - mergedHidden, appErr := a.mergeStoredPolicyExpressions(rctx, policy, resolver) - if appErr != nil { - return nil, appErr - } - - // Guard against persisting the sentinel as a real value. - if appErr := rejectMaskedTokens(policy); appErr != nil { - return nil, appErr - } - - // Self-inclusion check applies only to non-admins. System admins may - // legitimately set conditions for attributes they do not personally hold - // (e.g., creating a "Clearance == Top Secret" rule without holding that - // clearance themselves). Masking and write-path value validation still - // apply to system admins above. - if !a.HasPermissionTo(rctx, callerID, model.PermissionManageSystem) { - if appErr := a.checkSelfInclusion(rctx, policy, callerID, mergedHidden); appErr != nil { - return nil, appErr - } - } + // Channel/team UI GET masks values the caller cannot see, so mergeFromStore + // re-injects those hidden literals before persist. + var appErr *model.AppError + policy, appErr = a.enforceAccessControlPolicyWriteGuards(rctx, policy, callerID, true) + if appErr != nil { + return nil, appErr } - var appErr *model.AppError policy, appErr = acs.SavePolicy(rctx, policy) if appErr != nil { return nil, appErr @@ -452,6 +400,74 @@ func saveForbiddenError(rctx request.CTX, where, internalReason string) *model.A return model.NewAppError(where, "app.pap.save_policy.forbidden", nil, "", http.StatusForbidden) } +// enforceAccessControlPolicyWriteGuards runs shared save-path invariants for +// channel/team (CreateOrUpdateAccessControlPolicy) and plugin-owned policies +// (SavePluginAccessControlPolicy). +// +// When AttributeValueMasking is on (applies to all callers, including system +// admins): +// 1. validatePolicyExpressionValues — submitted literals must be held by caller +// 2. mergeStoredPolicyExpressions — only when mergeFromStore is true +// (channel/team UI round-trips masked GET responses; plugin GET is unmasked, +// so plugin saves pass mergeFromStore=false) +// 3. rejectMaskedTokens — never persist the masking sentinel +// +// Self-inclusion always runs for non-sysadmins with a non-empty callerID, even +// when AttributeValueMasking is off. That matches product intent (a non-sysadmin +// cannot save a policy that excludes them) and keeps the plugin path from +// weakening when masking is disabled. Sessionless/internal callers with an empty +// callerID skip self-inclusion. System admins are exempt from self-inclusion but +// remain subject to value-holding validation when masking is on. +// +// Masking with an empty callerID is rejected (session required), matching the +// historical CreateOrUpdateAccessControlPolicy contract. +func (a *App) enforceAccessControlPolicyWriteGuards( + rctx request.CTX, + policy *model.AccessControlPolicy, + callerID string, + mergeFromStore bool, +) (*model.AccessControlPolicy, *model.AppError) { + mergedHidden := false + + if a.Config().FeatureFlags.AttributeValueMasking { + if callerID == "" { + return nil, model.NewAppError("enforceAccessControlPolicyWriteGuards", "api.context.session_expired.app_error", nil, "session required for masking validation", http.StatusUnauthorized) + } + + resolver, appErr := newMaskingResolver(a, rctx, callerID) + if appErr != nil { + return nil, model.NewAppError("enforceAccessControlPolicyWriteGuards", "app.pap.save_policy.resolver_error", nil, "", http.StatusInternalServerError).Wrap(appErr) + } + + // Validate submitted values BEFORE merge: only the values the caller + // actually submitted should be checked against their holdings. Running + // validation after merge would reject the re-injected hidden values + // (e.g. Bravo, Charlie) that the caller legitimately cannot see. + if appErr = a.validatePolicyExpressionValues(rctx, policy, resolver); appErr != nil { + return nil, appErr + } + + if mergeFromStore { + mergedHidden, appErr = a.mergeStoredPolicyExpressions(rctx, policy, resolver) + if appErr != nil { + return nil, appErr + } + } + + if appErr := rejectMaskedTokens(policy); appErr != nil { + return nil, appErr + } + } + + if callerID != "" && !a.HasPermissionTo(rctx, callerID, model.PermissionManageSystem) { + if appErr := a.checkSelfInclusion(rctx, policy, callerID, mergedHidden); appErr != nil { + return nil, appErr + } + } + + return policy, nil +} + // checkSelfInclusion verifies the caller satisfies all policy rules after their edit. // When mergedHidden is true (hidden values were re-injected), a self-exclusion failure // returns the generic forbidden error; otherwise the specific self_exclusion error is used. diff --git a/server/channels/app/access_control_test.go b/server/channels/app/access_control_test.go index 96abcaf234d6..82e200196371 100644 --- a/server/channels/app/access_control_test.go +++ b/server/channels/app/access_control_test.go @@ -460,9 +460,8 @@ func TestDeleteAccessControlPolicy(t *testing.T) { // TestCheckSelfInclusion verifies the self-exclusion guard: non-admin callers must // satisfy their own policy after saving, or the save is refused with 403 -// self_exclusion. Sysadmins are exempt at the call site -// (CreateOrUpdateAccessControlPolicy), not inside checkSelfInclusion itself — this -// test exercises the function directly. +// self_exclusion. Sysadmins are exempt in enforceAccessControlPolicyWriteGuards, +// not inside checkSelfInclusion itself — this test exercises the function directly. func TestCheckSelfInclusion(t *testing.T) { t.Run("caller who satisfies the policy passes", func(t *testing.T) { th := Setup(t).InitBasic(t) diff --git a/server/channels/app/plugin_access_control.go b/server/channels/app/plugin_access_control.go index 2d5d046b65ef..1c1f200b0113 100644 --- a/server/channels/app/plugin_access_control.go +++ b/server/channels/app/plugin_access_control.go @@ -160,6 +160,13 @@ func (a *App) EvaluatePluginAccessRequest(rctx request.CTX, pluginID, userID, re // control policy. Version is forced to v0.5 and Active to true (plugin types // have no separate activation lifecycle); policy.ID must be the resource's // stable ID. +// +// Write guards go through enforceAccessControlPolicyWriteGuards with +// mergeFromStore=false: plugin GET returns unmasked policies, so there is no +// masked-value round-trip to repair. Non-system-admin acting users must always +// satisfy the saved expression (self-inclusion), including when +// AttributeValueMasking is off. System admins may author policies they do not +// match, but remain subject to value-holding validation when masking is on. func (a *App) SavePluginAccessControlPolicy(rctx request.CTX, pluginID, actingUserID string, policy *model.AccessControlPolicy) (*model.AccessControlPolicy, *model.AppError) { // Audit every attempt, including precondition failures. auditRec := a.MakeAuditRecord(rctx, model.AuditEventSavePluginAccessControlPolicy, model.AuditStatusFail) @@ -215,6 +222,10 @@ func (a *App) SavePluginAccessControlPolicy(rctx request.CTX, pluginID, actingUs return nil, appErr } + if _, appErr := a.enforceAccessControlPolicyWriteGuards(rctx, policy, actingUserID, false); appErr != nil { + return nil, appErr + } + // Enterprise SavePolicy derives the caller ID from the session, so // synthesize one for the acting user. saveCtx := rctx.WithSession(&model.Session{UserId: actingUserID}) @@ -399,6 +410,10 @@ func (a *App) QueryUsersForPluginAccessControlExpression(rctx request.CTX, plugi if appErr != nil { return nil, appErr } + // Normalize at the producer so gob/JSON never serialize Users as null. + if users == nil { + users = []*model.User{} + } return &model.AccessControlPolicyTestResponse{Users: users, Total: total}, nil } diff --git a/server/channels/app/plugin_access_control_gob_test.go b/server/channels/app/plugin_access_control_gob_test.go index ddc22c299e07..6af1f138634f 100644 --- a/server/channels/app/plugin_access_control_gob_test.go +++ b/server/channels/app/plugin_access_control_gob_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/mattermost/mattermost/server/public/model" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/v8/einterfaces/mocks" @@ -88,6 +89,32 @@ func TestPluginAccessControlGobSafety(t *testing.T) { requirePluginRPCGobSafe(t, &model.AccessControlPolicyTestResponse{Users: []*model.User{th.BasicUser}, Total: 1}) }) + t.Run("query users empty result is non-nil at producer and gob-safe", func(t *testing.T) { + mockACS := &mocks.AccessControlServiceInterface{} + th.App.Srv().ch.AccessControl = mockACS + mockACS.On("QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything). + Return(([]*model.User)(nil), int64(0), nil).Once() + + resp, appErr := th.App.QueryUsersForPluginAccessControlExpression( + th.Context, testAgentsPluginID, th.BasicUser.Id, testAgentResourceType, + `user.attributes.dept == "eng"`, "", "", 10) + require.Nil(t, appErr) + // Producer normalizes nil → empty slice so JSON/in-process callers never + // see null users. encoding/gob still decodes empty slices as nil; host + // and plugin UIs keep `users ?? []` as belt-and-braces for that quirk. + require.NotNil(t, resp.Users) + require.Empty(t, resp.Users) + requirePluginRPCGobSafe(t, resp) + + var decoded model.AccessControlPolicyTestResponse + var buf bytes.Buffer + require.NoError(t, gob.NewEncoder(&buf).Encode(resp)) + require.NoError(t, gob.NewDecoder(&buf).Decode(&decoded)) + require.Equal(t, int64(0), decoded.Total) + require.Empty(t, decoded.Users) + mockACS.AssertExpectations(t) + }) + t.Run("evaluation decision carrying a context reason", func(t *testing.T) { decision := model.NewNoPolicyAccessDecision() requirePluginRPCGobSafe(t, &decision) diff --git a/server/channels/app/plugin_access_control_save_test.go b/server/channels/app/plugin_access_control_save_test.go new file mode 100644 index 000000000000..aaac229fa9fe --- /dev/null +++ b/server/channels/app/plugin_access_control_save_test.go @@ -0,0 +1,112 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "net/http" + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost/server/v8/einterfaces/mocks" +) + +func TestSavePluginAccessControlPolicyWriteGuards(t *testing.T) { + notFoundErr := model.NewAppError("GetPolicy", "app.pap.get_policy.app_error", nil, "", http.StatusNotFound) + + t.Run("non-sysadmin self-excluding expression rejected", func(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.AttributeValueMasking = false + }).InitBasic(t) + actingUserID := th.BasicUser.Id + + mockACS := &mocks.AccessControlServiceInterface{} + th.App.Srv().ch.AccessControl = mockACS + + p := validPluginPolicy(model.NewId()) + mockACS.On("GetPolicy", mock.Anything, p.ID).Return(nil, notFoundErr).Once() + mockACS.On("QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything). + Return([]*model.User{}, int64(0), nil).Once() + + _, appErr := th.App.SavePluginAccessControlPolicy(th.Context, testAgentsPluginID, actingUserID, p) + require.NotNil(t, appErr) + assert.Equal(t, http.StatusForbidden, appErr.StatusCode) + assert.Equal(t, "app.pap.save_policy.self_exclusion", appErr.Id) + mockACS.AssertNotCalled(t, "SavePolicy", mock.Anything, mock.Anything) + mockACS.AssertExpectations(t) + }) + + t.Run("sysadmin may save self-excluding expression", func(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.AttributeValueMasking = false + }).InitBasic(t) + adminID := th.SystemAdminUser.Id + + mockACS := &mocks.AccessControlServiceInterface{} + th.App.Srv().ch.AccessControl = mockACS + + p := validPluginPolicy(model.NewId()) + mockACS.On("GetPolicy", mock.Anything, p.ID).Return(nil, notFoundErr).Once() + mockACS.On("SavePolicy", mock.MatchedBy(func(c request.CTX) bool { + return c.Session() != nil && c.Session().UserId == adminID + }), mock.Anything).Return(p, nil).Once() + + _, appErr := th.App.SavePluginAccessControlPolicy(th.Context, testAgentsPluginID, adminID, p) + require.Nil(t, appErr) + mockACS.AssertNotCalled(t, "QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything) + mockACS.AssertExpectations(t) + }) + + t.Run("masking on: non-admin cannot submit unheld literals", func(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.AttributeValueMasking = true + }).InitBasic(t) + actingUserID := th.BasicUser.Id + + mockACS := &mocks.AccessControlServiceInterface{} + th.App.Srv().ch.AccessControl = mockACS + + p := validPluginPolicy(model.NewId()) + valueErr := model.NewAppError("ValidateExpressionValuesForCaller", "app.pap.save_policy.forbidden", nil, "", http.StatusForbidden) + mockACS.On("GetPolicy", mock.Anything, p.ID).Return(nil, notFoundErr).Once() + mockACS.On("ValidateExpressionValuesForCaller", mock.Anything, p.Rules[0].Expression, mock.Anything). + Return(valueErr).Once() + + _, appErr := th.App.SavePluginAccessControlPolicy(th.Context, testAgentsPluginID, actingUserID, p) + require.NotNil(t, appErr) + assert.Equal(t, http.StatusForbidden, appErr.StatusCode) + assert.Equal(t, "app.pap.save_policy.forbidden", appErr.Id) + mockACS.AssertNotCalled(t, "SavePolicy", mock.Anything, mock.Anything) + mockACS.AssertNotCalled(t, "MergeExpressionWithMaskedValuesCanonical", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + mockACS.AssertNotCalled(t, "QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything) + mockACS.AssertExpectations(t) + }) + + t.Run("masking on: plugin path skips store merge", func(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.AttributeValueMasking = true + }).InitBasic(t) + actingUserID := th.BasicUser.Id + + mockACS := &mocks.AccessControlServiceInterface{} + th.App.Srv().ch.AccessControl = mockACS + + p := validPluginPolicy(model.NewId()) + mockACS.On("GetPolicy", mock.Anything, p.ID).Return(nil, notFoundErr).Once() + mockACS.On("ValidateExpressionValuesForCaller", mock.Anything, p.Rules[0].Expression, mock.Anything). + Return(nil).Once() + mockACS.On("QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything). + Return([]*model.User{{Id: actingUserID}}, int64(1), nil).Once() + mockACS.On("SavePolicy", mock.Anything, mock.Anything).Return(p, nil).Once() + + _, appErr := th.App.SavePluginAccessControlPolicy(th.Context, testAgentsPluginID, actingUserID, p) + require.Nil(t, appErr) + mockACS.AssertNotCalled(t, "MergeExpressionWithMaskedValuesCanonical", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + mockACS.AssertExpectations(t) + }) +} diff --git a/server/channels/app/plugin_access_control_test.go b/server/channels/app/plugin_access_control_test.go index 75ae5e73c889..6baa61f15e11 100644 --- a/server/channels/app/plugin_access_control_test.go +++ b/server/channels/app/plugin_access_control_test.go @@ -358,7 +358,12 @@ func TestEvaluatePluginAccessRequestStoreError(t *testing.T) { } func TestSavePluginAccessControlPolicy(t *testing.T) { - th := Setup(t).InitBasic(t) + // Write-guard self-inclusion and masking cases live in + // plugin_access_control_save_test.go. Keep this suite on ownership/versioning + // with masking off. + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.AttributeValueMasking = false + }).InitBasic(t) actingUserID := th.BasicUser.Id notFoundErr := model.NewAppError("GetPolicy", "app.pap.get_policy.app_error", nil, "", http.StatusNotFound) @@ -426,6 +431,8 @@ func TestSavePluginAccessControlPolicy(t *testing.T) { expression := p.Rules[0].Expression mockACS.On("GetPolicy", mock.Anything, p.ID).Return(nil, notFoundErr).Once() + mockACS.On("QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything). + Return([]*model.User{{Id: actingUserID}}, int64(1), nil).Once() mockACS.On("SavePolicy", mock.MatchedBy(func(c request.CTX) bool { return c.Session() != nil && c.Session().UserId == actingUserID }), mock.MatchedBy(func(saved *model.AccessControlPolicy) bool { @@ -450,6 +457,8 @@ func TestSavePluginAccessControlPolicy(t *testing.T) { p := validPluginPolicy(model.NewId()) existing := validPluginPolicy(p.ID) mockACS.On("GetPolicy", mock.Anything, p.ID).Return(existing, nil).Once() + mockACS.On("QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything). + Return([]*model.User{{Id: actingUserID}}, int64(1), nil).Once() mockACS.On("SavePolicy", mock.Anything, mock.Anything).Return(p, nil).Once() _, appErr := th.App.SavePluginAccessControlPolicy(th.Context, testAgentsPluginID, actingUserID, p) @@ -995,7 +1004,9 @@ func auditParam(t *testing.T, rec map[string]any, key string) any { // from entry and refines it to "create"/"update" once the existence probe // resolves; Delete stamps "delete". func TestPluginAccessControlAudit(t *testing.T) { - th := Setup(t).InitBasic(t) + th := SetupConfig(t, func(cfg *model.Config) { + cfg.FeatureFlags.AttributeValueMasking = false + }).InitBasic(t) capture := startPluginAuditCapture(t, th) actingUserID := th.BasicUser.Id @@ -1094,6 +1105,8 @@ func TestPluginAccessControlAudit(t *testing.T) { p := validPluginPolicy(model.NewId()) mockACS.On("GetPolicy", mock.Anything, p.ID). Return(nil, model.NewAppError("GetPolicy", "app.pap.get_policy.app_error", nil, "", http.StatusNotFound)).Once() + mockACS.On("QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything). + Return([]*model.User{{Id: actingUserID}}, int64(1), nil).Once() mockACS.On("SavePolicy", mock.Anything, mock.Anything).Return(p, nil).Once() rec := assertNextRecord(t, model.AuditEventSavePluginAccessControlPolicy, model.AuditStatusSuccess, func() { @@ -1104,6 +1117,26 @@ func TestPluginAccessControlAudit(t *testing.T) { mockACS.AssertExpectations(t) }) + t.Run("save: self-exclusion audits as fail", func(t *testing.T) { + mockACS := &mocks.AccessControlServiceInterface{} + th.App.Srv().ch.AccessControl = mockACS + + p := validPluginPolicy(model.NewId()) + mockACS.On("GetPolicy", mock.Anything, p.ID). + Return(nil, model.NewAppError("GetPolicy", "app.pap.get_policy.app_error", nil, "", http.StatusNotFound)).Once() + mockACS.On("QueryUsersForExpression", mock.Anything, mock.Anything, mock.Anything). + Return([]*model.User{}, int64(0), nil).Once() + + rec := assertNextRecord(t, model.AuditEventSavePluginAccessControlPolicy, model.AuditStatusFail, func() { + _, appErr := th.App.SavePluginAccessControlPolicy(th.Context, testAgentsPluginID, actingUserID, p) + require.NotNil(t, appErr) + assert.Equal(t, "app.pap.save_policy.self_exclusion", appErr.Id) + }) + assert.Equal(t, "create", auditParam(t, rec, "operation")) + mockACS.AssertNotCalled(t, "SavePolicy", mock.Anything, mock.Anything) + mockACS.AssertExpectations(t) + }) + t.Run("delete: service unavailable still audits as fail", func(t *testing.T) { th.App.Srv().ch.AccessControl = nil rec := assertNextRecord(t, model.AuditEventDeletePluginAccessControlPolicy, model.AuditStatusFail, func() { diff --git a/server/channels/app/team_access_control_test.go b/server/channels/app/team_access_control_test.go index f6002d1f3cfe..c015f254e460 100644 --- a/server/channels/app/team_access_control_test.go +++ b/server/channels/app/team_access_control_test.go @@ -646,10 +646,9 @@ func TestReconcilePolicyTeamScope(t *testing.T) { }) } -// TestCreateOrUpdateAccessControlPolicy_TeamSelfInclusion verifies that the -// team-specific self-inclusion guard at CreateOrUpdateAccessControlPolicy:124 -// is correctly wired into the create/update path. ValidateTeamAdminSelfInclusion -// is tested in isolation elsewhere; these tests confirm the integration. +// TestCreateOrUpdateAccessControlPolicy_TeamSelfInclusion verifies that +// enforceAccessControlPolicyWriteGuards self-inclusion applies to team policy +// saves. HTTP still uses ValidateTeamAdminSelfInclusion in the api4 handler. func TestCreateOrUpdateAccessControlPolicy_TeamSelfInclusion(t *testing.T) { t.Run("team admin excluded by own expression is rejected before save", func(t *testing.T) { // AttributeValueMasking defaults to true @@ -695,7 +694,7 @@ func TestCreateOrUpdateAccessControlPolicy_TeamSelfInclusion(t *testing.T) { result, appErr := th.App.CreateOrUpdateAccessControlPolicy(rctx, teamPolicy) require.NotNil(t, appErr) - assert.Equal(t, "app.team.access_policies.self_exclusion.app_error", appErr.Id) + assert.Equal(t, "app.pap.save_policy.self_exclusion", appErr.Id) assert.Nil(t, result) }) @@ -705,9 +704,8 @@ func TestCreateOrUpdateAccessControlPolicy_TeamSelfInclusion(t *testing.T) { cfg.FeatureFlags.AttributeValueMasking = false }).InitBasic(t) - callerID := th.BasicUser.Id rctx := th.Context.WithSession(&model.Session{ - UserId: callerID, + UserId: th.BasicUser.Id, Id: model.NewId(), Roles: model.SystemUserRoleId, }) @@ -720,15 +718,6 @@ func TestCreateOrUpdateAccessControlPolicy_TeamSelfInclusion(t *testing.T) { mockACS.AssertExpectations(t) }) - // Guard: caller satisfies the expression → guard passes. - mockACS.On("QueryUsersForExpression", - mock.AnythingOfType("*request.Context"), - "true", - mock.MatchedBy(func(opts model.SubjectSearchOptions) bool { - return opts.SubjectID == callerID - }), - ).Return([]*model.User{{Id: callerID}}, int64(1), nil).Once() - savedPolicy := &model.AccessControlPolicy{ ID: th.BasicTeam.Id, Type: model.AccessControlPolicyTypeTeam, @@ -736,7 +725,7 @@ func TestCreateOrUpdateAccessControlPolicy_TeamSelfInclusion(t *testing.T) { {Actions: []string{"membership"}, Expression: "true"}, }, } - // SavePolicy is reached after the guard passes. + // "true" skips checkSelfInclusion; SavePolicy is reached without a query. mockACS.On("SavePolicy", mock.AnythingOfType("*request.Context"), mock.Anything). Return(savedPolicy, (*model.AppError)(nil)).Once() @@ -753,6 +742,59 @@ func TestCreateOrUpdateAccessControlPolicy_TeamSelfInclusion(t *testing.T) { require.NotNil(t, result) }) + t.Run("team admin included by a non-true expression runs the shared guard and saves", func(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = true + cfg.FeatureFlags.AttributeValueMasking = false + }).InitBasic(t) + + callerID := th.BasicUser.Id + rctx := th.Context.WithSession(&model.Session{ + UserId: callerID, + Id: model.NewId(), + Roles: model.SystemUserRoleId, // not a system admin + }) + + mockACS := &mocks.AccessControlServiceInterface{} + originalACS := th.App.Srv().ch.AccessControl + th.App.Srv().ch.AccessControl = mockACS + t.Cleanup(func() { + th.App.Srv().ch.AccessControl = originalACS + mockACS.AssertExpectations(t) + }) + + expression := `user.attributes.dept == "eng"` + savedPolicy := &model.AccessControlPolicy{ + ID: th.BasicTeam.Id, + Type: model.AccessControlPolicyTypeTeam, + Rules: []model.AccessControlPolicyRule{ + {Actions: []string{"membership"}, Expression: expression}, + }, + } + // Non-"true" expression must go through checkSelfInclusion. + mockACS.On("QueryUsersForExpression", + mock.AnythingOfType("*request.Context"), + expression, + mock.MatchedBy(func(opts model.SubjectSearchOptions) bool { + return opts.SubjectID == callerID + }), + ).Return([]*model.User{{Id: callerID}}, int64(1), nil).Once() + mockACS.On("SavePolicy", mock.AnythingOfType("*request.Context"), mock.Anything). + Return(savedPolicy, (*model.AppError)(nil)).Once() + + teamPolicy := &model.AccessControlPolicy{ + ID: th.BasicTeam.Id, + Type: model.AccessControlPolicyTypeTeam, + Rules: []model.AccessControlPolicyRule{ + {Actions: []string{"membership"}, Expression: expression}, + }, + } + + result, appErr := th.App.CreateOrUpdateAccessControlPolicy(rctx, teamPolicy) + require.Nil(t, appErr) + require.NotNil(t, result) + }) + t.Run("system admin bypasses the team self-inclusion guard", func(t *testing.T) { th := SetupConfig(t, func(cfg *model.Config) { *cfg.AccessControlSettings.EnableAttributeBasedAccessControl = true diff --git a/server/go.mod b/server/go.mod index dd31340b2848..1fbcb6ba5abc 100644 --- a/server/go.mod +++ b/server/go.mod @@ -232,4 +232,4 @@ require ( ) // See MM-66167, MM-68222 for more details. -replace github.com/vmihailenco/msgpack/v5 => github.com/mattermost/msgpack/v5 v5.0.0-20260408165622-cadfad56a815 +replace github.com/vmihailenco/msgpack/v5 => github.com/mattermost/msgpack/v5 v5.0.0-20260813205620-e158e8d3647e diff --git a/server/go.sum b/server/go.sum index 8d9cbc5b0910..3e44a8c6e1b2 100644 --- a/server/go.sum +++ b/server/go.sum @@ -371,8 +371,8 @@ github.com/mattermost/mattermost/server/public v0.4.3 h1:vxdrD5j3+oVfrSCiSaX/lY6 github.com/mattermost/mattermost/server/public v0.4.3/go.mod h1:2z08gasPXqIIbzl/xf2/2Sfn5ITFLGC6tplhOklyAAQ= github.com/mattermost/morph v1.1.0 h1:Q9vrJbeM3s2jfweGheq12EFIzdNp9a/6IovcbvOQ6Cw= github.com/mattermost/morph v1.1.0/go.mod h1:gD+EaqX2UMyyuzmF4PFh4r33XneQ8Nzi+0E8nXjMa3A= -github.com/mattermost/msgpack/v5 v5.0.0-20260408165622-cadfad56a815 h1:uOi89NvrFmDngqMKjlLDxi+MNzJQLA3TqcU2p8czv34= -github.com/mattermost/msgpack/v5 v5.0.0-20260408165622-cadfad56a815/go.mod h1:GaZTsDaehaPpQVyxrf5mtQlH+pc21PIudVV/E3rRQok= +github.com/mattermost/msgpack/v5 v5.0.0-20260813205620-e158e8d3647e h1:tsfWJPgXPNhPT+kOkBmJJJDSjXSNAsweZrujWt8yfHk= +github.com/mattermost/msgpack/v5 v5.0.0-20260813205620-e158e8d3647e/go.mod h1:GaZTsDaehaPpQVyxrf5mtQlH+pc21PIudVV/E3rRQok= github.com/mattermost/pdf v0.0.0-20260828123129-5b7509a6ca01 h1:bXDcd5MREhXeaY+Gn1qixqN91GEClojJBG9v98cbB6s= github.com/mattermost/pdf v0.0.0-20260828123129-5b7509a6ca01/go.mod h1:pNks5J7leEpCnswIJaGfqiz/MQ0lExHgKl+A53aEXrg= github.com/mattermost/rsc v0.0.0-20160330161541-bbaefb05eaa0 h1:G9tL6JXRBMzjuD1kkBtcnd42kUiT6QDwxfFYu7adM6o= diff --git a/webapp/channels/src/components/admin_console/access_control/editors/cel_editor/editor.test.tsx b/webapp/channels/src/components/admin_console/access_control/editors/cel_editor/editor.test.tsx index 7a173d800f6c..79c59303fe5b 100644 --- a/webapp/channels/src/components/admin_console/access_control/editors/cel_editor/editor.test.tsx +++ b/webapp/channels/src/components/admin_console/access_control/editors/cel_editor/editor.test.tsx @@ -203,4 +203,15 @@ describe('buildCELSchemas', () => { expect(schemas['user.attributes']).toEqual(['valid']); expect(schemas['user.session']).toEqual(['ip_address']); }); + + test('skips null or undefined attribute names without throwing', () => { + const schemas = buildCELSchemas([ + {attribute: null, values: [], objectType: 'user'}, + {attribute: undefined, values: [], isNative: true}, + {attribute: 'department', values: [], objectType: 'user'}, + ]); + + expect(schemas.user).toEqual(['attributes']); + expect(schemas['user.attributes']).toEqual(['department']); + }); }); diff --git a/webapp/channels/src/components/admin_console/access_control/editors/cel_editor/editor.tsx b/webapp/channels/src/components/admin_console/access_control/editors/cel_editor/editor.tsx index 29732da90281..468db312f681 100644 --- a/webapp/channels/src/components/admin_console/access_control/editors/cel_editor/editor.tsx +++ b/webapp/channels/src/components/admin_console/access_control/editors/cel_editor/editor.tsx @@ -69,11 +69,14 @@ const MONACO_EDITOR_OPTIONS: monaco.editor.IStandaloneEditorConstructionOptions }; type CELUserAttribute = { - attribute: string; + + // Plugins may pass incomplete proxy rows; the host helper + // toCELEditorAttributes does not always run first. + attribute?: string | null; values: string[]; // 'session' marks a user.session.* attribute; 'user' marks a user.* / - // user.attributes.* attribute. Always populated by toCELEditorAttributes. + // user.attributes.* attribute. Plugin proxies may omit this. objectType?: string; // Native user attributes (e.g. user.email) complete directly off `user.` @@ -86,9 +89,12 @@ type CELUserAttribute = { // offered under user.session.* — the session bucket only appears when present. // Native attributes (isNative) complete directly off user.* (e.g. user.email). export function buildCELSchemas(userAttributes: CELUserAttribute[]): Record { + // Skip null/undefined/invalid names. Plugin proxies may pass incomplete + // rows that never went through toCELEditorAttributes; name.includes would + // throw during render. const cleanNames = (attrs: CELUserAttribute[]) => attrs. map((attr) => attr.attribute). - filter((name) => !name.includes(' ') && name.trim() !== ''); + filter((name): name is string => typeof name === 'string' && !name.includes(' ') && name.trim() !== ''); const sessionAttrNames = cleanNames(userAttributes.filter((attr) => attr.objectType === SESSION_ATTRIBUTES_OBJECT_TYPE)); const userAttrs = userAttributes.filter((attr) => !attr.objectType || attr.objectType === USER_OBJECT_TYPE); const nativeNames = cleanNames(userAttrs.filter((attr) => attr.isNative)); diff --git a/webapp/channels/src/components/admin_console/access_control/modals/policy_test/test_modal.test.tsx b/webapp/channels/src/components/admin_console/access_control/modals/policy_test/test_modal.test.tsx index 3fbe2ca8f2e2..a547f7730a5f 100644 --- a/webapp/channels/src/components/admin_console/access_control/modals/policy_test/test_modal.test.tsx +++ b/webapp/channels/src/components/admin_console/access_control/modals/policy_test/test_modal.test.tsx @@ -229,6 +229,22 @@ describe('TestResultsModal', () => { }); }); + it('should treat null users as an empty list', async () => { + // Plugin RPC (gob) can serialize an empty users slice as null. + mockSearchUsers.mockReturnValue(() => Promise.resolve({ + data: { + users: null, + total: 0, + }, + })); + + renderWithContext(); + + await waitFor(() => { + expect(screen.getByTestId('user-count')).toHaveTextContent('Showing 0 of 0 users'); + }); + }); + it('should handle search error gracefully', async () => { mockSearchUsers.mockReturnValue(() => Promise.resolve({ error: 'Search failed', diff --git a/webapp/channels/src/components/admin_console/access_control/modals/policy_test/test_modal.tsx b/webapp/channels/src/components/admin_console/access_control/modals/policy_test/test_modal.tsx index 527bd641043c..24a0b2bd33f8 100644 --- a/webapp/channels/src/components/admin_console/access_control/modals/policy_test/test_modal.tsx +++ b/webapp/channels/src/components/admin_console/access_control/modals/policy_test/test_modal.tsx @@ -74,7 +74,8 @@ function TestResultsModal({ return; } if (result?.data) { - const newUsers = result.data.users; + // Plugin RPC (gob) can turn an empty users slice into null on the wire. + const newUsers = result.data.users ?? []; if (reset) { setUsers(newUsers); } else { @@ -92,7 +93,7 @@ function TestResultsModal({ // The picker step defers the initial fetch until a channel is chosen // (handled in handleChannelSelected). if (!requireChannel) { - fetchUsers('', ''); + fetchUsers(term, '', true); } }, []); diff --git a/webapp/channels/src/components/channel_header_menu/channel_header_menu.tsx b/webapp/channels/src/components/channel_header_menu/channel_header_menu.tsx index ab1a2f5fc527..c7d6f2badb16 100644 --- a/webapp/channels/src/components/channel_header_menu/channel_header_menu.tsx +++ b/webapp/channels/src/components/channel_header_menu/channel_header_menu.tsx @@ -62,8 +62,6 @@ export default function ChannelHeaderMenu({dmUser, gmMembers, isMobile, archived const isChannelBookmarksEnabled = useSelector(getIsChannelBookmarksEnabled); const isChannelAutotranslated = useSelector((state: GlobalState) => (channel?.id ? isChannelAutotranslatedSelector(state, channel.id) : false)); - const isReadonly = false; - if (!channel) { return null; } @@ -181,7 +179,6 @@ export default function ChannelHeaderMenu({dmUser, gmMembers, isMobile, archived isFavorite={isFavorite} isMobile={isMobile || false} isDefault={isDefault} - isReadonly={isReadonly} isLicensedForLDAPGroups={isLicensedForLDAPGroups} isChannelBookmarksEnabled={isChannelBookmarksEnabled} isChannelAutotranslated={isChannelAutotranslated} diff --git a/webapp/channels/src/components/channel_header_menu/channel_header_menu_items/channel_header_public_private_menu.tsx b/webapp/channels/src/components/channel_header_menu/channel_header_menu_items/channel_header_public_private_menu.tsx index 6762597792de..56203be2868c 100644 --- a/webapp/channels/src/components/channel_header_menu/channel_header_menu_items/channel_header_public_private_menu.tsx +++ b/webapp/channels/src/components/channel_header_menu/channel_header_menu_items/channel_header_public_private_menu.tsx @@ -37,7 +37,6 @@ interface Props extends Menu.FirstMenuItemProps { channel: Channel; user: UserProfile; isMuted: boolean; - isReadonly: boolean; isDefault: boolean; isMobile: boolean; isFavorite: boolean; diff --git a/webapp/channels/src/components/channel_settings_modal/share_channel_with_workspaces/add_workspace_dropdown.tsx b/webapp/channels/src/components/channel_settings_modal/share_channel_with_workspaces/add_workspace_dropdown.tsx index c2222068bef2..48b3e01b768e 100644 --- a/webapp/channels/src/components/channel_settings_modal/share_channel_with_workspaces/add_workspace_dropdown.tsx +++ b/webapp/channels/src/components/channel_settings_modal/share_channel_with_workspaces/add_workspace_dropdown.tsx @@ -97,10 +97,11 @@ export default function AddWorkspaceDropdown({ disabled={true} /> )} + {/* A remote cluster is keyed on (remote_id, name), so remote_id alone can repeat here. */} {!loading && available.map((rc) => ( {rc.display_name || rc.name}} onClick={() => handleSelect(rc)} /> diff --git a/webapp/channels/src/components/suggestion/suggestion.tsx b/webapp/channels/src/components/suggestion/suggestion.tsx index fa84052eca0c..e8b10effe973 100644 --- a/webapp/channels/src/components/suggestion/suggestion.tsx +++ b/webapp/channels/src/components/suggestion/suggestion.tsx @@ -36,6 +36,10 @@ const SuggestionContainer = React.forwardRef { e.preventDefault(); diff --git a/webapp/channels/src/components/widgets/tag/tag.tsx b/webapp/channels/src/components/widgets/tag/tag.tsx index cdedf4b7c129..33039b2deb73 100644 --- a/webapp/channels/src/components/widgets/tag/tag.tsx +++ b/webapp/channels/src/components/widgets/tag/tag.tsx @@ -127,7 +127,7 @@ const TagText = styled.span` text-overflow: ellipsis; `; -const Tag = ({ +const Tag = React.forwardRef(({ variant, onClick, className, @@ -136,7 +136,7 @@ const Tag = ({ size = 'xs', uppercase = false, ...rest -}: Props) => { +}, ref) => { const Icon = iconName ? glyphMap[iconName] : null; const element = onClick ? 'button' : 'div'; @@ -157,6 +157,10 @@ const Tag = ({ return ( } as={element} uppercase={uppercase} onClick={onClick} @@ -166,6 +170,7 @@ const Tag = ({ {text} ); -}; +}); +Tag.displayName = 'Tag'; export default memo(Tag); diff --git a/webapp/channels/src/entry.tsx b/webapp/channels/src/entry.tsx index db0c05714814..3a9bc2675625 100644 --- a/webapp/channels/src/entry.tsx +++ b/webapp/channels/src/entry.tsx @@ -32,7 +32,9 @@ declare global { // This runs before we start to render anything. function preRenderSetup(onPreRenderSetupReady: () => void) { window.onerror = (msg, url, line, column, error) => { - if (msg === 'ResizeObserver loop limit exceeded') { + // Benign Chromium ResizeObserver noise (Monaco automaticLayout, etc.). + // Covers both "loop limit exceeded" and "loop completed with undelivered notifications." + if (typeof msg === 'string' && msg.startsWith('ResizeObserver loop')) { return; }