Skip to content

Commit 55cc333

Browse files
authored
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.
1 parent 90d1437 commit 55cc333

File tree

2 files changed

+38
-8
lines changed

2 files changed

+38
-8
lines changed

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -587,14 +587,23 @@ private static TokenCredentialData CreateTokenCredentialInstance(TokenCredential
587587
// specify 'Authentication = Active Directory Default' in
588588
// connection string.
589589
//
590-
// CodeQL Suppression - do not modify this comment:
591-
//
592-
// CodeQL [SM05137] Default Azure Credential is instantiated by
593-
// the calling application when using "Active Directory Default"
590+
// Default Azure Credential is instantiated by the calling
591+
// application when using "Active Directory Default"
594592
// authentication code to connect to Azure SQL instance.
595593
// SqlClient is a library, doesn't instantiate the credential
596594
// without running application instructions.
597-
return new TokenCredentialData(new DefaultAzureCredential(defaultAzureCredentialOptions), GetHash(secret));
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));
598607
}
599608

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

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -953,9 +953,30 @@ public override uint Receive(out SniPacket packet, int timeoutInMilliseconds)
953953
}
954954
finally
955955
{
956-
// Reset the socket timeout to Timeout.Infinite after the receive operation is done
957-
// to avoid blocking the thread in case of a timeout error.
958-
_socket.ReceiveTimeout = Timeout.Infinite;
956+
const int resetTimeout = Timeout.Infinite;
957+
958+
try
959+
{
960+
// Reset the socket timeout to Timeout.Infinite after
961+
// the receive operation is done to avoid blocking the
962+
// thread in case of a timeout error.
963+
_socket.ReceiveTimeout = resetTimeout;
964+
965+
}
966+
catch (SocketException ex)
967+
{
968+
// We sometimes see setting the ReceiveTimeout fail
969+
// on macOS. There's isn't much we can do about it
970+
// though, so just log and move on.
971+
SqlClientEventSource.Log.TrySNITraceEvent(
972+
nameof(SniTcpHandle),
973+
EventType.ERR,
974+
"Connection Id {0}, Failed to reset socket " +
975+
"receive timeout to {1}: {2}",
976+
_connectionId,
977+
resetTimeout,
978+
ex.Message);
979+
}
959980
}
960981
}
961982
}

0 commit comments

Comments
 (0)