fixes

scripts/images/rds-mariadb/rds-init

@@ -252,14 +252,29 @@ require_data_volume() {
     log "warning: ${DATA_MOUNT} is not a mount point; data will not survive a VM replace (RDS_ALLOW_LOCAL_DATADIR=1)"
 }
 
-# True when the datadir holds anything at all. A failed enumeration is neither
-# "empty" nor "has content": it is a refusal, because both callers below decide
-# whether something may be created or deleted from the answer.
-datadir_has_content() {
-    [ -d "${DATADIR}" ] || return 1
-    _contents=$(ls -A -- "${DATADIR}" 2>/dev/null) ||
-        fail "could not inspect datadir ${DATADIR}; refusing to decide whether it is ours"
-    [ -n "${_contents}" ]
+# True when the data volume holds anything that is not ours. The mount is what
+# is inspected rather than ${DATADIR}: another engine lays its data out under its
+# own name, so a PostgreSQL volume has content here while ${DATADIR} does not
+# exist at all.
+data_volume_has_content() {
+    [ -d "${DATA_MOUNT}" ] || return 1
+
+    # A mount that cannot be read is neither empty nor not: it is a refusal,
+    # because the caller decides from the answer whether to initialise.
+    ls -A -- "${DATA_MOUNT}" >/dev/null 2>&1 ||
+        fail "could not inspect ${DATA_MOUNT}; refusing to decide whether the data volume is ours"
+
+    # Globbed rather than split out of `ls`, so an entry carrying whitespace
+    # counts once. lost+found is the filesystem's, and the metadata and include
+    # directories are ours — none of the three says whose data the volume holds.
+    for _entry in "${DATA_MOUNT}"/* "${DATA_MOUNT}"/.[!.]* "${DATA_MOUNT}"/..?*; do
+        [ -e "${_entry}" ] || [ -L "${_entry}" ] || continue
+        case "${_entry##*/}" in
+            lost+found | .spinifex-rds | "${CONF_DIR##*/}") continue ;;
+        esac
+        return 0
+    done
+    return 1
 }
 
 # Written before the datadir is inspected, so a volume this image goes on to
@@ -304,11 +319,11 @@ require_engine_stamp() {
     fi
 
     # No MariaDB volume in the field predates the stamp, so unlike PostgreSQL
-    # there is nothing to backfill onto: an unstamped datadir with content in it
+    # there is nothing to backfill onto: an unstamped volume with content on it
     # is by definition not ours, and stamping it would turn the check into a
     # rubber stamp on exactly the volume it exists to refuse.
-    if datadir_has_content; then
-        fail "${DATADIR} holds data but the volume carries no ${ENGINE} engine stamp; refusing to touch it. Recover the volume out of band."
+    if data_volume_has_content; then
+        fail "${DATA_MOUNT} holds data but the volume carries no ${ENGINE} engine stamp; refusing to touch it. Recover the volume out of band."
     fi
 
     log "data volume carries no engine stamp — recording ${ENGINE}"
@@ -639,13 +654,24 @@ FLUSH PRIVILEGES;
 SQL
 }
 
-# The master user is administrative but deliberately holds none of the
-# privileges that reach outside the database: FILE is SELECT INTO OUTFILE and
-# LOAD_FILE against any path as the mysql OS user, INSERT on mysql.* is native
-# code execution through CREATE FUNCTION ... SONAME, SUPER and its 11.8 split-out
-# successors reach SET GLOBAL and plugin loading, and SHUTDOWN stops the server
-# underneath the control plane's lifecycle. Master credentials must be a database
-# privilege, not a shell in the DB VM.
+# The master user is administrative but is not granted the privileges that reach
+# outside the database: FILE is SELECT INTO OUTFILE and LOAD_FILE against any
+# path as the mysql OS user, SUPER and its 11.8 split-out successors reach SET
+# GLOBAL and plugin loading, and SHUTDOWN stops the server underneath the control
+# plane's lifecycle. Master credentials must be a database privilege, not a shell
+# in the DB VM.
+#
+# What the grant list cannot do is withhold the mysql schema. MariaDB privileges
+# are additive and it has no DENY, so the DML below reaches mysql.global_priv,
+# and REVOKE ... ON mysql.* is refused outright (ER_NONEXISTING_GRANT) because no
+# database-level row exists to revoke from. A master that writes that table and
+# runs FLUSH PRIVILEGES therefore holds ALL PRIVILEGES, including the three above.
+#
+# What still holds after that is the boundary this preset actually depends on:
+# secure_file_priv and plugin_dir are read-only variables, so 90-rds-init.cnf
+# pins them past any SET GLOBAL and FILE reaches only an empty root-owned
+# directory. The residual exposure is SHUTDOWN and unrecorded SET GLOBAL, not a
+# shell in the DB VM.
 apply_master_user() {
     log "creating the master user ${RDS_MASTER_USERNAME}"
     # Enumerated rather than GRANT ALL followed by revocations: a MariaDB release

scripts/images/rds-mariadb/rds-init_test.sh

@@ -387,15 +387,35 @@ if run_ok "initialize"; then
         || fail "initialize: the database was created under the compiled default"
 
     # Withheld deliberately: FILE is arbitrary host file access as the mysql user,
-    # SUPER reaches SET GLOBAL and plugin loading, and SHUTDOWN stops the server
-    # underneath the control plane's lifecycle.
-    for privilege in SUPER FILE SHUTDOWN 'ALL PRIVILEGES'; do
+    # SUPER and its 11.8 split-out successors reach SET GLOBAL and plugin loading,
+    # and SHUTDOWN stops the server underneath the control plane's lifecycle.
+    for privilege in SUPER FILE SHUTDOWN 'ALL PRIVILEGES' 'SET USER' \
+        'BINLOG ADMIN' 'BINLOG REPLAY' 'CONNECTION ADMIN' 'FEDERATED ADMIN' \
+        'READ_ONLY ADMIN' 'REPLICATION MASTER ADMIN' 'REPLICATION SLAVE ADMIN' \
+        'SLAVE MONITOR'; do
         grep -q "${privilege}" "${CLIENT_CALLS}" \
             && fail "initialize: the master grant includes ${privilege}" \
             || pass "initialize: master holds no ${privilege}"
     done
-    grep -q 'GRANT SELECT, INSERT, UPDATE, DELETE' "${CLIENT_CALLS}" \
-        && pass "initialize: the grant is enumerated" || fail "initialize: no enumerated grant"
+
+    # Asserted whole rather than by prefix. A privilege appended to the list is a
+    # privilege the master silently gains, and the enumeration exists precisely so
+    # that widening it has to be deliberate.
+    _grant=$(sed -n '/^GRANT /,/WITH GRANT OPTION;/p' "${CLIENT_CALLS}" | tr '\n' ' ' | tr -s ' ')
+    _expected_grant="GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, RELOAD, PROCESS, \
+REFERENCES, INDEX, ALTER, SHOW DATABASES, CREATE TEMPORARY TABLES, \
+LOCK TABLES, EXECUTE, REPLICATION SLAVE, REPLICATION CLIENT, \
+CREATE VIEW, SHOW VIEW, CREATE ROUTINE, ALTER ROUTINE, \
+CREATE USER, EVENT, TRIGGER ON *.* TO 'mulgamaster'@'%' WITH GRANT OPTION; "
+    [ "${_grant}" = "${_expected_grant}" ] \
+        && pass "initialize: the grant is exactly the enumerated list" \
+        || fail "initialize: the master grant has changed: ${_grant}"
+
+    # MariaDB privileges are additive and it has no DENY, so scope is the only
+    # control left: a second ON clause would widen the master beyond this list.
+    [ "$(grep -c '^  ON ' "${CLIENT_CALLS}")" = "1" ] \
+        && pass "initialize: the grant is scoped ON *.* and nothing else" \
+        || fail "initialize: the master carries more than one grant scope"
     grep -q "WITH GRANT OPTION" "${CLIENT_CALLS}" \
         && pass "initialize: master can delegate its own privileges" \
         || fail "initialize: master cannot grant"
@@ -699,7 +719,7 @@ unset LS_FAIL
 [ -s "${INSTALLDB_CALLS}" ] \
     && fail "datadir-probe-fail: initialised after the probe failed" \
     || pass "datadir-probe-fail: nothing created"
-grep -q 'could not inspect datadir' "${WORK}/out" \
+grep -q 'refusing to decide whether the data volume is ours' "${WORK}/out" \
     && pass "datadir-probe-fail: refusal explains why" || fail "datadir-probe-fail: no refusal message"
 
 # --- Case 6c: the bootstrap server never starts ---
@@ -1012,6 +1032,47 @@ if run_ok "stamp-absent-empty"; then
     grep -q 'no mariadb engine stamp' "${WORK}/out" \
         && pass "stamp-absent-nonempty: refusal names the missing stamp" \
         || fail "stamp-absent-nonempty: no refusal message"
+
+    # --- Case 12f: an unstamped PostgreSQL volume is refused ---
+    # The one unstamped volume that actually exists in the field, and the reason
+    # emptiness is decided on the mount: PostgreSQL keeps its cluster under
+    # ${DATA_MOUNT}/18/data, so ${DATADIR} is absent and the volume reads as new.
+    rm -rf "${DATADIR}" "${STAMP}"
+    mkdir -p "${DATA_MOUNT}/18/data"
+    echo 'customer cluster data' > "${DATA_MOUNT}/18/data/PG_VERSION"
+    : > "${INSTALLDB_CALLS}"
+    run_fails "stamp-absent-pgvolume"
+    [ -s "${INSTALLDB_CALLS}" ] \
+        && fail "stamp-absent-pgvolume: initialised over a PostgreSQL volume" \
+        || pass "stamp-absent-pgvolume: nothing created"
+    [ -f "${DATA_MOUNT}/18/data/PG_VERSION" ] \
+        && pass "stamp-absent-pgvolume: the PostgreSQL cluster is preserved" \
+        || fail "stamp-absent-pgvolume: the PostgreSQL cluster was touched"
+    # A stamp here is the worst outcome of all: the PostgreSQL image refuses a
+    # 'mariadb' stamp too, so the volume becomes unreachable from both engines.
+    [ -e "${STAMP}" ] \
+        && fail "stamp-absent-pgvolume: another engine's volume was stamped ours" \
+        || pass "stamp-absent-pgvolume: no stamp written"
+    grep -q 'no mariadb engine stamp' "${WORK}/out" \
+        && pass "stamp-absent-pgvolume: refusal names the missing stamp" \
+        || fail "stamp-absent-pgvolume: no refusal message"
+    rm -rf "${DATA_MOUNT}/18"
+
+    # --- Case 12g: lost+found alone does not make a fresh volume look used ---
+    # ext4 creates it at mkfs, so counting it as content would refuse every
+    # genuinely new data volume.
+    rm -rf "${DATADIR}" "${STAMP}"
+    mkdir -p "${DATA_MOUNT}/lost+found"
+    : > "${INSTALLDB_CALLS}"
+    write_handoff initialize 's3cr3t' appdb
+    if run_ok "stamp-absent-lostfound"; then
+        [ -s "${INSTALLDB_CALLS}" ] \
+            && pass "stamp-absent-lostfound: a fresh volume still initialises" \
+            || fail "stamp-absent-lostfound: nothing was initialised"
+        [ "$(cat "${STAMP}")" = "mariadb" ] \
+            && pass "stamp-absent-lostfound: the fresh volume was stamped" \
+            || fail "stamp-absent-lostfound: no stamp written"
+    fi
 fi
 
 # --- Case 12e: a stamp that cannot be written is fatal, before initialisation ---

spinifex/handlers/rds/engine.go

@@ -40,6 +40,9 @@ type Engine struct {
 	// What a snapshot taken without a quiesce actually recovers on restore, which
 	// is the engine's own guarantee and not a shared one.
 	crashRecoveryNote string
+	// The same guarantee stated for the next start rather than for a restore,
+	// which is what an engine that would not shut down cleanly gets.
+	uncleanStopNote string
 }
 
 var engines = map[string]Engine{
@@ -60,11 +63,15 @@ func indexEnginesByFamily() map[string]Engine {
 		if engine.validateDBName == nil {
 			panic("rds: engine " + engine.Name + " registers no DBName rule")
 		}
-		// An engine without one would tell a customer nothing about what a
-		// crash-consistent snapshot of it recovers, on the one event that says so.
+		// An engine without these would tell a customer nothing about what a
+		// crash-consistent snapshot or an unclean stop of it recovers, on the two
+		// events that say so.
 		if engine.crashRecoveryNote == "" {
 			panic("rds: engine " + engine.Name + " registers no crash-recovery note")
 		}
+		if engine.uncleanStopNote == "" {
+			panic("rds: engine " + engine.Name + " registers no unclean-stop note")
+		}
 		family := engine.ParameterGroupFamily()
 		if _, exists := out[family]; exists {
 			panic("rds: parameter group family " + family + " is claimed by two engines")

spinifex/handlers/rds/engine_mariadb.go

@@ -36,6 +36,8 @@ var engineMariaDB = Engine{
 	// mid-write can leave one of their tables inconsistent with no way back.
 	crashRecoveryNote: "InnoDB tables will recover from the redo log when it is restored; " +
 		"non-transactional tables such as Aria and MyISAM may be left inconsistent.",
+	uncleanStopNote: "InnoDB tables will recover from the redo log on the next start; " +
+		"non-transactional tables such as Aria and MyISAM may be left inconsistent.",
 }
 
 // The curated MariaDB 11.8 table, the same shape and approximate size as the

spinifex/handlers/rds/engine_postgres.go

@@ -31,6 +31,7 @@ var enginePostgres = Engine{
 	validateCombinations: validatePostgresParameterCombinations,
 	// Every table is WAL-logged, so a torn datadir recovers in full.
 	crashRecoveryNote: "It will recover from its write-ahead log when it is restored.",
+	uncleanStopNote:   "It will recover from its write-ahead log on the next start.",
 }
 
 // The curated PostgreSQL 18 table: a genuinely validated subset rather than the

spinifex/handlers/rds/enginefamily_test.go

@@ -42,6 +42,7 @@ var engineUnderTest = Engine{
 	),
 	validateCombinations: func([]Parameter) error { return nil },
 	crashRecoveryNote:    "It will recover when it is restored.",
+	uncleanStopNote:      "It will recover on the next start.",
 }
 
 // Registers the second engine for one test. Both indexes are rebuilt, because a

spinifex/handlers/rds/lifecycle.go

@@ -233,19 +233,34 @@ func (s *Service) confirmVMStopped(ctx context.Context, accountID, instanceID st
 }
 
 // Asks the engine to shut down cleanly, and records an event rather than
-// failing the call when it cannot. An engine that did not stop cleanly replays
-// its WAL on the next start; refusing to stop the VM over it would leave the
-// customer unable to stop an instance whose agent is wedged.
+// failing the call when it cannot. Refusing to stop the VM over it would leave
+// the customer unable to stop an instance whose agent is wedged.
 func (s *Service) stopEngineOrRecordFallback(ctx context.Context, accountID string, rec *DBInstanceRecord, operation string) {
 	if err := s.stopEngine(ctx, accountID, rec.DBInstanceIdentifier); err != nil {
 		slog.WarnContext(ctx, "rds: graceful engine stop failed; continuing",
 			"dbInstance", rec.DBInstanceIdentifier, "operation", operation, "err", err)
 		s.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, rec.DBInstanceIdentifier,
-			fmt.Sprintf("The database engine could not be shut down cleanly before %s; it will recover from its write-ahead log on the next start.", operation),
+			uncleanStopMessage(ctx, rec.Engine, operation),
 			EventCategoryNotification, EventCategoryAvailability)
 	}
 }
 
+// The half of the warning that holds for every engine. What the next start then
+// recovers does not: PostgreSQL replays every table from its write-ahead log,
+// MariaDB only its InnoDB ones.
+func uncleanStopMessage(ctx context.Context, engineName, operation string) string {
+	warning := fmt.Sprintf("The database engine could not be shut down cleanly before %s.", operation)
+	engine, err := LookupEngine(engineName)
+	if err != nil {
+		// The VM is going down either way, so the customer still gets the half of
+		// the warning that does not depend on knowing the engine.
+		slog.ErrorContext(ctx, "rds: the DB instance names an engine this build does not offer",
+			"engine", engineName, "err", err)
+		return warning
+	}
+	return warning + " " + engine.uncleanStopNote
+}
+
 // Moves the instance into a transitional state under CAS and returns the record
 // as written. from lists the statuses the operation is legal from, so a stop of
 // an already-stopping instance is rejected as a state error rather than racing

spinifex/handlers/rds/lifecycle_test.go

@@ -679,3 +679,23 @@ func TestReconciler_MarksFailedWhenARestartOverrunsItsBound(t *testing.T) {
 	assert.Equal(t, StatusFailed, rec.Status)
 	assert.Contains(t, rec.FailureReason, "did not report healthy")
 }
+
+// What the next start recovers is the engine's own guarantee. Telling a MariaDB
+// customer a write-ahead log will bring back an Aria or MyISAM table would be a
+// false assurance about exactly the data most at risk.
+func TestUncleanStopMessage_IsEngineAware(t *testing.T) {
+	postgres := uncleanStopMessage(t.Context(), "postgres", "stopping the instance")
+	assert.Contains(t, postgres, "could not be shut down cleanly before stopping the instance")
+	assert.Contains(t, postgres, "write-ahead log")
+
+	mariadb := uncleanStopMessage(t.Context(), "mariadb", "rebooting the instance")
+	assert.Contains(t, mariadb, "could not be shut down cleanly before rebooting the instance")
+	assert.Contains(t, mariadb, "InnoDB tables will recover")
+	assert.Contains(t, mariadb, "may be left inconsistent")
+	assert.NotContains(t, mariadb, "write-ahead log")
+
+	// The VM is going down either way, so an engine this build cannot resolve
+	// still gets the half of the warning that does not depend on knowing it.
+	unknown := uncleanStopMessage(t.Context(), "oracle", "stopping the instance")
+	assert.Equal(t, "The database engine could not be shut down cleanly before stopping the instance.", unknown)
+}