Skip to content

Commit 8e64aed

Browse files
souravcrlclaude
andcommitted
sql: add VIEWSCHEDULE system privilege for SHOW SCHEDULES
Previously, `SHOW SCHEDULES` was implicitly gated by `SELECT` on `system.scheduled_jobs` and `system.jobs`, which only `admin` has by default. This made it impossible to grant schedule visibility to non-admin users without granting broad access to system tables. This change introduces a new `VIEWSCHEDULE` system privilege (following the pattern of `VIEWJOB`, `VIEWACTIVITY`, etc.) that explicitly gates `SHOW SCHEDULES`. Admin users satisfy this check implicitly through `ALL` privileges. The privilege also grants implicit `SELECT` on `system.scheduled_jobs` and `system.jobs` so the delegated query executes successfully. The delegator check is kept (rather than relying solely on the authorization.go fallback) because without it the user would see a confusing "does not have SELECT privilege on relation scheduled_jobs" error instead of a clear message about the VIEWSCHEDULE privilege. On the DB Console side, the schedules API now checks for SQL execution errors before accessing results, preventing runtime crashes when the user lacks the required privilege. Fixes: #169420 Epic: none Release note (sql change): Added a new `VIEWSCHEDULE` system privilege that controls access to `SHOW SCHEDULES`. Non-admin users can be granted schedule visibility via `GRANT SYSTEM VIEWSCHEDULE TO <user>` without needing direct `SELECT` on system tables. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fc74b99 commit 8e64aed

9 files changed

Lines changed: 146 additions & 2 deletions

File tree

pkg/server/privchecker/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ go_test(
3939
"//pkg/server",
4040
"//pkg/sql",
4141
"//pkg/sql/isql",
42+
"//pkg/sql/privilege",
4243
"//pkg/testutils/serverutils",
4344
"//pkg/testutils/sqlutils",
4445
"//pkg/testutils/testcluster",

pkg/server/privchecker/privchecker_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/cockroachdb/cockroach/pkg/server/privchecker"
1616
"github.com/cockroachdb/cockroach/pkg/sql"
1717
"github.com/cockroachdb/cockroach/pkg/sql/isql"
18+
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
1819
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
1920
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
2021
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
@@ -56,6 +57,8 @@ func TestAdminPrivilegeChecker(t *testing.T) {
5657
sqlDB.Exec(t, "GRANT SYSTEM VIEWCLUSTERMETADATA TO withviewclustermetadata")
5758
sqlDB.Exec(t, "CREATE USER withviewdebug")
5859
sqlDB.Exec(t, "GRANT SYSTEM VIEWDEBUG TO withviewdebug")
60+
sqlDB.Exec(t, "CREATE USER withviewschedule")
61+
sqlDB.Exec(t, "GRANT SYSTEM VIEWSCHEDULE TO withviewschedule")
5962

6063
execCfg := ts.ExecutorConfig().(sql.ExecutorConfig)
6164
kvDB := ts.DB()
@@ -97,6 +100,25 @@ func TestAdminPrivilegeChecker(t *testing.T) {
97100
withVaAndRedactedGlobalPrivilege := username.MakeSQLUsernameFromPreNormalizedString("withvaandredactedglobalprivilege")
98101
withviewclustermetadata := username.MakeSQLUsernameFromPreNormalizedString("withviewclustermetadata")
99102
withViewDebug := username.MakeSQLUsernameFromPreNormalizedString("withviewdebug")
103+
withViewSchedule := username.MakeSQLUsernameFromPreNormalizedString("withviewschedule")
104+
105+
// Test HasGlobalPrivilege for VIEWSCHEDULE.
106+
globalPrivTests := []struct {
107+
name string
108+
privilege privilege.Kind
109+
username username.SQLUsername
110+
wantHas bool
111+
}{
112+
{"viewschedule-granted", privilege.VIEWSCHEDULE, withViewSchedule, true},
113+
{"viewschedule-not-granted", privilege.VIEWSCHEDULE, withoutPrivs, false},
114+
}
115+
for _, tt := range globalPrivTests {
116+
t.Run(tt.name, func(t *testing.T) {
117+
has, err := underTest.HasGlobalPrivilege(ctx, tt.username, tt.privilege)
118+
require.NoError(t, err)
119+
require.Equal(t, tt.wantHas, has)
120+
})
121+
}
100122

101123
tests := []struct {
102124
name string

pkg/sql/authorization.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,20 @@ func (p *planner) CheckPrivilegeForUser(
272272
if hasViewSystemTablePriv {
273273
return nil
274274
}
275+
// VIEWSCHEDULE grants implicit SELECT on system.scheduled_jobs and
276+
// system.jobs, the two tables underlying SHOW SCHEDULES.
277+
tableID := d.GetID()
278+
if tableID == keys.ScheduledJobsTableID || tableID == keys.JobsTableID {
279+
hasViewSchedule, err := p.HasPrivilege(
280+
ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWSCHEDULE, user,
281+
)
282+
if err != nil {
283+
return err
284+
}
285+
if hasViewSchedule {
286+
return nil
287+
}
288+
}
275289
}
276290
}
277291
return insufficientPrivilegeError(user, privilegeKind, privilegeObject)

pkg/sql/delegate/show_schedules.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,35 @@ import (
1010
"strings"
1111

1212
"github.com/cockroachdb/cockroach/pkg/jobs"
13+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
14+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
15+
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
1316
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
1417
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
18+
"github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege"
1519
)
1620

1721
// commandColumn converts executor execution arguments into jsonb representation.
1822
const commandColumn = `crdb_internal.pb_to_json('cockroach.jobs.jobspb.ExecutionArguments', execution_args, false, true)->'args'`
1923

2024
func (d *delegator) delegateShowSchedules(n *tree.ShowSchedules) (tree.Statement, error) {
25+
// Provide a clear privilege error upfront. Without this check, the
26+
// delegated query would fail with "does not have SELECT privilege on
27+
// relation scheduled_jobs", which doesn't tell the user what privilege
28+
// they actually need. The authorization.go layer grants implicit SELECT
29+
// on the underlying system tables when VIEWSCHEDULE is held.
30+
if err := d.catalog.CheckPrivilege(
31+
d.ctx, syntheticprivilege.GlobalPrivilegeObject,
32+
d.catalog.GetCurrentUser(), privilege.VIEWSCHEDULE,
33+
); err != nil {
34+
if pgerror.GetPGCode(err) == pgcode.InsufficientPrivilege {
35+
return nil, pgerror.Newf(pgcode.InsufficientPrivilege,
36+
"user %s does not have %s system privilege",
37+
d.catalog.GetCurrentUser(), privilege.VIEWSCHEDULE)
38+
}
39+
return nil, err
40+
}
41+
2142
sqltelemetry.IncrementShowCounter(sqltelemetry.Schedules)
2243

2344
columnExprs := []string{
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# LogicTest: local
2+
3+
# Verify that SHOW SCHEDULES requires the VIEWSCHEDULE privilege.
4+
5+
# testuser should not be able to SHOW SCHEDULES.
6+
user testuser
7+
8+
statement error user testuser does not have VIEWSCHEDULE system privilege
9+
SHOW SCHEDULES
10+
11+
user root
12+
13+
# Grant VIEWSCHEDULE to testuser.
14+
statement ok
15+
GRANT SYSTEM VIEWSCHEDULE TO testuser
16+
17+
user testuser
18+
19+
# testuser should now be able to SHOW SCHEDULES.
20+
statement ok
21+
SHOW SCHEDULES
22+
23+
user root
24+
25+
# Revoke VIEWSCHEDULE from testuser.
26+
statement ok
27+
REVOKE SYSTEM VIEWSCHEDULE FROM testuser
28+
29+
user testuser
30+
31+
# testuser should no longer be able to SHOW SCHEDULES.
32+
statement error user testuser does not have VIEWSCHEDULE system privilege
33+
SHOW SCHEDULES
34+
35+
user root
36+
37+
# Test that inheriting VIEWSCHEDULE through a role works.
38+
statement ok
39+
CREATE ROLE scheduleviewer
40+
41+
statement ok
42+
GRANT SYSTEM VIEWSCHEDULE TO scheduleviewer
43+
44+
statement ok
45+
GRANT scheduleviewer TO testuser
46+
47+
user testuser
48+
49+
# testuser should be able to SHOW SCHEDULES through role membership.
50+
statement ok
51+
SHOW SCHEDULES
52+
53+
user root
54+
55+
statement ok
56+
REVOKE scheduleviewer FROM testuser
57+
58+
user testuser
59+
60+
# testuser should no longer be able to SHOW SCHEDULES.
61+
statement error user testuser does not have VIEWSCHEDULE system privilege
62+
SHOW SCHEDULES

pkg/sql/logictest/tests/local/generated_test.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/sql/privilege/kind.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ const (
7373
BUILTIN_UNSAFE_ALLOWED Kind = 42
7474
MAINTAIN Kind = 43
7575
TEMPORARY Kind = 44
76-
largestKind = TEMPORARY
76+
VIEWSCHEDULE Kind = 45
77+
largestKind = VIEWSCHEDULE
7778

7879
// RULE, SET, and ALTERSYSTEM are PostgreSQL ACL-only pseudo-privileges.
7980
// They exist solely for ACL character mapping used by acldefault, aclexplode,
@@ -187,6 +188,8 @@ func (k Kind) InternalKey() KindInternalKey {
187188
return "MAINTAIN"
188189
case TEMPORARY:
189190
return "TEMPORARY"
191+
case VIEWSCHEDULE:
192+
return "VIEWSCHEDULE"
190193
case SET:
191194
return "SET"
192195
case ALTERSYSTEM:

pkg/sql/privilege/privilege.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ var (
101101
ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED,
102102
VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS, VIEWJOB,
103103
MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGEVIRTUALCLUSTER, VIEWSYSTEMTABLE, CREATEROLE, CREATELOGIN, CREATEDB, CONTROLJOB,
104-
REPAIRCLUSTER, BYPASSRLS, REPLICATIONDEST, REPLICATIONSOURCE, INSPECT,
104+
REPAIRCLUSTER, BYPASSRLS, REPLICATIONDEST, REPLICATIONSOURCE, INSPECT, VIEWSCHEDULE,
105105
}
106106
VirtualTablePrivileges = List{ALL, SELECT}
107107
ExternalConnectionPrivileges = List{ALL, USAGE, DROP, UPDATE}

pkg/ui/workspaces/cluster-ui/src/api/schedulesApi.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
executeInternalSql,
1313
LARGE_RESULT_SIZE,
1414
SqlExecutionRequest,
15+
sqlApiErrorMessage,
1516
sqlResultsAreEmpty,
1617
} from "./sqlApi";
1718

@@ -78,6 +79,9 @@ export function getSchedules(req: {
7879
execute: true,
7980
};
8081
return executeInternalSql<ScheduleColumns>(request).then(result => {
82+
if (result.error) {
83+
throw new Error(sqlApiErrorMessage(result.error.message));
84+
}
8185
const txnResults = result.execution.txn_results;
8286
if (sqlResultsAreEmpty(result)) {
8387
// No data.
@@ -122,6 +126,9 @@ export function getSchedule(id: Long): Promise<Schedule> {
122126
execute: true,
123127
};
124128
return executeInternalSql<ScheduleColumns>(request).then(result => {
129+
if (result.error) {
130+
throw new Error(sqlApiErrorMessage(result.error.message));
131+
}
125132
const txnResults = result.execution.txn_results;
126133
if (txnResults.length === 0 || !txnResults[0].rows) {
127134
// No data.

0 commit comments

Comments
 (0)