Accept true/false for --ssl_mode, keeping on/off

[rolandwalker]

Description

More liberally accept true/false/on/off (and capitalization variants) for --ssl_mode.

Some inconsistencies in the interface have crept in as features were added.

Some settings from ~/.myclirc especially, but also in the CLI arguments, use str_to_bool() for liberal boolean parsing. Other settings, such as --ssl_mode, are using on/off only. Other settings, such as --use_keyring, are using true/false only.

This change is a step towards converging all boolean settings on liberal str_to_bool(). It should ultimately include DSN query parameters, too.

The "auto" value for ssl_mode is still special-cased, and not affected by these changes.

The only loss is that click doesn't automatically display the choices in the helpdoc, but they are already listed in the description. We keep the description targeted to on/off, and only silently accept the other variants.

Checklist

• I added this contribution to the changelog.md file.
• I added my name to the AUTHORS file (or it's already there).
• To lint and format the code, I ran

  uv run ruff check && uv run ruff format && uv run mypy --install-types .

[github-actions]

Findings

1. Crash on startup when ssl_mode not set
   In MyCli.__init__, ssl_mode can be None. The new code does if ssl_mode is None: pass and then unconditionally calls ssl_mode.lower(), which will raise AttributeError when no config is set. This is a regression that will break default startup.
   File: mycli/main.py:228
   Action: Make the lower() check conditional (elif ssl_mode is not None:) or early-return on None.

2. Duplicate test name overwrites earlier test
   There are two functions named test_ssl_mode_false. Pytest will only keep the last one, so the ssl_mode='true' case never runs.
   File: test/test_main.py:154 and test/test_main.py:176
   Action: Rename one test (e.g. test_ssl_mode_true) to keep both cases.

3. Invalid --ssl-mode now raises unhandled ValueError
   CLI parsing changed from click.Choice to type=str, and invalid values now flow to str_to_on_off which raises ValueError. There’s no handling to convert this into a user-friendly Click error, so the CLI will crash with a traceback.
   File: mycli/main.py:1995
   Action: Wrap str_to_on_off in a try/except and raise click.BadParameter with a clear message.

Open questions / assumptions

• Is it acceptable for ssl_mode to be boolean in config? If so, the config validation should handle non-string values safely before calling .lower().

Test gaps

• Add a test for invalid --ssl-mode to ensure a clean CLI error (not a traceback).

[github-actions]

Findings

1. Correctness: potential crash on non-string ssl_mode values

   • ssl_mode.lower() is called without guarding the type in both config validation and CLI processing. If ssl_mode is ever a non-string (e.g., a boolean from programmatic use or a list from a malformed config), this will raise AttributeError and bypass the intended “invalid value” handling.
   • Files: mycli/main.py:233, mycli/main.py:2008
   • Fix: normalize first, e.g., ssl_mode_str = str(ssl_mode).lower() (or use str_to_bool/str_to_on_off with a safe type check) and keep the error path.

2. Missing tests: invalid --ssl-mode value handling

   • Previously Click rejected invalid choices; now it is runtime-validated. There should be a test that an invalid --ssl-mode (e.g., --ssl-mode maybe) exits non-zero and prints the error.
   • File: test/test_main.py
   • Add a test that asserts failure and error message.

Notes

• No security concerns found in this change.

Potential next steps

1. Add a guard/normalization helper for ssl_mode to avoid type-related crashes.
2. Add a CLI test for invalid --ssl-mode values to lock in the new runtime validation.

[scottnemes]

Agree consistency is good, but --ssl_mode = true/false doesn't make sense from a naming perspective (it's not like --ssl = true/false, which makes more sense). A mode isn't true or false, it's on / off / auto. Converting that to true/false on the backend could make sense if desired for type checking, but I wouldn't expose it to the user.

Also the point of choice was to restrict it to those values, so isn't any issue with people putting in unexpected values.