support ALTER DEFAULT PRIVILEGES

[jennifersp]

No description provided.

[github-actions]

	Main	PR

[github-actions]

	Main	PR
Total	42090	42090
Successful	18203	18211
Failures	23887	23879
Partial Successes1	5305	5305

	Main	PR
Successful	43.2478%	43.2668%
Failures	56.7522%	56.7332%

${\color{lightgreen}Progressions (8)}$

dependency

QUERY: ALTER DEFAULT PRIVILEGES FOR ROLE regress_dep_user1 IN SCHEMA deptest
  GRANT ALL ON TABLES TO regress_dep_user2;

event_trigger

QUERY: alter default privileges for role regress_evt_user
 revoke delete on tables from regress_evt_user;

matview

QUERY: ALTER DEFAULT PRIVILEGES FOR ROLE regress_matview_user
  REVOKE INSERT ON TABLES FROM regress_matview_user;

QUERY: ALTER DEFAULT PRIVILEGES FOR ROLE regress_matview_user
  GRANT INSERT ON TABLES TO regress_matview_user;

object_address

QUERY: ALTER DEFAULT PRIVILEGES FOR ROLE regress_addr_user IN SCHEMA public GRANT ALL ON TABLES TO regress_addr_user;

QUERY: ALTER DEFAULT PRIVILEGES FOR ROLE regress_addr_user REVOKE DELETE ON TABLES FROM regress_addr_user;

select_into

QUERY: ALTER DEFAULT PRIVILEGES FOR ROLE regress_selinto_user
	  REVOKE INSERT ON TABLES FROM regress_selinto_user;

QUERY: ALTER DEFAULT PRIVILEGES FOR ROLE regress_selinto_user
	  GRANT INSERT ON TABLES TO regress_selinto_user;

Footnotes

1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct. ↩

[itoqa]

Ito Test Report ❌

17 test cases ran. 3 failed, 3 additional findings, 11 passed.

Across 17 test cases, 11 passed and 6 failed: core default-privilege inheritance behaviors were mostly validated end-to-end (including tables, wildcard schemas, sequences, routines, a race scenario, and a non-enumerating unauthorized error path), and one previously unstable privilege-inheritance path now executes successfully in a stabilized environment. The most important confirmed defects are a critical startup crash on malformed auth.db deserialization, a high-severity legacy v1 auth.db role-ID collision issue that can break role lookups, and four medium-severity logic flaws (ALTER DEFAULT PRIVILEGES without FOR ROLE/USER being wrongly denied for non-superusers, in-memory/on-disk ACL drift when persistence fails, CREATE TABLE default ACL persistence asymmetry versus CREATE SEQUENCE, and ALTER INDEX ATTACH PARTITION parsing but always failing due to hardcoded unsupported execution).

❌ Failed (3)

Category	Summary	Screenshot
Creation	🟠 CREATE TABLE objects did not retain expected inherited table ACLs while CREATE SEQUENCE objects retained inherited sequence ACLs.
Privileges	🟠 ALTER DEFAULT PRIVILEGES without FOR ROLE/USER is denied for non-superusers due to empty auth target handling.
Privileges	🟠 Default ACL state mutates before persistence and can drift from disk when persistence fails.

🟠 Default table privileges are not persisted during CREATE TABLE flow

• What failed: New sequences retained inherited access, but new tables did not inherit/persist expected default SELECT access for readonly_user; both object types should apply and persist their configured defaults.
• Impact: Teams depending on default table ACL inheritance can silently lose expected access on newly created tables, requiring manual grants. This creates inconsistent permission behavior between object types in the same workflow.
• Steps to reproduce:
  1. Configure default SELECT on tables and default USAGE on sequences for the same owner and schema.
  2. Create multiple tables and sequences in parallel as that owner.
  3. Restart the server and compare inherited grantee access on the new tables versus new sequences.
• Stub / mock context: Authentication was bypassed via server/authentication_scram.go to run deterministic seeded-role SQL checks, and local auth bootstrap state was repaired before executing this scenario. No route interception or API response stubbing was used; the defect finding comes from direct source-code inspection of the production create paths.
• Code analysis: I inspected default-privilege application paths for table and sequence creation. The table path defers applying defaults until the iterator returns io.EOF, while the sequence path applies defaults and persists changes directly in RowIter, creating an asymmetric and brittle persistence point for tables.
• Why this is likely a bug: The table ACL apply/persist hook is coupled to iterator EOF instead of the successful create path, so table defaults can be skipped while equivalent sequence defaults are persisted immediately.

Relevant code:

server/node/create_table.go (lines 120-133)

func (i *createTableDefaultPrivsIter) Next(ctx *sql.Context) (sql.Row, error) {
    row, err := i.inner.Next(ctx)
    if err == io.EOF && !i.applied {
        i.applied = true
        var applyErr error
        auth.LockWrite(func() {
            auth.ApplyDefaultPrivilegesForNewTable(i.ownerID, i.schemaName, i.tableName)
            applyErr = auth.PersistChanges()
        })
        if applyErr != nil {
            return nil, applyErr
        }
    }
    return row, err
}

server/node/create_sequence.go (lines 229-236)

var authErr error
auth.LockWrite(func() {
    ownerRole := auth.GetRole(ctx.Client().User)
    if ownerRole.IsValid() {
        auth.ApplyDefaultPrivilegesForNewSequence(ownerRole.ID(), c.sequence.Id.SchemaName(), c.sequence.Id.SequenceName())
    }
    authErr = auth.PersistChanges()
})

server/auth/default_privileges.go (lines 135-156)

// ApplyDefaultPrivilegesForNewTable applies any matching default privileges to a newly created table.
// Must be called under LockWrite.
func ApplyDefaultPrivilegesForNewTable(ownerRoleID RoleID, schemaName, tableName string) {
    for key, dpv := range globalDatabase.defaultPrivileges.Data {
        if key.OwnerRole != ownerRoleID || key.ObjectType != PrivilegeObject_TABLE {
            continue
        }
        if key.Schema != "" && key.Schema != schemaName {
            continue
        }
        for granteeID, granteeValue := range dpv.Grantees {
            for _, privilegeMap := range granteeValue.Privileges {
                for grantedPriv, withGrantOption := range privilegeMap {

🟠 No-FOR-ROLE statement denied before owner fallback resolution

• What failed: The command is denied with a permission error for an empty target instead of treating the session user as owner; expected behavior is self-context authorization without requiring explicit FOR ROLE/USER.
• Impact: Non-superusers cannot use a valid ALTER DEFAULT PRIVILEGES form unless they add an explicit owner clause. This breaks expected SQL compatibility for a meaningful privilege-management workflow.
• Steps to reproduce:
  1. Connect as a non-superuser role in a clean session.
  2. Run ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT ON TABLES TO readonly_user without a FOR ROLE/USER clause.
  3. Create a table as that role and verify readonly_user still lacks inherited SELECT privileges.
• Stub / mock context: Authentication was bypassed by disabling SCRAM checks in server/authentication_scram.go so local SQL role behavior could be tested without external auth dependencies. Local roles and schema permissions were seeded before the authorization scenario was executed.
• Code analysis: I inspected AST conversion, auth gate checks, and execution fallback in server/ast/alter_default_privileges.go, server/auth/auth_handler.go, and server/node/alter_default_privileges.go. The auth metadata always includes one target name even when empty, and authorization rejects that empty target before executor fallback can resolve the current user.
• Why this is likely a bug: Production code proves a valid empty-owner form is rejected in auth before the built-in current-user fallback runs, so this is a logic-order defect rather than test setup noise.

Relevant code:

server/ast/alter_default_privileges.go (lines 43-50)

return vitess.InjectedStatement{
	Auth: vitess.AuthInformation{
		AuthType:    auth.AuthType_CREATE,
		TargetType:  auth.AuthTargetType_AlterDefaultPrivilegesIdentifiers,
		TargetNames: []string{node.TargetRole},
	},
	Statement: &pgnodes.AlterDefaultPrivileges{

server/auth/auth_handler.go (lines 224-235)

case AuthTargetType_AlterDefaultPrivilegesIdentifiers:
	nTargets := len(auth.TargetNames)
	if nTargets > 1 {
		return errors.Errorf("function identifiers has an unsupported count: %d", len(auth.TargetNames))
	}
	if state.role.IsSuperUser {
		return nil
	}
	if nTargets == 1 && state.role.Name == auth.TargetNames[0] {
		return nil
	}
	return errors.Errorf("permission denied for %s", auth.TargetNames[0])

server/node/alter_default_privileges.go (lines 147-155)

func (n *AlterDefaultPrivileges) resolveOwnerRole(ctx *sql.Context) (auth.Role, error) {
	// empty means current user
	if n.OwnerRole == "" {
		userRole := auth.GetRole(ctx.Client().User)
		if !userRole.IsValid() {
			return auth.Role{}, errors.Errorf(`role "%s" does not exist`, ctx.Client().User)
		}
		return userRole, nil

🟠 Persist error after ALTER leaves in-memory and on-disk ACL drift

• What failed: In-memory default privileges are updated before persistence, and on persistence error there is no rollback; expected behavior is atomic failure semantics with no durable/in-memory drift.
• Impact: Operators can observe conflicting privilege behavior before and after restart after a failed ALTER write. This can cause inconsistent access control outcomes during incident recovery.
• Steps to reproduce:
  1. Induce an auth.db write failure while the process remains running.
  2. Execute ALTER DEFAULT PRIVILEGES and capture the returned persistence outcome.
  3. Compare effective default privilege behavior before restart and after restart to detect memory-disk divergence.
• Stub / mock context: Authentication was bypassed by disabling SCRAM checks in server/authentication_scram.go to keep role/ACL verification deterministic in local execution. The run also simulated auth persistence failures by changing filesystem write behavior, including redirecting the auth database path to a full device.
• Code analysis: I inspected RowIter execution ordering in server/node/alter_default_privileges.go, write behavior in server/auth/serialization.go, and default privilege mutation in server/auth/default_privileges.go. The node executes mutations first, then persists, and mutation helpers directly write global in-memory state.
• Why this is likely a bug: The production write path has no transaction or rollback around mutable global ACL state, so persistence failure can deterministically leave memory and disk out of sync.

Relevant code:

server/node/alter_default_privileges.go (lines 63-70)

var err error
auth.LockWrite(func() {
	err = n.execute(ctx)
	if err != nil {
		return
	}
	err = auth.PersistChanges()
})

server/auth/serialization.go (lines 25-28)

func PersistChanges() error {
	if fileSystem != nil {
		return fileSystem.WriteFile(authFileName, globalDatabase.serialize(), 0644)
	}

server/auth/default_privileges.go (lines 53-77)

// AddDefaultPrivilege adds a default privilege entry to the global database.
func AddDefaultPrivilege(key DefaultPrivilegeKey, grantee RoleID, privilege GrantedPrivilege, withGrantOption bool) {
	dpv, ok := globalDatabase.defaultPrivileges.Data[key]
	if !ok {
		dpv = DefaultPrivilegeValue{
			Key:      key,
			Grantees: make(map[RoleID]DefaultPrivilegeGranteeValue),
		}
	}
	granteeValue, ok := dpv.Grantees[grantee]
	if !ok {
		granteeValue = DefaultPrivilegeGranteeValue{
			Grantee:    grantee,
			Privileges: make(map[Privilege]map[GrantedPrivilege]bool),
		}
	}
	privilegeMap, ok := granteeValue.Privileges[privilege.Privilege]
	if !ok {

✅ Passed (11)

Category	Summary	Screenshot
Creation	New public table inherited configured default SELECT privileges for readonly_user.
Creation	Schema-wildcard default table privileges applied to both public and alt schema tables.
Creation	New sequence inherited configured USAGE privileges for seq_reader and nextval succeeded.
Creation	ALTER DEFAULT PRIVILEGES for routines applied as expected; func_reader executed both newly created routine objects.
Creation	Concurrent ALTER DEFAULT PRIVILEGES and CREATE TABLE operations produced consistent ACL outcomes before and after restart in this run.
Parser	Single-owner FOR ROLE syntax parsed and executed successfully, and inherited SELECT access worked on a newly created table.
Parser	ALTER TABLE ATTACH PARTITION and DETACH PARTITION both executed successfully using the intended partition identifier.
Parser	Legacy multi-owner FOR USER syntax was rejected with a controlled syntax error, which matches expected behavior.
Privileges	Re-executed successfully in a stabilized environment: ALTER DEFAULT PRIVILEGES FOR USER auth_test_super ... GRANT SELECT ON TABLES TO readonly_user succeeded, and readonly_user could SELECT from newly created another_table without manual GRANT.
Privileges	Unauthorized ALTER checks stop at permission denial before role existence lookup.
Serialization	Default privileges persisted across restart and were still enforced for readonly access after reload.

ℹ️ Additional Findings (3)

These findings are unrelated to the current changes but were observed during testing.

Category	Summary	Screenshot
Parser	🟠 ALTER INDEX ATTACH PARTITION parses, but execution is hard-blocked by an unconditional unsupported error in production conversion logic.
Serialization	⚠️ Legacy v1 auth.db startup can degrade auth state and trigger repeated role public does not exist failures.
Serialization	🚨 Truncated auth.db bytes can crash startup with an unhandled panic instead of a controlled error.

🟠 ALTER INDEX ATTACH PARTITION cannot execute

• What failed: The statement shape is accepted by parser/AST construction, but execution always fails before planning because ALTER INDEX conversion is hardcoded to return NotYetSupportedError; expected behavior is successful execution for supported ATTACH PARTITION flows.
• Impact: Users cannot complete a meaningful partition-management workflow with ALTER INDEX ATTACH PARTITION even when SQL parses correctly. This blocks expected DDL behavior without a direct in-product workaround.
• Steps to reproduce:
  1. Create parent and child index objects for a partition attach flow.
  2. Run ALTER INDEX public.parent_idx ATTACH PARTITION public.child_idx.
  3. Observe that execution returns unsupported instead of applying the attach operation.
• Stub / mock context: Real SCRAM sign-in was bypassed to keep local SQL sessions deterministic in this run; no API response mocking or route interception was applied for this test.
• Code analysis: I verified parser and conversion paths for ALTER INDEX in the repo: parser grammar builds an AlterIndexAttachPartition node, conversion dispatch reaches nodeAlterIndex, and that converter unconditionally returns an unsupported error for all ALTER INDEX statements.
• Why this is likely a bug: Production code accepts this statement shape in parser/dispatch but then always rejects execution, indicating missing implementation in a live code path rather than test setup noise.

Relevant code:

postgres/parser/parser/sql.y (lines 2029-2032)

| ALTER INDEX table_index_name ATTACH PARTITION db_object_name
  {
    $$.val = &tree.AlterIndex{Index: $3.tableIndexName(), Cmd: &tree.AlterIndexAttachPartition{Index: $6.unresolvedObjectName()}}
  }

server/ast/convert.go (lines 46-47)

case *tree.AlterIndex:
		return nodeAlterIndex(ctx, stmt)

server/ast/alter_index.go (lines 24-30)

func nodeAlterIndex(ctx *Context, node *tree.AlterIndex) (vitess.Statement, error) {
	if node == nil {
		return nil, nil
	}

	return NotYetSupportedError("ALTER INDEX is not yet supported")
}

⚠️ Legacy auth database load can break role mapping

• What failed: Legacy auth metadata load is expected to preserve a valid role graph, but role identity state can drift after load and break core auth lookups.
• Impact: Upgrades from legacy auth metadata can leave the server in a degraded authentication/authorization state where normal SQL access fails. This breaks a primary workflow with no practical user workaround.
• Steps to reproduce:
  1. Prepare a data directory with a v1 auth.db fixture.
  2. Start doltgres with that directory and perform basic authenticated SQL operations.
  3. Observe repeated role lookup errors and degraded auth behavior.
• Stub / mock context: The run intentionally used a legacy v1 auth metadata fixture to simulate upgrade behavior, and no route mocks or auth bypasses were used.
• Code analysis: I inspected auth deserialization and role-ID assignment paths. deserializeV1 repopulates rolesByID but does not restore userIDCounter; new roles still use userIDCounter.Add(1) and SetRole overwrites existing ID slots, which can clobber built-in role mappings such as public.
• Why this is likely a bug: The code permits ID collisions after legacy deserialization, and those collisions can delete or replace existing role-name mappings, which directly explains missing-role auth failures.

Relevant code:

server/auth/serialization.go (lines 108-118)

func (db *Database) deserializeV1(reader *utils.Reader) error {
	// Read the roles
	clear(db.rolesByName)
	clear(db.rolesByID)
	roleCount := reader.Uint32()
	for i := uint32(0); i < roleCount; i++ {
		r := Role{}
		r.deserialize(1, reader)
		db.rolesByName[r.Name] = r.id
		db.rolesByID[r.id] = r

server/auth/role.go (lines 47-50)

func CreateDefaultRole(name string) Role {
	r := createDefaultRoleWithoutID(name)
	r.id = RoleID(userIDCounter.Add(1))
	return r
}

server/auth/database.go (lines 116-120)

if existingRole, ok := globalDatabase.rolesByID[role.id]; ok {
	delete(globalDatabase.rolesByName, existingRole.Name)
}
globalDatabase.rolesByName[role.Name] = role.id
globalDatabase.rolesByID[role.ID()] = role

🚨 Malformed auth metadata can panic server startup

• What failed: Malformed persistence input should be rejected with a controlled error path, but unchecked byte reads can trigger runtime panics and abort process startup.
• Impact: A malformed auth metadata file can crash the server process during startup. This is a denial-of-service condition for affected deployments.
• Steps to reproduce:
  1. Truncate auth.db to an undersized byte payload.
  2. Start doltgres with the tampered file in the configured data directory.
  3. Observe panic-style crash behavior instead of controlled rejection.
• Stub / mock context: The test intentionally tampered auth metadata bytes to validate resilience against malformed persistence input, without route interception or bypass hooks.
• Code analysis: Reader helpers perform direct byte indexing and slicing without bounds checks, so truncated payloads can raise index out of range; initialization then panics on deserialize errors instead of returning a recoverable startup error.
• Why this is likely a bug: Production startup code has unchecked deserialization reads and panic-on-error handling, making malformed auth.db input capable of causing uncontrolled process termination.

Relevant code:

utils/reader.go (lines 40-43)

func (reader *Reader) Bool() bool {
	reader.offset += 1
	if reader.buf[reader.offset-1] == 1 {
		return true

utils/reader.go (lines 196-200)

func (reader *Reader) String() string {
	length := reader.VariableUint()
	reader.offset += length
	return string(reader.buf[reader.offset-length : reader.offset])
}

server/auth/database.go (lines 176-177)

} else if err = globalDatabase.deserialize(authData); err != nil {
	panic(err)
}

Commit: 2c6eb26

View Full Run

---

Tell us how we did: Give Ito Feedback