Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sql: Block IMPORT statements for RLS tables #136807

Closed
Tracked by #73596
spilchen opened this issue Dec 5, 2024 · 1 comment · Fixed by #143066
Closed
Tracked by #73596

sql: Block IMPORT statements for RLS tables #136807

spilchen opened this issue Dec 5, 2024 · 1 comment · Fixed by #143066
Assignees
@spilchen
Labels
A-sql-privileges SQL privilege handling and permission checks. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) v25.2.0-prerelease

Comments

@spilchen
Copy link
Contributor

spilchen commented Dec 5, 2024 •
edited
Loading

Data ingestion methods other than INSERT could inadvertently bypass RLS policy enforcement. To align with PostgreSQL behavior, we will prevent IMPORT and COPY operations on tables with row-level security (RLS) enabled.

PostgreSQL returns an error similar to this, which we should replicate:

postgres=> COPY cities FROM '/c.csv';  
ERROR:  COPY FROM not supported with row-level security  
HINT:  Use INSERT statements instead.

Jira issue: CRDB-45262
@spilchen spilchen added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Dec 5, 2024
@spilchen spilchen mentioned this issue Dec 5, 2024
Open
37 tasks
@spilchen spilchen added the A-sql-privileges SQL privilege handling and permission checks. label Dec 5, 2024
@spilchen
Copy link
Contributor Author

I want to allow the COPY command now since it does go through the query engine. I verified that we still enforce RLS policies. Given this, I'll pivot and add tests to ensure policies are properly enforced.

We still want to block IMPORT though. A PR for that should be coming soon.

@spilchen spilchen changed the title sql: Block IMPORT and COPY statements for RLS tables sql: Block IMPORT statements for RLS tables Feb 21, 2025
@spilchen spilchen mentioned this issue Feb 21, 2025
Merged
spilchen added a commit to spilchen/cockroach that referenced this issue Feb 21, 2025
@spilchen
sql/importer: Block IMPORT INTO on RLS tables
c8a11d6 
Tables enabled for row-level security (RLS) have policies that restrict
which rows can be added. Since IMPORT bypasses the query engine where
these policies are enforced, we need to block IMPORT INTO for RLS
tables. There is one exception: admin users can still run this command,
as RLS policies are not enforced for administrators.

Epic: CRDB-45203
Release note: none
Informs cockroachdb#136807
spilchen added a commit to spilchen/cockroach that referenced this issue Feb 25, 2025
@spilchen
sql/importer: Block IMPORT INTO on RLS tables
e6c7e33 
Tables enabled for row-level security (RLS) have policies that restrict
which rows can be added. Since IMPORT bypasses the query engine where
these policies are enforced, we need to block IMPORT INTO for RLS
tables. There is one exception: admin users can still run this command,
as RLS policies are not enforced for administrators.

Epic: CRDB-45203
Release note: none
Informs cockroachdb#136807
craig bot pushed a commit that referenced this issue Feb 25, 2025
@spilchen
Merge #141844
c32ba04 
141844: sql/importer: Block IMPORT INTO on RLS tables r=spilchen a=spilchen

Tables enabled for row-level security (RLS) have policies that restrict which rows can be added. Since IMPORT bypasses the query engine where these policies are enforced, we need to block IMPORT INTO for RLS tables. There is one exception: admin users can still run this command, as RLS policies are not enforced for administrators.

Epic: CRDB-45203
Release note: none
Informs #136807

Co-authored-by: Matt Spilchen <[email protected]>
@spilchen spilchen mentioned this issue Mar 18, 2025
Merged
craig bot pushed a commit that referenced this issue Mar 20, 2025
@spilchen
Merge #142983 #143066
4f514e3 
142983: workload/schemachange: improve diagnostics for unique constraint failures r=spilchen a=spilchen

Previously, when adding a unique constraint to a table, failures due to duplicate key checks were diagnosed by comparing table row counts against a DISTINCT check. However, when this failed, it provided little insight into the cause.

This change improves diagnostics by modifying the query to explicitly extract the components of the check, ensuring they are included in test logs. This additional context will make debugging easier if the issue recurs.

Closes #142458

Epic: none
Release note: none

143066: sql/copy: validate RLS policies for COPY FROM r=spilchen a=spilchen

COPY FROM goes through the optbuilder, applying any applicable row-level security (RLS) policies. This commit adds tests to ensure that RLS policies are correctly enforced for COPY FROM.

Additionally, a stale comment in logic.go has been removed. The COPY FROM tests originally resided in logic tests before being moved to the sql/copy package.

Closes #136807

Epic: CRDB-45203
Release note: none

Co-authored-by: Matt Spilchen <[email protected]>
@craig craig bot closed this as completed in 6c2c343 Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@spilchen spilchen

Labels
A-sql-privileges SQL privilege handling and permission checks. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) v25.2.0-prerelease
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql/copy: validate RLS policies for COPY FROM spilchen/cockroach
1 participant
@spilchen