diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index d30ed765c2..987e96f345 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -12,10 +12,19 @@ name: "CodeQL Advanced" on: + # Scan on pushes to main only. push: - branches: [ "main", "feat/*", "dev/**/*" ] + branches: + - main + + # Scan on PRs that target matching branches. pull_request: - branches: [ "main", "feat/*", "dev/**/*" ] + branches: + - main + - feat/** + - dev/** + + # Scan weekly on Saturdays at 23:33 UTC schedule: - cron: '33 23 * * 6' diff --git a/Directory.Packages.props b/Directory.Packages.props index c0637bf065..e4953215a7 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -87,7 +87,6 @@ - @@ -116,7 +115,9 @@ - + + + diff --git a/eng/pipelines/common/templates/jobs/build-signed-package-job.yml b/eng/pipelines/common/templates/jobs/build-signed-package-job.yml index 22e6758c3e..bf5ee07ebb 100644 --- a/eng/pipelines/common/templates/jobs/build-signed-package-job.yml +++ b/eng/pipelines/common/templates/jobs/build-signed-package-job.yml @@ -36,9 +36,11 @@ jobs: - script: SET displayName: 'Print Environment Variables' - - powershell: | + - pwsh: | Write-Host "##vso[task.setvariable variable=CDP_BUILD_TYPE_COPY;isOutput=true]$($env:CDP_BUILD_TYPE)" + # This variable is used far away in validate-signed-packages-job.yml. name: GetBuildType + displayName: Get Build Type # Install the .NET SDK. - template: /eng/pipelines/steps/install-dotnet.yml@self diff --git a/eng/pipelines/common/templates/jobs/run-tests-package-reference-job.yml b/eng/pipelines/common/templates/jobs/run-tests-package-reference-job.yml index a6e7755c6b..6d07e60ff7 100644 --- a/eng/pipelines/common/templates/jobs/run-tests-package-reference-job.yml +++ b/eng/pipelines/common/templates/jobs/run-tests-package-reference-job.yml @@ -4,14 +4,9 @@ # See the LICENSE file in the project root for more information. # ################################################################################# parameters: - - name: downloadPackageStep - type: step - default: - script: echo - - name: dependsOn + - name: artifactName type: string - default: empty - name: isPreview type: boolean @@ -21,10 +16,9 @@ parameters: type: number jobs: + - job: run_tests_package_reference displayName: 'Run tests with package reference' - ${{ if ne(parameters.dependsOn, 'empty')}}: - dependsOn: '${{parameters.dependsOn }}' # Some of our tests take longer than the default 60 minutes to run on some # OSes and configurations. @@ -42,7 +36,10 @@ jobs: steps: - template: /eng/pipelines/common/templates/steps/pre-build-step.yml - - ${{parameters.downloadPackageStep }} + - download: current + artifact: ${{parameters.artifactName}} + patterns: '**/*.*nupkg' + displayName: 'Download NuGet Package' - template: /eng/pipelines/common/templates/steps/update-config-file-step.yml parameters: @@ -52,7 +49,7 @@ jobs: - template: /eng/pipelines/common/templates/steps/prepare-test-db-step.yml -# build & test + # build & test - template: /eng/pipelines/common/templates/steps/build-and-run-tests-netfx-step.yml parameters: ${{ if parameters.isPreview }}: diff --git a/eng/pipelines/common/templates/jobs/validate-signed-package-job.yml b/eng/pipelines/common/templates/jobs/validate-signed-package-job.yml index 6dba0a8eab..237d4798f6 100644 --- a/eng/pipelines/common/templates/jobs/validate-signed-package-job.yml +++ b/eng/pipelines/common/templates/jobs/validate-signed-package-job.yml @@ -4,26 +4,9 @@ # See the LICENSE file in the project root for more information. # ################################################################################# parameters: - - name: downloadPackageStep - type: step - default: - script: echo - - name: packageFolderName + - name: artifactName type: string - default: drop_build_build_signed_package - - - name: dependsOn - type: string - default: '' - - - name: packageType - type: string - default: both - values: - - dll - - pdb - - both - name: isPreview type: boolean @@ -31,8 +14,7 @@ parameters: jobs: - job: validate_signed_package displayName: 'Verify signed package' - ${{ if ne(parameters.dependsOn, '')}}: - dependsOn: '${{parameters.dependsOn }}' + pool: type: windows # read more about custom job pool types at https://aka.ms/obpipelines/yaml/jobs isCustom: true @@ -43,9 +25,11 @@ jobs: - template: /eng/pipelines/libraries/mds-validation-variables.yml@self - name: pathToDownloadedNuget # path to the downloaded nuget files - value: $(Pipeline.Workspace)\${{parameters.packageFolderName }} + value: $(Pipeline.Workspace)\${{parameters.artifactName }} - name: BuildType + # This variable is set in the build-signed-package-job.yml job. Any changes + # to the names of the stage, job, or variable must be reflected here. value: $[ stageDependencies.buildMDS.build_signed_package.outputs['GetBuildType.CDP_BUILD_TYPE_COPY'] ] - ${{ if parameters.isPreview }}: @@ -69,7 +53,10 @@ jobs: nuget locals all -Clear displayName: 'Clear local cache' - - ${{parameters.downloadPackageStep }} + - download: current + artifact: ${{parameters.artifactName}} + patterns: '**/*.*nupkg' + displayName: 'Download NuGet Package' - powershell: | # Install nuget package @@ -82,21 +69,12 @@ jobs: displayName: 'Extract Nuget in temp folder' - powershell: | - # Artifact is stored in the Nuget folder - $packageType = '${{parameters.packageType}}' - Write-Host "--------------------------------------------------" Write-Host "This will verify the artifact signature" -ForegroundColor Green Write-Host "--------------------------------------------------" - if ($packageType -eq 'dll' -or $packageType -eq 'both') - { - nuget verify -All $(pathToDownloadedNuget)\*.nupkg - } - if ($packageType -eq 'pdb' -or $packageType -eq 'both') - { - nuget verify -All $(pathToDownloadedNuget)\*.snupkg - } + nuget verify -All $(pathToDownloadedNuget)\*.nupkg + nuget verify -All $(pathToDownloadedNuget)\*.snupkg displayName: 'Verify nuget signature' - powershell: | diff --git a/eng/pipelines/common/templates/steps/code-analyze-step.yml b/eng/pipelines/common/templates/steps/code-analyze-step.yml index ec226e134c..3ee9b4e92e 100644 --- a/eng/pipelines/common/templates/steps/code-analyze-step.yml +++ b/eng/pipelines/common/templates/steps/code-analyze-step.yml @@ -32,7 +32,7 @@ steps: msBuildCommandline: >- msbuild ${{parameters.sourceRoot}}\build.proj + -t:BuildAllConfigurations -p:configuration=Release -p:GenerateNuget=false -p:BuildTools=false - -p:SigningKeyPath=$(Agent.TempDirectory)\netfxKeypair.snk diff --git a/eng/pipelines/dotnet-sqlclient-signing-pipeline.yml b/eng/pipelines/dotnet-sqlclient-signing-pipeline.yml index 78c0f230b6..2c27a1c407 100644 --- a/eng/pipelines/dotnet-sqlclient-signing-pipeline.yml +++ b/eng/pipelines/dotnet-sqlclient-signing-pipeline.yml @@ -159,21 +159,12 @@ extends: jobs: - template: /eng/pipelines/common/templates/jobs/validate-signed-package-job.yml@self parameters: - packageFolderName: $(mdsArtifactName) + artifactName: $(mdsArtifactName) isPreview: ${{ parameters['isPreview'] }} - downloadPackageStep: - download: current - artifact: $(mdsArtifactName) - patterns: '**/*.*nupkg' - displayName: 'Download NuGet Package' # Disabling as of 10/15/2025 due to OneBranch apparently disallowing MSBuild tasks in validation stages. # - template: /eng/pipelines/common/templates/jobs/run-tests-package-reference-job.yml@self # parameters: +# artifactName: $(mdsArtifactName) # isPreview: ${{ parameters['isPreview'] }} # timeout: ${{ parameters.testJobTimeout }} -# downloadPackageStep: -# download: current -# artifact: $(mdsArtifactName) -# patterns: '**/*.nupkg' -# displayName: 'Download NuGet Package' diff --git a/eng/pipelines/jobs/build-akv-official-job.yml b/eng/pipelines/jobs/build-akv-official-job.yml index fab4c1aa9c..c7ed0a7354 100644 --- a/eng/pipelines/jobs/build-akv-official-job.yml +++ b/eng/pipelines/jobs/build-akv-official-job.yml @@ -89,6 +89,9 @@ jobs: $jsonParams | ConvertFrom-Json | Format-List displayName: 'Output Job Parameters' + # Install the .NET SDK. + - template: /eng/pipelines/steps/install-dotnet.yml@self + # Perform analysis before building, since this step will clobber build # output. - template: /eng/pipelines/steps/roslyn-analyzers-akv-step.yml@self diff --git a/eng/pipelines/jobs/test-azure-package-ci-job.yml b/eng/pipelines/jobs/test-azure-package-ci-job.yml index 2d2da14435..93b8b60132 100644 --- a/eng/pipelines/jobs/test-azure-package-ci-job.yml +++ b/eng/pipelines/jobs/test-azure-package-ci-job.yml @@ -273,10 +273,10 @@ jobs: # List the DLLs in the output directory for debugging purposes. - ${{ if eq(parameters.debug, true) }}: - - pwsh: > + - pwsh: >- Get-ChildItem - -Path "src/Microsoft.Data.SqlClient.Extensions/Azure/test/bin/${{ parameters.buildConfiguration }}" - -Recurse + -Path "src/Microsoft.Data.SqlClient.Extensions/Azure/test/bin/${{ parameters.buildConfiguration }}" + -Recurse displayName: '[Debug] List Output DLLs' # Run the tests for each .NET runtime. diff --git a/eng/pipelines/sqlclient-pr-package-ref-pipeline.yml b/eng/pipelines/sqlclient-pr-package-ref-pipeline.yml index 5b75f1f2de..456cf4fc8f 100644 --- a/eng/pipelines/sqlclient-pr-package-ref-pipeline.yml +++ b/eng/pipelines/sqlclient-pr-package-ref-pipeline.yml @@ -30,7 +30,7 @@ pr: branches: include: # GitHub repo branch targets that will trigger PR validation builds. - - dev/**/* + - dev/* - feat/* - main diff --git a/eng/pipelines/sqlclient-pr-project-ref-pipeline.yml b/eng/pipelines/sqlclient-pr-project-ref-pipeline.yml index 2f4e8a2ee5..bfd19af682 100644 --- a/eng/pipelines/sqlclient-pr-project-ref-pipeline.yml +++ b/eng/pipelines/sqlclient-pr-project-ref-pipeline.yml @@ -30,7 +30,7 @@ pr: branches: include: # GitHub repo branch targets that will trigger PR validation builds. - - dev/**/* + - dev/* - feat/* - main diff --git a/eng/pipelines/steps/compound-build-akv-step.yml b/eng/pipelines/steps/compound-build-akv-step.yml index 53b3d32371..9cb977ca34 100644 --- a/eng/pipelines/steps/compound-build-akv-step.yml +++ b/eng/pipelines/steps/compound-build-akv-step.yml @@ -31,9 +31,6 @@ steps: retryCount: 5 secureFile: 'netfxKeypair.snk' - # Install the .NET SDK. - - template: /eng/pipelines/steps/install-dotnet.yml@self - - task: MSBuild@1 displayName: 'Build.proj - BuildAkv' inputs: @@ -46,6 +43,6 @@ steps: -p:MdsPackageVersion=${{ parameters.mdsPackageVersion }} -p:ReferenceType=Package -p:SigningKeyPath=$(Agent.TempDirectory)/netfxKeypair.snk - + - script: tree /a /f $(BUILD_OUTPUT) displayName: Output Build Output Tree diff --git a/src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderException.cs b/src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderException.cs index ebcd89fa0c..30209f70ce 100644 --- a/src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderException.cs +++ b/src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderException.cs @@ -11,7 +11,7 @@ public abstract class SqlAuthenticationProviderException : Exception /// The string value used when the failure code is not known. /// private const string Unknown = "Unknown"; - + /// protected SqlAuthenticationProviderException( string message, @@ -37,7 +37,7 @@ protected SqlAuthenticationProviderException( Method = method; FailureCode = failureCode; ShouldRetry = shouldRetry; - RetryPeriod = shouldRetry && retryPeriod > 0? retryPeriod : 0; + RetryPeriod = (shouldRetry && retryPeriod > 0) ? retryPeriod : 0; } /// diff --git a/src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs b/src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs index f383ca600f..03222adf61 100644 --- a/src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs +++ b/src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs @@ -358,20 +358,36 @@ previousPw is byte[] previousPwBytes && if (ex is MsalServiceException svcEx && svcEx.StatusCode == MsalRetryStatusCode) { - int retryPeriod = 0; - var retryAfter = svcEx.Headers.RetryAfter; if (retryAfter is not null) { + // Prefer the Delta value over Date. + double totalMilliseconds = 0; + if (retryAfter.Delta.HasValue) { - retryPeriod = retryAfter.Delta.Value.Milliseconds; + totalMilliseconds = retryAfter.Delta.Value.TotalMilliseconds; } else if (retryAfter.Date.HasValue) { - retryPeriod = Convert.ToInt32(retryAfter.Date.Value.Offset.TotalMilliseconds); + var now = DateTimeOffset.UtcNow; + if (retryAfter.Date.Value > now) + { + totalMilliseconds = (retryAfter.Date.Value - now).TotalMilliseconds; + } } + int retryPeriod = + // Ignore nonsensical values. + totalMilliseconds <= 0 + ? 0 + // Avoid overflow when converting to an int. + : totalMilliseconds > int.MaxValue + ? int.MaxValue + // Convert from double to int safely. + : Convert.ToInt32(totalMilliseconds); + + // Report the retryable error. throw new Extensions.Azure.AuthenticationException( parameters.AuthenticationMethod, ex.ErrorCode, diff --git a/src/Microsoft.Data.SqlClient.Extensions/Azure/src/Azure.csproj b/src/Microsoft.Data.SqlClient.Extensions/Azure/src/Azure.csproj index 4fd4d4860e..c22bfdce3c 100644 --- a/src/Microsoft.Data.SqlClient.Extensions/Azure/src/Azure.csproj +++ b/src/Microsoft.Data.SqlClient.Extensions/Azure/src/Azure.csproj @@ -95,6 +95,7 @@ + diff --git a/src/Microsoft.Data.SqlClient.Extensions/Azure/test/StringExtensions.cs b/src/Microsoft.Data.SqlClient.Extensions/Azure/test/StringExtensions.cs index fc7801ee78..a3045d579e 100644 --- a/src/Microsoft.Data.SqlClient.Extensions/Azure/test/StringExtensions.cs +++ b/src/Microsoft.Data.SqlClient.Extensions/Azure/test/StringExtensions.cs @@ -3,11 +3,16 @@ // See the LICENSE file in the project root for more information. /// -/// Adds the missing IsEmpty() method to string that doesn't waste time on null checks like -/// String.IsNullOrEmpty() does, and has a nice shorter name. +/// String extensions used by our tests. /// internal static class StringExtensions { + /// + /// Adds the IsEmpty() method to string that doesn't waste time on null checks + /// like String.IsNullOrEmpty() does, and has a nice short name. + /// + /// The string to check; must not be null. + /// True if the string is empty, false otherwise. internal static bool IsEmpty(this string str) { return str.Length == 0; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index ffc411b8c0..35367ed82b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -2845,7 +2845,10 @@ private SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) if (_timeout.IsExpired || _timeout.MillisecondsRemaining <= 0) { // No, so we throw. - SqlClientEventSource.Log.TryTraceEvent(" Attempt: {0}, Timeout: {1}", attempt, ex.FailureCode); + SqlClientEventSource.Log.TryTraceEvent( + " Attempt: {0}, FailureCode: {1}", + attempt, + ex.FailureCode); throw SQL.ActiveDirectoryTokenRetrievingTimeout(Enum.GetName(typeof(SqlAuthenticationMethod), ConnectionOptions.Authentication), ex.FailureCode, ex); } @@ -2864,6 +2867,28 @@ private SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) // Fall through to retry... } + // If the provider throws anything else, it's an API violation, which we must + // consume to avoid breaking our API promise. + catch (Exception ex) + { + // Some exceptions should escape as-is. + if (! ADP.IsCatchableExceptionType(ex)) + { + throw; + } + + // Wrap the exception in a SqlAuthenticationProviderException to maintain our + // API promise. + throw ADP.CreateSqlException( + new ProviderApiViolationException( + message: + "API violation; provider threw unexpected exception " + + ex.GetType().FullName + ": " + ex.Message, + causedBy: ex), + ConnectionOptions, + this, + username); + } } // Nullable context has exposed that _fedAuthToken may be null here, @@ -2891,6 +2916,15 @@ private SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) return _fedAuthToken; } + // Thrown when an authentication provider violates the expected API contract. + private class ProviderApiViolationException : SqlAuthenticationProviderException + { + public ProviderApiViolationException(string message, Exception causedBy) + : base(message, causedBy) + { + } + } + #nullable disable private void Login( diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlAuthenticationProviderManagerTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlAuthenticationProviderManagerTests.cs index 5409b0f7e4..778d69991b 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlAuthenticationProviderManagerTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlAuthenticationProviderManagerTests.cs @@ -6,7 +6,7 @@ using System.Threading.Tasks; using Xunit; -namespace Microsoft.Data.SqlClient.Test.UnitTests; +namespace Microsoft.Data.SqlClient.UnitTests; public class SqlAuthenticationProviderManagerTests { @@ -67,7 +67,7 @@ public void Abstractions_And_Manager_GetSetProvider_Equivalent() provider2, SqlAuthenticationProviderManager.GetProvider( SqlAuthenticationMethod.ActiveDirectoryDeviceCodeFlow)); - + Assert.Same( provider2, SqlAuthenticationProvider.GetProvider( diff --git a/tools/specs/Microsoft.Data.SqlClient.nuspec b/tools/specs/Microsoft.Data.SqlClient.nuspec index 079e16d41c..01bcaa1aa2 100644 --- a/tools/specs/Microsoft.Data.SqlClient.nuspec +++ b/tools/specs/Microsoft.Data.SqlClient.nuspec @@ -36,8 +36,6 @@ - - @@ -54,8 +52,6 @@ - - @@ -67,8 +63,6 @@ - - @@ -80,8 +74,6 @@ - -