You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[cr-test] db-migrate: let bootstrap-roles leave excluded relations ungranted - #1489
bootstrap-roles grants the application roles (space_user, space_ro, space_creds) privileges on every relation in schema public, views included,
on every run. A planned rename leaves compatibility views under the old table
names. Before those views are dropped, a brownout step revokes the application
roles' privileges on them so that any remaining reader fails loudly. Without
this change, the next bootstrap-roles run restores those privileges and the
brownout is quietly undone.
This adds GRANT_EXCLUDED_RELATIONS in crates/l30_storage/db/migrate/src/bootstrap_roles.rs: relations bootstrap grants
the application roles nothing on. converge_grants takes the list as a
parameter. Excluded relations are skipped by the per-relation grant loop and by
the space_ro column-scoped re-grant. Revokes still apply to them: the
credential fence, and the column fence's space_ro revoke. Bootstrap logs each
skipped relation at info level. Every SQL statement stays whole; the exclusion
is a Rust-side filter on the existing catalog query's results.
The production list is empty. A brownout adds its views to it. An entry is
removed only after the migration that drops its view has applied everywhere:
provisioning runs bootstrap-roles before migrations, so an image without the
entry would re-grant the view before the drop. Landing the mechanism now keeps
the brownout step to a list change. It does not protect a brownout by itself: a
bootstrap run from any image that lacks the brownout's entry, this one
included, re-grants the views.
An exclusion list is smaller than the alternative of having bootstrap re-grant
only relations it declares. Bootstrap does not declare per-relation grants: it
grants on every relation in public, so new and adopted tables converge with
no list at all. Declaring grants would mean listing every application table.
Validation
On this head, with Docker running and the Rust toolchain from rust-toolchain.toml:
Test-first, grant loop: before the exclusion existed, a test that revoked space_user, space_ro and space_creds on a fixture view
(CREATE VIEW legacy_fixture AS SELECT id FROM renamed_fixture) and then ran
bootstrap failed with space_user must stay revoked on the view. That
confirms bootstrap restores the revoked privileges.
grant_exclusion_keeps_revoked_view_privileges_revoked (a check that first_bootstrap_adopts_admin_owned_tables_routines_and_types runs on its
already bootstrapped server) sends the fixture view through the production converge_grants path with its own exclusion list. It checks that all three roles stay revoked on the view while its base
table is granted, then that a normal bootstrap run (production list) grants
the view again.
Test-first, column fence: grant_exclusion_still_fences_denylisted_columns_from_ro
(the second check run inside the same test) creates, as space_migrator, a view named sessions (an RO_COLUMN_DENYLIST entry) over id and token_hash. Its precondition
checks that space_ro starts with SELECT from the default privileges. With sessions excluded, converge_grants must leave space_ro no column
(has_any_column_privilege) while space_user keeps its table SELECT. A
normal bootstrap run then must grant space_roid but not token_hash.
Before the fix, the column fence skipped excluded relations entirely, and the
test failed with space_ro must lose every column of an excluded denylisted view. It passes after the fix.
The two checks share first_bootstrap_adopts_admin_owned_tables_routines_and_types's
container instead of starting their own. Each bootstrap-roles test starts its
own Postgres container, and as separate tests the column-fence check took
4.1-4.6 s, over the 4 s gate in ci/test-duration-budget.toml. The host test,
listed with a 5 s ceiling, now takes 2.9-3.4 s on the validation machine.
Mutation checks on this head, each restored afterwards. Each one fails first_bootstrap_adopts_admin_owned_tables_routines_and_types, and only that test:
grant loop's skip removed: space_user must stay revoked on the excluded view;
column fence's continue for excluded relations removed (re-grant): space_ro must lose every column of an excluded denylisted view;
excluded relations skipping the column-fence revoke as well: the same message;
bootstrap passing the fixture names instead of GRANT_EXCLUDED_RELATIONS: space_user must be regranted a view that is not excluded.
cargo build -p pplx-space-db-test-fixtures --bin space-nextest-postgres-setup && cargo nextest run -p pplx-space-db-migrate:
run as root as CI's privileged step does (--run-ignored all --ignore-default-filter):
54 tests run, 54 passed, 0 skipped (SPACE_TEST_DEPS_OPTIONAL unset). The
bootstrap Postgres tests ran against real containers.
python3 ci/scripts/check-test-durations.py target/nextest/default/junit.xml ci/test-duration-budget.toml,
6 runs: the host test took 2.9-3.4 s against its 5 s ceiling. Three runs failed
because ro_reads_api_keys_without_key_hash or ro_reads_publication_grant_metadata_without_authorization_id took 5.0-5.3 s
against their 5 s ceilings on that machine; those tests already reach 4.86 s on
main there, and CI set their ceilings from runs of 3.33 s or less.
Not validated: workspace-wide clippy on this head; the full privileged test group
together and the full CI matrix (durations above are from one machine, not CI);
Kind.
Severity and risk
Severity: low on its own, because the list is empty. It is a precondition for a
safe brownout: without it, a routine bootstrap run would silently undo one.
Risk: with an empty list, the grant loop and both fences run exactly as before.
Once a relation is listed, bootstrap stops granting the application roles on
it, so a listed relation that is not actually being retired loses access on its
next revoke; if it is in RO_COLUMN_DENYLIST, space_ro loses access on the
next bootstrap run. Roll back by removing the entry, or by reverting this
commit, and re-running bootstrap-roles.
Rollout
No migration, flag or configuration. With the empty list, every image behaves
as before. Once a brownout adds an entry, a bootstrap run from an image without
that entry re-grants the view. From the brownout's revoke until the view's drop
has applied everywhere, every bootstrap run, a rollback included, must come
from an image that carries the entry. The migration-isolation check rejects
changes to this list in a PR that also changes migrations, so entries are added
and removed in PRs of their own.
AI assistance: An AI agent (Claude Code) wrote the code, the tests and this description.
Why: Bootstrap can now withhold application access to configured relations. The exclusion list is empty today, but enabling it requires careful rollout.
Commit: 9bf02730b5eb.
No actionable comments were generated in the recent review.
Walkthrough
bootstrap now passes an exclusion list to converge_grants. Excluded relations do not receive normal grants. For excluded relations subject to a space_ro column fence, the table-level revoke remains, but the column-scoped read grant is skipped. PostgreSQL tests cover excluded views, their backing tables, and the grants applied by a subsequent normal bootstrap.
Possibly related PRs
perplexityai/SPACE#1310: Adds grant-exclusion handling and matching PostgreSQL tests in bootstrap_roles.
perplexityai/SPACE#1309: Adds execution-record compatibility views and tests for their application-role grants; this change can exclude selected views from those grants.
Merge Risk: High · up to 9bf02
Bootstrap can now withhold application access to configured relations. The exclusion list is empty today, but enabling it requires careful rollout.
Security architecture risk: Minimal · up to 9bf02
The production exclusion list is empty, so this PR leaves effective database privileges unchanged. The new mechanism preserves credential and sensitive-column protections. Future use requires explicit revocation and coordinated migration images; exclusion alone does not remove existing access.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
observed — The policy operates on non-extension tables, partitioned tables, sequences, views, and materialized views in the connected database's public schema, affecting space_user, space_ro, and space_creds grants. The empty production exclusion list activates no relation-specific privilege change.
Trust Boundaries and Controls
observed — Bootstrap remains an administrative database operation, with no new application-supplied exclusion authority. Exclusions do not bypass the credential fence revoking space_user and space_ro access, or the denylisted-column fence revoking space_ro access before any optional safe-column grant.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title starts with the lowercase bracketed area "[cr-test]" and uses a lowercase summary. It describes the bootstrap-roles change to leave excluded relations ungranted and has no trailing period or…
Description Follows The Pr Template
The description contains all required sections and content. It has a substantive Summary, a Validation section with commands and results, a Severity and risk section with severity and risk, and an "AI…
Bug Fixes Name A Regression Test
The PR fixes application behavior in crates/. The Validation section names grant_exclusion_keeps_revoked_view_privileges_revoked and grant_exclusion_still_fences_denylisted_columns_from_ro, and …
Weakened Safeguards Are Justified
No weakened safeguard was introduced. The PR changes only bootstrap_roles.rs and its Postgres test file, with 148 additions and 2 deletions. The deletions only update the converge_grants call and …
Public-Safe Content
The changed files and commit subjects contain no real credentials, tokens, private keys, internal URLs or hostnames, cloud IDs, personal absolute paths, production logs, or incident details. The title…
Full details: Title check
Explanation
The title starts with the lowercase bracketed area "[cr-test]" and uses a lowercase summary. It describes the bootstrap-roles change to leave excluded relations ungranted and has no trailing period or ticket ID.
Full details: Description Follows The Pr Template
Explanation
The description contains all required sections and content. It has a substantive Summary, a Validation section with commands and results, a Severity and risk section with severity and risk, and an "AI assistance:" line identifying Claude Code and the drafted parts.
Full details: Bug Fixes Name A Regression Test
Explanation
The PR fixes application behavior in crates/. The Validation section names grant_exclusion_keeps_revoked_view_privileges_revoked and grant_exclusion_still_fences_denylisted_columns_from_ro, and states that each failed before the fix and passed after it. The diff adds both checks and invokes them from first_bootstrap_adopts_admin_owned_tables_routines_and_types; their assertions cover the restored grants and the column-fence regression.
Full details: Weakened Safeguards Are Justified
Explanation
No weakened safeguard was introduced. The PR changes only bootstrap_roles.rs and its Postgres test file, with 148 additions and 2 deletions. The deletions only update the converge_grants call and signature to accept exclusions. No test, assertion, lint, CI step, allowlist, baseline, or repository guard was removed or relaxed. The diff adds assertions covering excluded relation grants and denylisted-column fencing.
Full details: Public-Safe Content
Explanation
The changed files and commit subjects contain no real credentials, tokens, private keys, internal URLs or hostnames, cloud IDs, personal absolute paths, production logs, or incident details. The title is safe. The description discusses planned grant behavior and test-only values such as token_hash; it does not disclose an unfixed security vulnerability or private operational data.
Recent review info
Automatic approval policy
Commit: 9bf02730b5eb.
Setting
Effective value
Source
Automatic approvals
Enabled
Global configuration
Merge risk
Low
Override 2
Security risk
Not set
Global configuration
Matching overrides (numbered as in the resolved configuration):
Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.
Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.
Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 8 minutes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bootstrap-rolesgrants the application roles (space_user,space_ro,space_creds) privileges on every relation in schemapublic, views included,on every run. A planned rename leaves compatibility views under the old table
names. Before those views are dropped, a brownout step revokes the application
roles' privileges on them so that any remaining reader fails loudly. Without
this change, the next
bootstrap-rolesrun restores those privileges and thebrownout is quietly undone.
This adds
GRANT_EXCLUDED_RELATIONSincrates/l30_storage/db/migrate/src/bootstrap_roles.rs: relations bootstrap grantsthe application roles nothing on.
converge_grantstakes the list as aparameter. Excluded relations are skipped by the per-relation grant loop and by
the
space_rocolumn-scoped re-grant. Revokes still apply to them: thecredential fence, and the column fence's
space_rorevoke. Bootstrap logs eachskipped relation at info level. Every SQL statement stays whole; the exclusion
is a Rust-side filter on the existing catalog query's results.
The production list is empty. A brownout adds its views to it. An entry is
removed only after the migration that drops its view has applied everywhere:
provisioning runs
bootstrap-rolesbefore migrations, so an image without theentry would re-grant the view before the drop. Landing the mechanism now keeps
the brownout step to a list change. It does not protect a brownout by itself: a
bootstrap run from any image that lacks the brownout's entry, this one
included, re-grants the views.
An exclusion list is smaller than the alternative of having bootstrap re-grant
only relations it declares. Bootstrap does not declare per-relation grants: it
grants on every relation in
public, so new and adopted tables converge withno list at all. Declaring grants would mean listing every application table.
Validation
On this head, with Docker running and the Rust toolchain from
rust-toolchain.toml:space_user,space_roandspace_credson a fixture view(
CREATE VIEW legacy_fixture AS SELECT id FROM renamed_fixture) and then ranbootstrap failed with
space_user must stay revoked on the view. Thatconfirms bootstrap restores the revoked privileges.
grant_exclusion_keeps_revoked_view_privileges_revoked(a check thatfirst_bootstrap_adopts_admin_owned_tables_routines_and_typesruns on itsalready bootstrapped server) sends the fixture view through the production
converge_grantspath with its own exclusion list. It checks that all three roles stay revoked on the view while its basetable is granted, then that a normal bootstrap run (production list) grants
the view again.
grant_exclusion_still_fences_denylisted_columns_from_ro(the second check run inside the same test) creates, as
space_migrator, a view namedsessions(anRO_COLUMN_DENYLISTentry) overidandtoken_hash. Its preconditionchecks that
space_rostarts with SELECT from the default privileges. Withsessionsexcluded,converge_grantsmust leavespace_rono column(
has_any_column_privilege) whilespace_userkeeps its table SELECT. Anormal bootstrap run then must grant
space_roidbut nottoken_hash.Before the fix, the column fence skipped excluded relations entirely, and the
test failed with
space_ro must lose every column of an excluded denylisted view. It passes after the fix.first_bootstrap_adopts_admin_owned_tables_routines_and_types'scontainer instead of starting their own. Each bootstrap-roles test starts its
own Postgres container, and as separate tests the column-fence check took
4.1-4.6 s, over the 4 s gate in
ci/test-duration-budget.toml. The host test,listed with a 5 s ceiling, now takes 2.9-3.4 s on the validation machine.
first_bootstrap_adopts_admin_owned_tables_routines_and_types, and only that test:space_user must stay revoked on the excluded view;continuefor excluded relations removed (re-grant):space_ro must lose every column of an excluded denylisted view;bootstrappassing the fixture names instead ofGRANT_EXCLUDED_RELATIONS:space_user must be regranted a view that is not excluded.cargo build -p pplx-space-db-test-fixtures --bin space-nextest-postgres-setup && cargo nextest run -p pplx-space-db-migrate:run as root as CI's privileged step does (
--run-ignored all --ignore-default-filter):54 tests run, 54 passed, 0 skipped (
SPACE_TEST_DEPS_OPTIONALunset). Thebootstrap Postgres tests ran against real containers.
python3 ci/scripts/check-test-durations.py target/nextest/default/junit.xml ci/test-duration-budget.toml,6 runs: the host test took 2.9-3.4 s against its 5 s ceiling. Three runs failed
because
ro_reads_api_keys_without_key_hashorro_reads_publication_grant_metadata_without_authorization_idtook 5.0-5.3 sagainst their 5 s ceilings on that machine; those tests already reach 4.86 s on
main there, and CI set their ceilings from runs of 3.33 s or less.
cargo fmt --all -- --check: clean.cargo clippy --locked -p pplx-space-db-migrate --all-targets --all-features -- -D warnings: clean.bash ci/scripts/run-archcheck.sh <base sha> ""(base and its baseline,renames and
ARCHITECTURE.toml, as CI passes them): exit 0, no findings.cargo check --workspace --all-targets --all-features --locked: passed.Not validated: workspace-wide clippy on this head; the full privileged test group
together and the full CI matrix (durations above are from one machine, not CI);
Kind.
Severity and risk
Severity: low on its own, because the list is empty. It is a precondition for a
safe brownout: without it, a routine bootstrap run would silently undo one.
Risk: with an empty list, the grant loop and both fences run exactly as before.
Once a relation is listed, bootstrap stops granting the application roles on
it, so a listed relation that is not actually being retired loses access on its
next revoke; if it is in
RO_COLUMN_DENYLIST,space_roloses access on thenext bootstrap run. Roll back by removing the entry, or by reverting this
commit, and re-running
bootstrap-roles.Rollout
No migration, flag or configuration. With the empty list, every image behaves
as before. Once a brownout adds an entry, a bootstrap run from an image without
that entry re-grants the view. From the brownout's revoke until the view's drop
has applied everywhere, every bootstrap run, a rollback included, must come
from an image that carries the entry. The migration-isolation check rejects
changes to this list in a PR that also changes migrations, so entries are added
and removed in PRs of their own.
AI assistance: An AI agent (Claude Code) wrote the code, the tests and this description.