fix(msdsn): preserve connection settings in DSN URLs

[sary (sary8)]

Fixes #455.

Config.URL() kept its own list of what to write while Parse grew new parameters, so a
Parse -> URL().String() -> Parse round trip reset whatever the serializer had never been told
about. The worst case is an explicit encrypt=false. It came back with certificate
verification off, because parseTLS reads a missing encrypt as a reason to trust the server:

before: encryption=0 trustServerCertificate=false skipVerify=false
after:  encryption=0 trustServerCertificate=true  skipVerify=true

The fix writes every setting that has a Config field from that field, whenever it differs
from what reparsing the URL without it would produce. Settings with no field (the certificate
paths, fedauth, authenticator, krb5-*) are carried from an allowlist rather than by
copying Parameters, because a copy would pass systemtoken, clientassertion and
userassertion through Redacted() unmasked; it masks only the userinfo password. The
certificate rules follow what crypto/tls actually does with the tls.Config rather than any
one field on it.

What changes for users

• URL() writes more parameters. Signature unchanged; a downstream golden-string test will see
  the difference. fix: expose TrustServerCertificate in msdsn.Config and URL round-trip #312 and fix: preserve strict encryption in DSN URLs #452 changed this output the same way and shipped as fix:. Inside
  the repo the callers are the integration harness (makeConnStr in tds_test.go) and the
  NewConnector example, both of which build a Config by hand and round-trip it.
• A Config built as a struct literal serializes the zero-valued fields whose parser defaults
  differ (AppName: "", KeepAlive: 0, MultiSubnetFailover: false) instead of picking up
  those defaults, which is what such a Config already connected with. TLS is the exception:
  a Config with no tls.Config, TrustServerCertificate at its zero value and no parameter
  has no view on trust, so nothing is written and it reparses to the parser's default (trust),
  exactly as on main. The first push wrote trustservercertificate=false there and every
  AppVeyor job failed with "certificate signed by unknown authority", because the harness
  builds its Config that way against a self-signed SQL Server; the second commit fixes it.
• tlsmin is preserved downward (1.0, 1.1), and gains one spelling: 0x and four hex
  digits, read only above the highest version tlsmin has a name for (TLS 1.3 on supported
  builds), so a floor it cannot name round-trips instead of being dropped (decision 8).
• One hand-built shape, a servercertificate pin with verification turned back on, now
  produces a DSN Parse refuses, because every alternative widens what it accepts (decision 6).
• URL() now reads the certificate file, os.Hostname() and MSSQL_USE_EPA on every call.
• An Azure Pipelines DSN round-trips to the ambient SYSTEM_ACCESSTOKEN rather than to SQL
  authentication, since fedauth now travels and systemtoken deliberately does not
  (decision 7).

Windows: mildly affected. defaultWorkstation() calls os.Hostname(), the authenticator and
krb5-* settings now carried are consumed by integratedauth, and pipe/protocol
serialization feeds the Windows-only namedpipe parser. None of that is platform-specific code,
and none of it runs in the Linux matrix, so AppVeyor is the check.

Decisions

Three I would like a maintainer to make:

1. trustservercertificate is written from what the handshake does, not from the field. A
   verifying value whenever it differs from what reparsing would produce; a trusting value only
   when the connection string asked for it. A round trip can therefore never turn verification
   off. The one carve-out is a hand-built Config with no view on TLS at all, which stays on
   the parser's default as it always has (see above). This is a judgment call about TLS
   defaults, so I want a human on it.
2. A pin beside ordinary verification is written so that Parse refuses it. crypto/tls
   runs the chain first and the pin after, so the Config accepts only what passes both. The
   pin alone accepts the self-signed certificate a pin usually names; the chain alone accepts
   every public-CA certificate for the host. Naming both, which parseTLS rejects, is the only
   output that accepts nothing the Config would not. Alternatives: the pin alone, or the chain
   alone.
3. tlsmin learns 0x + four hex digits, above the highest name only. The one addition to
   the grammar. The bound matters: crypto/tls keeps TLS 1.0/1.1 out only while MinVersion is
   zero, so a number read into the named range would loosen a floor a DSN has always had. A test
   drives a TLS 1.0-1.1-only server to show none of 0x0300, 0x0301, 0x300, 0x1 does.
   Alternative: round down to 1.3, a documented widening.

Five I made and want a maintainer to see:

2. epa enabled: an explicit choice travels; a value MSSQL_USE_EPA supplied is left for the
   reading environment to supply again.
3. The drift guard (TestEveryParameterIsClassified) will make feat: add SQL Server 2025 VECTOR type support #307 fail, once it is
   rebased onto this change, until vectortypesupport is classified. That is one map entry and
   one sample value.
4. A trust anchor nothing can name falls back to system roots (a caller-built RootCAs, a
   missing file). That is honest, since the URL states nothing untrue, but not safe, since the
   system pool accepts more than a private one. The alternative is writing a stale file name a
   reader cannot notice. main writes certificate in no case, so it lands here too.
5. A DSN names a certificate file, not its contents: a servercertificate pin follows the
   file if it is rotated between Parse and URL(). No inline spelling exists to write instead.
6. Azure Pipelines falls back to the ambient token when systemtoken is dropped. This is the
   one path this PR makes newly reachable, since main dropped fedauth too. On-behalf-of has
   no such fallback and fails naming userassertion. Both are asserted.

Tests

msdsn/conn_str_roundtrip_test.go is new; azuread/configuration_test.go gains two tests.
There is one case per row of the issue, and a 3780-combination sweep over every encrypt x
trustservercertificate x certificate/servercertificate x hostnameincertificate a
connection string can carry, crossed with fifteen edits a caller can make afterwards, compared
on what the connection would actually do with the certificate. Where a claim rests on
crypto/tls behaviour the test runs a real handshake. I reverted every rule one at a time to
confirm a test catches it. msdsn coverage 90.9% to 95.2%, URL() 84.6% to 98.1%.

Validation skipped: no local SQL Server here, so every integration test skipped and integration
coverage is left to CI.

Out of scope

While checking that a synonym could not produce a duplicate query key I found that
adoSynonyms is applied by splitConnectionString but not by splitConnectionStringURL or
splitConnectionStringOdbc, so sqlserver://host?app=x&connect+timeout=7 silently ignores
both settings while the ADO spelling honours them. Filed separately as #464 and left out of
this diff.

#307 inserts into URL() at the same anchor as the block added here, so whichever lands second
wants a trivial both-keep resolution. #368's only overlap with this change in
msdsn/conn_str.go is an adoSynonyms entry, so it should not conflict.

---

The rest is the full reasoning, for whoever wants to check it.

How each setting is written

A setting with a field on Config is written from that field, whenever the field disagrees
with the value reparsing the URL without it would produce. An edit to a Config therefore
survives whether or not the connection string named the setting, which a "was the parameter
supplied" test cannot do, because Parse fills several fields in regardless: AppName from a
constant, KeepAlive from 30s, MultiSubnetFailover from true, and TrustServerCertificate
from the absence of encrypt. Where the field holds something the grammar cannot spell (a
negative or sub-second duration, read-only intent with no database), the parameter is dropped
rather than emitted, because Parse would reject the URL or read it back as something else.
dial timeout goes through the same check, which is slightly beyond the issue's table:
DialTimeout is documented as negative to disable, and that value used to serialize to
dial timeout=-1, which Parse rejects.

A packet size is not in that category. Zero is the sentinel for the driver's own default, so it
stays out; anything else is written as the size the connection would actually use, because
Parse and the login path clamp to the same TDS range. Dropping an out-of-range value would
have come back as the 4096 default instead of the 512 or 32767 a direct connection uses.

Two settings need the parameter as well as the field, because their default is ambient
rather than a constant.

workstation id is filled from the local host name, so an explicit value that happens to match
it is indistinguishable from an unset one by comparison alone; without the presence test it
would come back as the reading machine's name.

epa enabled falls back to MSSQL_USE_EPA. The contract I settled on is: an explicit choice
travels with the URL; a value the environment supplied is left for the reading environment to
supply again. A connection string that never named the setting was already asking whichever
process read it, so freezing this process's answer into the URL would invent a decision nobody
made. What has to survive is the explicit choice, the loss the issue reports,
and any value a caller set on the Config, which is why the comparison is against the
environment rather than against false. If you would rather the URL always pinned the
effective value, that is a one-line change, but it puts epa enabled in every URL and
breaks the two existing whole-Config round-trip tests.

encrypt is written only when the connection string supplied it: EncryptionOff is also the
zero value, and emitting it for every unconfigured Config would turn a default into a
choice, which the issue warns against. That costs nothing, because EncryptionOff is already
what a URL without the parameter reparses to; the half of the distinction that decides whether
the certificate is verified rides on trustservercertificate, which follows the rule above.

Settings with no field (the certificate paths, and the fedauth, authenticator and
krb5-* settings azuread and integratedauth read out of Parameters) are carried from a
list, carriedVerbatim, not by copying Parameters.

Why a list and not a copy

Copying every retained parameter closes the same rows with less code, and I tried that first.
It publishes credentials: azuread reads systemtoken, clientassertion and userassertion
out of the same map, and url.URL.Redacted() masks only the userinfo password, not query
parameters, so those would come through Redacted() unmasked.

An unrecognized parameter is therefore dropped. It is a real cost, and it leaves the issue's closing
paragraph (a new parser setting can be forgotten by the serializer) technically true. The
trade is not free: a dropped setting can itself be a security loss, and epa enabled above is
exactly that. What makes it the right way round is that the failure is visible and recoverable,
while a leaked credential is neither, with one qualification: where the consumer has an ambient
source for the secret, as azuread does for systemtoken, the loss is a substitution rather
than a failure; see decision 7.

TestEveryParameterIsClassified pins the drift down: it reads the
parameter constants out of conn_str.go with go/ast and fails if one is not classified as
emitted, carried in the URL's structure, or deliberately dropped. It also collects the names the
parser reaches for with a string literal rather than a constant, columnencryption is the only
one today, and a guard that missed it would miss the kind of name it exists for.

This will make #307 fail, once it is rebased onto this change, until vectortypesupport is
classified. That is a map entry plus a sample value in
TestClassifiedQueryParametersAreEmitted, which checks the classification against what
URL() really writes. I ran it: both failures name what to add. #307 already collides
with this change inside URL(), so its author is in this code either way, but it is still a
tripwire on somebody else's branch. I can drop it if you prefer.

The decisions, in full

1. trustservercertificate. Config.TrustServerCertificate is not read at connect time, so
   serializing it is the only way setting it can reach a connection. I made writing it
   asymmetric, verifying values always, trusting ones only when the connection string asked,
   so a round trip can never turn verification off. This is a call I want a maintainer to
   make. Details below.
2. epa enabled. An explicit choice travels with the URL; a value MSSQL_USE_EPA supplied
   is left for the reading environment to supply again. Tell me if you would rather the URL
   always pinned the effective value.
3. The drift guard. TestEveryParameterIsClassified will make feat: add SQL Server 2025 VECTOR type support #307 fail, once it is
   rebased onto this change, until vectortypesupport is classified. I can drop it or move it
   to a follow-up test: PR.
4. A trust anchor that cannot be written falls back to system roots. A caller who replaces
   RootCAs with a pool they built, or whose certificate file has gone, gets a URL with no
   certificate in it, and a reader chains to system roots. It is honest, since it states
   nothing untrue about the Config, but not safe, because the system pool accepts
   more than a private one does. The alternative is writing the stale file name, which describes
   an anchor the connection has stopped using and which a reader cannot notice. I picked
   honesty. The other option, or having URL() emit something Parse refuses so that it fails
   closed, are both easy to switch to.
5. A DSN names a certificate file, not its contents. Parse reads the file again, so a
   servercertificate pin follows the file rather than the bytes captured when the Config was
   built: rotate the file between Parse and URL() and the round trip pins the new
   certificate. readCertificate takes a .pem or .der path and nothing else, so there is no
   inline form to write instead, and dropping the parameter would land on system roots, which
   can accept public-CA certificates the pinned configuration would reject. Documented and
   tested rather than fixed.
6. One Config is written so that Parse refuses it. A servercertificate pin with
   ordinary verification turned back on accepts only a certificate that passes both the chain
   check and the byte comparison. No connection string produces that, because the pin path
   turns verification off, and either half alone accepts certificates the Config rejects: the pin
   alone accepts the self-signed certificate a pin usually names, which the chain would refuse;
   the chain alone accepts every certificate a public CA has issued for the host. So the URL
   names both, and parseTLS rejects the pair with an error naming them. It is the only output
   that accepts nothing the Config would not, at the cost of a DSN that does not read back.
   I went back and forth between the two halves before settling here, so I would rather a
   maintainer chose: this, the pin alone, or the chain alone.
7. Azure Pipelines falls back to the ambient token. systemtoken is a secret and is not
   written (see below), and azuread then reads SYSTEM_ACCESSTOKEN, as it already does for
   any connection string that never named a token. So a round trip of a DSN that named token A,
   in an environment holding token B, carries on with B. On-behalf-of has no such fallback and
   fails naming userassertion. Both are asserted. Whether a serializer that cannot carry an
   explicit secret should let the consumer substitute an ambient one is a judgment call; the
   alternative is to also drop fedauth for that workflow so the round trip fails.
8. tlsmin learns one spelling, so that a version floor round-trips whether or not it has a
   name. tlsmin names TLS 1.0 to 1.3; a MinVersion above those had no spelling, and on
   today's crypto/tls such a floor is a Config no handshake can satisfy. Dropping the
   parameter came back as the TLS 1.2 default and rounding it down came back as TLS 1.3, either
   way a Config that connects where the original refused. So URL() writes such a floor as
   its protocol number, tlsmin=0x0305, and Parse reads 0x and exactly four hex digits back
   as that number, only above the highest version it has a name for. That bound is the
   security-relevant part: crypto/tls keeps TLS 1.0 and 1.1 out only while MinVersion is
   zero, so a number read into the named range would give a string that meant the default
   yesterday (0x0300, 0x0301) a looser meaning today. Those, and anything that is not
   exactly the spelling (0x300, 0x1, 0X0305, 12), still read as the default, and a test
   drives a server that speaks nothing newer than TLS 1.1 to show none of them loosens the
   floor. This is the only grammar change in this PR, and it is here because nothing else
   avoids both a wider connection and an error. If you would rather
   tlsmin stayed names-only, the fallback is rounding down to 1.3, which I can restore and
   document as a widening. MaxVersion and CipherSuites have no parameter at all and are
   simply gone after a round trip; carrying them would mean new parameters, which I have not
   added, and a test pins that they are lost.

Decisions 4, 5 and 6 narrow or refuse where the grammar cannot follow the Config, and on all
three main silently widened further, since it writes certificate and servercertificate in
no case at all. Decision 7 is different, and the only place this PR makes something newly
reachable: main dropped fedauth as well, so no round trip ever reached the Azure Pipelines
workflow, whereas preserving fedauth while omitting its secret makes that workflow's existing
environment fallback observable across a round trip for the first time. These are limits of the
grammar, and I would rather state them here than have them found in review.

The certificate rules

trustservercertificate is written against the runtime's notion of trust, not the
field's, and I want a maintainer to sanity-check that. Config.TrustServerCertificate is not read at
connect time (getTLSConn is), so serializing the field is the only way setting it can reach
a connection, and the only standard the serializer can be held to is what the connection would
actually do. A nil tls.Config is not "no opinion" there: getTLSConn replaces it with
SetupTLS("", "", false, …), so such a Config verifies. A verification callback counts as
verification even alongside InsecureSkipVerify, which is the combination SetupTLS itself
uses for a servercertificate pin and for a common name containing a colon, and the one a
caller uses for their own check.

On top of that: a verifying value is written whenever it differs from what reparsing would
produce, which closes the issue's headline case and lets a caller turn verification on; a
trusting value only when the connection string asked for it, so a round trip can never be
the thing that turns verification off. encrypt=strict is skipped entirely, because
parseTLS forces the value to false there whatever the parameter said.

One deliberate cost: a trustservercertificate=true supplied next to a servercertificate
pin does not survive, because the pin's callback makes the config "verifying" by the rule
above. The pin governs the handshake either way, so what the connection does is unchanged,
but it is a parameter the connection string named, so it is worth stating.

hostnameincertificate is written from HostInCertificateProvided alone, not from
ServerName differing from Host. The two also diverge when a caller moves Host and leaves
the tls.Config alone (failoverPartnerParams does this), and writing the old name
there would check the certificate of a server the connection is no longer going to, and would
come back with HostInCertificateProvided set, which is the flag connect() reads to decide
whether to retarget ServerName after a routing redirect. The cost is that a ServerName set
straight onto the tls.Config does not travel; set HostInCertificateProvided alongside it if
it should, except next to a servercertificate pin that is actually written, where the
parameter is skipped because parseTLS rejects the two together and the pin compares raw bytes
rather than checking a name. It has to be a pin that is written, not one the connection string
once named: a caller who removed the pin and chose a name has a Config that checks that name,
and the stale parameter says nothing about it (TestConfigURLWritesTheNameChosenAfterAPinIsRemoved).

certificate and servercertificate travel while they are still true about what the
connection does with the certificate, not while they are the whole truth. A caller can add a
check no connection string can spell, and the parameter is then incomplete rather than wrong.
Dropping it for that costs more than it saves: nothing else in the grammar names a pin or a
private CA, so a URL without the parameter verifies against system roots, which accepts every
certificate a public CA has issued for the host. Trading an incomplete truth for that is a
wider connection, not a safer one.

The pin travels while a VerifyPeerCertificate is there at all. crypto/tls calls one
on every handshake, after any chain check and whether or not verification is on, and nothing
else on the config takes that away: a pool is read only where a chain is built, and a
VerifyConnection is a second check rather than a replacement. Requiring the rest of the shape
setupTLSServerCertificateOnly builds would drop the pin in those cases, and the chain it
would be traded for is to system roots, because the pin path sets no RootCAs.

Verification turned back on beside the pin is the only shape written so that Parse refuses
it (decision 6). crypto/tls runs the chain first and the pin after, so that Config accepts
only a certificate that passes both, and each half on its own accepts certificates it rejects.
The URL names the pin and the name the chain checks, and parseTLS rejects the pair.
TestConfigURLRefusesToWriteAChainAndAPin runs a real handshake against the pinned self-signed
server to show the two halves disagree: the Config refuses it, the pin alone accepts it.

A root pool is different, because it is read only where a chain is built against it: with
InsecureSkipVerify on the file names nothing the connection enforces. Where a chain is built,
URL() reads the file back and compares the pool, so the parameter travels only while it still
describes the roots in use; a caller who swapped the pool has left it describing certificates
the connection no longer trusts. An extra callback beside the pool is not a reason to stop
believing the pool.

The exception to both is the common name path. SetupTLS forks on the name it checks
against, and a colon in it routes the file through setupTLSCommonName, which builds its
callback out of that same file and turns verification off so the callback can do the checking;
there the file names the check even though no chain is built. A callback a caller swapped in
over that shape is indistinguishable from the one SetupTLS built, because Go function values
are not comparable; that limit is noted on URL() and pinned by
TestConfigURLCannotTellACommonNameCallbackApart.

Where the parameter has stopped being true there is nothing to write, and the reader chains to
system roots. Honest, but not safe. A reader can see that no certificate was named, whereas writing a
stale file name instead would state something about the Config with nothing to give it away.
But a private pool accepts less than the system pool does, so this direction can end up
accepting a publicly issued certificate for the host that the Config itself would reject.
TestConfigURLCannotCarryAPoolTheCallerBuilt asserts that outcome, and
TestConfigURLDropsACertificateItCannotReadBack checks what the reader is left with too. This is decision 4 above.

And the parameters name a file, not its contents. Parse reads the file again, so a
servercertificate pin follows the file rather than the bytes setupTLSServerCertificateOnly
captured in its callback (those live in a closure URL() cannot read), and there is no inline
spelling to write instead. Following the file is the narrower of the two available answers:
dropping the parameter lands on system roots, and on the ordinary reason for a file to change, a
rotated certificate, the file holds what the operator now means.
TestConfigURLNamesACertificateFileRatherThanItsContents pins it. certificate differs only
because its pool can be read back out of the tls.Config and compared. This is decision 5
above.

None of that touches the rest of carriedVerbatim. fedauth, authenticator and the krb5-*
settings have nothing to do with TLS, and dropping one because the tls.Config changed would
pick a different authentication method.

change password is deliberately not serialized. It is the only setting with a Config
field that is still dropped: a login-time credential, and replaying a serialized DSN should
not reissue a password change. That leaves one row of the table unfixed by choice.

Log surface. No credential reaches the query, and Redacted() is unchanged in what it
masks. But a serialized DSN now also carries local filesystem paths (certificate,
clientcertpath, krb5-keytabfile, ...), an Azure resource id and
additionallyallowedtenants. Infrastructure identifiers, not secrets, and dropping them breaks
authentication.

Every test

msdsn/conn_str_roundtrip_test.go is new; azuread/configuration_test.go gains two tests.
Every regression case here fails against the pre-fix serializer; I checked by running the new
tests against the old URL(). Every rule in
retainedCertificateParameterApplies and the hostnameincertificate emission has been reverted
one at a time to confirm a test catches it.

• TestConfigURLRoundTripPreservesSettings: one case per setting in the issue, asserting
  effective configuration rather than URL spelling. The encryption cases assert
  TrustServerCertificate and TLSConfig.InsecureSkipVerify, not the enum, because the enum
  survived already and the verification behavior did not.
• TestConfigURLRoundTripPreservesCertificateTrust: generates a certificate at run time; the
  pin case calls the restored VerifyPeerCertificate, accepting the configured certificate
  and rejecting another. A nil check would have passed against a pool that loaded nothing.
• TestConfigURLCarriesAnExplicitEpaChoice and
  TestConfigURLLeavesAnAmbientEpaValueToTheReader: the two halves of the contract above,
  across MSSQL_USE_EPA combinations at serialize and parse time, including an unreadable
  value, which is the case that otherwise produces a URL Parse rejects.
• TestConfigURLKeepsWorkstationIDMatchingTheLocalHost: the other ambient default.
• TestConfigURLLeavesAZeroValueTLSChoiceToTheReader: the harness's own Config, built from
  HOST and DATABASE with no view on TLS, round-trips to the parser's default rather than to a
  verifying DSN; a tls.Config on the same Config is a view and is written. This is the
  AppVeyor failure.
• TestConfigURLKeepsAuthenticationSettingsWhenTheTLSConfigIsCleared: the mistake that is easy
  to make while tightening the certificate rules. Clearing a tls.Config says nothing about how
  the caller authenticates, and integratedauth falls back to its platform default without
  authenticator.
• TestConfigURLDropsACertificateItCannotReadBack: the file behind the parameter is gone or
  holds nothing a pool will take. Each case asserts what the reader is left with, not only that
  the parameter went missing.
• TestConfigURLCannotCarryAPoolTheCallerBuilt and
  TestConfigURLNamesACertificateFileRatherThanItsContents: decisions 4 and 5, asserted
  explicitly. The second rotates the certificate
  behind a pin between Parse and URL() and calls the rebuilt pin on both certificates.
• TestConfigURLKeepsAnAnchorItCanStillName: the three ways a caller adds a check that cannot
  travel, each asserting that the anchor underneath it does. One calls the restored pin on the
  pinned certificate and on another; two compare the restored pool against the file.
• TestConfigURLRefusesToWriteAChainAndAPin: decision 6. A real TLS handshake against the
  pinned self-signed server: the Config refuses it, the pin alone accepts it, and the URL names
  both halves so that Parse refuses the pair.
• TestConfigURLWritesTheNameChosenAfterAPinIsRemoved: a stale servercertificate in
  Parameters must not suppress the name a caller chose after taking the pin out.
• TestConfigURLKeepsAPinBesideAPoolTheHandshakeIgnores: the same point for a RootCAs the
  handshake never reads.
• TestConfigURLDropsACertificateItNoLongerNames: the other half, a parameter that has stopped
  being true, including a pool the caller swapped under a check of their own, which is the case
  where a callback must not answer before the pool comparison runs.
• TestConfigURLCarriesCertificateSettingsWithEncryptionOff: with no TLS there is no
  tls.Config to read the settings out of, and the parsed text is the only record they have.
• TestConfigURLCannotTellACommonNameCallbackApart: the limit above, asserted so that
  changing it has to be deliberate.
• TestFedAuthSurvivesConfigURLRoundTrip (azuread): these settings have no field on
  msdsn.Config, so this is the layer where losing them matters: validateParameters sees no
  fedauth and falls back to SQL authentication.
• TestFedAuthSecretsDoNotSurviveConfigURLRoundTrip (azuread): the other side of the trade
  below, asserted per workflow: ActiveDirectoryOnBehalfOf fails naming
  userassertion; ActiveDirectoryAzurePipelines fails naming systemtoken when
  SYSTEM_ACCESSTOKEN is unset, and carries on with the environment's token when it is set
  (decision 7). It is there so that anyone who "fixes" the loss by copying Parameters wholesale
  has to delete a test that says why not.
• TestConfigURLOmitsSecretParameters: systemtoken, clientassertion, userassertion and
  change password reach neither String() nor Redacted(). This is the test that catches
  the copy-everything design.
• TestConfigURLPreservesCertificatePolicy: the sweep, and the most useful thing here. Every
  encrypt × trustservercertificate × certificate/servercertificate ×
  hostnameincertificate combination a connection string can carry (including a name with a
  colon in it, the only one that reaches setupTLSCommonName), crossed with fifteen
  edits a caller can make afterwards, compared on what the connection would actually do with the
  certificate: which checks run, what a chain is built against (the pool compared, not merely
  whether one is set), and under which name. 3780 combinations. Every part of that is there
  because a coarser version let something through: asking only "does it verify" missed system
  roots turning into a private CA; ignoring the chain flag missed "chain and a pin" turning into
  the pin alone, since crypto/tls runs VerifyPeerCertificate only after ordinary
  verification has succeeded; treating any non-nil pool as the same pool missed a swap; leaving
  colon names out of the matrix missed setupTLSCommonName entirely; and counting a pool
  that no chain is built against made throwing a pin away for system roots look correct. Where
  an edit leaves a policy no connection string produces, the sweep requires that the narrowest
  anchor still nameable survives rather than waiving the comparison: a pin names one
  certificate and a private pool names one CA, while a chain to system roots accepts every
  certificate a public CA has issued for the host, so falling past one of those to the next
  accepts more than the Config ever did. Only an anchor with no spelling at all leaves the
  ordinary chain as the answer. I checked it catches each fix by reverting them one at a time.
• TestConfigURLCarriesAVersionFloorItCannotName: decision 8. A MinVersion above what
  tlsmin names fails a real handshake with "no supported versions", is written as
  tlsmin=0x0305, comes back unchanged, and the handshake after fails for the same
  reason. TestParseReadsTheVersionFloorURLWrites pins the spelling's edges (exactly four hex
  digits, above the highest name only), and TestParseDoesNotReadANumberIntoTheNamedRange
  drives a TLS 1.0–1.1-only server to show that 0x0300, 0x0301, 0x300 and 0x1 still
  negotiate nothing below TLS 1.2, as they did before.
• TestConfigURLCannotCarryACeilingOrCipherSuites: a MaxVersion and a cipher suite list are
  gone after a round trip, asserted explicitly.
• TestConfigURLOmitsUnrepresentableValues: negative and sub-second durations including
  dial timeout, packet size zero, read-only intent with the database cleared; each asserts
  the parameter is absent and the URL still parses.
• TestConfigURLWritesThePacketSizeTheConnectionWouldUse: Parse clamps to the TDS range, so
  a size outside it only reaches URL() from a hand-built Config. Writing the clamped size
  rather than dropping it is what keeps the round trip agreeing with a direct connection, which
  clamps the same way.
• TestConfigURLNeverAdvertisesTrustOnStrictEncryption: strict forces verification whatever
  the parameter says, so the URL should not advertise a trust setting this driver ignores and
  another client might not.
• TestConfigURLPrefersEditedFields: edits after parsing, including settings the connection
  string never mentioned, a field cleared back to empty, and TLSConfig.ServerName, which
  decides which name the certificate is checked against.
• TestConfigURLOmitsUnsuppliedSettings,
  TestConfigURLWithNilParameters, TestConfigURLIsIdempotent,
  TestEveryParameterIsClassified, TestClassifiedQueryParametersAreEmitted,
  TestTLSVersionToString, TestTLSMinParameter, TestWholeSeconds.

msdsn coverage goes from 90.9% to 95.2% and URL() from 84.6% to 98.1%, measured against a
clean export of main. Every statement added here is covered; the only uncovered statement left
in URL() is pre-existing (the protocol-mismatch panic), plus one deliberate gap: the
carriedVerbatim loop skips a name something above it has already written, which nothing does
today. It is there because a duplicate key is a hard Parse failure rather than a merge, so the
day one of those names gains a field the failure would be an unreadable DSN.
TestConfigURLNeverWritesAKeyTwice asserts the property it protects.

Validation notes

go build ./... and the unit suites (./msdsn, ./internal/..., ./integratedauth,
./azuread) are clean. gofmt -l is clean on the five changed files. Note that go fmt ./...
as written in pr-workflow.instructions.md rewrites ten unrelated committed files that have trailing
whitespace, so I ran it against the diff only. go vet ./... reports only
integratedauth/winsspi/winsspi.go:24:36: possible misuse of unsafe.Pointer, which is on
main before this change and which .github/copilot-instructions.md already notes.

tlsVersionToString and highestNamedTLSVersion are added to both build-tagged files, and
TLSVersionFromString in both learns the numeric spelling above that bound, so the !go1.12
variant, which the go 1.25 floor in go.mod no longer builds, is kept in step; there the
bound would be TLS 1.2, the highest name it has.

[Copilot]

🟡 Changes recommended

Certificate fallback can widen trust, and some reachable configurations produce URLs that Parse rejects.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR expands DSN URL serialization to preserve connection, TLS, authentication, and certificate settings across round trips while avoiding credential leakage.

Changes:

• Serializes additional configuration and retained parameters.
• Preserves TLS minimum versions and certificate policies.
• Adds extensive round-trip and Azure AD coverage.
• Requires resolution of certificate fallback and unparsable URL behavior.

File summaries

File	Description
msdsn/conn_str.go	Expands URL serialization and TLS handling.
msdsn/conn_str_roundtrip_test.go	Adds comprehensive round-trip tests.
msdsn/conn_str_go112.go	Supports numeric TLS version serialization.
msdsn/conn_str_go112pre.go	Supports numeric TLS versions for older builds.
azuread/configuration_test.go	Tests federated authentication preservation and secret omission.

Review details

• Files reviewed: 4/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sary (sary8)]

AppVeyor failed on the first commit: every integration test hit certificate signed by unknown authority. The harness builds its Config by hand from HOST and DATABASE (GetConnParams in tds_test.go) and round-trips it through URL(). That Config has no tls.Config, TrustServerCertificate at its zero value and no parameter, and the first commit wrote trustservercertificate=false for it, so the reparsed DSN verified the self-signed server. On main nothing was written and the reader fell back to its default, which is to trust.

2f3d0a4 restores that for a Config with no view on TLS at all. A Config that holds a view (a tls.Config, the field, or the parameter) is written as before, and parsed DSNs are not affected because Parse always builds a tls.Config. TestConfigURLLeavesAZeroValueTLSChoiceToTheReader covers the harness's exact shape; TestConfigURLWithNilParameters had encoded the old expectation and now asserts the new one. The description is updated under "What changes for users".

[Copilot]

🔵 Needs a closer look

TLS, certificate, authentication, and environment-sensitive behavior require final human review.

Review details

• Files reviewed: 4/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.81%. Comparing base (1f52296) to head (696e3d4).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #465      +/-   ##
==========================================
+ Coverage   77.91%   82.81%   +4.90%     
==========================================
  Files          35       35              
  Lines        7222     7222              
==========================================
+ Hits         5627     5981     +354     
+ Misses       1332      982     -350     
+ Partials      263      259       -4     

see 9 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

🔵 Needs a closer look

A moderate DER certificate handling issue remains, and the TLS trust-policy changes require human review.

Review details

Suppressed comments (1)

msdsn/conn_str.go:797

• readCertificate explicitly accepts .der files, but this comparison always feeds the returned bytes to AppendCertsFromPEM, which returns false for DER. Consequently a parsed certificate=/path/file.der is omitted by URL() and the reparsed DSN falls back to system roots, changing the effective trust policy (and widening it). Reconstruct the comparison pool using the same PEM/DER handling as SetupTLS (and ensure the setup path itself preserves DER roots), then add a DER round-trip case.

	fromFile := x509.NewCertPool()
	if !fromFile.AppendCertsFromPEM(pemBytes) {
		return false

• Files reviewed: 4/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[sary (sary8)]

Refuted, for the suppressed comment on msdsn/conn_str.go:797 in the latest Copilot review: readCertificate does not hand DER bytes to AppendCertsFromPEM. Its .der branch goes through readDERFile, which parses the file with x509.ParseCertificate and re-encodes it as a PEM block, so SetupTLS and the pool comparison see the same PEM. A parsed certificate=/path/file.der is written by URL() and comes back with an equal pool. There was no .der round-trip test to cite, so d03aa5c adds TestConfigURLReadsBackADERCertificate. No code change.

[Copilot]

🔵 Needs a closer look

The unresolved TLS callback and certificate-policy behavior requires maintainer review.

Review details

Suppressed comments (1)

msdsn/conn_str.go:758

• A caller can replace the callback that SetupTLS installed for a parsed servercertificate with their own VerifyPeerCertificate function while leaving Parameters[ServerCertificate] unchanged. This branch treats any non-nil callback as proof that the retained file is still enforced, so URL() emits the stale pin and reparsing installs a new pin callback, changing the caller's effective TLS policy (and potentially rejecting certificates the edited Config accepts). The same reachability is exercised by the new sweep's callback-edit case, but its assertion only checks that some anchor remains, so it does not catch this policy change. Please track whether the callback is the driver-installed pin (or otherwise surface this unrepresentable state) instead of using non-nil as that distinction.

	if name == ServerCertificate {
		// crypto/tls calls VerifyPeerCertificate on every handshake, after any
		// chain check and whether or not verification is on, so while one is
		// there the file still names a certificate the connection compares
		// against. Nothing else on the config takes that away: a pool is read
		// only where a chain is built, a VerifyConnection is a second check
		// rather than a replacement, and verification turned back on adds the
		// chain rather than removing the pin.
		//
		// Requiring the rest of the shape setupTLSServerCertificateOnly builds
		// would drop the pin in each of those cases, and this parameter is the
		// only thing in the grammar that can name one. What it would be traded
		// for is a chain to system roots - the pin path sets no RootCAs - so the
		// URL would accept every certificate a public CA has issued for the host
		// in place of the single certificate named here.
		return p.TLSConfig.VerifyPeerCertificate != nil

• Files reviewed: 4/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[sary (sary8)]

Refuted, for the suppressed comment on msdsn/conn_str.go:758 in the latest Copilot review: the behaviour is intentional and is the documented limit on callbacks. Go function values cannot be compared, so URL() cannot tell a callback a caller swapped in over the pin from the pin itself; the file travels and reparsing rebuilds the pin from it. The alternative, dropping servercertificate whenever any callback is present, falls back to system roots, which accepts every public-CA certificate for the host rather than the one, so it is the wider answer, not the safer one. Tracking whether the callback is the driver's own would need provenance the tls.Config does not carry. The limit is recorded on URL() and, from this push, pinned for the pin by TestConfigURLCannotTellAPinCallbackApart alongside the existing TestConfigURLCannotTellACommonNameCallbackApart; the sweep marks its callback edit opaque for exactly this case. No code change.

[Copilot]

🟡 Changes recommended

Critical TLS behavior and test compilation issues must be fixed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 4/5 changed files
• Comments generated: 2
• Review effort level: Lite

[azure-pipelines]

There was an error handling pipeline event c1c3417e-395f-4977-8742-11c4b95cb118.

[Copilot]

🔵 Needs a closer look

TLS and authentication behavior changes require maintainer review; integration tests were not run locally.

Review details

• Files reviewed: 4/5 changed files
• Comments generated: 0 new
• Review effort level: Lite