Skip to content

GetFunctionSecretsAsync(merged: true) can throw a duplicate-key ArgumentException #11996

Description

Description

SecretManager.GetFunctionSecretsAsync has an optional merged parameter. When merged: true, it combines the function-specific keys with the host-level function keys:

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);

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<string,string>, because GetFunctionSecretsOrNull passes the deserialized dictionary through without normalizing its comparer:

public virtual IDictionary<string, IDictionary<string, string>> GetFunctionSecretsOrNull()
{
if (Context?.Secrets?.Function != null)
{
var functionKeys = Context.Secrets.Function.ToDictionary(p => p.Name, p => p.Secrets);
_logger.LogDebug($"Loaded keys for {functionKeys.Keys.Count} functions from startup context");
return functionKeys;
}
return null;
}

(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).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions