Skip to content

Commit 213f68b

Browse files
committed
FIX+SECURITY: PR #744 review (Sumit) — PE presence gate, secret TLS conn, small fixes
- assert_pe_machine.py: assert the native binding (ddbc_bindings*.pyd) AND the vendored ODBC driver DLLs are BOTH present, independently (win-arm64 skips the runtime import, so this is its presence gate); +2 tests. - conda-build-pipeline.yml: source the TLS probe connection string from a SECRET variable (variable group / Key Vault), not a plaintext queue-time parameter that ADO leaves unmasked in the run UI/logs. - driver_load_probe.py: drop "missing companion" from the failure label (self-contained package -- vendors the ODBC payload; there is no separate companion). - tests/test_028: add a }} escaped-brace case for _split_top_level (MS-ODBCSTR: '}}' is a literal '}', so a value with '}}' + internal ';' must not be mis-split). - audit_bundled_binaries.py: document that the Linux audit does not assert a required distro SET (the repackaged wheel is the source of truth). 96 probe/audit unit tests pass; black clean.
1 parent eef020f commit 213f68b

6 files changed

Lines changed: 73 additions & 14 deletions

File tree

‎OneBranchPipelines/conda-build-pipeline.yml‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,6 @@ parameters:
4040
displayName: 'Python versions (comma-separated)'
4141
type: string
4242
default: '3.10,3.11,3.12,3.13,3.14'
43-
# H1: the Encrypt=yes probe connection string. OpenSSL is dlopen'd lazily (not in
44-
# DT_NEEDED), so ONLY a live TLS handshake proves libssl/libcrypto resolve from
45-
# $PREFIX/lib -- no static audit can. Supply a reachable server (from a secret var)
46-
# to activate conda/tls_connect_probe.py on the Linux leg; empty = the probe SKIPs.
47-
- name: condaTlsProbeConn
48-
displayName: 'TLS probe: a full SQL Server connection string; NOT a yes/no toggle; empty = skip'
49-
type: string
50-
default: ''
5143
# H1: enable the minimal-base ldd reachability gate (CONDA_ASSERT_PREFIX_REACHABLE).
5244
# It fails CLOSED if the driver binds a system (or absent) krb5/gssapi/libltdl, so it
5345
# is only valid on a leg with NO system copies of those libs -- set true ONLY when
@@ -261,10 +253,14 @@ extends:
261253
vmImage: 'ubuntu-latest'
262254
variables:
263255
ob_outputDirectory: '$(Build.ArtifactStagingDirectory)'
264-
# H1: activate the runtime gates in build-conda-packages.sh. The TLS probe
265-
# runs when a connection string is supplied; the ldd reachability gate runs
266-
# (fail-closed) only when explicitly enabled on a minimal base.
267-
CONDA_TLS_PROBE_CONN: ${{ parameters.condaTlsProbeConn }}
256+
# H1: activate the runtime gates in build-conda-packages.sh. The TLS probe's
257+
# Encrypt=yes connection string carries a password, so it is sourced from a
258+
# SECRET variable -- NOT a queue-time parameter, which ADO leaves UNMASKED in
259+
# the run UI/logs. Define `condaTlsProbeConn` as a secret in a variable group /
260+
# Key Vault linked to this pipeline; unset = the probe SKIPs loudly. The ldd
261+
# reachability gate runs (fail-closed) only when explicitly enabled on a
262+
# minimal base.
263+
CONDA_TLS_PROBE_CONN: $(condaTlsProbeConn)
268264
${{ if parameters.enableMinimalReachabilityGate }}:
269265
CONDA_ASSERT_PREFIX_REACHABLE: '1'
270266
steps:

‎conda/driver_load_probe.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ def main():
109109
if driver_loaded(outcome):
110110
print("DRIVER_LOADED (" + describe(outcome) + ")")
111111
return
112-
sys.exit("DRIVER DID NOT LOAD / wrong arch / missing companion: " + describe(outcome))
112+
sys.exit("DRIVER DID NOT LOAD / wrong arch: " + describe(outcome))
113113

114114

115115
if __name__ == "__main__":

‎eng/scripts/assert_pe_machine.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,17 @@ def audit_package(path: str) -> list[str]:
141141

142142
errors: list[str] = []
143143
native_seen = 0
144+
binding_seen = 0
145+
driver_dll_seen = 0
144146
for name, data in _iter_payload_members(path):
145147
if not name.lower().endswith(_NATIVE_SUFFIXES):
146148
continue
147149
native_seen += 1
150+
low = name.replace("\\", "/").lower()
151+
if "/mssql_python/" in low and "ddbc_bindings" in low and low.endswith(".pyd"):
152+
binding_seen += 1
153+
if "/mssql_python_odbc/libs/windows/" in low and low.endswith(".dll"):
154+
driver_dll_seen += 1
148155
machine = pe_machine(data)
149156
if machine is None:
150157
errors.append(f"{name}: not a valid PE binary (no MZ/PE header).")
@@ -157,11 +164,25 @@ def audit_package(path: str) -> list[str]:
157164
else:
158165
print(f" {subdir}/{os.path.basename(name)}: PE machine={_MACHINES[expected]} OK")
159166

167+
# Presence: assert BOTH required binary categories independently, not just >=1 native
168+
# file -- win-arm64 skips the runtime import, so this IS its presence gate. A package
169+
# with the binding .pyd but missing driver DLLs (or vice versa) must fail here.
160170
if native_seen == 0:
161171
errors.append(
162172
f"{base_name}: no .pyd/.dll found in a '{subdir}' package -- the native binding "
163173
f"(ddbc_bindings*.pyd) + the vendored ODBC driver DLLs must be present."
164174
)
175+
else:
176+
if binding_seen == 0:
177+
errors.append(
178+
f"{base_name}: no native binding (mssql_python/ddbc_bindings*.pyd) found in a "
179+
f"'{subdir}' package."
180+
)
181+
if driver_dll_seen == 0:
182+
errors.append(
183+
f"{base_name}: no vendored ODBC driver DLL "
184+
f"(mssql_python_odbc/libs/windows/**/*.dll) found in a '{subdir}' package."
185+
)
165186
return errors
166187

167188

‎eng/scripts/audit_bundled_binaries.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,10 @@ def audit_package(path: str) -> list[str]:
464464
)
465465
# Per-subdir presence: EVERY discovered driver lib dir must ship BOTH a driver and
466466
# libodbcinst.so.2. A package-global count would let a driver missing from ONE
467-
# distro subdir (alpine/debian_ubuntu/rhel/suse) slip past.
467+
# distro subdir (alpine/debian_ubuntu/rhel/suse) slip past. NOTE: this checks every
468+
# DISCOVERED distro/arch dir but does NOT assert a required distro SET -- the
469+
# repackaged wheel is the source of truth for which distro payloads exist, so a
470+
# wholesale-missing distro is a wheel-build concern, not enforced here.
468471
if not lib_dirs:
469472
errors.append(
470473
f"{base_name}: no mssql_python_odbc/libs/linux/*/*/lib directory found in a "

‎tests/test_028_tls_connect_probe.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,17 @@ def test_split_top_level_respects_braces():
187187
]
188188

189189

190+
def test_split_top_level_handles_escaped_braces():
191+
"""MS-ODBCSTR: '}}' inside a braced value is a literal '}', not a close -- so a value
192+
containing '}}' and an internal ';' must NOT be split (matches the shipped parser)."""
193+
probe = _load_probe()
194+
assert probe._split_top_level("Server=x;Pwd={p}}w;d};Encrypt=no") == [
195+
"Server=x",
196+
"Pwd={p}}w;d}",
197+
"Encrypt=no",
198+
]
199+
200+
190201
def test_redact_masks_values_and_flags_bare_segments():
191202
"""The debug line must never leak a value and must surface a no-value segment."""
192203
probe = _load_probe()

‎tests/test_030_pe_machine_assert.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,34 @@ def test_win_package_without_native_fails(tmp_path):
160160
assert any("no .pyd/.dll" in e for e in errors)
161161

162162

163+
@pytest.mark.skipif(not _zstd_available(), reason="no zstandard backend available")
164+
def test_win_missing_driver_dll_fails(tmp_path):
165+
# Correct-arch binding present, but the vendored ODBC driver DLLs are missing.
166+
p = _make_conda(
167+
tmp_path,
168+
"win-arm64",
169+
{"Lib/site-packages/mssql_python/ddbc_bindings.cp312-arm64.pyd": _fake_pe(_ARM64)},
170+
)
171+
errors = ape.audit_package(p)
172+
assert any("driver DLL" in e for e in errors)
173+
174+
175+
@pytest.mark.skipif(not _zstd_available(), reason="no zstandard backend available")
176+
def test_win_missing_binding_fails(tmp_path):
177+
# Correct-arch driver DLL present, but the native binding .pyd is missing.
178+
p = _make_conda(
179+
tmp_path,
180+
"win-arm64",
181+
{
182+
"Lib/site-packages/mssql_python_odbc/libs/windows/arm64/msodbcsql18.dll": _fake_pe(
183+
_ARM64
184+
)
185+
},
186+
)
187+
errors = ape.audit_package(p)
188+
assert any("native binding" in e for e in errors)
189+
190+
163191
@pytest.mark.skipif(not _zstd_available(), reason="no zstandard backend available")
164192
def test_non_windows_package_skipped(tmp_path):
165193
# A linux-64 package has no PE payload -> skipped clean (not failed).

0 commit comments

Comments
 (0)