Skip to content

fix(calling): serialize concurrent fetchEncryptionKeyUrl calls to prevent duplicate KMS keys and contact groups - #5178

Draft
mkesavan13 wants to merge 1 commit into
nextfrom
jira/CAI-8465
Draft

fix(calling): serialize concurrent fetchEncryptionKeyUrl calls to prevent duplicate KMS keys and contact groups#5178
mkesavan13 wants to merge 1 commit into
nextfrom
jira/CAI-8465

Conversation

@mkesavan13

Copy link
Copy Markdown
Contributor

COMPLETES CAI-8465

This pull request addresses

A TOCTOU (time-of-check/time-of-use) concurrency bug in ContactsClient.fetchEncryptionKeyUrl where concurrent callers racing on an empty group list each create a separate KMS key and a duplicate "Other contacts" default group. This produces duplicate KMS resources and duplicate default contact groups in the contacts service.

Root Cause: fetchEncryptionKeyUrl had no concurrency guard on the KMS-key-creation + default-group-creation path. Multiple concurrent callers (e.g., concurrent createContact or fetchDefaultGroup calls on a fresh client) each checked this.groups as empty, each called createUnboundKeys/createResource, and each called createContactGroup(DEFAULT_GROUP_NAME) independently before any had stored the result.

by making the following changes

  • packages/calling/src/Contacts/ContactsClient.ts — Added a private encryptionKeyUrlPromise: Promise<string> | undefined single-flight guard in fetchEncryptionKeyUrl. The first caller to reach the no-groups branch stores an IIFE promise; concurrent callers await the same promise instead of starting independent KMS/group-creation sequences. On transient failure the guard is cleared so subsequent callers can retry.
  • packages/calling/src/Contacts/ContactsClient.test.ts — Two new unit tests: (1) concurrent calls resolve to the same URL with only one KMS key and one default group created; (2) a failed resolution clears the in-flight promise so the next call retries.
  • packages/calling/src/Contacts/ai-docs/contacts-spec.md — Spec updated to document the encryptionKeyUrlPromise field, single-flight resolution behavior, and retry-after-failure semantics.

Change Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Tooling change
  • Internal code refactor

The following scenarios were tested

  • Gate 1 (compile): PASSED
  • Gate 2 (unit tests): PASSED — 26/26 ContactsClient.test.ts tests pass, including 2 new concurrency tests for AC-2
  • Gate 3: not run
  • Tests added: 2 new unit tests (UT-2)

Testing

  • Tests added: 2
  • Gate 1 verification: PASSED
  • Gate 2 verification: PASSED — 26/26 ContactsClient.test.ts tests pass including 2 new concurrency tests for AC-2
  • Gate 3 verification: not run

Acceptance Criteria

ID Criterion Source JiraToPr status Evidence
AC-2 Concurrent fetchEncryptionKeyUrl calls resolve to a single URL; at most one KMS key and one default contact group created per resolution cycle. CAI-8465 security finding U-09 TOCTOU Unit validated concurrent fetchEncryptionKeyUrl creates one key and one default group passed in Gate 2 (26/26 tests)
AC-1 BroadWorks XSI requests only send the Bearer access token to a URL whose host is on an approved trusted-host allowlist. CAI-8465 security finding U-08 SSRF/token-exfiltration External validation required Not run by JiraToPr — no trusted-host allowlist source exists in packages/calling

External Validation Required

  • AC-1 — BroadWorks XSI requests only send Bearer token to an allowlisted host.
    • Severity: Important
    • File: packages/calling/src/Voicemail/BroadworksBackendConnector.ts:194-220
    • Reason: external-dependency — no trusted-host allowlist source exists in packages/calling; the only BWRKS XSI host is the backend-controlled WDM BW_XSI_URL that is itself the attack vector (Utils.ts:1300-1327).
    • Details: A WxCC/Webex stakeholder must confirm the authoritative trusted-host source, after which host validation before any token-bearing fetch plus a negative test (non-allowlisted host rejected, no Bearer fetch) close U-08. A scheme-only check does not.
    • Source: CAI-8465 security finding U-08 SSRF/token-exfiltration

External validation never uses completion language. Human review performs the external validation. Acceptance criteria and recorded evidence are disclosed for human review; external validation remains explicitly unrun by JiraToPr.

Contract Discovery Warnings

  • Manifest reference discovery capped at 100 strings

The GAI Coding Policy And Copyright Annotation Best Practices

  • GAI was not used (or, no additional notation is required)
  • Code was generated entirely by GAI
  • GAI was used to create a draft that was subsequently customized or modified
  • Coder created a draft manually that was non-substantively modified by GAI (e.g., refactoring was performed by GAI on manually written code)
  • Tool used for AI assistance (GitHub Copilot / Other - specify)
    • Github Copilot
    • Other - Please Specify: Claude Code (Anthropic)
  • This PR is related to
    • Feature
    • Defect fix
    • Tech Debt
    • Automation

I certified that

  • I have read and followed contributing guidelines
  • I discussed changes with code owners prior to submitting this pull request
  • I have not skipped any automated checks
  • All existing and new tests passed
  • I have updated the documentation accordingly

Jira: https://jira-eng-sjc12.cisco.com/jira/browse/CAI-8465

🤖 Generated with Claude Code

@mkesavan13 mkesavan13 added the jira-to-pr Automated PR from JiraToPr workflow label Aug 19, 2026
@mkesavan13 mkesavan13 closed this Aug 19, 2026
@mkesavan13 mkesavan13 reopened this Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira-to-pr Automated PR from JiraToPr workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant