feat(SITES-44690): add spacecat-shared-ticket-client package with Jira Cloud integration

[prithipalpatwal]

Summary

• Scaffolds the spacecat-shared-ticket-client package implementing BaseTicketClient, JiraCloudClient, OAuthCredentialManager, TicketClientFactory, and CredentialManagerFactory
• JiraCloudClient targets the Jira REST API v3 (ADF description, plain-text-to-ADF conversion with URL link detection, 255-char summary truncation, uploadAttachment with MIME whitelist + filename sanitization)
• RateLimitAwareHttpClient wraps the HTTP client with automatic 429 retry and exponential backoff (up to 4 retries, honours Retry-After header)
• OAuthCredentialManager handles OAuth 2.0 3LO token lifecycle: expiry check, concurrent-safe refresh via SM pre-read, and setRequiresReauth on unrecoverable failure
• createTicket returns ticketId (Jira internal numeric ID) alongside ticketKey, ticketUrl, ticketStatus — required by the API service Ticket entity persistence
• CredentialManagerFactory uses CREDENTIAL_MAP for OCP-compliant provider dispatch
• Code clarity pass: grouped constants (Routing / Validation / Attachment Rules), intent-driven method names (#requireOk, #fetchNewTokens, #buildTicketResult, #validateAttachment, blockToAdfParagraph, buildSecretPath)

Alignment

Implements the v1 scope from mysticat-architecture PR #150:

• All API endpoints, field mappings, error handling, and retry flows covered
• Secret path pattern: /mysticat/{env}/task-management/{orgId}/{connectionId}
• SSRF protection: all calls route through fixed Atlassian gateway; cloudId validated as UUID; siteUrl validated as https://*.atlassian.net (display-only, never used as request target)

Concurrent refresh design

Atlassian uses rotating refresh tokens (single-use). If two concurrent Lambda invocations both see an expired token, both may call Atlassian. The second caller's refresh token is already consumed → Atlassian returns 401 → naively this would mark a healthy connection as requires_reauth.

The original spec described using SM PutSecretValue with ExpectedVersionId as an optimistic lock. AWS SM does not expose ExpectedVersionId on PutSecretValue — ClientRequestToken is an idempotency token (deduplicates SM retries with the same payload), not a conditional lock.

Chosen mitigation: #refreshAndGetHeaders re-reads SM immediately before calling Atlassian. This collapses the race window from the full 60-second expiry buffer down to one SM round-trip (~200ms). If another caller already refreshed between the two reads, the stale-check exits early without calling Atlassian.

Accepted v1 risk: Two callers completing their pre-reads within ~200ms of each other can still both reach Atlassian. This requires two concurrent requests in a ~200ms window near token expiry — low probability in practice. Eliminating it fully requires a distributed lock (e.g. DynamoDB conditional write) not available in SM. Accepted as v1 risk; tracked for v2 if monitoring shows spurious requires_reauth events.

Test plan

• 51 unit tests, 100% statement/branch/function/line coverage across all files
• Constructor validation (cloudId UUID, siteUrl host, missing OAuth env vars)
• createTicket: ADF description, URL-to-link conversion, summary truncation, ticketId extraction, non-2xx error handling
• listProjects: project list mapping, empty-values fallback
• uploadAttachment: all allowed MIME types, oversized/empty content, invalid ticketKey, path traversal filename, Jira error response
• OAuthCredentialManager: valid token, expired token refresh, pre-read early-exit (concurrent winner path), unrecoverable failure, requires-reauth short-circuit, missing env vars
• RateLimitAwareHttpClient: success passthrough, 429 retry, Retry-After header, exponential backoff, max-retry exhaustion
• TicketClientFactory and CredentialManagerFactory: provider dispatch and unsupported-provider errors

Made with Cursor

[github-actions]

This PR will trigger a minor release when merged.

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Request changes - solid scaffolding with a few security and correctness gaps to close before merge.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (23 files).

Must fix before merge

1. [Critical] Secret path injection - buildSecretPath interpolates organizationId/connectionId without validation; a malformed DB record with ../ can traverse the Secrets Manager namespace - src/ticket-client-factory.js:44 (details inline)
2. [Important] Unused production dependencies - @adobe/fetch and @adobe/spacecat-shared-utils are declared in dependencies but never imported anywhere in src/; ships dead weight to consumers - package.json:41 (details inline)
3. [Important] Uncapped Retry-After - #resolveWaitMs trusts the server-sent Retry-After header without an upper bound; a value of 86400 blocks the Lambda for a full day - src/http/rate-limit-aware-http-client.js:62 (details inline)
4. [Important] ClientRequestToken misuse - access_token.slice(0, 32) may collide across refreshes or contain invalid characters for SM's idempotency token; use a proper UUID or deterministic hash instead - src/credentials/oauth-credential-manager.js:90 (details inline)

Non-blocking (5): minor issues and suggestions

• nit: content?.length works for Buffer/Uint8Array but will silently under-count multi-byte strings if the type contract ever loosens - consider Buffer.byteLength or a type guard - src/clients/jira-cloud-client.js:203
• nit: c8, mocha, mocha-multi-reporters are used in scripts/config but not in devDependencies - works via workspace hoisting but explicit is better - package.json:44
• suggestion: add jitter to exponential backoff (waitMs * (0.5 + Math.random() * 0.5)) to avoid thundering herd on shared rate limits - src/http/rate-limit-aware-http-client.js:60
• nit: URL_REGEX greedily captures trailing punctuation (e.g. ) or . at end of sentence) - consider a negative lookbehind or trimming trailing non-URL chars - src/clients/jira-cloud-client.js:32
• nit: sanitizeFilename strips ../ sequences but not standalone .. (no trailing slash) - edge case for directory traversal on Windows-style paths - src/clients/jira-cloud-client.js:124

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 10m 30s | Cost: $8.16 | Commit: 2541c41333939018ad13d14c8cdba405e63ee6a8
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

issue (blocking): buildSecretPath interpolates organizationId and connectionId directly into the Secrets Manager path without any validation.

What: If a compromised or malformed DB record contains path-traversal sequences (../) or unexpected characters, the resulting secret path could escape the intended /mysticat/{env}/task-management/ namespace.

Why it matters: This is the trust boundary between the DB layer and the secrets store. The cloudId already has UUID validation in JiraCloudClient - these parameters need equivalent protection.

Fix: Add a UUID regex check (or at minimum a / and .. rejection) for both parameters before interpolation:

const ID_REGEX = /^[0-9a-f-]{36}$/;
if (!ID_REGEX.test(organizationId) || !ID_REGEX.test(connectionId)) {
  throw new Error('Invalid path segment: organizationId or connectionId failed validation');
}

[MysticatBot]

issue (blocking): @adobe/fetch (4.3.0) and @adobe/spacecat-shared-utils (1.81.1) are listed as production dependencies but are never imported in any source file under src/.

Why it matters: Every consumer of this package will install these unused transitive trees. It also signals to future maintainers that these are load-bearing when they are not.

Fix: Remove both entries from dependencies. If they are planned for future use, add them when the import lands.

[MysticatBot]

issue (blocking): #resolveWaitMs trusts the Retry-After header value without enforcing an upper bound.

What: A misconfigured upstream (or a malicious response in a testing/staging environment) could send Retry-After: 3600 and block the Lambda execution for an hour, consuming compute cost and timing out the caller.

Why it matters: Lambda has a hard timeout (typically 30s-15min depending on config), but waiting 8+ seconds per retry (the current max backoff) is already aggressive. An unbounded server-controlled delay is a denial-of-service vector against your own compute budget.

Fix: Cap the parsed value:

const MAX_WAIT_MS = 30_000; // never wait more than 30s per retry
const seconds = parseInt(retryAfter, 10);
if (Number.isFinite(seconds) && seconds > 0) {
  return Math.min(seconds * 1000, MAX_WAIT_MS);
}

[MysticatBot]

issue (blocking): ClientRequestToken: refreshed.access_token.slice(0, 32) is used as the Secrets Manager idempotency key, but this has two problems:

1. Collision risk: If Atlassian ever rotates tokens with a shared prefix (common with JWT-style tokens that share header bytes), different token values could map to the same ClientRequestToken, causing SM to silently reject a valid update.
2. Format violation: SM requires ClientRequestToken to be 32-64 characters matching [a-zA-Z0-9-]. Access tokens commonly contain characters outside this range (., _, +, /).

Fix: Use a deterministic hash of the full access token, or generate a UUID per refresh attempt:

import { createHash } from 'node:crypto';
const idempotencyToken = createHash('sha256')
  .update(refreshed.access_token)
  .digest('hex')
  .slice(0, 64);

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - all four prior blocking findings are correctly fixed with adequate tests.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (23 files).

Previously flagged, now resolved

• Secret path injection: buildSecretPath now validates both IDs as UUIDs before interpolation
• Unused production dependencies: @adobe/fetch and @adobe/spacecat-shared-utils removed; c8, mocha, mocha-multi-reporters added to devDependencies
• Uncapped Retry-After: MAX_WAIT_MS = 30_000 ceiling applied via Math.min
• ClientRequestToken misuse: SHA-256 hash of full access token (64-char hex) replaces naive .slice(0, 32)

Non-blocking (4): minor issues and suggestions

• nit: sanitizeFilename .replace(/\.\./g, '') strips ALL .. sequences including legitimate ones in filenames like report..final.pdf - stripping path separators (/ and \) instead would be more targeted - src/clients/jira-cloud-client.js:125
• nit: UUID_REGEX comment says "Matches lowercase UUIDs" but the regex uses the /i flag accepting uppercase - align the comment with the code or remove the flag - src/ticket-client-factory.js:34
• suggestion: validate process.env.NODE_ENV against an allowlist or /^[a-z]+$/ pattern in buildSecretPath for defense-in-depth - the other two path segments are UUID-validated but the env segment is not - src/ticket-client-factory.js:56
• nit: URL_REGEX lookbehind will strip trailing ) from legitimate URLs with balanced parens like Wikipedia links https://en.wikipedia.org/wiki/Foo_(bar) - acceptable tradeoff for the Jira description use case but worth a code comment - src/clients/jira-cloud-client.js:30

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 5s | Cost: $6.08 | Commit: 0303efc11adae7029511b667a03e529fcdd7d5dc
If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[MysticatBot]

Mysticat review failed: Claude CLI crashed (exit 1): stderr= stdout={"type":"result","subtype":"error_max_budget_usd","duration_ms":6,"duration_api_ms":1560718,"is_error":true,"num_turns":1,"stop_reason":null,"session_id":"b18831f0-22a7-4862-a39b-cdbc7351939d","total_cost_usd":10.4506784,"usage":{"input_tokens":0,"cache_creation_input_tokens":0,"cache_read_input_tok

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Request changes - prior blocking findings are fixed; a few remaining correctness issues in filename sanitization, environment validation, and token lifecycle handling.
Complexity: HIGH - large diff, new package scaffolding across 24 files.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (24 files).

Must fix before merge

1. [Important] sanitizeFilename strips all .. sequences and allows path separators through - src/clients/jira-cloud-client.js:127 (details inline)
2. [Important] process.env.NODE_ENV interpolated into SM secret path without validation - src/ticket-client-factory.js:56 (details inline)
3. [Important] Token refresh 401 from Atlassian never triggers setRequiresReauth - src/credentials/oauth-credential-manager.js:84 (details inline)
4. [Important] siteUrl validation uses string suffix matching instead of hostname parsing - src/clients/jira-cloud-client.js:97 (details inline)

Non-blocking (5): minor issues and suggestions

• nit: index.test.js does not verify RateLimitAwareHttpClient export despite it being exported from src/index.js - test/index.test.js:22
• nit: CLOUD_ID_REGEX is lowercase-only (no /i) while UUID_REGEX in the factory has /i - inconsistent casing contract - src/clients/jira-cloud-client.js:19 vs src/ticket-client-factory.js:34
• nit: #buildAdfDescription JSDoc says "URLs are converted to clickable link nodes" but implementation emits plain-text nodes only - src/clients/jira-cloud-client.js:282
• suggestion: add ticket-client to the Package Catalog's API clients row in the root CLAUDE.md so the discovery index stays current
• nit: test/setup-env.js copyright header says 2026, rest of files say 2025 - test/setup-env.js:2

Previously flagged, now resolved

• Secret path injection: buildSecretPath now validates both IDs as UUIDs before interpolation
• Unused production dependencies: removed from dependencies; test tooling moved to devDependencies
• Uncapped Retry-After: MAX_WAIT_MS = 30_000 ceiling applied via Math.min
• ClientRequestToken misuse: SHA-256 hash of full access token replaces naive .slice(0, 32)

[MysticatBot]

issue (blocking): sanitizeFilename has two problems: (1) .replace(/\.\./g, "") strips ALL .. sequences, so a legitimate filename like report..v2.pdf becomes reportv2.pdf; (2) path separator characters (/, \) pass through unstripped.

Why it matters: Problem 1 corrupts valid filenames. Problem 2 means a filename like foo/bar.txt is sent to Jira unchanged, which may cause unexpected behavior in downstream consumers that trust Jira-sourced filenames.

Fix: Replace the current strip with targeted traversal removal plus separator stripping:

.replace(/\.\.[/\\]/g, "")  // strip traversal sequences only
.replace(/[/\\]/g, "_")     // replace separators with underscore

[MysticatBot]

issue (blocking): process.env.NODE_ENV is interpolated into the Secrets Manager path without any validation. If NODE_ENV is unset, the path becomes /mysticat/undefined/task-management/... which points at a non-existent secret, producing a confusing runtime error. If it ever contained a slash, it would traverse the SM namespace.

Why it matters: Both UUID path segments are validated, but the environment segment is not - inconsistent trust boundary.

Fix: Validate against the known set:

const ALLOWED_ENVS = new Set(["dev", "stage", "prod", "ephemeral", "test"]);
const env = process.env.NODE_ENV;
if (!ALLOWED_ENVS.has(env)) {
  throw new Error("Invalid NODE_ENV for secret path: " + env);
}

[MysticatBot]

issue (blocking): #refreshAndGetHeaders calls #fetchNewTokens first. If that throws (e.g. Atlassian returns 401 for a revoked refresh token), the error propagates directly to the caller without entering the catch (smErr) block. This means repeated refresh failures from a revoked token never trigger setRequiresReauth(). The connection stays in a "looks valid but every call fails" state indefinitely.

Why it matters: When a Jira admin revokes an OAuth app or a user deauthorizes, the refresh endpoint returns 401 permanently. Without marking requiresReauth, the system retries on every ticket creation attempt and never surfaces the re-authorization prompt to the user.

Fix: Wrap #fetchNewTokens in a try/catch. On 401 specifically, call setRequiresReauth() before re-throwing:

let refreshed;
try {
  refreshed = await this.#fetchNewTokens(staleSecret.refreshToken);
} catch (err) {
  if (err.message?.includes("401")) {
    await this.setRequiresReauth();
    throw new Error("OAuth token refresh failed - connection requires re-authorization");
  }
  throw err;
}

[MysticatBot]

issue (blocking): The siteUrl validation uses string matching (startsWith("https://") + endsWith(".atlassian.net")). A URL like https://evil.com/.atlassian.net satisfies both checks because the path portion ends with .atlassian.net. Since siteUrl is used to build ticketUrl (returned to callers and likely rendered as a link in the UI), a malicious DB record could produce a phishing URL.

Why it matters: While exploitation requires a compromised DB record and siteUrl is never used as a request target, the constructed ticketUrl is surfaced to end users. Proper hostname validation costs one line and closes the gap.

Fix:

const hostname = new URL(config.siteUrl).hostname;
if (!hostname.endsWith(".atlassian.net")) {
  throw new Error("Invalid siteUrl: hostname must be *.atlassian.net, got: " + hostname);
}

[MysticatBot]

Hey @prithipalpatwal,

Verdict: Approve - all four prior blocking findings are correctly fixed with appropriate test coverage.
Complexity: HIGH - large diff; new package scaffolding with auth + secrets management.
Changes: adds the spacecat-shared-ticket-client package with Jira Cloud integration via OAuth 2.0 3LO, including rate-limit-aware HTTP, credential management, ADF formatting, and attachment upload (24 files).

Previously flagged, now resolved

• sanitizeFilename now strips path separators (/, \) instead of all .. sequences - preserves legitimate filenames while neutralizing traversal
• NODE_ENV removed from SM secret path entirely - account boundary provides env isolation, eliminating the unvalidated interpolation
• Token refresh 401 now triggers setRequiresReauth via try/catch on #fetchNewTokens with .status property on the thrown error
• siteUrl validation upgraded to new URL() hostname parsing - blocks path-based and subdomain-based spoofing attempts

Non-blocking (2): minor issues and suggestions

• nit: UUID_REGEX comment says "Matches lowercase UUIDs" but the regex uses the /i flag accepting uppercase - align the comment or remove the flag for consistency with CLOUD_ID_REGEX - src/ticket-client-factory.js:34
• nit: sanitizeFilename comment "Strip path separators" is on the line above the eslint-disable, making it visually ambiguous which .replace it describes - consider moving it directly above the regex - src/clients/jira-cloud-client.js:120

---

_{Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 1m 1s | Cost: $3.07 | Commit: 15094660ec79f5d55cbd5652798e3ee31763514f
If this code review was useful, please react with 👍. Otherwise, react with 👎.}