Revoke the reader's grants as their own grantors and read them back, alter its role only on drift, and report definer functions and views (review of hq issue 337)
mesh/merge-gate pass: builds postgres, sent nowhere; no bus step; 2 wait(s) for a person; every machine composes with the change as it did without (4 of 4 …
mesh/repo-check pass: its merge-check.sh passed
mesh/delivery superseded: a newer head of the same pull request

A superuser's REVOKE acts as the table owner, so a grant to the reader made
by a role holding GRANT OPTION survived it while the report said revoked.
Each grant is now revoked under SET ROLE to its grantor, the grants are read
back, and anything still held is reported as a failure. The role was
altered every minute with its password in the statement, which a server
logging DDL would record and which could collide with a first query; it is
now altered only when an attribute, a setting or the password has drifted,
one ensure at a time. SECURITY DEFINER functions and views the reader may
use reach data without a grant on it, so every pass lists them. The README
says the name filter is a backstop and that the read-only session default
can be turned off by a statement.
This commit is contained in:
jochen
2026-10-08 22:01:57 +02:00
parent fb07d552a0
commit 8bf50f7555
6 changed files with 431 additions and 92 deletions
@@ -70,7 +70,7 @@ func TestTheReaderIsGivenOnlyWhatIsDeclaredAndNeverASecret(t *testing.T) {
{"public", "signing_key", "public"}, {"public", "signing_key", "private"},
{"public", "build", "id"}, {"public", "build", "sealed"}, {"public", "build", "log"},
}
stmts, rep := planDatabase(decl, existing, held{Tables: map[table]string{}, Columns: map[column][]string{}}, map[string]bool{})
stmts, rep := planDatabase(decl, existing, newHeld(), map[string]bool{})
sql := strings.Join(stmts, "\n")
if !strings.Contains(sql, `GRANT SELECT ("name", "role") ON "public"."node" TO "mesh_store_reader"`) ||
!strings.Contains(sql, `GRANT SELECT ("id") ON "public"."build" TO "mesh_store_reader"`) {
@@ -92,22 +92,28 @@ func TestTheReaderIsGivenOnlyWhatIsDeclaredAndNeverASecret(t *testing.T) {
func TestWhatTheReaderHoldsAndIsNotDeclaredIsRevoked(t *testing.T) {
// An install before issue 337, or a grant by hand: a table-level grant, a column no longer
// declared, and a declared column held with more than SELECT.
sel := func(by string) grant { return grant{"SELECT", by} }
h := held{
Tables: map[table]string{{"public", "secret"}: "r", {"public", "seq"}: "S"},
Columns: map[column][]string{
{"public", "node", "name"}: {"SELECT"},
{"public", "node", "role"}: {"SELECT", "UPDATE"},
{"public", "node", "old"}: {"SELECT"},
Tables: map[table]tableGrants{
{"public", "secret"}: {Kind: "r", Grants: []grant{sel("postgres")}},
{"public", "seq"}: {Kind: "S", Grants: []grant{{"USAGE", "postgres"}}},
},
Columns: map[column][]grant{
{"public", "node", "name"}: {sel("postgres")},
{"public", "node", "role"}: {sel("postgres"), {"UPDATE", "postgres"}},
{"public", "node", "old"}: {sel("postgres")},
},
}
decl, _ := ParseReadable(map[string]any{"readable": []any{"node(name, role)"}})
existing := []column{{"public", "node", "name"}, {"public", "node", "role"}, {"public", "node", "old"}}
stmts, rep := planDatabase(decl, existing, h, map[string]bool{})
want := []string{
`REVOKE ALL ON TABLE "public"."secret" FROM "mesh_store_reader"`,
`REVOKE ALL ON SEQUENCE "public"."seq" FROM "mesh_store_reader"`,
`SET ROLE "postgres"`,
`REVOKE ALL ("old", "role") ON "public"."node" FROM "mesh_store_reader"`,
`GRANT SELECT ("role") ON "public"."node" TO "mesh_store_reader"`,
`REVOKE ALL ON SEQUENCE "public"."seq" FROM "mesh_store_reader"`,
`REVOKE ALL ON TABLE "public"."secret" FROM "mesh_store_reader"`,
`RESET ROLE`,
`GRANT SELECT ("name", "role") ON "public"."node" TO "mesh_store_reader"`,
}
if strings.Join(stmts, "\n") != strings.Join(want, "\n") {
t.Fatalf("got\n%s\nwant\n%s", strings.Join(stmts, "\n"), strings.Join(want, "\n"))
@@ -122,20 +128,74 @@ func TestWhatTheReaderHoldsAndIsNotDeclaredIsRevoked(t *testing.T) {
t.Fatal(s)
}
}
if len(stmts) != 3 {
if len(stmts) != 5 {
t.Fatal(stmts)
}
// Held exactly as declared: nothing to do.
exact := held{Tables: map[table]string{}, Columns: map[column][]string{{"public", "node", "name"}: {"SELECT"}}}
exact := held{Tables: map[table]tableGrants{}, Columns: map[column][]grant{{"public", "node", "name"}: {sel("postgres")}}}
decl, _ = ParseReadable(map[string]any{"readable": []any{"node(name)"}})
if stmts, _ := planDatabase(decl, existing, exact, map[string]bool{}); len(stmts) != 0 {
t.Fatal(stmts)
}
}
// A superuser's REVOKE acts as the table's owner, so a grant to the reader made by another role
// holding GRANT OPTION survives it. Each grant is revoked as its own grantor.
func TestAGrantIsRevokedAsItsOwnGrantor(t *testing.T) {
h := held{
Tables: map[table]tableGrants{{"public", "vault"}: {Kind: "r", Grants: []grant{{"SELECT", "middle"}, {"SELECT", "owner1"}}}},
Columns: map[column][]grant{{"public", "vault", "name"}: {{"SELECT", "middle"}}},
}
stmts, _ := planDatabase(nil, nil, h, map[string]bool{})
want := []string{
`SET ROLE "middle"`,
`REVOKE ALL ("name") ON "public"."vault" FROM "mesh_store_reader"`,
`REVOKE ALL ON TABLE "public"."vault" FROM "mesh_store_reader"`,
`RESET ROLE`,
`SET ROLE "owner1"`,
`REVOKE ALL ON TABLE "public"."vault" FROM "mesh_store_reader"`,
`RESET ROLE`,
}
if strings.Join(stmts, "\n") != strings.Join(want, "\n") {
t.Fatalf("got\n%s\nwant\n%s", strings.Join(stmts, "\n"), strings.Join(want, "\n"))
}
if left := unwanted(h, nil); strings.Join(left, ",") != "public.vault,public.vault.name" {
t.Fatal(left)
}
}
// The role is altered only when it drifted: the statement carries the password.
func TestTheReadersRoleIsAlteredOnlyWhenItDrifted(t *testing.T) {
conn := admin
conn.ReaderPassword = "r"
for _, c := range []struct {
row []string
alter bool
}{
{[]string{"f", "f", "f", "f", "f", "t", "t", "t", "default_transaction_read_only=on,statement_timeout=60s"}, false},
{[]string{"f", "f", "f", "f", "t", "t", "t", "t", "default_transaction_read_only=on,statement_timeout=60s"}, true},
{[]string{"f", "f", "f", "f", "f", "t", "t", "t", "statement_timeout=60s"}, true},
} {
f, cl := newFake(conn)
f.answer(`^SELECT rolsuper`, []string{"a", "b", "c", "d", "e", "f", "g", "h", "i"}, c.row)
if err := cl.ensureReaderRole(ctx); err != nil {
t.Fatal(err)
}
altered := false
for _, k := range f.calls {
if strings.HasPrefix(k.SQL, "ALTER ROLE") && strings.Contains(k.SQL, "PASSWORD") {
altered = true
}
}
if altered != c.alter {
t.Errorf("%v: altered %v", c.row, altered)
}
}
}
func TestASchemaTheReaderCannotUseIsGrantedUsage(t *testing.T) {
decl, _ := ParseReadable(map[string]any{"readable": []any{"audit.events"}})
stmts, _ := planDatabase(decl, []column{{"audit", "events", "at"}}, held{Tables: map[table]string{}, Columns: map[column][]string{}},
stmts, _ := planDatabase(decl, []column{{"audit", "events", "at"}}, newHeld(),
map[string]bool{"audit": true})
if len(stmts) != 2 || stmts[0] != `GRANT USAGE ON SCHEMA "audit" TO "mesh_store_reader"` {
t.Fatal(stmts)