[tobyhede]

Requested by Toby

SettingsCommand::from_statement discarded SettingChange::parse errors with .ok(), so an invalid CIPHERSTASH.* setting silently became SettingsCommand::Other.
Every current caller runs validate_setting first, so this was unreachable, but a future caller that skipped validation would treat an invalid setting as an ordinary statement.

Changes:

• SettingsCommand::from_statement now returns Result<Self, Error>.
• SettingsProtocol::prepare, SettingsProtocol::simple_query, project_settings_effects and Context::settings_simple_query_requires_fail_closed propagate the error.
• New unit test settings_command_reports_an_invalid_cipherstash_setting covers an invalid keyset id, an invalid mapping value and an unknown CIPHERSTASH.* name.

No behaviour change for clients, so no changelog entry.

Validated locally with cargo test -p cipherstash-proxy --lib (424 passed), cargo clippy --all-targets --all-features -- -D warnings and cargo fmt --check.

Acknowledgment

By submitting this pull request, I confirm that CipherStash can use, modify, copy, and redistribute this contribution, under the terms of CipherStash's choice.

---

Generated by Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	🔄 Running since 2026-10-22T03:19:29.010204Z	bf4322f	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[cipherstash-agent]

Review attestation: Claude Code /code-review medium followed by company-skills:cross-check-review-findings, against head bf4322f.

Result: no findings. No critical or blocking issues.

• All callers of SettingsCommand::from_statement (prepare, simple_query, project_settings_effects, settings_simple_query_requires_fail_closed) now propagate the error. No .ok() discard remains.
• Error paths leave no partial state. prepare builds the command before inserting into self.prepared, and simple_query fails in project_settings_effects before touching pending.
• Existing callers still run validate_setting first (lifecycle.rs:158, frontend.rs:350), so there is no behaviour change for clients, as the description says.
• Existing PR feedback: only a Codex status comment (review still running), nothing to cross-check against.

Generated by Claude Code

---

Generated by Claude Code

[freshtonic]

Reviewed head 235f796b. I found no problems.

What I checked

• SettingsCommand::from_statement has two callers outside the tests: SettingsProtocol::prepare and project_settings_effects. Both now propagate the error. No .ok() discard is left.
• The change is unreachable for clients today, as the description says. validate_setting (lifecycle.rs:521) calls the same SettingChange::parse with ?, so every input that from_statement now rejects is rejected first, by validate_setting:
  • Extended protocol: prepare_parse calls validate_setting (lifecycle.rs:158) before prepare_settings_statement (:159).
  • Simple protocol: query_handler validates every parsed statement (frontend.rs:349-351) before settings_simple_query_requires_fail_closed and before both register_query calls (:416, :516). register_query is the only caller of execute_simple_settings_statements, and it receives a subset of the validated statements.
• No partial state is left after an error. prepare builds the command before it inserts into prepared. simple_query returns an error from project_settings_effects before it changes pending.
• project_settings_effects now collects into a Vec and does not return a lazy iterator. This is correct: a later statement's projection depends on the earlier statements, and the batch is small.
• The new test fails if the .ok() discard comes back. With .ok(), all three inputs become SettingsCommand::Other, so is_err() is false.
• CI passed on this head, with all 9 checks.

Approved.