Skip to content

Commit 57e7209

Browse files
dylan-mccarthydylan-mccarthy
committed
fix(security): replace ContainsKey+indexer with TryGetValue pattern
Replace inefficient Dictionary access in 16 locations: - LeastLoadedScheduler.cs (4): region, environment, slots, metadata - InvoiceClassifierAgentTests.cs (11): model, config, metadata checks - DeploymentEndpointsTests.cs (1): placement region Refs: E7-T1
1 parent a79ed3e commit 57e7209

File tree

3 files changed

+32
-33
lines changed

3 files changed

+32
-33
lines changed

‎src/ControlPlane.Api/Services/LeastLoadedScheduler.cs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,9 @@ private List<Node> FilterNodesByConstraints(
161161
private bool MatchesPlacementConstraints(Node node, Dictionary<string, object> placementConstraints)
162162
{
163163
// Handle region constraint (most common)
164-
if (placementConstraints.ContainsKey("region"))
164+
if (placementConstraints.TryGetValue("region", out var regionValue))
165165
{
166-
var requiredRegions = GetRegionConstraint(placementConstraints["region"]);
166+
var requiredRegions = GetRegionConstraint(regionValue);
167167
var nodeRegion = GetNodeRegion(node);
168168

169169
if (nodeRegion == null || !requiredRegions.Contains(nodeRegion))
@@ -175,9 +175,9 @@ private bool MatchesPlacementConstraints(Node node, Dictionary<string, object> p
175175
}
176176

177177
// Handle environment constraint
178-
if (placementConstraints.ContainsKey("environment"))
178+
if (placementConstraints.TryGetValue("environment", out var envValue))
179179
{
180-
var requiredEnvironment = placementConstraints["environment"]?.ToString();
180+
var requiredEnvironment = envValue?.ToString();
181181
var nodeEnvironment = GetNodeMetadataValue(node, "environment");
182182

183183
if (requiredEnvironment != null && nodeEnvironment != requiredEnvironment)
@@ -217,12 +217,11 @@ private bool MatchesPlacementConstraints(Node node, Dictionary<string, object> p
217217

218218
private int GetSlotsFromCapacity(Dictionary<string, object>? capacity)
219219
{
220-
if (capacity == null || !capacity.ContainsKey("slots"))
220+
if (capacity == null || !capacity.TryGetValue("slots", out var slotsValue))
221221
{
222222
return 0;
223223
}
224224

225-
var slotsValue = capacity["slots"];
226225
if (slotsValue is int intValue)
227226
{
228227
return intValue;
@@ -263,11 +262,11 @@ private List<string> GetRegionConstraint(object regionConstraint)
263262

264263
private string? GetNodeMetadataValue(Node node, string key)
265264
{
266-
if (node.Metadata == null || !node.Metadata.ContainsKey(key))
265+
if (node.Metadata == null || !node.Metadata.TryGetValue(key, out var value))
267266
{
268267
return null;
269268
}
270269

271-
return node.Metadata[key]?.ToString();
270+
return value?.ToString();
272271
}
273272
}

‎tests/ControlPlane.Api.Tests/DeploymentEndpointsTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ public async Task CreateDeployment_ReturnsCreatedDeployment()
124124
Assert.NotNull(deployment.Target);
125125
Assert.Equal(3, deployment.Target.Replicas);
126126
Assert.NotNull(deployment.Target.Placement);
127-
Assert.True(deployment.Target.Placement.ContainsKey("region"));
128-
Assert.Equal("us-east-1", deployment.Target.Placement["region"].ToString());
127+
Assert.True(deployment.Target.Placement.TryGetValue("region", out var regionValue));
128+
Assert.Equal("us-east-1", regionValue.ToString());
129129
Assert.NotNull(deployment.Status);
130130
Assert.Equal("pending", deployment.Status.State);
131131
Assert.Equal(0, deployment.Status.ReadyReplicas);

‎tests/ControlPlane.Api.Tests/InvoiceClassifierAgentTests.cs‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ public async Task InvoiceClassifierDefinition_HasValidModelProfile()
6565
// Assert
6666
Assert.NotNull(agent);
6767
Assert.NotNull(agent.ModelProfile);
68-
Assert.True(agent.ModelProfile.ContainsKey("model"));
69-
Assert.Equal("gpt-4", agent.ModelProfile["model"].ToString());
68+
Assert.True(agent.ModelProfile.TryGetValue("model", out var modelValue));
69+
Assert.Equal("gpt-4", modelValue.ToString());
7070
Assert.True(agent.ModelProfile.ContainsKey("temperature"));
7171
Assert.True(agent.ModelProfile.ContainsKey("maxTokens"));
7272
}
@@ -95,8 +95,8 @@ public async Task InvoiceClassifierDefinition_HasServiceBusInput()
9595
Assert.NotNull(agent.Input);
9696
Assert.Equal("ServiceBus", agent.Input.Type);
9797
Assert.NotNull(agent.Input.Config);
98-
Assert.True(agent.Input.Config.ContainsKey("queueName"));
99-
Assert.Equal("invoices", agent.Input.Config["queueName"].ToString());
98+
Assert.True(agent.Input.Config.TryGetValue("queueName", out var queueNameValue));
99+
Assert.Equal("invoices", queueNameValue.ToString());
100100
}
101101

102102
[Fact]
@@ -112,18 +112,18 @@ public async Task InvoiceClassifierDefinition_ServiceBusInput_HasCorrectConfigur
112112

113113
// Verify Service Bus specific configuration
114114
Assert.True(agent.Input.Config.ContainsKey("connectionString"));
115-
Assert.True(agent.Input.Config.ContainsKey("prefetchCount"));
116-
Assert.True(agent.Input.Config.ContainsKey("maxDeliveryCount"));
117-
Assert.True(agent.Input.Config.ContainsKey("receiveMode"));
115+
Assert.True(agent.Input.Config.TryGetValue("prefetchCount", out var prefetchCountValue));
116+
Assert.True(agent.Input.Config.TryGetValue("maxDeliveryCount", out var maxDeliveryCountValue));
117+
Assert.True(agent.Input.Config.TryGetValue("receiveMode", out var receiveModeValue));
118118

119119
// Verify values
120-
var prefetchCount = ((JsonElement)agent.Input.Config["prefetchCount"]).GetInt32();
120+
var prefetchCount = ((JsonElement)prefetchCountValue).GetInt32();
121121
Assert.Equal(16, prefetchCount);
122122

123-
var maxDeliveryCount = ((JsonElement)agent.Input.Config["maxDeliveryCount"]).GetInt32();
123+
var maxDeliveryCount = ((JsonElement)maxDeliveryCountValue).GetInt32();
124124
Assert.Equal(3, maxDeliveryCount);
125125

126-
var receiveMode = agent.Input.Config["receiveMode"].ToString();
126+
var receiveMode = receiveModeValue.ToString();
127127
Assert.Equal("PeekLock", receiveMode);
128128
}
129129

@@ -138,8 +138,8 @@ public async Task InvoiceClassifierDefinition_HasHttpOutput()
138138
Assert.NotNull(agent.Output);
139139
Assert.Equal("Http", agent.Output.Type);
140140
Assert.NotNull(agent.Output.Config);
141-
Assert.True(agent.Output.Config.ContainsKey("method"));
142-
Assert.Equal("POST", agent.Output.Config["method"].ToString());
141+
Assert.True(agent.Output.Config.TryGetValue("method", out var methodValue));
142+
Assert.Equal("POST", methodValue.ToString());
143143
}
144144

145145
[Fact]
@@ -152,9 +152,9 @@ public async Task InvoiceClassifierDefinition_HttpOutput_HasRetryPolicy()
152152
Assert.NotNull(agent);
153153
Assert.NotNull(agent.Output);
154154
Assert.NotNull(agent.Output.Config);
155-
Assert.True(agent.Output.Config.ContainsKey("retryPolicy"));
155+
Assert.True(agent.Output.Config.TryGetValue("retryPolicy", out var retryPolicyValue));
156156

157-
var retryPolicyJson = (JsonElement)agent.Output.Config["retryPolicy"];
157+
var retryPolicyJson = (JsonElement)retryPolicyValue;
158158
var maxRetries = retryPolicyJson.GetProperty("maxRetries").GetInt32();
159159
Assert.Equal(3, maxRetries);
160160

@@ -172,9 +172,9 @@ public async Task InvoiceClassifierDefinition_HttpOutput_HasIdempotencyKey()
172172
Assert.NotNull(agent);
173173
Assert.NotNull(agent.Output);
174174
Assert.NotNull(agent.Output.Config);
175-
Assert.True(agent.Output.Config.ContainsKey("idempotencyKeyFormat"));
175+
Assert.True(agent.Output.Config.TryGetValue("idempotencyKeyFormat", out var idempotencyKeyValue));
176176

177-
var idempotencyKeyFormat = agent.Output.Config["idempotencyKeyFormat"].ToString();
177+
var idempotencyKeyFormat = idempotencyKeyValue.ToString();
178178
Assert.Contains("RunId", idempotencyKeyFormat);
179179
Assert.Contains("MessageId", idempotencyKeyFormat);
180180
}
@@ -200,11 +200,11 @@ public async Task InvoiceClassifierDefinition_HasMetadata()
200200
// Assert
201201
Assert.NotNull(agent);
202202
Assert.NotNull(agent.Metadata);
203-
Assert.True(agent.Metadata.ContainsKey("owner"));
204-
Assert.True(agent.Metadata.ContainsKey("epic"));
205-
Assert.True(agent.Metadata.ContainsKey("version"));
206-
Assert.Equal("Platform Engineering", agent.Metadata["owner"]);
207-
Assert.Equal("E3-T6", agent.Metadata["epic"]);
203+
Assert.True(agent.Metadata.TryGetValue("owner", out var ownerValue));
204+
Assert.True(agent.Metadata.TryGetValue("epic", out var epicValue));
205+
Assert.True(agent.Metadata.TryGetValue("version", out _));
206+
Assert.Equal("Platform Engineering", ownerValue);
207+
Assert.Equal("E3-T6", epicValue);
208208
}
209209

210210
[Fact]
@@ -288,9 +288,9 @@ public async Task InvoiceClassifierDefinition_ModelProfile_HasLowTemperature()
288288
// Assert
289289
Assert.NotNull(agent);
290290
Assert.NotNull(agent.ModelProfile);
291-
Assert.True(agent.ModelProfile.ContainsKey("temperature"));
291+
Assert.True(agent.ModelProfile.TryGetValue("temperature", out var temperatureValue));
292292

293-
var temperature = ((JsonElement)agent.ModelProfile["temperature"]).GetDouble();
293+
var temperature = ((JsonElement)temperatureValue).GetDouble();
294294
Assert.True(temperature <= 0.5,
295295
$"Temperature should be low (≤0.5) for consistent classification results, but was {temperature}");
296296
}

0 commit comments

Comments
 (0)