Harden mysql client execution for AzureMysqlDeploymentV2 task - #22484
Gayatri Jaiswal (v-gayatrij) wants to merge 26 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
…tps://github.com/microsoft/azure-pipelines-tasks into users/v-gayjaiswal/AzureMySqlDeployment-msrc-fix
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Gayatri Jaiswal (v-gayatrij)
changed the title
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
…tps://github.com/microsoft/azure-pipelines-tasks into users/v-gayjaiswal/AzureMySqlDeployment-msrc-fix
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
| /** | ||
| * Reject a bare "--" token in user-supplied additional arguments, which | ||
| * is not a supported value for this input and could interfere with | ||
| * enforced security options. See internal security tracking for details. |
There was a problem hiding this comment.
Please keep public source comments neutral and remove the reference to internal security tracking. This can describe the validation as preventing unsupported arguments from interfering with task-managed client options without exposing private tracking context.
There was a problem hiding this comment.
updated comment
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
…tps://github.com/microsoft/azure-pipelines-tasks into users/v-gayjaiswal/AzureMySqlDeployment-msrc-fix
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
…tps://github.com/microsoft/azure-pipelines-tasks into users/v-gayjaiswal/AzureMySqlDeployment-msrc-fix
| return; | ||
| } | ||
| const tokens = Utility.argStringToArray(additionalArguments); | ||
| if (tokens.some(token => token === '--')) { |
There was a problem hiding this comment.
-- is not the only additional argument that can negate the task-managed behavior here. Beginning with MySQL 8.4.6, --commands can re-enable client commands even when --binary-mode is set. Because this validation still permits command-enabling forms such as --commands and --commands=ON, the intended restriction is not guaranteed on those clients. Please reject those forms as well and add a file-execution regression test.
There was a problem hiding this comment.
Added commands options to rejection list and file-execution regression test
…tps://github.com/microsoft/azure-pipelines-tasks into users/v-gayjaiswal/AzureMySqlDeployment-msrc-fix
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
| */ | ||
| private _validateAdditionalArguments(additionalArguments: string): void { | ||
| for (const token of Utility.argStringToArray(additionalArguments)) { | ||
| const optionName = token.split('=', 1)[0].toLowerCase(); |
There was a problem hiding this comment.
MySQL accepts unambiguous prefixes for long options, so --command=ON is resolved as --commands=ON. Here, however, optionName remains --command, which is not in the exact-name set, so the task-managed behavior can still be overridden. Please reject spellings that MySQL resolves to --commands (including valid abbreviations) and add a regression test for --command=ON.
There was a problem hiding this comment.
As per this, commands option cannot enable client commands when --binary-mode is set.
This branch has not been deployed
2 participants
Context
Associated WI: AB#2459100
Task Name
AzureMysqlDeploymentV2
Description
This PR adds the
--binary-modeflag to everymysqlclient invocation performed by the task (both inline-eexecution and file/stdin-based script execution). This causes the client to treat script content strictly as SQL, disabling client-side meta-commands.The flag is applied unconditionally by the task and cannot be overridden or disabled via any task input — user-supplied additional arguments are appended before the enforced flag, so the secure behavior always takes effect regardless of what a script or caller attempts to pass.
No changes were made to how ordinary SQL (including quoted string literals, SQL comments,
and
DELIMITER-based multi-statement definitions) is parsed or executed.Risk Assessment (Low / Medium / High)
Low
Change Behind Feature Flag (Yes / No)
No
Tech Design / Approach
Documentation Changes Required (Yes/No)
Indicate whether related documentation needs to be updated.
Unit Tests Added or Updated (Yes / No)
Yes
Additional Testing Performed
Manual regression + security validation performed on test organization
mysqlclient invocation the task performs (both inline execute and file/stdin execution paths), on Windows and Linux agents.DELIMITER-based multi-statement scripts (e.g., trigger/procedure definitions) all continue to execute successfully and unmodified.Logging Added/Updated (Yes/No)
Telemetry Added/Updated (Yes/No)
Rollback Scenario and Process (Yes/No)
Dependency Impact Assessed and Regression Tested (Yes/No)
Yes. Only the task's own invocation of the
mysqlclient is affected; no other task, module, or shared package is impacted. Minimum supportedmysqlclient version confirmed to support the flag used.Checklist