Dedupe concurrent Key Vault secret requests when multiple references resolve to the same secret

[jimmythomson]

Dedupe concurrent Key Vault secret requests when multiple references resolve to the same secret

Summary

When an App Configuration store contains multiple Key Vault references that point to the same secret (same sourceId), the provider can issue redundant getSecret requests to Azure Key Vault. The in-memory cache in AzureKeyVaultSecretProvider only stores resolved values, not in-flight requests, so concurrent resolutions of the same secret are not deduplicated.

Current behavior

The cache lives in src/keyvault/keyVaultSecretProvider.ts:

async getSecretValue(secretIdentifier) {
    const identifierKey = secretIdentifier.sourceId;
    if (this.#cachedSecretValues.has(identifierKey) &&
        (!this.#secretRefreshTimer || !this.#secretRefreshTimer.canRefresh())) {
            return this.#cachedSecretValues.get(identifierKey);
    }
    const secretValue = await this.#getSecretValueFromKeyVault(secretIdentifier);
    this.#cachedSecretValues.set(identifierKey, secretValue);
    return secretValue;
}

Secret references are dispatched from AppConfigurationImpl.#resolveSecretReferences:

• Sequential mode (resolveSecretsInParallel = false, default): references are awaited one at a time, so the first call populates the cache and subsequent duplicates are served from it. ✅ One Key Vault request.
• Parallel mode (resolveSecretsInParallel = true): all processKeyValue promises are started before any resolve, then awaited with Promise.all. Because the cache is only written after await #getSecretValueFromKeyVault(...) completes, every duplicate reference that starts before the first one returns makes its own network call. ❌ N concurrent Key Vault requests for N duplicates.

The same multiplication occurs on refresh ticks: when secretRefreshIntervalInMs is set and #secretRefreshTimer.canRefresh() is true, the cache is bypassed for every reference in that round, so duplicates fan out again.

Impact

• Unnecessary load on Azure Key Vault (latency, throttling risk against per-vault TPS limits).
• Higher cost / quota consumption for customers with many references to the same secret.
• Slower load() / refresh() when the duplicates trigger throttling or retries.
• Inconsistent behavior between sequential and parallel modes for what should be a transparent performance toggle.

Expected behavior

For a given secretIdentifier.sourceId, the provider should issue at most one outstanding Key Vault request at a time, regardless of:

• how many configuration settings reference that secret,
• whether resolveSecretsInParallel is enabled,
• whether the call is part of an initial load() or a refresh cycle.

Additional resolutions for the same sourceId while a request is in flight should await the existing promise and receive the same result (or the same error).

Proposed fix

Track in-flight requests in addition to resolved values in AzureKeyVaultSecretProvider:

#inflightRequests: Map<string, Promise<unknown>> = new Map();

async getSecretValue(secretIdentifier) {
    const key = secretIdentifier.sourceId;

    if (this.#cachedSecretValues.has(key) &&
        (!this.#secretRefreshTimer || !this.#secretRefreshTimer.canRefresh())) {
        return this.#cachedSecretValues.get(key);
    }

    let pending = this.#inflightRequests.get(key);
    if (!pending) {
        pending = this.#getSecretValueFromKeyVault(secretIdentifier)
            .then(value => {
                this.#cachedSecretValues.set(key, value);
                return value;
            })
            .finally(() => {
                this.#inflightRequests.delete(key);
            });
        this.#inflightRequests.set(key, pending);
    }
    return pending;
}

Notes:

• Failures are not cached — the finally removes the in-flight entry so the next call can retry.
• clearCache() should continue to clear #cachedSecretValues; in-flight entries naturally drain via finally.
• Behavior in sequential mode is unchanged.

Acceptance criteria

• Multiple references to the same sourceId (same vault, name, and version) resolved concurrently result in exactly one call to SecretClient.getSecret / secretResolver.
• Different versions of the same secret are still fetched independently (cache/in-flight keyed on full sourceId).
• If the underlying Key Vault call rejects, all awaiting references receive the same error and a subsequent call retries (no negative caching).
• Sequential mode behavior is unchanged.
• Refresh cycles (secretRefreshIntervalInMs elapsed) still re-fetch, but only once per unique sourceId per round.
• clearCache() / onChangeDetected() continues to work as today.

Tests to add (in test/keyvault.test.ts)

• Load a configuration with N settings all referencing the same secret URI:
  • With resolveSecretsInParallel: true, assert the mocked SecretClient.getSecret is called exactly once and all settings receive the resolved value.
  • With resolveSecretsInParallel: false, assert the same (regression guard).
• Same scenario using secretResolver instead of a SecretClient, asserting the resolver is invoked once.
• Two references with different versions of the same secret → two calls, one per version.
• Failure case: the underlying call rejects → all concurrent awaiters reject with the same error; a follow-up load() retries (no cached failure).
• Refresh case: after secretRefreshIntervalInMs elapses, duplicates trigger one fetch per unique sourceId, not one per reference.

References

• src/keyvault/keyVaultSecretProvider.ts — AzureKeyVaultSecretProvider.getSecretValue
• src/appConfigurationImpl.ts — #resolveSecretReferences (parallel vs. sequential dispatch)
• src/keyvault/keyVaultOptions.ts — secretRefreshIntervalInMs, secretResolver, secretClients

[jimmythomson]

Potential fix PR here: #326

[linglingye001]

Hi @jimmythomson, thanks for reaching out and detailed analysis. Appreciate the linked PR, we'll follow up.

[jimmythomson]

Hi @jimmythomson, thanks for reaching out and detailed analysis. Appreciate the linked PR, we'll follow up.

No problem. That PR was a quick Copilot generated one, so take it as a reference rather than finalised code.