sql: add system privileges to error messages in statement stats tables

This commit implements the VIEWACTIVITY and VIEWACTIVITYREDACTED system
privileges to the crdb_internal.node_statement_statistics table to
provide fine-grained permissions to view the error message for failed
statements.

Release note (sql change): the crdb_interanal.node_statement_statistics
table redact the error message if the user has VIEWACTIVITYREDACTED,
and do not redact the error message if the user has VIEWACTIVITY. If the
user has both, VIEWACTIVITYREDACTED takes precedence and the last error
is redacted.

Author: gtr

pkg/ccl/logictestccl/tests/3node-tenant/generated_test.go

@@ -1516,12 +1516,24 @@ CREATE TABLE crdb_internal.node_statement_statistics (
   latency_seconds_p99 FLOAT
 )`,
 	populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error {
-		hasViewActivityOrViewActivityRedacted, err := p.HasViewActivityOrViewActivityRedactedRole(ctx)
-		if err != nil {
+		shouldRedactError := false
+		// Check if the user is admin.
+		if isAdmin, err := p.HasAdminRole(ctx); err != nil {
 			return err
-		}
-		if !hasViewActivityOrViewActivityRedacted {
-			return noViewActivityOrViewActivityRedactedRoleError(p.User())
+		} else if !isAdmin {
+			// If the user is not admin, check the individual VIEWACTIVITY and VIEWACTIVITYREDACTED
+			// privileges.
+			if hasViewActivityRedacted, err := p.HasViewActivityRedacted(ctx); err != nil {
+				return err
+			} else if hasViewActivityRedacted {
+				shouldRedactError = true
+			} else if hasViewActivity, err := p.HasViewActivity(ctx); err != nil {
+				return err
+			} else if !hasViewActivity {
+				// If the user is not admin and does not have VIEWACTIVITY or VIEWACTIVITYREDACTED,
+				// return insufficient privileges error.
+				return noViewActivityOrViewActivityRedactedRoleError(p.User())
+			}
 		}
 
 		var alloc tree.DatumAlloc
@@ -1530,8 +1542,12 @@ CREATE TABLE crdb_internal.node_statement_statistics (
 
 		statementVisitor := func(_ context.Context, stats *appstatspb.CollectedStatementStatistics) error {
 			errString := tree.DNull
-			if stats.Stats.SensitiveInfo.LastErr != "" {
-				errString = alloc.NewDString(tree.DString(stats.Stats.SensitiveInfo.LastErr))
+			if shouldRedactError {
+				errString = alloc.NewDString(tree.DString("<redacted>"))
+			} else {
+				if stats.Stats.SensitiveInfo.LastErr != "" {
+					errString = alloc.NewDString(tree.DString(stats.Stats.SensitiveInfo.LastErr))
+				}
 			}
 
 			errCode := tree.DNull

pkg/sql/crdb_internal.go

@@ -6,14 +6,16 @@
 # by CRDB ("$ internal-migration-manager-find-jobs" for example) since they can be flaky. We are
 # also ordering by "last_error_code" to maintain consistency.
 
+user root
+
 # Test 1: division by zero. Error code "22012" should be added.
 statement error division by zero
 SELECT 2/0;
 
-query T
-SELECT last_error_code FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+query TT
+SELECT last_error_code, last_error FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
 ----
-22012
+22012  division by zero
 
 # Test 2: database does not exist. Error code "3D000" should be added.
 query TTTTTT colnames,rowsort
@@ -28,34 +30,34 @@ test           root   NULL            NULL              {}       NULL
 statement error pq: database "posgres" does not exist
 use posgres
 
-query T
-SELECT last_error_code FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+query TT
+SELECT last_error_code, last_error FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
 ----
-22012
-3D000
+22012  division by zero
+3D000  database "posgres" does not exist
 
 # Test 3: Nonexistant user. Error code "42704" should be added.
 statement error pq: role/user "who" does not exist
 ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES to who
 
-query T
-SELECT last_error_code FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+query TT
+SELECT last_error_code, last_error FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
 ----
-22012
-3D000
-42704
+22012  division by zero
+3D000  database "posgres" does not exist
+42704  role/user "who" does not exist 
 
 # Test 4: Insufficient privilege. Error code "42501" should be added.
 statement error schema cannot be modified: "crdb_internal"
 CREATE TABLE crdb_internal.example (abc INT)
 
-query T
-SELECT last_error_code FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+query TT
+SELECT last_error_code, last_error FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
 ----
-22012
-3D000
-42501
-42704
+22012  division by zero
+3D000  database "posgres" does not exist
+42501  schema cannot be modified: "crdb_internal"
+42704  role/user "who" does not exist
 
 # Test 5: Foreign key violation. Error code "23503" should be added.
 statement ok
@@ -67,11 +69,11 @@ INSERT INTO dst(x) VALUES ('example')
 statement error foreign key
 UPDATE dst SET x = 'xyz'
 
-query T
-SELECT last_error_code FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+query TT
+SELECT last_error_code, last_error   FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
 ----
-22012
-23503
-3D000
-42501
-42704
+22012  division by zero
+23503  update on table "dst" violates foreign key constraint "dst_x_fkey"
+3D000  database "posgres" does not exist
+42501  schema cannot be modified: "crdb_internal"
+42704  role/user "who" does not exist

pkg/sql/logictest/testdata/logic_test/statement_statistics_errors

@@ -0,0 +1,96 @@
+# Check that error messages and error codes are being surfaced correctly
+#
+# For these tests, a statement containing an intentional error is made and the
+# crdb_internal.node_statement_statistics virtual table is queried to validate that the error code
+# is recorded. When querying the node statistics table, we are ignoring statements made internally
+# by CRDB ("$ internal-migration-manager-find-jobs" for example) since they can be flaky. We are
+# also ordering by "last_error_code" to maintain consistency. Here we are also testing that the
+# error message is redacted for the VIEWACTIVITYREDACTED privilege.
+
+
+# Grant testuser the VIEWACTIVITYREDACTED system privilege.
+statement ok
+GRANT SYSTEM VIEWACTIVITYREDACTED TO testuser;
+
+query TTB
+SHOW SYSTEM GRANTS
+----
+testuser  VIEWACTIVITYREDACTED  false
+
+
+# Switch to testuser
+user testuser
+
+# Test 1: division by zero. Error code "22012" should be added.
+statement error division by zero
+SELECT 2/0;
+
+query TT
+SELECT last_error_code, last_error FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+----
+22012  <redacted>
+
+# Test 2: database does not exist. Error code "3D000" should be added.
+query TTTTTT colnames,rowsort
+SHOW DATABASES
+----
+database_name  owner  primary_region  secondary_region  regions  survival_goal
+defaultdb      root   NULL            NULL              {}       NULL
+postgres       root   NULL            NULL              {}       NULL
+test           root   NULL            NULL              {}       NULL
+
+statement error pq: database "posgres" does not exist
+use posgres
+
+query TT
+SELECT last_error_code, last_error FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+----
+22012  <redacted>
+3D000  <redacted>
+
+# Test 3: Nonexistant user. Error code "42704" should be added.
+statement error pq: role/user "who" does not exist
+ALTER DEFAULT PRIVILEGES GRANT SELECT ON TABLES to who
+
+query TT
+SELECT last_error_code, last_error FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+----
+22012  <redacted>
+3D000  <redacted>
+42704  <redacted>
+
+# Test 4: Give testuser the VIEWACTIVITY system privilege and remove the VIEWACTIVITYREDACTED privilege.
+user root
+
+statement ok
+GRANT SYSTEM VIEWACTIVITY TO testuser
+
+statement ok
+REVOKE SYSTEM VIEWACTIVITYREDACTED FROM testuser
+
+user testuser
+
+# Now the error message should not be redacted.
+
+query TT
+SELECT last_error_code, last_error FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+----
+22012  division by zero
+3D000  database "posgres" does not exist
+42704  role/user "who" does not exist
+
+# Test 5: Give tesuser the VIEWACTIVITYREDACTED privilege again. This time, the error message should be redacted,
+# as VIEWACTIVITYREDACTED takes precedence over VIEWACTIVITY.
+user root
+
+statement ok
+GRANT SYSTEM VIEWACTIVITYREDACTED TO testuser
+
+user testuser
+
+query TT
+SELECT last_error_code, last_error FROM crdb_internal.node_statement_statistics WHERE last_error_code!='NULL' AND application_name NOT LIKE '$ %' ORDER BY last_error_code ASC;
+----
+22012  <redacted>
+3D000  <redacted>
+42704  <redacted>