Skip to content

Commit ea12280

Browse files
authored
[6.0] Add CodeQL suppression for DefaultAzureCredential and fix macOS CI failures (#3551)
* Add CodeQL suppression for DefaultAzureCredential use in Production (#3542) * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adjusted CodeQL suppression to meet the strict requirements of where it may appear relative to the flagged code. * Task 37261: [S360] [SM05137] DefaultAzureCredential use in Production - Adding catch for macOS socket error to log and ignore. * Fixed SniTcpHandle -> SNITCPHandle class case difference.
1 parent 392002e commit ea12280

File tree

2 files changed

+46
-4
lines changed

2 files changed

+46
-4
lines changed

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -895,9 +895,30 @@ public override uint Receive(out SNIPacket packet, int timeoutInMilliseconds)
895895
}
896896
finally
897897
{
898-
// Reset the socket timeout to Timeout.Infinite after the receive operation is done
899-
// to avoid blocking the thread in case of a timeout error.
900-
_socket.ReceiveTimeout = Timeout.Infinite;
898+
const int resetTimeout = Timeout.Infinite;
899+
900+
try
901+
{
902+
// Reset the socket timeout to Timeout.Infinite after
903+
// the receive operation is done to avoid blocking the
904+
// thread in case of a timeout error.
905+
_socket.ReceiveTimeout = resetTimeout;
906+
907+
}
908+
catch (SocketException ex)
909+
{
910+
// We sometimes see setting the ReceiveTimeout fail
911+
// on macOS. There's isn't much we can do about it
912+
// though, so just log and move on.
913+
SqlClientEventSource.Log.TrySNITraceEvent(
914+
nameof(SNITCPHandle),
915+
EventType.ERR,
916+
"Connection Id {0}, Failed to reset socket " +
917+
"receive timeout to {1}: {2}",
918+
_connectionId,
919+
resetTimeout,
920+
ex.Message);
921+
}
901922
}
902923
}
903924
}

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ActiveDirectoryAuthenticationProvider.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,28 @@ private static TokenCredentialData CreateTokenCredentialInstance(TokenCredential
582582
defaultAzureCredentialOptions.WorkloadIdentityClientId = tokenCredentialKey._clientId;
583583
}
584584

585-
return new TokenCredentialData(new DefaultAzureCredential(defaultAzureCredentialOptions), GetHash(secret));
585+
// SqlClient is a library and provides support to acquire access
586+
// token using 'DefaultAzureCredential' on user demand when they
587+
// specify 'Authentication = Active Directory Default' in
588+
// connection string.
589+
//
590+
// Default Azure Credential is instantiated by the calling
591+
// application when using "Active Directory Default"
592+
// authentication code to connect to Azure SQL instance.
593+
// SqlClient is a library, doesn't instantiate the credential
594+
// without running application instructions.
595+
//
596+
// Note that CodeQL suppression support can only detect
597+
// suppression comments that appear immediately above the
598+
// flagged statement, or appended to the end of the statement.
599+
// Multi-line justifications are not supported.
600+
//
601+
// https://eng.ms/docs/cloud-ai-platform/devdiv/one-engineering-system-1es/1es-docs/codeql/codeql-semmle#guidance-on-suppressions
602+
//
603+
// CodeQL [SM05137] See above for justification.
604+
DefaultAzureCredential cred = new(defaultAzureCredentialOptions);
605+
606+
return new TokenCredentialData(cred, GetHash(secret));
586607
}
587608

588609
TokenCredentialOptions tokenCredentialOptions = new() { AuthorityHost = new Uri(tokenCredentialKey._authority) };

0 commit comments

Comments
 (0)