Skip to content

FIX: Remove credential exposure from CI/CD pipelines - #715

Open
gargsaumya wants to merge 4 commits into
mainfrom
saumya/fix-ci-credential-exposure
Open

FIX: Remove credential exposure from CI/CD pipelines#715
gargsaumya wants to merge 4 commits into
mainfrom
saumya/fix-ci-credential-exposure

Conversation

@gargsaumya

@gargsaumya gargsaumya commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Work Item / Issue Reference

AB#46465

GitHub Issue: #<ISSUE_NUMBER>


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:

  • In .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/environment and shell RC files, and ensures credentials are sourced securely in new shell sessions. [1] [2]
  • Docker containers for SQL Server in CI pipelines now receive passwords via environment variables (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:

  • Shell scripts in CI pipelines (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:

  • Windows and Linux pipelines now consistently use environment variables for passing sensitive information to SQL Server and 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:

  • The devcontainer script now removes legacy plaintext password files and entries, ensuring no old credentials are left behind after upgrades.

These changes collectively make the development and CI environments more secure and maintainable by standardizing credential handling and improving script reliability.

Copilot AI lite review requested due to automatic review settings August 12, 2026 05:52

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.

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 -x tracing from multiple bash blocks (set -euo pipefail) and switches several Docker env injections to -e VAR (inherit by name).
  • Stops passing SQL passwords on sqlcmd command lines by using SQLCMDPASSWORD / SQLCMDPASSWORD-style env flows and stdin for sensitive SQL statements.
  • Updates devcontainer SQL startup to use a restricted --env-file and stores DB_CONNECTION_STRING in 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.

Comment thread .devcontainer/post-create.sh Outdated
Comment thread OneBranchPipelines/stress-test-pipeline.yml
Comment thread OneBranchPipelines/stress-test-pipeline.yml
Comment thread OneBranchPipelines/stages/wheel-installation-test-windows-stage.yml Outdated
Comment thread OneBranchPipelines/stages/build-windows-single-stage.yml
Comment thread eng/pipelines/pr-validation-pipeline.yml
Comment thread eng/pipelines/pr-validation-pipeline.yml
Comment thread eng/pipelines/pr-validation-pipeline.yml
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
@gargsaumya
gargsaumya force-pushed the saumya/fix-ci-credential-exposure branch from e0efe43 to ebe74ee Compare August 12, 2026 05:58

@github-advanced-security github-advanced-security 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.

devskim found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@github-actions github-actions Bot added the pr-size: medium Moderate update size label Aug 12, 2026
Avoid mutating the global umask; create per-file with owner-only perms via a scoped subshell and pre-write chmod.
@gargsaumya

Copy link
Copy Markdown
Contributor Author

Re: DevSkim ("20+ potential problems") — these are informational (non-blocking per repo convention) and are the expected local-test-scaffolding patterns: TrustServerCertificate=yes, sqlcmd invocations, and the masked Pwd=*** template strings (literal, no secret expansion). None expose credentials. Existing # DevSkim: ignore suppressions already cover these categories; will add targeted suppressions for any genuinely new line rather than change behavior.

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.
@github-actions

Copy link
Copy Markdown

📊 Code Coverage Report

🔥 Diff Coverage

100%


🎯 Overall Coverage

82%


📈 Total Lines Covered: 7368 out of 8963
📁 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: 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

⚙️ Build Summary 📋 Coverage Details

View Azure DevOps Build

Browse Full Coverage Report

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

Labels

pr-size: medium Moderate update size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants