diff --git a/pkg/cmd/mirror/go/mirror.go b/pkg/cmd/mirror/go/mirror.go index a39189f301e5..82cd4b545656 100644 --- a/pkg/cmd/mirror/go/mirror.go +++ b/pkg/cmd/mirror/go/mirror.go @@ -157,7 +157,12 @@ func downloadZips( cmd.Dir = tmpdir jsonBytes, err := cmd.Output() if err != nil { - return nil, err + var stderr []byte + if exitErr := (*exec.ExitError)(nil); errors.As(err, &exitErr) { + stderr = exitErr.Stderr + } + return nil, fmt.Errorf("failed to run go with arguments %+v; got stdout %s, stderr %s; %w", + downloadArgs, string(jsonBytes), string(stderr), err) } var jsonBuilder strings.Builder ret := make(map[string]downloadedModule) @@ -184,7 +189,12 @@ func listAllModules(tmpdir string) (map[string]listedModule, error) { cmd.Dir = tmpdir jsonBytes, err := cmd.Output() if err != nil { - return nil, err + var stderr []byte + if exitErr := (*exec.ExitError)(nil); errors.As(err, &exitErr) { + stderr = exitErr.Stderr + } + return nil, fmt.Errorf("failed to run `go list -mod=readonly -m -json all`; got stdout %s, stderr %s; %w", + string(jsonBytes), string(stderr), err) } ret := make(map[string]listedModule) var jsonBuilder strings.Builder diff --git a/pkg/cmd/roachtest/tests/unoptimized_query_oracle.go b/pkg/cmd/roachtest/tests/unoptimized_query_oracle.go index 7bea9cabb011..1c6dd4566fd3 100644 --- a/pkg/cmd/roachtest/tests/unoptimized_query_oracle.go +++ b/pkg/cmd/roachtest/tests/unoptimized_query_oracle.go @@ -120,10 +120,19 @@ func runUnoptimizedQueryOracleImpl( if err := h.execStmt(disableVectorizeStmt); err != nil { return h.makeError(err, "failed to disable the vectorized engine") } + disableDistSQLStmt := "SET distsql = off" + if err := h.execStmt(disableDistSQLStmt); err != nil { + return h.makeError(err, "failed to disable DistSQL") + } unoptimizedRows, err := h.runQuery(stmt) if err != nil { - // Skip unoptimized statements that fail with an error. + // Skip unoptimized statements that fail with an error (unless it's an + // internal error). + if es := err.Error(); strings.Contains(es, "internal error") { + verboseLogging = true + return h.makeError(err, "internal error while running unoptimized statement") + } //nolint:returnerrcheck return nil } @@ -151,6 +160,15 @@ func runUnoptimizedQueryOracleImpl( return h.makeError(err, "failed to disable not visible index feature") } } + if roll := rnd.Intn(4); roll > 0 { + distSQLMode := "auto" + if roll == 3 { + distSQLMode = "on" + } + if err := h.execStmt(fmt.Sprintf("SET distsql = %s", distSQLMode)); err != nil { + return h.makeError(err, "failed to re-enable DistSQL") + } + } // Then, rerun the statement with optimization and/or vectorization enabled // and/or not visible index feature disabled. diff --git a/pkg/sql/authorization.go b/pkg/sql/authorization.go index c6ef3253af1e..853d9f11d655 100644 --- a/pkg/sql/authorization.go +++ b/pkg/sql/authorization.go @@ -262,14 +262,32 @@ func (p *planner) CheckPrivilegeForUser( privilegeKind privilege.Kind, user username.SQLUsername, ) error { - ok, err := p.HasPrivilege(ctx, privilegeObject, privilegeKind, user) + hasPriv, err := p.HasPrivilege(ctx, privilegeObject, privilegeKind, user) if err != nil { return err } - if !ok { - return insufficientPrivilegeError(user, privilegeKind, privilegeObject) + if hasPriv { + return nil } - return nil + // Special case for system tables. The VIEWSYSTEMTABLE system privilege is + // equivalent to having SELECT on all system tables. This is because it is not + // possible to dynamically grant SELECT privileges system tables, but in the + // context of support escalations, we need to be able to grant the ability to + // view system tables without granting the entire admin role. + if d, ok := privilegeObject.(catalog.Descriptor); ok { + if catalog.IsSystemDescriptor(d) && privilegeKind == privilege.SELECT { + hasViewSystemTablePriv, err := p.HasPrivilege( + ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWSYSTEMTABLE, user, + ) + if err != nil { + return err + } + if hasViewSystemTablePriv { + return nil + } + } + } + return insufficientPrivilegeError(user, privilegeKind, privilegeObject) } // CheckPrivilege implements the AuthorizationAccessor interface. diff --git a/pkg/sql/logictest/testdata/logic_test/synthetic_privileges b/pkg/sql/logictest/testdata/logic_test/synthetic_privileges index af20fa591a6d..58f25f71d7f5 100644 --- a/pkg/sql/logictest/testdata/logic_test/synthetic_privileges +++ b/pkg/sql/logictest/testdata/logic_test/synthetic_privileges @@ -397,3 +397,29 @@ statement ok REVOKE SYSTEM ALL FROM testuser subtest end + +subtest view_system_table + +user testuser + +# Make sure testuser does not have privileges first. +statement error user testuser does not have SELECT privilege on relation users +SELECT * FROM system.users WHERE username = 'testuser' + +user root + +statement ok +GRANT SYSTEM VIEWSYSTEMTABLE TO testuser + +user testuser + +query TT +SELECT username, "hashedPassword" FROM system.users WHERE username = 'testuser' +---- +testuser NULL + +# testuser still should not have write privileges. +statement error user testuser does not have INSERT privilege on relation users +INSERT INTO system.users VALUES ('cat', null, true, 200) + +subtest end diff --git a/pkg/sql/privilege/kind_string.go b/pkg/sql/privilege/kind_string.go index 3962e9017605..3c951f66323a 100644 --- a/pkg/sql/privilege/kind_string.go +++ b/pkg/sql/privilege/kind_string.go @@ -38,6 +38,7 @@ func _() { _ = x[MODIFYSQLCLUSTERSETTING-28] _ = x[REPLICATION-29] _ = x[MANAGETENANT-30] + _ = x[VIEWSYSTEMTABLE-31] } func (i Kind) String() string { @@ -102,6 +103,8 @@ func (i Kind) String() string { return "REPLICATION" case MANAGETENANT: return "MANAGETENANT" + case VIEWSYSTEMTABLE: + return "VIEWSYSTEMTABLE" default: return "Kind(" + strconv.FormatInt(int64(i), 10) + ")" } diff --git a/pkg/sql/privilege/privilege.go b/pkg/sql/privilege/privilege.go index 5b0f2fa7bf0d..d4fd304f4b2f 100644 --- a/pkg/sql/privilege/privilege.go +++ b/pkg/sql/privilege/privilege.go @@ -71,6 +71,7 @@ const ( MODIFYSQLCLUSTERSETTING Kind = 28 REPLICATION Kind = 29 MANAGETENANT Kind = 30 + VIEWSYSTEMTABLE Kind = 31 ) // Privilege represents a privilege parsed from an Access Privilege Inquiry @@ -149,8 +150,12 @@ var ( // before v22.2 we treated Sequences the same as Tables. This is to avoid making // certain privileges unavailable after upgrade migration. // Note that "CREATE, CHANGEFEED, INSERT, DELETE, ZONECONFIG" are no-op privileges on sequences. - SequencePrivileges = List{ALL, USAGE, SELECT, UPDATE, CREATE, CHANGEFEED, DROP, INSERT, DELETE, ZONECONFIG} - GlobalPrivileges = List{ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS, VIEWJOB, MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT} + SequencePrivileges = List{ALL, USAGE, SELECT, UPDATE, CREATE, CHANGEFEED, DROP, INSERT, DELETE, ZONECONFIG} + GlobalPrivileges = List{ + ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, + VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS, VIEWJOB, + MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT, VIEWSYSTEMTABLE, + } VirtualTablePrivileges = List{ALL, SELECT} ExternalConnectionPrivileges = List{ALL, USAGE, DROP} ) @@ -196,6 +201,7 @@ var ByName = map[string]Kind{ "MODIFYSQLCLUSTERSETTING": MODIFYSQLCLUSTERSETTING, "REPLICATION": REPLICATION, "MANAGETENANT": MANAGETENANT, + "VIEWSYSTEMTABLE": VIEWSYSTEMTABLE, } // List is a list of privileges. diff --git a/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/index.tsx b/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/index.tsx index 19db708c5c62..b6a1bbb867c3 100644 --- a/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/index.tsx +++ b/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodeGraphs/index.tsx @@ -328,7 +328,7 @@ export class NodeGraphs extends React.Component< const nodeSources = selectedNode !== "" ? [selectedNode] : null; const selectedTenant = isSystemTenant(currentTenant) ? getMatchParamByName(match, tenantNameAttr) || "" - : currentTenant; + : undefined; // When "all" is the selected source, some graphs display a line for every // node in the cluster using the nodeIDs collection. However, if a specific // node is already selected, these per-node graphs should only display data