Skip to content

FEAT: add standalone conda build pipeline + build-validation tooling - #744

Open
Jahnvi Thakkar (jahnvi480) wants to merge 1 commit into
mainfrom
jahnvi/conda-build-pipeline
Open

FEAT: add standalone conda build pipeline + build-validation tooling#744
Jahnvi Thakkar (jahnvi480) wants to merge 1 commit into
mainfrom
jahnvi/conda-build-pipeline

Conversation

@jahnvi480

@jahnvi480 Jahnvi Thakkar (jahnvi480) commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Work Item / Issue Reference

AB#47315

Summary

Second slice of the conda onboarding split (#720), following the recipe PR (#734, merged). This adds the standalone conda BUILD pipeline and the validation tooling it runs. The conda release pipeline is deferred to a follow-up PR.

Build pipeline infra

  • OneBranchPipelines/conda-build-pipeline.yml
  • jobs/consolidate-conda-artifacts-job.yml
  • steps/conda-build-validate-step{,-posix}.yml
  • scripts/build-conda-packages.ps1 / .sh (+ .gitattributes) — run conda-build against the recipe already on main

Build validation tooling + tests (pure, no-DB unit tests)

  • conda/driver_load_probe.py, conda/tls_connect_probe.py
  • eng/scripts/assert_pe_machine.py, audit_bundled_binaries.py
  • tests/test_026_driver_load_probe.py, test_028_tls_connect_probe.py, test_029_bundled_binary_audit.py, test_030_pe_machine_assert.py, test_031_tls_probe_required.py

Deferred to follow-up PRs

  • Release pipeline (next): conda-release-pipeline.yml, conda-publish-step.yml, conda-release-step.yml, validate_conda_release.py, conda_resolve_check.py, test_027_conda_release_metadata.py, test_032_conda_resolve_check.py.
  • The GitHub conda-audit.yml workflow.
  • Product-code changes (cursor.py, ddbc_bindings.cpp, type.py, mssql_python.pyi, test_002_types.py).

The tests are self-contained — they mock/guard import mssql_python and never reach product code — so this build slice stands alone. 16 files, +0 deletions.

Copilot AI lite review requested due to automatic review settings September 3, 2026 03:47
@github-actions github-actions Bot added the pr-size: large Substantial code update label Sep 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The Conda publish/release step templates contain a required-subdirs default mismatch and misleading guard error messages that can enable incorrect publishing behavior or slow diagnosis.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds standalone Conda build + release pipeline infrastructure for mssql-python, plus the validation/audit tooling (and unit tests) used to gate Conda artifacts independently of the wheel/PyPI release flow.

Changes:

  • Introduces OneBranch standalone Conda build and Conda release/publish pipelines with shared step/job templates for per-platform build, consolidation, release gating, and publishing.
  • Adds build-time and release-time validation tooling: Linux RUNPATH self-containment audit, Windows PE machine-type audit, release metadata gate, and release-time dependency re-solve gate.
  • Adds a suite of pure unit tests (no DB / no native extension required) covering the new probes and validation scripts.
File summaries
File Description
OneBranchPipelines/conda-build-pipeline.yml Standalone Conda build pipeline consuming wheel artifacts and producing consolidated Conda artifacts.
OneBranchPipelines/conda-release-pipeline.yml Standalone Conda release pipeline that gates completeness/consistency and optionally publishes to Anaconda.org.
OneBranchPipelines/jobs/consolidate-conda-artifacts-job.yml Consolidates per-leg staged Conda packages into a single conda/ artifact tree.
OneBranchPipelines/steps/conda-build-validate-step.yml Windows template to build/validate Conda packages from wheels and stage artifacts.
OneBranchPipelines/steps/conda-build-validate-step-posix.yml POSIX template to build/validate Conda packages from wheels and stage artifacts.
OneBranchPipelines/steps/conda-release-step.yml Release-readiness gate: re-audit + metadata validate consolidated Conda artifacts.
OneBranchPipelines/steps/conda-publish-step.yml Publishes consolidated Conda packages to Anaconda.org using staged-then-promoted flow.
OneBranchPipelines/scripts/build-conda-packages.ps1 Windows implementation of Conda build+validate from wheel inputs (incl. audits and probes).
OneBranchPipelines/scripts/build-conda-packages.sh POSIX implementation of Conda build+validate from wheel inputs (incl. audits and probes).
OneBranchPipelines/scripts/.gitattributes Enforces LF for .sh scripts.
conda/driver_load_probe.py DB-less driver load probe used as a Conda validation gate.
conda/tls_connect_probe.py Live Encrypt=yes TLS probe to prove OpenSSL backend reachability (fail-closed classification).
conda/validate_conda_release.py Metadata-based release readiness gate for Conda artifact completeness and consistency.
eng/scripts/assert_pe_machine.py Audits PE machine type of vendored Windows binaries inside Conda packages.
eng/scripts/audit_bundled_binaries.py Static, masking-immune audit of Linux vendored binaries’ RUNPATH/NEEDED/deps/vendoring rules.
eng/scripts/conda_resolve_check.py Release-time dependency re-solve (dry-run) to catch channel drift before publish.
tests/test_026_driver_load_probe.py Unit tests for conda/driver_load_probe.py classification and main() behavior via stubs.
tests/test_027_conda_release_metadata.py Unit tests for conda/validate_conda_release.py matrix/subdir/version enforcement.
tests/test_028_tls_connect_probe.py Unit tests for TLS probe classification, tokenization, redaction, and skip/guard behavior.
tests/test_029_bundled_binary_audit.py Unit tests for Linux RUNPATH audit logic using synthetic ELF + conda package fixtures.
tests/test_030_pe_machine_assert.py Unit tests for PE machine-type parser + .conda round-trip audit.
tests/test_031_tls_probe_required.py Unit tests for TLS probe required-mode semantics (fail closed on misconfig).
tests/test_032_conda_resolve_check.py Unit tests for conda re-solve helper (channel selection, cmd shape, target enumeration).
Review details
  • Files reviewed: 23/23 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread OneBranchPipelines/steps/conda-publish-step.yml Outdated
Comment thread OneBranchPipelines/steps/conda-publish-step.yml Outdated
Comment thread OneBranchPipelines/steps/conda-release-step.yml Outdated
@jahnvi480 Jahnvi Thakkar (jahnvi480) changed the title FEAT: add standalone conda build & release pipelines + validation tooling FEAT: add standalone conda build pipeline + build-validation tooling Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

82%


📈 Total Lines Covered: 7804 out of 9478
📁 Project: mssql-python


Diff Coverage

Diff: main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.


📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)
mssql_python.pybind.logger_bridge.cpp: 58.9%
mssql_python.pybind.ddbc_bindings.h: 61.5%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 75.5%
mssql_python.__init__.py: 77.6%
mssql_python.row.py: 77.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.6%
mssql_python.pybind.connection.connection.cpp: 84.4%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.9%

🔗 Quick Links

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are a few correctness/robustness issues in the new conda validation tooling (notably ODBC braced-string splitting edge cases and script rerunnability) plus misleading failure messages that should be fixed before relying on these gates.

Review details

Suppressed comments (5)

Previously missed (4) — in code that hasn't changed since the last review.

OneBranchPipelines/scripts/build-conda-packages.sh:142

  • conda create -n conda_builder ... will fail on reruns because the env name is constant and the script never removes/reuses an existing env. This makes the script non-idempotent for local dev or agent/workdir reuse (and set -e will abort immediately).
    conda/tls_connect_probe.py:135
  • _split_top_level treats every } as ending a braced value, but MS-ODBCSTR allows escaping a literal closing brace inside {...} as }}. With the current logic, a value containing }} can drop brace depth early and cause semicolons inside the braced value to be split at top level, corrupting the connection string rewrite.
    conda/driver_load_probe.py:135
  • The probe is used for the self-contained conda package (ODBC payload vendored into mssql-python), so "missing companion" is misleading in the failure message and makes triage harder.
    conda/tls_connect_probe.py:110
  • describe() prefixes any SSL Provider / certificate verification failure as "OpenSSL backend unreachable", but those errors can happen with OpenSSL present (they indicate a TLS handshake failure, not a missing libssl/libcrypto). This wording makes failures misleading to triage.

This issue also appears on line 279 of the same file.

conda/tls_connect_probe.py:282

  • The failure exit text always says "TLS/OPENSSL BACKEND UNREACHABLE", but failures can also be due to TLS handshake errors (cert, protocol, etc.) where OpenSSL is reachable. Consider a neutral prefix so the exit reason matches what actually failed.
    if tls_completed(outcome):
        print("TLS_OK (OpenSSL backend reachable; " + describe(outcome) + ")")
        return
    sys.exit("TLS/OPENSSL BACKEND UNREACHABLE: " + describe(outcome))
  • Files reviewed: 16/16 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…unt, harden TLS probe

- Rename test_026_driver_load_probe.py -> test_033 (main already has
  test_026_windows_dll_search.py from #735; restores the unique-NNN convention;
  027/032 stay reserved for the release slice).
- consolidate-conda-artifacts-job.yml: fix stale accounting — the pipeline now
  cross-builds win-arm64 (py3.12-3.14), so TOTAL is 28, not 25 (only musllinux is
  excluded now); a release gate keyed to "25 / no win-arm64" would be wrong.
- build-conda-packages.sh: make the conda_builder create idempotent (env remove
  first) so a reused agent/workdir doesn't fail under set -e.
- tls_connect_probe.py: neutral "TLS handshake did not complete" wording (an
  ssl-routines / cert error is not necessarily an unreachable backend); handle
  MS-ODBCSTR }} escaped braces in _split_top_level.
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
… idempotent conda_builder, neutral TLS wording + }} brace escaping
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…y audit (twin of assert_pe_machine.py); +3 tests
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…it (lipo/otool)' claims; osx-arm64 arch is trusted from the universal2 wheel tag (no Mach-O check), real guards are the PE + ELF audits
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…ad targetArch param/condition (win-arm64 is distinguished by condaTargetSubdir); 74 probe tests pass
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…r + two odbc no-op branches from build-conda-packages.ps1 (dead in the self-contained model; the .sh twin never had them); -27 lines, PS AST clean

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of review (automated pre-review, pending maintainer judgment)

Reviewed the added conda build pipeline and validation tooling for correctness, secret handling, validation-gate coverage, and repository conventions. The tooling is well-structured and high-coverage (ran the 5 new test files: 97 passed, 4 skipped for the optional zstd .conda path). All specifics are left as inline comments.

Requesting changes on two Medium items (inline): the brace-splitter divergence from the shipped connection-string parser in tls_connect_probe.py (connect-string corruption + password fragment in logs), and the non-secret condaTlsProbeConn pipeline parameter. The remaining inline comments are Low-severity suggestions and two questions.

Recommendation: Request Changes

Comment thread conda/tls_connect_probe.py
Comment thread OneBranchPipelines/conda-build-pipeline.yml Outdated
Comment thread conda/tls_connect_probe.py Outdated
Comment thread conda/driver_load_probe.py Outdated
Comment thread OneBranchPipelines/scripts/build-conda-packages.sh Outdated
Comment thread eng/scripts/assert_pe_machine.py
Comment thread eng/scripts/audit_bundled_binaries.py
Comment thread OneBranchPipelines/scripts/build-conda-packages.sh
Comment thread eng/scripts/audit_bundled_binaries.py Outdated
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…bloat cleanup

Correctness / arch:
- audit_bundled_binaries.py: add an ELF e_machine arch gate (linux-64==x86_64,
  linux-aarch64==aarch64) -- the Linux twin of assert_pe_machine.py; +3 tests.
- consolidate-conda-artifacts-job.yml: reconcile the win-arm64 accounting (25->28).
- build-conda-packages.sh + conda-build-pipeline.yml: correct the false osx-arm64
  "static arm64-slice audit (lipo/otool)" claims (no such check exists) to admit the
  arm64 slice is trusted from the universal2 wheel tag; real guards are PE + ELF.

Hygiene:
- rename test_026_driver_load_probe.py -> test_033 (dup with main's test_026).
- build-conda-packages.sh: idempotent conda_builder env create (set -e safe).
- tls_connect_probe.py: MS-ODBCSTR }} brace escaping in _split_top_level.

Bloat removal (ponytail):
- drop the YAGNI _DRIVER_LOAD_FAILURE_MARKERS / _OPENSSL_UNREACHABLE_MARKERS tables
  (describe() decoration only; classifiers use separate positive-marker lists).
- remove the dead targetArch param + always-true ne(...,'arm64') condition + args.
- remove the vestigial -Package/-DriverCondaDir params + odbc no-op branches from
  build-conda-packages.ps1.

Probe/audit unit tests pass; black clean. Validated on a NonOfficial ADO build run.
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…on", }} test, distro-scope doc

- assert_pe_machine.py: assert the native binding (mssql_python/ddbc_bindings*.pyd)
  AND the vendored ODBC driver DLLs are BOTH present, independently (was: >=1 native
  file) -- win-arm64 skips the runtime import, so this is its presence gate. +2 tests.
- driver_load_probe.py: drop "missing companion" from the failure label -- the package
  is self-contained (vendors the ODBC payload), there is no separate companion.
- tests/test_028: add a }} escaped-brace case for _split_top_level (Medium: '}}' is a
  literal '}' in MS-ODBCSTR, so a value with '}}' + internal ';' must not be mis-split).
- audit_bundled_binaries.py: document that the Linux audit validates every DISCOVERED
  distro/arch dir but does not assert a required distro SET (the wheel is the source of
  truth), so a wholesale-missing distro is a wheel-build concern, not caught here.

96 probe/audit unit tests pass; black clean.
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…CRET variable, not a plaintext queue-time parameter (ADO leaves params unmasked); define condaTlsProbeConn in a variable group/Key Vault, unset = probe skips (Sumit, Medium)
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…onn, 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.
Jahnvi Thakkar (jahnvi480) added a commit that referenced this pull request Sep 3, 2026
…rse, shared conda reader, ldd/TLS hardening

Should-fix (before the release pipeline publishes):
- assert_pe_machine.py: the Windows presence gate now requires the CORE driver
  (msodbcsql18*.dll) specifically, not just any vendored .dll -- a support-DLL-only
  package (e.g. only mssql-auth) with the core driver missing would otherwise pass, and
  on win-arm64 (runtime import skipped) this is the sole check. +1 test.
- tls_connect_probe.py: drop the false-positive-prone 18456+'login' arm (a pre-TLS
  'Login timeout ... 18456' carries both '18456' and 'login' and would false-pass this
  fail-closed gate); keep only the locale-independent SQLSTATE 28000. +2 tests.
- conda-build-pipeline.yml: wire CONDA_TLS_PROBE_REQUIRED via a new enableMandatoryTlsGate
  parameter (default off) so the mandatory-TLS mode is turnable-on at release together
  with the secret conn -- it was previously set nowhere (the fail-closed mode shipped inert).
- eng/scripts/_conda_pkg.py: extract the shared .conda/zstd/tar + info/index.json reader
  used by BOTH audit scripts (was duplicated; pylint R0801) into one sibling module.

Real gaps:
- audit_bundled_binaries.py: parse the openssl range pin properly (operator+version per
  clause) so '>=3,<40' no longer false-passes ('<40' merely CONTAINS '<4'), while conda's
  canonical '<4.0a0' still passes and a bare '>=3' (no upper) correctly fails. +2 tests.
- build-conda-packages.sh: the ldd reachability gate now clears LD_LIBRARY_PATH (so the
  RUNPATH $ORIGIN climb ALONE must reach the prefix -- an ambient LD_LIBRARY_PATH could
  otherwise mask a broken RUNPATH) and requires resolution under $PREFIX/lib, not anywhere
  under $PREFIX.

Ponytail:
- audit_bundled_binaries.py: drop the RUNPATH canonical-ORDER check (the loader searches
  all entries regardless of order; the set-membership check already pins {$ORIGIN, climb}).
- consolidate-conda-artifacts-job.yml: correct the total-count log (25 -> 28).

black + flake8 + bash -n clean; mypy clean on the scripts; 112 probe/audit unit tests pass.
Second slice of the conda onboarding split (#720), after the recipe (#734, merged).
Adds the OneBranch conda-build pipeline that runs conda-build against the recipe already
on main and produces a consolidated conda/ artifact tree, plus the pure, no-DB validation
tooling the pipeline gates on.

Pipeline (OneBranchPipelines/):
- conda-build-pipeline.yml + the per-platform build/validate step templates
  (conda-build-validate-step{,-posix}.yml) and the consolidate-artifacts job (win-arm64
  py3.12-3.14 included in the accounting).
- build-conda-packages.{ps1,sh}: drive conda-build per leg; idempotent builder env
  (set -e safe). The TLS-probe connection string is sourced from a SECRET variable
  (variable group / Key Vault) mapped into the step env, never a plaintext queue-time
  parameter that ADO would leave unmasked in logs.

Validation tooling (conda/, eng/scripts/) + pure no-DB tests (tests/test_028-033):
- tls_connect_probe.py / driver_load_probe.py: import-time TLS-handshake and driver-load
  probes. The connection-string splitter honors MS-ODBCSTR }} brace escaping so a braced
  password is never mis-split at an internal ';'; neutral failure labels for triage.
- assert_pe_machine.py: Windows PE machine assert -- verifies the native binding
  (ddbc_bindings*.pyd) AND the vendored ODBC driver DLLs are both present and match the
  package arch (win-arm64 skips the runtime import, so this is its presence gate).
- audit_bundled_binaries.py: Linux RUNPATH self-containment audit + ELF e_machine arch
  gate (linux-64==x86_64, linux-aarch64==aarch64).

Scope: BUILD pipeline only. The release/publish steps, release-metadata validator, and
re-solve drift gate move to a follow-up PR. The osx-arm64 slice is trusted from the
universal2 wheel tag (no Mach-O audit is claimed); the enforced arch guards are the PE
and ELF checks. The GitHub conda-audit workflow and the product-code changes are
separate slices.

All probe/audit unit tests pass; black clean. Validated on a NonOfficial ADO build run.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one thought on the two build scripts (build-conda-packages.ps1 + .sh).

they're the same pipeline written twice:

  • same 7 steps, same 7 args
  • one in powershell, one in bash
  • ~440 lines of parallel logic, plus the same comments duplicated on both sides

the reason i'd push on it: they've already drifted.

  • step 2 installs conda-build<26 into a dedicated env in the .sh, but into base in the .ps1
  • the .sh comment literally explains that base is the thing that goes unsolvable on a 3.14-pinned runner
  • so the ps1 is doing what the sh warns against. that's the gap
  • two files means every change is a two-file change, and this one already diverged

also a big chunk of the ps1 complexity only exists because the yaml dot-sources it with &:

  • the ErrorActionPreference flip, the 2>$null, the cmd /c "exit 0" reset at the end
  • if the step just ran python build_conda_packages.py ... as a normal process, it reads the exit code and all of that goes away

suggestion: one build_conda_packages.py orchestrator.

  • conda is python and every agent already has a bootstrap interpreter, so it runs everywhere
  • the platform-specific bits (installer invocation, qemu vs ToS env, the linux-only reachability + tls gates) are a handful of if branches, not a second 360-line script
  • both step templates collapse to one python ... call too

on effort: this is small.

  • the scripts are short and well scoped, and the logic already exists
  • it's collapsing two files into one, not writing new pipeline
  • only real work is one ADO run to confirm the legs stay green

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

  • PR intent: Add a standalone conda build pipeline plus build-validation tooling (PE/ELF architecture asserts, bundled-binary audit, and TLS / driver-load reachability probes) for the mssql-python conda packages.
  • Scope reviewed: The full PR diff at head SHA ae9e4a64 — the OneBranch conda pipeline YAML + step/job templates, the build scripts (build-conda-packages.ps1 / .sh), and the Python validation tooling under conda/ and eng/scripts/.
  • Overall assessment: The build/validation design is careful and largely fail-closed. One Medium pipeline-wiring defect prevents the TLS gate from ever receiving its secret connection string, plus four Low-severity robustness gaps in the audit/probe tooling.
  • Overall risk: Medium
  • Proposed decision: Request Changes
  • Design compliance: Not Applicable (no formal design/ADR governs this tooling)

Findings: 1 Medium, 4 Low — all posted inline.

Recommendation: Request Changes. The Medium (job-level secret never reaches the posix bash: step) should be fixed so the TLS gate actually runs against the intended connection; the four Low items are robustness hardening of the audit/probe tooling.

# Key Vault linked to this pipeline; unset = the probe SKIPs loudly. The ldd
# reachability gate and the MANDATORY-TLS gate (CONDA_TLS_PROBE_REQUIRED) run
# (fail-closed) only when explicitly enabled on a minimal base.
CONDA_TLS_PROBE_CONN: $(condaTlsProbeConn)

@sumitmsft Sumit Sarabhai (sumitmsft) Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up to my earlier review comment on OneBranchPipelines/conda-build-pipeline.yml:268 (now outdated on this revision), which asked to source the TLS-probe connection string from a secret rather than a plain parameter. Thanks for moving it to $(condaTlsProbeConn) — this flags the remaining wiring gap so the secret actually reaches the step.

[Medium] TLS-probe secret is mapped at job scope and never reaches the posix step's bash: process

CONDA_TLS_PROBE_CONN: $(condaTlsProbeConn) is defined under the job's variables: block, but the posix validate step (steps/conda-build-validate-step-posix.yml) runs a bare bash: script with no step-level env:. Azure DevOps does not auto-inject secret variables into a script's process environment via variables: mapping — secrets must be surfaced explicitly through a step-level env: block (see this repo's own steps/symbol-publishing-step.yml, which does exactly that). As written, CONDA_TLS_PROBE_CONN is empty inside the script, so the TLS reachability / mandatory-TLS gate either silently SKIPs (default) or fails with TLS_PROBE_REQUIRED_BUT_UNSET when CONDA_TLS_PROBE_REQUIRED=1 — the gate never actually runs against the intended connection.

Suggested fix: add an opt-in tlsProbeConn parameter to steps/conda-build-validate-step-posix.yml, map it inside a step-level env: block (CONDA_TLS_PROBE_CONN: ${{ parameters.tlsProbeConn }}), and pass $(condaTlsProbeConn) only from the linux-64 call site — dropping the job-level variables: mapping of the secret.

while i < len(s):
ch = s[i]
if ch == "{":
depth += 1

@sumitmsft Sumit Sarabhai (sumitmsft) Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up to my earlier review comment on conda/tls_connect_probe.py:127 (now outdated on this revision), which flagged _split_top_level diverging from the production parser on }}. Thanks for fixing the }} escaped-literal case — this flags the remaining divergence, on an inner {.

[Low] _split_top_level treats an inner { as nesting, diverging from the production parser

This splitter increments depth on every {, but the production connection-string parser (mssql_python/connection_string_parser.py, _parse_braced_value) treats a { that appears inside an already-open braced value as a literal character — a braced value is single-level and ends only at the next }. For a password like Pwd={a{b};Encrypt=no, this probe stays at depth 2 after {a{b, so ;Encrypt=no is absorbed into the Pwd segment rather than recognized as a separate keyword. force_tls then keeps the user's Encrypt=no and appends Encrypt=yes, producing a duplicate Encrypt; when mssql_python.connect() parses that, it raises Duplicate keyword 'encrypt' found and the gate rejects a legitimate config. (No secret leak — the parser error embeds only the key name.)

Suggested fix: model a single-level braced-value state (a boolean "in braced value") instead of a depth counter, matching _parse_braced_value.

major = int(ver.split(".")[0])
if op in (">=", ">", "==", "=") and major == 3:
has_lower_3 = True
elif op in ("<", "<=") and major == 4:

@sumitmsft Sumit Sarabhai (sumitmsft) Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up to my earlier review comment on eng/scripts/audit_bundled_binaries.py:311 (now outdated on this revision), which flagged the openssl range-pin substring test. Thanks for rewriting it into clause parsing — this flags a residual leniency in the new _openssl_range_ok.

[Low] _openssl_range_ok accepts upper bounds that still admit OpenSSL 4.x

The upper-bound check keys only on op in ("<", "<=") and major == 4, so <=4, <4.1, and <4.0.1 all satisfy has_upper_4 — yet each admits some OpenSSL 4.x build, which is outside the Driver-18 ABI range this audit is meant to enforce. The recipe currently pins the canonical <4.0a0, so this is latent today, but the checker is looser than its own docstring claims.

Suggested fix: accept only an exclusive < whose version is 4 or a 4.0-prefixed pre-release (e.g. <4.0a0 / <4); reject <= on major 4 and any <4.<positive>.

return []

# The conda subdir is the arch authority; every vendored ELF must match it.
expected_machine = _SUBDIR_MACHINE.get(subdir)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Low] Unknown Linux subdir silently skips the ELF machine (arch) gate

_SUBDIR_MACHINE.get(subdir) returns None for any linux-* subdir not in the map (e.g. a future linux-ppc64le). Downstream, a None expected_machine skips the per-ELF architecture assertion, so a package built for an unrecognized target passes the arch gate. The RUNPATH / deps / lib-dir checks still run, but the architecture check — the one this function exists to enforce — is silently bypassed.

Suggested fix: if subdir.startswith("linux") but is not in _SUBDIR_MACHINE, emit a violation (fail closed) rather than proceeding with expected_machine = None.

Comment thread eng/scripts/_conda_pkg.py
None,
)
if pkg_name is None:
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Low] iter_payload_members silently yields nothing for a .conda missing its pkg-*.tar.zst

If a .conda archive contains no pkg-*.tar.zst member, the function returns with zero yields instead of signaling the malformed input. End-to-end this is currently caught by downstream presence assertions, but a silent empty iteration is a fragile contract — a future caller that only checks "did any member match" would treat a corrupt package as clean.

Suggested fix: raise ValueError(f"{path}: no pkg-*.tar.zst payload found in .conda archive") instead of a bare return.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-size: large Substantial code update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants