Skip to content

Commit 4198d37

Browse files
souravcrlclaude
andcommitted
sql: gate SHOW SCHEDULES behind VIEWJOB system privilege
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 gates `SHOW SCHEDULES` behind the existing `VIEWJOB` system privilege (which already controls job visibility and is a natural fit for schedule visibility). 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 performs an upfront privilege check (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 VIEWJOB privilege. The check properly distinguishes InsufficientPrivilege from transient errors to avoid swallowing operational failures. 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): `SHOW SCHEDULES` now requires the `VIEWJOB` system privilege. Non-admin users can be granted schedule visibility via `GRANT SYSTEM VIEWJOB 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 4198d37

7 files changed

Lines changed: 167 additions & 0 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 withviewjob")
61+
sqlDB.Exec(t, "GRANT SYSTEM VIEWJOB TO withviewjob")
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+
withViewJob := username.MakeSQLUsernameFromPreNormalizedString("withviewjob")
104+
105+
// Test HasGlobalPrivilege for VIEWJOB.
106+
globalPrivTests := []struct {
107+
name string
108+
privilege privilege.Kind
109+
username username.SQLUsername
110+
wantHas bool
111+
}{
112+
{"viewjob-granted", privilege.VIEWJOB, withViewJob, true},
113+
{"viewjob-not-granted", privilege.VIEWJOB, 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+
// VIEWJOB 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+
hasViewJob, err := p.HasPrivilege(
280+
ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWJOB, user,
281+
)
282+
if err != nil {
283+
return err
284+
}
285+
if hasViewJob {
286+
return nil
287+
}
288+
}
275289
}
276290
}
277291
return insufficientPrivilegeError(user, privilegeKind, privilegeObject)

pkg/sql/delegate/show_schedules.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,45 @@ 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 VIEWJOB or VIEWSYSTEMTABLE
30+
// is held, so we let those users through.
31+
cat := d.catalog
32+
user := cat.GetCurrentUser()
33+
globalPrivObj := syntheticprivilege.GlobalPrivilegeObject
34+
hasViewJob := false
35+
if err := cat.CheckPrivilege(d.ctx, globalPrivObj, user, privilege.VIEWJOB); err == nil {
36+
hasViewJob = true
37+
} else if pgerror.GetPGCode(err) != pgcode.InsufficientPrivilege {
38+
return nil, err
39+
}
40+
if !hasViewJob {
41+
if err := cat.CheckPrivilege(d.ctx, globalPrivObj, user, privilege.VIEWSYSTEMTABLE); err == nil {
42+
// VIEWSYSTEMTABLE grants access too.
43+
} else if pgerror.GetPGCode(err) != pgcode.InsufficientPrivilege {
44+
return nil, err
45+
} else {
46+
return nil, pgerror.Newf(pgcode.InsufficientPrivilege,
47+
"user %s does not have %s system privilege",
48+
user, privilege.VIEWJOB)
49+
}
50+
}
51+
2152
sqltelemetry.IncrementShowCounter(sqltelemetry.Schedules)
2253

2354
columnExprs := []string{
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
# LogicTest: local
2+
3+
# Verify that SHOW SCHEDULES requires the VIEWJOB privilege.
4+
5+
# testuser should not be able to SHOW SCHEDULES.
6+
user testuser
7+
8+
statement error user testuser does not have VIEWJOB system privilege
9+
SHOW SCHEDULES
10+
11+
user root
12+
13+
# Grant VIEWJOB to testuser.
14+
statement ok
15+
GRANT SYSTEM VIEWJOB 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 VIEWJOB from testuser.
26+
statement ok
27+
REVOKE SYSTEM VIEWJOB 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 VIEWJOB system privilege
33+
SHOW SCHEDULES
34+
35+
user root
36+
37+
# Test that inheriting VIEWJOB through a role works.
38+
statement ok
39+
CREATE ROLE scheduleviewer
40+
41+
statement ok
42+
GRANT SYSTEM VIEWJOB 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 VIEWJOB system privilege
62+
SHOW SCHEDULES
63+
64+
user root
65+
66+
# Test that VIEWSYSTEMTABLE also grants access to SHOW SCHEDULES (no
67+
# regression for users who previously relied on this broader privilege).
68+
statement ok
69+
GRANT SYSTEM VIEWSYSTEMTABLE TO testuser
70+
71+
user testuser
72+
73+
statement ok
74+
SHOW SCHEDULES
75+
76+
user root
77+
78+
statement ok
79+
REVOKE SYSTEM VIEWSYSTEMTABLE FROM testuser
80+
81+
user testuser
82+
83+
statement error user testuser does not have VIEWJOB system privilege
84+
SHOW SCHEDULES

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

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

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)