Skip to content

release-25.4: sql: check view owner privileges when selecting from a view#166730

Merged
trunk-io[bot] merged 1 commit intocockroachdb:release-25.4from
shghasemi:backport25.4-164664
Apr 14, 2026
Merged

release-25.4: sql: check view owner privileges when selecting from a view#166730
trunk-io[bot] merged 1 commit intocockroachdb:release-25.4from
shghasemi:backport25.4-164664

Conversation

@shghasemi
Copy link
Copy Markdown
Contributor

@shghasemi shghasemi commented Mar 25, 2026

sql: check view owner privileges when selecting from a view

Previously, when a user selected from a view, privilege checks on the
view's underlying tables were entirely skipped. This diverged
from PostgreSQL, where the view owner's (definer's) privileges are used
to access underlying tables, while the invoker only needs SELECT on the
view itself.

Fixed this by introducing definer privilege checking in the builder:

  1. When building a view's query, set dataSourcePrivilegeUserOverride to
    the view owner so that SELECT privilege on underlying tables is
    checked as the definer. A SkipUnderlyingViewPrivilegeChecks cluster
    setting was added to preserve the current behavior.

  2. Store the definer's username in the privilege dependency key
    (privilegeKey) so that memo re-validation checks the correct user.
    For invoker contexts such as udfs, an empty username is stored and
    re-validation uses the current session user via GetCurrentUser().
    This avoids unnecessary memo invalidation on SET ROLE.

  3. Separate EXECUTE privilege checks from data source privilege checks.
    Views override data source privileges to the view owner, but
    EXECUTE privilege on functions called by the view is still checked
    against the invoker, matching PostgreSQL. SECURITY DEFINER routines
    set both overrides to the routine owner. SECURITY INVOKER routines
    reset the data source override back to the invoker.

Informs: #164426

Release note (bug fix): When selecting from a view,
the view owner's privileges on the underlying tables are not checked.
This means no privilege checks are performed on the underlying tables,
so a view would continue to work even after the owner lost access to the
underlying tables. This also affects row-level security (RLS): the view
invoker's RLS policies are enforced instead of the view owner's. If you
need to enforce privilege checks on the underlying tables, you can set
cluster setting sql.auth.skip_underlying_view_privilege_checks.enabled
to false. This setting was enabled by default to prevent backward
incompatible behavior.

Release justification: Security bug fix. Changes are gated by a cluster setting that is off by default.

@shghasemi shghasemi requested a review from a team March 25, 2026 18:24
@shghasemi shghasemi self-assigned this Mar 25, 2026
@trunk-io
Copy link
Copy Markdown
Contributor

trunk-io Bot commented Mar 25, 2026

😎 Merged successfully - details.

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Mar 25, 2026

Thanks for opening a backport.

Before merging, please confirm that it falls into one of the following categories (select one):

  • Non-production code changes OR fixes for serious issues. Non-production includes test-only changes, build system changes, etc. Serious issues are defined in the policy as correctness, stability, or security issues, data corruption/loss, significant performance regressions, breaking working and widely used functionality, or an inability to detect and debug production issues.
  • Other approved changes. These changes must be gated behind a disabled-by-default feature flag unless there is a strong justification not to. Reference the approved ENGREQ ticket in the PR body (e.g., "Fixes ENGREQ-123").

Add a brief release justification to the PR description explaining your selection.

Also, confirm that the change does not break backward compatibility and complies with all aspects of the backport policy.

All backports must be reviewed by the TL and EM for the owning area.

@blathers-crl blathers-crl Bot added backport Label PR's that are backports to older release branches T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Mar 25, 2026
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Mar 25, 2026

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Mar 25, 2026

✅ PR #166730 is compliant with backport policy

Confidence: high
Critical bug criteria met: [Stability or security issues]
Feature flag detected: Yes
Backward compatible: true
Explanation: The PR is compliant with the backport policy. It addresses a security bug associated with privilege checks when selecting from a view, a deviation from expected PostgreSQL behavior. The security fix involves using the view owner's privileges for underlying tables, which could otherwise lead to unauthorized data access. Moreover, the changes are gated by a new cluster setting (sql.auth.skip_underlying_view_privilege_checks.enabled) which is off by default, satisfying the feature flag requirement of the backport policy. The inclusion of a 'Release justification: Security bug fix' in the PR body further exempts the need for other compliance checks, affirming this change has been scrutinized for its appropriateness in the backport process. The PR modifies primarily production files but also includes test adjustments to validate the changes.

ENGREQ Check Passed: No ENGREQ required (non-production code or serious issues).

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@shghasemi shghasemi force-pushed the backport25.4-164664 branch 2 times, most recently from cc0bd01 to f861c6d Compare March 25, 2026 18:31
@rafiss rafiss changed the title sql: check view owner privileges when selecting from a view release-25.4: sql: check view owner privileges when selecting from a view Mar 25, 2026
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Mar 25, 2026

Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link)

1 similar comment
@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Mar 25, 2026

Detected infrastructure failure (matched: self-hosted runner lost communication with the server). Automatically rerunning failed jobs. (run link)

Previously, when a user selected from a view, privilege checks on the
view's underlying tables were either entirely. This diverged
from PostgreSQL, where the view owner's (definer's) privileges are used
to access underlying tables, while the invoker only needs SELECT on the
view itself.

Fixed this by introducing definer privilege checking in the builder:

1. When building a view's query, set dataSourcePrivilegeUserOverride to
   the view owner so that SELECT privilege on underlying tables is
   checked as the definer. A SkipUnderlyingViewPrivilegeChecks cluster
   setting was added to preserve the current behavior.

2. Store the definer's username in the privilege dependency key
   (privilegeKey) so that memo re-validation checks the correct user.
   For invoker contexts such as udfs, an empty username is stored and
   re-validation uses the current session user via GetCurrentUser().
   This avoids unnecessary memo invalidation on SET ROLE.

3. Separate EXECUTE privilege checks from data source privilege checks.
   Views override data source privileges to the view owner, but
   EXECUTE privilege on functions called by the view is still checked
   against the invoker, matching PostgreSQL. SECURITY DEFINER routines
   set both overrides to the routine owner. SECURITY INVOKER routines
   reset the data source override back to the invoker.

Informs: cockroachdb#164426

Release note (bug fix): When selecting from a view,
the view owner's privileges on the underlying tables are not checked.
This means no privilege checks are performed on the underlying tables,
so a view would continue to work even after the owner lost access to the
underlying tables. This also affects row-level security (RLS): the view
invoker's RLS policies are enforced instead of the view owner's. If you
need to enforce privilege checks on the underlying tables, you can set
cluster setting `sql.auth.skip_underlying_view_privilege_checks.enabled`
to false. This setting was enabled by default to prevent backward
incompatible behavior.
@shghasemi shghasemi force-pushed the backport25.4-164664 branch from f861c6d to 0c4d687 Compare April 10, 2026 19:28
@shghasemi shghasemi marked this pull request as ready for review April 13, 2026 19:54
@shghasemi shghasemi requested a review from a team as a code owner April 13, 2026 19:54
@shghasemi shghasemi requested review from mw5h and removed request for a team April 13, 2026 19:54
Copy link
Copy Markdown
Contributor

@spilchen spilchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, just had the same comment as other backports that we are referencing a future release (26.2).

@spilchen made 2 comments.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on mw5h and shghasemi).


docs/generated/settings/settings.html line 217 at r1 (raw file):

<tr><td><div id="setting-sql-auth-grant-option-inheritance-enabled" class="anchored"><code>sql.auth.grant_option_inheritance.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether the GRANT OPTION for privileges is inherited through role membership</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-auth-public-schema-create-privilege-enabled" class="anchored"><code>sql.auth.public_schema_create_privilege.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether to grant all users the CREATE privileges on the public schema when it is created</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-auth-skip-underlying-view-privilege-checks-enabled" class="anchored"><code>sql.auth.skip_underlying_view_privilege_checks.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>determines whether to skip privilege checks on tables underlying views. When enabled, users with SELECT privileges on a view can query it regardless of their privileges on the underlying tables, and row-level security policies are evaluated as the invoking user rather than the view owner. This restores pre-v26.2 behavior.</td><td>Basic/Standard/Advanced/Self-Hosted</td></tr>

nit: we still reference v26.2 but this is a backport to 25.4

Copy link
Copy Markdown
Contributor

@spilchen spilchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@spilchen made 2 comments and resolved 1 discussion.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on mw5h).


docs/generated/settings/settings.html line 217 at r1 (raw file):

Previously, spilchen wrote…

nit: we still reference v26.2 but this is a backport to 25.4

Clarified here: #166744 (review)

@shghasemi
Copy link
Copy Markdown
Contributor Author

TFTR!

/trunk merge

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 14, 2026

Detected infrastructure failure on trunk-merge branch (matched: self-hosted runner lost communication with the server). Automatically resubmitting to merge queue (attempt 1 of 2). (run link)

@blathers-crl
Copy link
Copy Markdown

blathers-crl Bot commented Apr 14, 2026

/trunk merge

@trunk-io trunk-io Bot merged commit c0a301c into cockroachdb:release-25.4 Apr 14, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Label PR's that are backports to older release branches T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) target-release-25.4.10

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants