FIX: Remove credential exposure from CI/CD pipelines - #715
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens CI/CD and devcontainer workflows to reduce credential exposure (avoiding bash xtrace, avoiding secrets in process arguments, and limiting secret persistence to restricted files/env).
Changes:
- Removes
-xtracing from multiple bash blocks (set -euo pipefail) and switches several Docker env injections to-e VAR(inherit by name). - Stops passing SQL passwords on
sqlcmdcommand lines by usingSQLCMDPASSWORD/SQLCMDPASSWORD-style env flows and stdin for sensitive SQL statements. - Updates devcontainer SQL startup to use a restricted
--env-fileand storesDB_CONNECTION_STRINGin a user-only env file instead of/etc/environment.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| OneBranchPipelines/stress-test-pipeline.yml | Reduces secret exposure during stress-test DB/container setup (env injection + SQLCMDPASSWORD). |
| OneBranchPipelines/stages/wheel-installation-test-windows-stage.yml | Avoids embedding DB passwords directly in the sqlcmd -Q command line. |
| OneBranchPipelines/stages/wheel-installation-test-stage.yml | Removes bash tracing and shifts Docker/sqlcmd password passing to inherited env vars. |
| OneBranchPipelines/stages/wheel-installation-test-macos-stage.yml | Uses inherited MSSQL_SA_PASSWORD / SQLCMDPASSWORD for container startup and readiness checks. |
| OneBranchPipelines/stages/build-windows-single-stage.yml | Avoids embedding DB passwords directly in the sqlcmd -Q command line. |
| OneBranchPipelines/stages/build-macos-single-stage.yml | Uses SQLCMDPASSWORD instead of -P for container readiness checks. |
| OneBranchPipelines/stages/build-linux-single-stage.yml | Removes bash tracing and shifts Docker/sqlcmd password passing to inherited env vars. |
| eng/pipelines/pypi-package-smoketest.yml | Stops echoing the connection string and passes it into containers via inherited env. |
| eng/pipelines/pr-validation-pipeline.yml | Moves SQL installer password to ACL-restricted config files and switches sqlcmd auth to SQLCMDPASSWORD. |
| .devcontainer/post-create.sh | Uses a restricted Docker env file and stores the connection string in a user-only env file sourced by shells. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Eliminate secret leakage via bash xtrace, process arguments, and world-readable files across ADO/OneBranch pipelines and the devcontainer: - Replace 'set -eux' with 'set -euo pipefail' in all secret-handling blocks and nested shells - Map secrets through step env: and reference shell vars instead of ADO \ macros - Use SQLCMDPASSWORD and stdin for sqlcmd; inherit Docker env vars by name (-e VAR) - Replace SQL Server installer /SAPWD= with an ACL-restricted /ConfigurationFile= deleted in finally - devcontainer: use --env-file (0600) for docker run and a user-only env file instead of world-readable /etc/environment
e0efe43 to
ebe74ee
Compare
There was a problem hiding this comment.
devskim found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Avoid mutating the global umask; create per-file with owner-only perms via a scoped subshell and pre-write chmod.
|
Re: DevSkim ("20+ potential problems") — these are informational (non-blocking per repo convention) and are the expected local-test-scaffolding patterns: |
The PowerShell here-string content sat at column 0, which dedented out of the YAML '|' block and made the ADO parser reject the pipeline. Replace it with an indented PowerShell array written via Set-Content, keeping clean INI output.
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.ddbc_bindings.cpp: 76.6%
mssql_python.__init__.py: 77.6%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.pybind.connection.connection_pool.cpp: 81.4%
mssql_python.pybind.connection.connection.cpp: 84.3%
mssql_python.logging.py: 85.5%🔗 Quick Links
|
Work Item / Issue Reference
Summary
This pull request improves the handling of sensitive credentials and environment variables for SQL Server setup and testing across development scripts and CI pipelines. The changes enhance security by avoiding plaintext password exposure, standardize environment variable usage, and make scripts more robust and portable.
Credential Management and Security Improvements:
.devcontainer/post-create.sh, SQL Server passwords are now written to a secure temporary environment file (umask 077), and the credentials are kept in a user-only file (~/.mssql_python_env). The script also removes plaintext credentials from/etc/environmentand shell RC files, and ensures credentials are sourced securely in new shell sessions. [1] [2]MSSQL_SA_PASSWORD,SQLCMDPASSWORD) instead of command-line arguments, reducing the risk of leaking secrets in process lists or logs. [1] [2] [3] [4]Pipeline and Script Robustness:
build-linux-single-stage.yml,stress-test-pipeline.yml) have been updated to use stricter error handling (set -euo pipefail) for improved reliability and easier debugging. [1] [2] [3] [4] [5] [6] [7] [8]Cross-Platform and Pipeline Consistency:
sqlcmd, including for database and user creation. This includes changes to how PowerShell scripts interpolate passwords, and how SQL Server is installed/configured on Windows using a secure configuration file. [1] [2] [3] [4] [5] [6] [7] [8] [9]Backward Compatibility and Cleanup:
These changes collectively make the development and CI environments more secure and maintainable by standardizing credential handling and improving script reliability.