Skip to content

fix(bff): managed-schema privilege test works when the DB role is a superuser - #85

Merged
ovander merged 1 commit into
mainfrom
fix/postgres-store-test-superuser
Oct 4, 2026
Merged

ovander merged 1 commit into
mainfrom
fix/postgres-store-test-superuser

Conversation

@ovander

@ovander ovander commented Oct 4, 2026

Copy link
Copy Markdown
Owner

What and why

main is red since #83. TestPostgresStore_ManagedSchemaChecksTheTable fails in CI with "missing DELETE privilege: , want a start-up error naming it".

The test revoked DELETE from its own database role and expected NewPostgresStore (managed mode) to refuse to start. In CI that role is a superuser, which holds every privilege no matter what is revoked, so has_table_privilege stays true. Locally the role was not a superuser, so the test passed there; #83 was merged with this check red. The library code is correct; only the test was wrong for that setup.

  • TestPostgresStore_ManagedSchemaChecksTheTable: when the role is a superuser, it creates a real NOLOGIN role holding SELECT, INSERT, UPDATE and runs the store as that role (SET ROLE on a one-connection pool). Start-up must then fail naming DELETE. As a non-superuser it keeps revoking from itself. The columns check now uses its own table.
  • New TestPostgresStore_ManagedSchemaWithACRUDOnlyRole: the exact bff: let PostgresStore run on a migration-owned table with a CRUD-only role #82 layout, now automated in CI: a role holding only SELECT, INSERT, UPDATE, DELETE on a table someone else created, with its index. The default mode cannot start, because it runs DDL. The managed mode starts and runs Put, Get, Delete and Sweep. It needs a role that can create roles, which CI's can; elsewhere it skips with that reason.

How it was tested

The failure is reproduced locally as a PostgreSQL superuser (the CI setup) on main, then fixed. All 9 PostgresStore tests pass in both setups:

  • As a superuser: every test runs, including the new CRUD-only-role test.
  • As a non-superuser: the CRUD-only-role test skips with its reason.

No test roles are left behind.

  • go mod tidy && git diff --exit-code go.sum leaves go.sum unchanged
  • go build ./... passes
  • go vet ./... passes
  • go test -race -count=1 -timeout=120s ./... passes (as a superuser, like CI)
  • golangci-lint run ./... (v2.14.0) reports no issue
  • govulncheck ./...: CI
  • A line is added under ## [Unreleased] in CHANGELOG.md

Compatibility

  • Exported-API change: no.
  • Behaviour change for existing callers: none (tests only).
  • Breaking change: none.

Release: hold the v1.20.0 cut (#84) until this is merged. I'll then update #84 so the release includes this fix.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GKRxaeYxyDhmt42cehLsGA


Generated by Claude Code

…uperuser

TestPostgresStore_ManagedSchemaChecksTheTable revoked DELETE from the
test's own role and expected the store to refuse to start. In CI that
role is a superuser, which holds every privilege whatever is revoked,
so has_table_privilege stayed true and the test failed on main (#83).

When the role is a superuser, the test now creates a real NOLOGIN role
holding SELECT, INSERT and UPDATE, and runs the store as that role
(SET ROLE on a one-connection pool); otherwise it keeps revoking from
itself. A new test runs the exact #82 layout in CI: a CRUD-only role on
a table it does not own, where the default mode cannot start and the
managed mode serves sessions. No library change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GKRxaeYxyDhmt42cehLsGA
@ovander
ovander merged commit 9456518 into main Oct 4, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants