From 0586f178899641f9b6542d3fd5013d5809661db6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 29 May 2026 20:15:34 +0000 Subject: [PATCH] feat(coderd/database): add DeleteMCPServerUserHeaderValuesByConfigID query Admin-only query used by the MCP server config update path to clear every user's stored user-set header values when the auth_type changes away from custom_headers or the custom_headers_user_keys list is altered. Without this, stale per-user credentials silently reactivate if the previous key set is later restored. --- coderd/database/dbauthz/dbauthz.go | 11 +++++++++++ coderd/database/dbauthz/dbauthz_test.go | 5 +++++ coderd/database/dbmetrics/querymetrics.go | 8 ++++++++ coderd/database/dbmock/dbmock.go | 14 ++++++++++++++ coderd/database/querier.go | 5 +++++ coderd/database/queries.sql.go | 16 ++++++++++++++++ coderd/database/queries/mcpserverconfigs.sql | 10 ++++++++++ 7 files changed, 69 insertions(+) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index abb8af1d77..909d1f6400 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2121,6 +2121,17 @@ func (q *querier) DeleteMCPServerUserHeaderValues(ctx context.Context, arg datab return fetchAndExec(q.log, q.auth, policy.ActionUpdatePersonal, fetch, q.db.DeleteMCPServerUserHeaderValues)(ctx, arg) } +func (q *querier) DeleteMCPServerUserHeaderValuesByConfigID(ctx context.Context, mcpServerConfigID uuid.UUID) error { + // Admin-only operation. Called from the admin MCP server config + // update path when auth_type or custom_headers_user_keys changes, + // so stale per-user header values do not silently reactivate when + // the key set is restored. + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil { + return err + } + return q.db.DeleteMCPServerUserHeaderValuesByConfigID(ctx, mcpServerConfigID) +} + func (q *querier) DeleteMCPServerUserToken(ctx context.Context, arg database.DeleteMCPServerUserTokenParams) error { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil { return err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index cf4b646f97..9b2917ae6c 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1697,6 +1697,11 @@ func (s *MethodTestSuite) TestChats() { dbm.EXPECT().DeleteMCPServerUserHeaderValues(gomock.Any(), arg).Return(nil).AnyTimes() check.Args(arg).Asserts(value, policy.ActionUpdatePersonal).Returns() })) + s.Run("DeleteMCPServerUserHeaderValuesByConfigID", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + id := uuid.New() + dbm.EXPECT().DeleteMCPServerUserHeaderValuesByConfigID(gomock.Any(), id).Return(nil).AnyTimes() + check.Args(id).Asserts(rbac.ResourceDeploymentConfig, policy.ActionUpdate).Returns() + })) s.Run("InsertMCPServerConfig", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { arg := database.InsertMCPServerConfigParams{ DisplayName: "Test MCP Server", diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 0e7a333cc0..f980a20cb7 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -585,6 +585,14 @@ func (m queryMetricsStore) DeleteMCPServerUserHeaderValues(ctx context.Context, return r0 } +func (m queryMetricsStore) DeleteMCPServerUserHeaderValuesByConfigID(ctx context.Context, mcpServerConfigID uuid.UUID) error { + start := time.Now() + r0 := m.s.DeleteMCPServerUserHeaderValuesByConfigID(ctx, mcpServerConfigID) + m.queryLatencies.WithLabelValues("DeleteMCPServerUserHeaderValuesByConfigID").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "DeleteMCPServerUserHeaderValuesByConfigID").Inc() + return r0 +} + func (m queryMetricsStore) DeleteMCPServerUserToken(ctx context.Context, arg database.DeleteMCPServerUserTokenParams) error { start := time.Now() r0 := m.s.DeleteMCPServerUserToken(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index bb09bdb26c..b981e243b6 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -974,6 +974,20 @@ func (mr *MockStoreMockRecorder) DeleteMCPServerUserHeaderValues(ctx, arg any) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteMCPServerUserHeaderValues", reflect.TypeOf((*MockStore)(nil).DeleteMCPServerUserHeaderValues), ctx, arg) } +// DeleteMCPServerUserHeaderValuesByConfigID mocks base method. +func (m *MockStore) DeleteMCPServerUserHeaderValuesByConfigID(ctx context.Context, mcpServerConfigID uuid.UUID) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteMCPServerUserHeaderValuesByConfigID", ctx, mcpServerConfigID) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteMCPServerUserHeaderValuesByConfigID indicates an expected call of DeleteMCPServerUserHeaderValuesByConfigID. +func (mr *MockStoreMockRecorder) DeleteMCPServerUserHeaderValuesByConfigID(ctx, mcpServerConfigID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteMCPServerUserHeaderValuesByConfigID", reflect.TypeOf((*MockStore)(nil).DeleteMCPServerUserHeaderValuesByConfigID), ctx, mcpServerConfigID) +} + // DeleteMCPServerUserToken mocks base method. func (m *MockStore) DeleteMCPServerUserToken(ctx context.Context, arg database.DeleteMCPServerUserTokenParams) error { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 312b7deddc..6ba5435732 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -139,6 +139,11 @@ type sqlcQuerier interface { DeleteLicense(ctx context.Context, id int32) (int32, error) DeleteMCPServerConfigByID(ctx context.Context, id uuid.UUID) error DeleteMCPServerUserHeaderValues(ctx context.Context, arg DeleteMCPServerUserHeaderValuesParams) error + // Deletes every user's stored header values for the given MCP server + // config. Use when the admin changes auth_type away from custom_headers + // or alters custom_headers_user_keys so stale credentials do not + // silently reactivate when the key set is restored. + DeleteMCPServerUserHeaderValuesByConfigID(ctx context.Context, mcpServerConfigID uuid.UUID) error DeleteMCPServerUserToken(ctx context.Context, arg DeleteMCPServerUserTokenParams) error DeleteOAuth2ProviderAppByClientID(ctx context.Context, id uuid.UUID) error DeleteOAuth2ProviderAppByID(ctx context.Context, id uuid.UUID) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 14e46530b2..b53cd07702 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -15057,6 +15057,22 @@ func (q *sqlQuerier) DeleteMCPServerUserHeaderValues(ctx context.Context, arg De return err } +const deleteMCPServerUserHeaderValuesByConfigID = `-- name: DeleteMCPServerUserHeaderValuesByConfigID :exec +DELETE FROM + mcp_server_user_header_values +WHERE + mcp_server_config_id = $1::uuid +` + +// Deletes every user's stored header values for the given MCP server +// config. Use when the admin changes auth_type away from custom_headers +// or alters custom_headers_user_keys so stale credentials do not +// silently reactivate when the key set is restored. +func (q *sqlQuerier) DeleteMCPServerUserHeaderValuesByConfigID(ctx context.Context, mcpServerConfigID uuid.UUID) error { + _, err := q.db.ExecContext(ctx, deleteMCPServerUserHeaderValuesByConfigID, mcpServerConfigID) + return err +} + const deleteMCPServerUserToken = `-- name: DeleteMCPServerUserToken :exec DELETE FROM mcp_server_user_tokens diff --git a/coderd/database/queries/mcpserverconfigs.sql b/coderd/database/queries/mcpserverconfigs.sql index 192c8a71f0..57a290e731 100644 --- a/coderd/database/queries/mcpserverconfigs.sql +++ b/coderd/database/queries/mcpserverconfigs.sql @@ -260,6 +260,16 @@ WHERE mcp_server_config_id = @mcp_server_config_id::uuid AND user_id = @user_id::uuid; +-- name: DeleteMCPServerUserHeaderValuesByConfigID :exec +-- Deletes every user's stored header values for the given MCP server +-- config. Use when the admin changes auth_type away from custom_headers +-- or alters custom_headers_user_keys so stale credentials do not +-- silently reactivate when the key set is restored. +DELETE FROM + mcp_server_user_header_values +WHERE + mcp_server_config_id = @mcp_server_config_id::uuid; + -- name: CleanupDeletedMCPServerIDsFromChats :exec UPDATE chats SET mcp_server_ids = (