Propagate settings parse errors from SettingsCommand::from_statement

packages/cipherstash-proxy/src/postgresql/context/mod.rs

@@ -1449,8 +1449,7 @@ where
         let settings = settings_protocol::EffectiveSettings::new(&state.settings);
         state
             .settings_protocol
-            .prepare(operation, name, statement, settings);
-        Ok(())
+            .prepare(operation, name, statement, settings)
     }
 
     fn bind_settings_statement(
@@ -1525,26 +1524,26 @@ where
             settings_protocol,
             ..
         } = &mut *state;
-        Ok(settings_protocol.simple_query(operation, statements, settings))
+        settings_protocol.simple_query(operation, statements, settings)
     }
 
     pub fn settings_simple_query_requires_fail_closed(
         &self,
         statements: &[sqltk::parser::ast::Statement],
-    ) -> bool {
+    ) -> Result<bool, Error> {
         let Ok(state) = self.protocol_state.read() else {
-            return true;
+            return Ok(true);
         };
         let mut changed = false;
         for (statement, _, changes) in
-            settings_protocol::project_settings_effects(&state.settings, statements)
+            settings_protocol::project_settings_effects(&state.settings, statements)?
         {
             if changed && eql_mapper::requires_type_check(statement) {
-                return true;
+                return Ok(true);
             }
             changed |= changes;
         }
-        false
+        Ok(false)
     }
 
     pub fn discarding_extended_messages(&self) -> Result<bool, Error> {
@@ -2559,9 +2558,11 @@ mod tests {
             "RESET ALL; SELECT 1",
         ] {
             assert!(
-                context.settings_simple_query_requires_fail_closed(
-                    &crate::postgresql::parser::SqlParser::parse_statements(sql).unwrap()
-                ),
+                context
+                    .settings_simple_query_requires_fail_closed(
+                        &crate::postgresql::parser::SqlParser::parse_statements(sql).unwrap()
+                    )
+                    .unwrap(),
                 "{sql}"
             );
         }
@@ -2629,9 +2630,11 @@ mod tests {
         })
         .join();
 
-        assert!(context.settings_simple_query_requires_fail_closed(
-            &crate::postgresql::parser::SqlParser::parse_statements("SELECT 1").unwrap()
-        ));
+        assert!(context
+            .settings_simple_query_requires_fail_closed(
+                &crate::postgresql::parser::SqlParser::parse_statements("SELECT 1").unwrap()
+            )
+            .unwrap());
     }
 
     #[test]
@@ -2662,7 +2665,9 @@ mod tests {
         ] {
             let statements = crate::postgresql::parser::SqlParser::parse_statements(sql).unwrap();
             assert!(
-                !context.settings_simple_query_requires_fail_closed(&statements),
+                !context
+                    .settings_simple_query_requires_fail_closed(&statements)
+                    .unwrap(),
                 "{sql}"
             );
         }

packages/cipherstash-proxy/src/postgresql/context/settings.rs

@@ -167,8 +167,8 @@ pub enum SettingsCommand {
 }
 
 impl SettingsCommand {
-    pub fn from_statement(statement: &Statement) -> Self {
-        match statement {
+    pub fn from_statement(statement: &Statement) -> Result<Self, Error> {
+        Ok(match statement {
             Statement::StartTransaction { .. } => Self::Begin,
             Statement::Commit { .. } => Self::Commit,
             Statement::Rollback {
@@ -180,12 +180,10 @@ impl SettingsCommand {
             } => Self::RollbackTo(postgres_identifier(name)),
             Statement::Savepoint { name } => Self::Savepoint(postgres_identifier(name)),
             Statement::ReleaseSavepoint { name } => Self::Release(postgres_identifier(name)),
-            _ => SettingChange::parse(statement)
-                .ok()
-                .flatten()
+            _ => SettingChange::parse(statement)?
                 .map(Self::Change)
                 .unwrap_or(Self::Other),
-        }
+        })
     }
 
     pub fn may_change_effective(&self, settings: &ConnectionSettings) -> bool {
@@ -310,7 +308,10 @@ impl ConnectionSettings {
     /// Applies only a command confirmed successful by PostgreSQL.
     #[cfg(test)]
     pub fn execution_succeeded(&mut self, statement: &Statement, command_tag: &str) {
-        self.command_succeeded(&SettingsCommand::from_statement(statement), command_tag);
+        self.command_succeeded(
+            &SettingsCommand::from_statement(statement).unwrap(),
+            command_tag,
+        );
     }
 
     pub fn command_succeeded(&mut self, command: &SettingsCommand, command_tag: &str) {
@@ -875,6 +876,21 @@ mod tests {
         }
     }
 
+    #[test]
+    fn settings_command_reports_an_invalid_cipherstash_setting() {
+        for sql in [
+            "SET CIPHERSTASH.KEYSET_ID = 'invalid'",
+            "SET CIPHERSTASH.UNSAFE_DISABLE_MAPPING = 'maybe'",
+            "SET CIPHERSTASH.UNKNOWN = 'value'",
+        ] {
+            let statements = crate::postgresql::parser::SqlParser::parse_statements(sql).unwrap();
+            assert!(
+                SettingsCommand::from_statement(&statements[0]).is_err(),
+                "{sql}"
+            );
+        }
+    }
+
     fn parse_one(sql: &str) -> Result<Option<SettingChange>, Error> {
         let statements = crate::postgresql::parser::SqlParser::parse_statements(sql).unwrap();
         SettingChange::parse(&statements[0])
@@ -1017,7 +1033,9 @@ mod tests {
             "SET",
         );
         let commit = crate::postgresql::parser::SqlParser::parse_statement("COMMIT").unwrap();
-        assert!(SettingsCommand::from_statement(&commit).may_change_effective(&settings));
+        assert!(SettingsCommand::from_statement(&commit)
+            .unwrap()
+            .may_change_effective(&settings));
     }
 
     #[test]