Skip to content

Commit 74a0760

Browse files
fix(service): merge sslmode params into one connection setting so host survives (#694)
A non-`unset` Postgres TLS mode dropped the `host`/`port`/`dbname` params and libpq fell back to the local Unix socket, so `DB_SSL_MODE=require` (and every verifying mode) failed to start against a remote host -- the documented ADR-0064 / #684 TLS-hardening path and the Azure Flexible Server golden path. Root cause: `toConnectionParams` emitted the base params and each TLS param as SEPARATE `Hasql.Connection.Setting.connection` values. hasql's `staticConnectionSettings` applies each one by REPLACING the whole connection string (`setConnectionString config {connectionString}` folded left -- last wins, not merge), so the `sslmode`-only setting wiped out host/port/dbname. `SslModeUnset` emitted no TLS setting, which is why only the unset path worked. Fix: build every libpq param -- base AND conditional TLS -- into ONE `ConnectionSettingConnection.params [...]`, i.e. a single `connection` Setting. The three pools (EventStore, FileUpload, QueryObjectStore) all route through this builder, so all three are fixed. The `unset` path stays byte-identical (same 9 params in one setting), preserving the ADR-0064 §2 no-regression guarantee. To make the contract testable after collapsing to one opaque setting, the param set is now produced as an inspectable `toParamPairs :: ... -> LinkedList (Text, Text)` (the hasql Param/Setting types have no Eq/Show and their rendering modules are internal). Tests assert directly that `host` survives every sslMode and that no `sslcert`/`sslkey` ever leaks in (ADR-0064 §3). Tests: - ConnectionConfigSpec: new #694 regression (exactly ONE connection setting for every sslMode) + `toParamPairs` host-survival and exact-param-set assertions, replacing the prior length-based ssl tests that encoded the multi-setting structure. Verified red: reintroducing the split setting yields 2 settings for `require` / 4 for `verify-full`+cert and fails the regression. - FileUpload FileStateStore PostgresSpec: the `#684` sslMode-threading test that asserted `Ok 2` settings (the buggy shape) now asserts `Ok 1` and verifies the `sslmode=require` param via `toParamPairs`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 54bb118 commit 74a0760

3 files changed

Lines changed: 249 additions & 90 deletions

File tree

core/service/Service/Infra/Postgres/ConnectionConfig.hs

Lines changed: 75 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ module Service.Infra.Postgres.ConnectionConfig (
66
sslModeToText,
77
textToSslMode,
88
resolveParams,
9+
toParamPairs,
910
toConnectionParams,
1011
toPoolConfig,
1112
logPoolObservation,
@@ -22,6 +23,7 @@ import Hasql.Connection.Setting.Connection.Param qualified as Param
2223
import Hasql.Pool.Config (Config)
2324
import Hasql.Pool.Config qualified as HasqlPoolConfig
2425
import Hasql.Pool.Observation (ConnectionStatus (..), ConnectionTerminationReason (..), Observation (..))
26+
import LinkedList qualified
2527
import Log qualified
2628
import Result qualified
2729
import Task qualified
@@ -114,73 +116,105 @@ resolveParams cfg =
114116
}
115117

116118

117-
-- | Conditional TLS 'Setting' list appended after the keepalive Setting.
119+
-- | The base libpq (key, value) params every pool always sends: the five
120+
-- connection coordinates plus the four ADR-0037 keepalive entries. Plain
121+
-- inspectable data (see 'toParamPairs').
122+
baseParamPairs :: ResolvedParams -> LinkedList (Text, Text)
123+
baseParamPairs resolved =
124+
[ ("host", resolved.host),
125+
("port", portToText resolved.port),
126+
("dbname", resolved.databaseName),
127+
("user", resolved.user),
128+
("password", resolved.password),
129+
-- TCP keepalive: detect dead connections in cloud environments
130+
-- (ADR-0037, #397) -- on ALL three pools.
131+
("keepalives", resolved.keepalives),
132+
("keepalives_idle", resolved.keepalivesIdle),
133+
("keepalives_interval", resolved.keepalivesInterval),
134+
("keepalives_count", resolved.keepalivesCount)
135+
]
136+
137+
138+
-- | Render the validated port to its libpq decimal text form. A standalone
139+
-- helper so the 'fmt' splice only ever sees a bare variable -- the quasiquoter
140+
-- does not parse a 'resolved.port' record-dot accessor (OverloadedRecordDot).
141+
portToText :: Word16 -> Text
142+
portToText p = [fmt|#{p}|]
143+
144+
145+
-- | Conditional TLS (key, value) params appended AFTER the base params.
118146
-- Returns [] for 'SslModeUnset' (byte-identical to pre-WI-5 — the dev/CI
119147
-- no-regression guarantee, ADR-0064 §2). For any set mode it emits the
120-
-- sslmode param as its own Setting; for a verifying mode WITH a root cert it
121-
-- additionally emits sslrootcert (and for verify-full: channel_binding=require)
122-
-- each as their own Setting. NEVER emits sslcert/sslkey (ADR-0064 §3).
123-
sslParams :: ResolvedParams -> LinkedList Setting
124-
sslParams resolved =
148+
-- sslmode param; for a verifying mode WITH a root cert it additionally emits
149+
-- sslrootcert (and for verify-full: channel_binding=require). NEVER emits
150+
-- sslcert/sslkey (ADR-0064 §3).
151+
sslParamPairs :: ResolvedParams -> LinkedList (Text, Text)
152+
sslParamPairs resolved =
125153
case sslModeToText resolved.sslMode of
126154
Nothing -> []
127155
Just token ->
128-
paramSetting (Param.other "sslmode" token) : rootCertParams resolved.sslMode resolved.sslRootCert
156+
("sslmode", token) : rootCertParamPairs resolved.sslMode resolved.sslRootCert
129157

130158

131-
-- | The verifying-mode companion Settings. 'sslrootcert' is emitted ONLY for
132-
-- the verifying modes ('SslModeVerifyCa' / 'SslModeVerifyFull') and only when a
159+
-- | The verifying-mode companion params. 'sslrootcert' is emitted ONLY for the
160+
-- verifying modes ('SslModeVerifyCa' / 'SslModeVerifyFull') and only when a
133161
-- root cert path is supplied — a root cert is meaningless for the non-verifying
134162
-- modes ('disable'/'allow'/'prefer'/'require'), so it is NEVER sent for them
135163
-- even if the environment provides a path (ADR-0064 §3). 'verify-full'
136164
-- additionally emits channel_binding=require. Forwards a PATH only — pins no
137165
-- cert (ADR-0064 §"Root-only trust"). NEVER emits sslcert/sslkey.
138-
rootCertParams :: SslMode -> Maybe Text -> LinkedList Setting
139-
rootCertParams mode maybeRootCert =
166+
rootCertParamPairs :: SslMode -> Maybe Text -> LinkedList (Text, Text)
167+
rootCertParamPairs mode maybeRootCert =
140168
case (mode, maybeRootCert) of
141169
(SslModeVerifyCa, Just path) ->
142-
[paramSetting (Param.other "sslrootcert" path)]
170+
[("sslrootcert", path)]
143171
(SslModeVerifyFull, Just path) ->
144-
[ paramSetting (Param.other "sslrootcert" path),
145-
paramSetting (Param.other "channel_binding" "require")
172+
[ ("sslrootcert", path),
173+
("channel_binding", "require")
146174
]
147175
_ ->
148176
[]
149177

150178

151-
-- | Wrap a single 'Param.Param' into a 'Setting' so ssl params are individual
152-
-- list entries (each adds 1 to 'LinkedList.length', making the length assertions
153-
-- in the spec numerically meaningful).
154-
paramSetting :: Param.Param -> Setting
155-
paramSetting p =
156-
ConnectionSettingConnection.params [p]
157-
|> ConnectionSetting.connection
179+
-- | The complete, ORDERED libpq (key, value) parameter set — base params first,
180+
-- then any conditional TLS params — that the single connection 'Setting' is
181+
-- built from (ADR-0062 section 2). Carries the four ADR-0037 keepalives for
182+
-- every pool and fails fast on an out-of-range port (see 'validatePort').
183+
--
184+
-- Exposed as plain inspectable data because the hasql 'Param'/'Setting' types
185+
-- are opaque (no Eq/Show) and the modules that render them to a connection
186+
-- string are internal to hasql, so this is the ONLY place a test can assert the
187+
-- exact params — including that 'host' survives a set sslMode (the #694
188+
-- regression) and that no sslcert/sslkey ever leaks in (ADR-0064 §3).
189+
toParamPairs :: ConnectionParams -> Result Text (LinkedList (Text, Text))
190+
toParamPairs cfg =
191+
cfg
192+
|> resolveParams
193+
|> Result.map \resolved -> baseParamPairs resolved ++ sslParamPairs resolved
158194

159195

160196
-- | The single place libpq connection params are constructed (ADR-0062
161-
-- section 2). Carries the four ADR-0037 keepalives for every pool and fails
162-
-- fast on an out-of-range port (see 'validatePort'). Returns a 'Result' so
163-
-- the three pools surface the typed error in their own error channel.
197+
-- section 2). Returns a 'Result' so the three pools surface the typed error in
198+
-- their own error channel.
199+
--
200+
-- Every param — base AND the conditional TLS params — goes into ONE
201+
-- 'ConnectionSettingConnection.params', i.e. a SINGLE 'ConnectionSetting.connection'
202+
-- (#694). hasql's 'Hasql.Pool.Config.staticConnectionSettings' applies each
203+
-- 'ConnectionSetting.connection' by REPLACING the whole connection string
204+
-- (last-wins, not merge), so emitting the TLS params as a second connection
205+
-- Setting silently dropped host/port/dbname and libpq fell back to the local
206+
-- Unix socket. Returning a one-element list keeps the 'LinkedList Setting'
207+
-- shape every pool already threads through 'toPoolConfig'.
164208
toConnectionParams :: ConnectionParams -> Result Text (LinkedList Setting)
165209
toConnectionParams cfg =
166210
cfg
167-
|> resolveParams
168-
|> Result.map \resolved -> do
169-
let baseParams =
170-
[ Param.host resolved.host,
171-
Param.port resolved.port,
172-
Param.dbname resolved.databaseName,
173-
Param.user resolved.user,
174-
Param.password resolved.password,
175-
Param.other "keepalives" resolved.keepalives,
176-
Param.other "keepalives_idle" resolved.keepalivesIdle,
177-
Param.other "keepalives_interval" resolved.keepalivesInterval,
178-
Param.other "keepalives_count" resolved.keepalivesCount
179-
]
180-
let baseSetting =
181-
ConnectionSettingConnection.params baseParams
182-
|> ConnectionSetting.connection
183-
baseSetting : sslParams resolved
211+
|> toParamPairs
212+
|> Result.map \pairs ->
213+
[ pairs
214+
|> LinkedList.map (\(key, value) -> Param.other key value)
215+
|> ConnectionSettingConnection.params
216+
|> ConnectionSetting.connection
217+
]
184218

185219

186220
-- | The single place 'Hasql.Pool.Config' is constructed (ADR-0062 section 3).

0 commit comments

Comments
 (0)