Azure / Azure/azure-functions-host
GetFunctionSecretsAsync(merged: true) can throw a duplicate-key ArgumentException
- Dominant language
- C#
- Stars
- 2k
- Forks
- 482
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 36
Description
### Description
`SecretManager.GetFunctionSecretsAsync` has an optional `merged` parameter. When `merged: true`, it combines the function-specific keys with the host-level function keys:
https://github.com/Azure/azure-functions-host/blob/03797f60f190b80cdf28b30efd1b75efac67c9fb/src/WebJobs.Script.WebHost/Security/KeyManagement/SecretManager.cs#L318-L324
```csharp
if (merged)
{
// If merged is true, we combine function specific keys with host level function keys,
// prioritizing function specific keys
var hostSecrets = await GetHostSecretsAsync();
functionSecrets = functionSecrets.Union(hostSecrets.FunctionKeys.Where(s => !functionSecrets.ContainsKey(s.Key)))
.ToDictionary(kv => kv.Key, kv => kv.Value, StringComparer.OrdinalIgnoreCase);
}
```
The final `ToDictionary(...)` uses `StringComparer.OrdinalIgnoreCase`, but the `!functionSecrets.ContainsKey(s.Key)` filter uses the comparer of the `functionSecrets` instance. When `functionSecrets` was populated from the startup context cache, it is a plain (case-sensitive) `Dictionary`, because `GetFunctionSecretsOrNull` passes the deserialized dictionary through without normalizing its comparer:
https://github.com/Azure/azure-functions-host/blob/03797f60f190b80cdf28b30efd1b75efac67c9fb/src/WebJobs.Script.WebHost/StartupContextProvider.cs#L82-L94
(Note the host-secrets path a few lines above *does* normalize to `OrdinalIgnoreCase`, but the function-secrets path does not.)
### Result
If a function-scoped key and a host-scoped function key have names that differ only by case (e.g. function key `foo` and host function key `FOO`):
1. `functionSecrets.ContainsKey("FOO")` returns `false` (case-sensitive lookup), so the host key survives the filter.
2. The final `.ToDictionary(..., OrdinalIgnoreCase)` then receives both `foo` and `FOO`, which collide under `OrdinalIgnoreCase` → `ArgumentException: An item with the same key has already been added.`
The two key sets live in separate scopes and are deduplicated independently, so nothing prevents this cross-scope name overlap.
### Scope / impact
This only affects the `merged: true` code path. As far as I can tell, **no production/API code path calls `GetFunctionSecretsAsync` with `merged: true`** — all production callers (`KeysController`, `FunctionsSyncManager`, the internal authorization-level helper) use the default `merged: false`. The only callers passing `merged: true` are unit tests (`SecretManagerTests`).
### Proposed fix
Since the `merged` behavior isn't used by any production path, the simplest option is to **remove the `merged` parameter and the merge branch entirely** (and the tests that exercise it). Alternatively, if the behavior should be retained, make the filter and the final projection use a consistent `OrdinalIgnoreCase` comparer (e.g. normalize `functionSecrets` to `OrdinalIgnoreCase`, mirroring the host-secrets path in `GetFunctionSecretsOrNull`).
Contributor guide
Assessment
This issue has not been assessed yet.