Skip to content

fix(plugin-redis)!: full SSL refactor with typed SSLConfiguration across plugin boundary (#1247)#1248

Merged
datlechin merged 15 commits into
mainfrom
refactor/typed-ssl-config
May 13, 2026
Merged

fix(plugin-redis)!: full SSL refactor with typed SSLConfiguration across plugin boundary (#1247)#1248
datlechin merged 15 commits into
mainfrom
refactor/typed-ssl-config

Conversation

@datlechin
Copy link
Copy Markdown
Member

Closes #1247.

Full refactor of the plugin SSL surface. Fixes the user-reported bug (Redis "Required (skip verify)" never actually skipped verification) plus three sibling bugs in the same code path, and replaces the stringly-typed dictionary at the plugin boundary with a typed SSLConfiguration.

Root cause

RedisSSLConfig read verifyPeer from additionalFields["sslVerifyPeer"] with a fallback of "true". The app core never wrote that key, so the fallback always won and every SSL mode performed verification.

But the real anti-pattern is the boundary itself: PluginDatabaseDriver.connect() received additionalFields: [String: String] and every plugin parsed the SSL strings differently. Six plugins, four working approaches, one broken (Redis), one missing TLS entirely (MSSQL). No compiler help, no shared semantics.

Refactor scope

  1. Typed SSLConfiguration boundary. New Plugins/TableProPluginKit/SSLConfiguration.swift defines the canonical SSLConfiguration struct + SSLMode enum with isEnabled, verifiesCertificate, verifiesHostname helpers. DriverConnectionConfig carries it as a first-class field. The app-side SSLConfiguration/SSLMode collapsed into typealiases of the PluginKit types so there is one canonical type; the app-side localized labels remain as an extension. PluginKit ABI bumped 11 → 12; every bundled plugin's Info.plist bumped.

  2. Redis ("Required (skip verify)" SSL mode does not actually skip certificate verification for Redis connections #1247). RedisSSLConfig extracted to its own file, drives directly from typed SSLMode. connectSSL maps per hiredis_ssl.h: .disabled skips TLS; .preferred/.required enable TLS with REDIS_SSL_VERIFY_NONE; .verifyCa/.verifyIdentity enable TLS with REDIS_SSL_VERIFY_PEER plus CA path; SNI hostname set only for .verifyIdentity. CA path gated on verifiesCertificate, matching PostgreSQL.

  3. MSSQL TLS, fresh implementation. Previously SSL was completely absent from FreeTDS dblib setup; the connection form's SSL pane did nothing. Now MSSQLSSLMapping.freetdsEncryptionFlag(for:) maps to FreeTDS strings and is passed via dbsetlname(login, value, DBSETENCRYPT). Documented limitation: dblib does not expose per-connection cert verification, so .verifyCa/.verifyIdentity both map to require and verification depends on the system trust store and freetds.conf.

  4. MongoDB. URI builder switches on SSLMode per libmongoc semantics. .preferred/.required set tlsAllowInvalidCertificates=true (no TLS fallback in libmongoc, so these behave identically). .verifyCa sets tlsAllowInvalidHostnames=true. CA file loaded only when verifying. mongodb+srv URIs force .required if user picked .disabled, per spec.

  5. PostgreSQL, MySQL, ClickHouse. Switched from String parsing to SSLMode switch. MySQL no longer loads CA file when not verifying.

  6. Tests. New SSLConfigurationTests (7 cases, computed helpers, Codable round-trip, raw-value stability). New RedisSSLConfigTests (6 cases, one per mode + path passthrough, regression cover for "Required (skip verify)" SSL mode does not actually skip certificate verification for Redis connections #1247). New MSSQLSSLMappingTests (5 cases, one per mode). Obsolete SSLModeStringTests deleted (it duplicated parsing logic the refactor eliminated).

  7. Docs. docs/databases/redis.mdx and docs/databases/mssql.mdx document the per-mode behavior tables. CHANGELOG entries under [Unreleased] cover Fixed ("Required (skip verify)" SSL mode does not actually skip certificate verification for Redis connections #1247, MSSQL, MongoDB, MySQL) and Changed (PluginKit ABI bump with registry-republish note).

Verification

  • swiftlint lint --strict on every touched file is clean. Other plugins have pre-existing violations on main that this PR does not change.
  • xcodebuild build succeeds (the two SwiftLint plugin failures in CodeEditSourceEditor/CodeEditTextView are the known headless sandbox issue, unrelated).
  • 28 SSL-related tests pass: SSLConfigurationTests (7), RedisSSLConfigTests (6), MSSQLSSLMappingTests (5), PostgresDumpServiceCommandTests including SSL mapping (10).

Post-merge action required

PluginKit ABI bumped to v12. Per CLAUDE.md post-bump checklist, every registry plugin (MongoDB, Oracle, DuckDB, MSSQL, Cassandra, Etcd, CloudflareD1, DynamoDB, BigQuery, LibSQL) must be re-tagged with a bumped patch version and plugins.json updated, otherwise users hit "Plugin was built with PluginKit version 11, but version 12 is required" on next app update. Do this before cutting the next app release.

Test plan

  • Smoke: PostgreSQL connection with .required mode still connects (verify-skip path).
  • Smoke: PostgreSQL with .verifyCa still connects (verify path).
  • Smoke: MySQL with .required connects against a self-signed server.
  • "Required (skip verify)" SSL mode does not actually skip certificate verification for Redis connections #1247 repro: Redis to Upstash (dear-toad-*.upstash.io:6379) with .required (skip verify) connects.
  • Redis to private CA endpoint with .verifyCa connects when CA is provided.
  • MSSQL to Azure SQL with .required connects.
  • MongoDB mongodb+srv:// connects when SSL was left .disabled (auto-upgraded to .required).

@datlechin datlechin merged commit 6b64534 into main May 13, 2026
1 check passed
@datlechin datlechin deleted the refactor/typed-ssl-config branch May 13, 2026 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Required (skip verify)" SSL mode does not actually skip certificate verification for Redis connections

1 participant