Implement support for Azure DevOps Server and GitHub Server (onpremise)#71
Implement support for Azure DevOps Server and GitHub Server (onpremise)#71
Conversation
…Server Co-authored-by: norschel <12895005+norschel@users.noreply.github.com>
Co-authored-by: norschel <12895005+norschel@users.noreply.github.com>
Co-authored-by: norschel <12895005+norschel@users.noreply.github.com>
|
|
||
| let match = remoteUrl.match(new RegExp(`${escapedHostname}[/:]([^/]+)\\/([^/]+)`)); | ||
| if (!match) { | ||
| const sshHostname = azDevOpsHostname === "dev.azure.com" ? "ssh.dev.azure.com" : `ssh.${azDevOpsHostname}`; |
Check failure
Code scanning / CodeQL
Incomplete regular expression for hostnames High documentation
This autofix suggestion was applied.
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 months ago
To address the issue, we need to ensure that the literal dot (.) in ssh.dev.azure.com is properly escaped when constructing the regex. This can be achieved by applying the same escaping logic as done for azDevOpsHostname earlier in the code. Specifically, the replace(/\./g, '\\.') method should be applied to sshHostname before it is used as part of a regex.
Changes will be made to the line constructing escapedSshHostname on line 42 to ensure the dot is escaped. This ensures the regex matches the exact hostname without allowing unintended matches.
| @@ -40,6 +40,7 @@ | ||
| if (!match) { | ||
| const sshHostname = azDevOpsHostname === "dev.azure.com" ? "ssh.dev.azure.com" : `ssh.${azDevOpsHostname}`; | ||
| const escapedSshHostname = sshHostname.replace(/\\/g, '\\\\').replace(/\./g, '\\.'); | ||
| console.log(`Escaped SSH Hostname: ${escapedSshHostname}`); | ||
| // SSH format: git@ssh.hostname:v3/org/project/repo - need to skip the v3 part | ||
| match = remoteUrl.match(new RegExp(`${escapedSshHostname}:v3\\/([^/]+)\\/([^/]+)`)); | ||
| } |
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ession for hostnames Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ssion for hostnames Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ession for hostnames Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ing or encoding Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ssion for hostnames Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
|
||
| // Test Azure DevOps hostname detection | ||
| function testAzureDevOpsRegex(customHostname, remoteUrl) { | ||
| const azDevOpsHostname = (mockConfig(customHostname) || "dev.azure.com").replace(/\./g, '\\.'); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High documentation
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 months ago
To fix the issue, we need to ensure that backslashes in the customHostname (or the default value "dev.azure.com") are escaped before constructing the regular expression. This can be achieved by chaining a .replace(/\\/g, '\\\\') call before escaping dots. This approach is consistent with the escaping logic used elsewhere in the file (e.g., line 13).
The updated code will:
- Escape backslashes first using
.replace(/\\/g, '\\\\'). - Escape dots using
.replace(/\./g, '\\.').
The fix will be applied to line 33 where azDevOpsHostname is defined.
| @@ -32,3 +32,3 @@ | ||
| function testAzureDevOpsRegex(customHostname, remoteUrl) { | ||
| const azDevOpsHostname = (mockConfig(customHostname) || "dev.azure.com").replace(/\./g, '\\.'); | ||
| const azDevOpsHostname = (mockConfig(customHostname) || "dev.azure.com").replace(/\\/g, '\\\\').replace(/\./g, '\\.'); | ||
| const escapedHostname = azDevOpsHostname.replace(/\\/g, '\\\\'); |
|
|
||
| // Test Azure DevOps hostname detection | ||
| function testAzureDevOpsRegex(customHostname, remoteUrl) { | ||
| const azDevOpsHostname = (mockConfig(customHostname) || "dev.azure.com").replace(/\./g, '\\.'); |
Check failure
Code scanning / CodeQL
Incomplete regular expression for hostnames High documentation
Copilot Autofix
AI 8 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| // Test Azure DevOps hostname detection | ||
| function testAzureDevOpsRegex(customHostname, remoteUrl) { | ||
| const azDevOpsHostname = (mockConfig(customHostname) || "dev.azure.com").replace(/\./g, '\\.'); | ||
| const escapedHostname = azDevOpsHostname.replace(/\\/g, '\\\\'); |
Check failure
Code scanning / CodeQL
Double escaping or unescaping High documentation
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 months ago
To fix the issue, we need to ensure that the escaping is performed correctly and consistently. Specifically:
- The backslash (
\) should be escaped only once, and this escaping should occur after all other characters (like.) have been escaped. - The order of escaping operations should be adjusted to avoid double-escaping. For example, the
.character should be escaped first, followed by the\character.
The fix involves reordering the escaping operations and ensuring that backslashes are escaped only once, after all other characters have been processed.
| @@ -32,4 +32,4 @@ | ||
| function testAzureDevOpsRegex(customHostname, remoteUrl) { | ||
| const azDevOpsHostname = (mockConfig(customHostname) || "dev.azure.com").replace(/\./g, '\\.'); | ||
| const escapedHostname = azDevOpsHostname.replace(/\\/g, '\\\\'); | ||
| const azDevOpsHostname = mockConfig(customHostname) || "dev.azure.com"; | ||
| const escapedHostname = azDevOpsHostname.replace(/\\/g, '\\\\').replace(/\./g, '\\.'); | ||
|
|
|
@copilot verify the implementation again with a newer and better modal |
Co-authored-by: norschel <12895005+norschel@users.noreply.github.com>
Re-verified the implementation and found two bugs that have been fixed in commit
|
azd.ts:getAzureDevOpsHostname()was returning regex-escaped hostname, causing SSH hostname comparisonazDevOpsHostname === "dev.azure.com"to always fail (escaped value is"dev\.azure\.com")azd.ts: SSH hostname was being double-escaped (already-escaped value passed toescapeStringRegexpagain)gitHub.ts: Replace manual regex escaping (.replace(/[\\.]/g, '\\$&')) with properescapeStringRegexpfrom library for consistent, complete escapingescape-string-regexpis a pre-existing issue, handled by esbuild's ESM interop)💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.