diff --git a/cdk/src/constructs/jira-integration.ts b/cdk/src/constructs/jira-integration.ts index b39560d2..528c0b8a 100644 --- a/cdk/src/constructs/jira-integration.ts +++ b/cdk/src/constructs/jira-integration.ts @@ -25,6 +25,7 @@ import * as dynamodb from 'aws-cdk-lib/aws-dynamodb'; import * as iam from 'aws-cdk-lib/aws-iam'; import { Runtime, Architecture } from 'aws-cdk-lib/aws-lambda'; import * as lambda from 'aws-cdk-lib/aws-lambda-nodejs'; +import * as s3 from 'aws-cdk-lib/aws-s3'; import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager'; import { NagSuppressions } from 'cdk-nag'; import { Construct } from 'constructs'; @@ -35,8 +36,14 @@ import { JiraWorkspaceRegistryTable } from './jira-workspace-registry-table'; /** Default task-record retention used for TTL computation (days). */ const DEFAULT_TASK_RETENTION_DAYS = 90; -/** Webhook-processor Lambda timeout (seconds). */ -const WEBHOOK_PROCESSOR_TIMEOUT_SECONDS = 30; +/** + * Webhook-processor Lambda timeout (seconds). The processor is invoked + * asynchronously (not behind the API Gateway 30s deadline), so it can run + * longer than the receiver. #577 added serial authenticated download + + * Bedrock screening of up to 10 Jira attachments, so 60s leaves headroom over + * the per-attachment fetch (10s) + screening-retry budget. + */ +const WEBHOOK_PROCESSOR_TIMEOUT_SECONDS = 60; /** * Marker key embedded in the auto-generated stack-wide webhook-secret @@ -87,6 +94,14 @@ export interface JiraIntegrationProps { /** Bedrock Guardrail version for input screening. */ readonly guardrailVersion?: string; + /** + * S3 bucket for task attachment storage. Required for the webhook processor + * to fetch, screen, and store Jira `media` file attachments at task-admission + * time (issue #577). When omitted, issues carrying supported file attachments + * are rejected with a Jira comment rather than silently dropping them. + */ + readonly attachmentsBucket?: s3.IBucket; + /** Task retention in days for TTL computation. */ readonly taskRetentionDays?: number; @@ -206,6 +221,9 @@ export class JiraIntegration extends Construct { createTaskEnv.GUARDRAIL_ID = props.guardrailId; createTaskEnv.GUARDRAIL_VERSION = props.guardrailVersion; } + if (props.attachmentsBucket) { + createTaskEnv.ATTACHMENTS_BUCKET_NAME = props.attachmentsBucket.bucketName; + } // --- Cognito Authorizer (for /jira/link) --- const cognitoAuthorizer = new apigw.CognitoUserPoolsAuthorizer(this, 'JiraCognitoAuthorizer', { @@ -280,6 +298,13 @@ export class JiraIntegration extends Construct { ], })); } + // The processor downloads Jira `media` attachments, screens them, and + // writes the cleaned bytes to the attachments bucket before creating the + // task (#577). ReadWrite mirrors the confirm-uploads path (Put + Get for + // multipart/versioned writes). + if (props.attachmentsBucket) { + props.attachmentsBucket.grantReadWrite(webhookProcessorFn); + } // --- Webhook receiver (verifies HMAC, dedups, invokes processor) --- const webhookFn = new lambda.NodejsFunction(this, 'WebhookFn', { diff --git a/cdk/src/constructs/task-api.ts b/cdk/src/constructs/task-api.ts index d2f67993..1c5ad7f4 100644 --- a/cdk/src/constructs/task-api.ts +++ b/cdk/src/constructs/task-api.ts @@ -326,6 +326,18 @@ export class TaskApi extends Construct { textTransformations: [{ priority: 0, type: 'NONE' }], }, }, + { + // Jira issue payloads include the full issue field set. + // Attachment metadata can push them over 8 KB. The + // receiver HMAC-verifies the raw body and the priority-4 + // rule below still rate-limits this route. + byteMatchStatement: { + fieldToMatch: { uriPath: {} }, + positionalConstraint: 'EXACTLY', + searchString: '/v1/jira/webhook', + textTransformations: [{ priority: 0, type: 'NONE' }], + }, + }, { // GitHub deployment_status webhook (preview-deploy // screenshot pipeline). The full payload (workflow run @@ -376,6 +388,18 @@ export class TaskApi extends Construct { }, }, }, + { + notStatement: { + statement: { + byteMatchStatement: { + fieldToMatch: { uriPath: {} }, + positionalConstraint: 'EXACTLY', + searchString: '/v1/jira/webhook', + textTransformations: [{ priority: 0, type: 'NONE' }], + }, + }, + }, + }, { notStatement: { statement: { diff --git a/cdk/src/handlers/jira-webhook-processor.ts b/cdk/src/handlers/jira-webhook-processor.ts index 3f32c631..d7adb966 100644 --- a/cdk/src/handlers/jira-webhook-processor.ts +++ b/cdk/src/handlers/jira-webhook-processor.ts @@ -18,13 +18,25 @@ */ import * as crypto from 'crypto'; +import { BedrockRuntimeClient } from '@aws-sdk/client-bedrock-runtime'; import { DynamoDBClient } from '@aws-sdk/client-dynamodb'; +import { S3Client } from '@aws-sdk/client-s3'; import { DynamoDBDocumentClient, GetCommand, ScanCommand } from '@aws-sdk/lib-dynamodb'; +import { ulid } from 'ulid'; +import type { ScreeningConfig } from './shared/attachment-screening'; import { createTaskCore } from './shared/create-task-core'; +import { extractDescriptionMarkdown } from './shared/jira-adf'; +import { + downloadScreenAndStoreJiraAttachments, + fetchRecentHumanComments, + JiraAttachmentError, + type RenderedComment, +} from './shared/jira-attachments'; import { reportIssueFailure } from './shared/jira-feedback'; import { resolveJiraOauthToken } from './shared/jira-oauth-resolver'; import { logger } from './shared/logger'; -import type { Attachment } from './shared/types'; +import type { Attachment, PassedAttachmentRecord } from './shared/types'; +import { MAX_TASK_DESCRIPTION_LENGTH } from './shared/validation'; import { CODING_WORKFLOW_ID } from './shared/workflows'; const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({})); @@ -34,8 +46,20 @@ const USER_MAPPING_TABLE = process.env.JIRA_USER_MAPPING_TABLE_NAME!; const WORKSPACE_REGISTRY_TABLE = process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME; const DEFAULT_LABEL_FILTER = 'bgagent'; -/** Deepest markdown heading level (`######`) ADF heading nodes are clamped to. */ -const MAX_MARKDOWN_HEADING_LEVEL = 6; +// Attachment enrichment (#577). The processor downloads Jira `media` file +// attachments, screens them through the Bedrock Guardrail, and uploads the +// cleaned bytes to S3 before creating the task. All three must be configured; +// when they aren't, an issue carrying supported file attachments is rejected +// (fail-closed) rather than silently dropping them. +const ATTACHMENTS_BUCKET = process.env.ATTACHMENTS_BUCKET_NAME; +const GUARDRAIL_ID = process.env.GUARDRAIL_ID; +const GUARDRAIL_VERSION = process.env.GUARDRAIL_VERSION; +const s3Client = ATTACHMENTS_BUCKET ? new S3Client({}) : undefined; +const bedrockClient = GUARDRAIL_ID && GUARDRAIL_VERSION ? new BedrockRuntimeClient({}) : undefined; +const screeningConfig: ScreeningConfig | undefined = + bedrockClient && GUARDRAIL_ID && GUARDRAIL_VERSION + ? { bedrockClient, guardrailId: GUARDRAIL_ID, guardrailVersion: GUARDRAIL_VERSION } + : undefined; /** * Post a Jira comment without ever propagating an error. Mirrors the @@ -142,6 +166,8 @@ interface JiraIssueEvent { readonly summary?: string; readonly description?: unknown; // ADF document readonly labels?: string[]; + /** Jira `media` file attachments. Shape validated in jira-attachments.ts. */ + readonly attachment?: unknown[]; readonly creator?: { readonly accountId?: string }; readonly reporter?: { readonly accountId?: string }; readonly project?: { @@ -334,7 +360,6 @@ export async function handler(event: ProcessorEvent): Promise { // Convert the ADF description to markdown once and reuse it for both the // task body and image-attachment extraction. const descriptionMarkdown = extractDescriptionMarkdown(issue.fields?.description); - const taskDescription = buildTaskDescription(issue, descriptionMarkdown); const channelMetadata: Record = { jira_cloud_id: cloudId, @@ -374,7 +399,71 @@ export async function handler(event: ProcessorEvent): Promise { channelMetadata.jira_site_url = resolved.siteUrl; } - const attachments = extractImageUrlAttachments(descriptionMarkdown); + // Embedded HTTPS image URLs from the description (unchanged, #577 preserves). + const urlAttachments = extractImageUrlAttachments(descriptionMarkdown); + + // Mint the task ID up front so pre-screened attachment S3 keys match the + // eventual task record (createTaskCore honors context.taskId, #577). + const taskId = ulid(); + + // Context enrichment (#577). Both need the workspace registry to resolve an + // OAuth token. Comments are fail-open (advisory); attachments are + // fail-closed (a selected-but-unscreenable attachment rejects the task). + let comments: RenderedComment[] = []; + let preScreenedAttachments: PassedAttachmentRecord[] = []; + if (WORKSPACE_REGISTRY_TABLE) { + const tenantCtx = { cloudId, registryTableName: WORKSPACE_REGISTRY_TABLE }; + + // Recent human comments — advisory context, never gate task creation. + comments = await fetchRecentHumanComments(tenantCtx, issue.key); + + const rawAttachments = issue.fields?.attachment; + if (Array.isArray(rawAttachments) && rawAttachments.length > 0) { + if (!s3Client || !ATTACHMENTS_BUCKET || !screeningConfig) { + // Fail-closed: the issue has attachments but the processor can't + // screen/store them. Don't silently drop selected context — reject + // with a clear comment so the operator can fix configuration. + logger.error('Jira issue has attachments but screening/storage is not configured (fail-closed)', { + issue_key: issue.key, + jira_cloud_id: cloudId, + has_bucket: Boolean(ATTACHMENTS_BUCKET), + has_guardrail: Boolean(screeningConfig), + }); + await safeReportIssueFailure( + issue.key, + cloudId, + '❌ This Jira issue has file attachments, but ABCA attachment screening is not configured. Contact your ABCA admin.', + ); + return; + } + // Combined cap: URL image attachments already consume slots. + const remainingSlots = 10 - urlAttachments.length; + try { + preScreenedAttachments = await downloadScreenAndStoreJiraAttachments( + rawAttachments, + remainingSlots, + { ...tenantCtx, s3Client, bucketName: ATTACHMENTS_BUCKET, screeningConfig, userId: platformUserId, taskId }, + ); + } catch (err) { + if (err instanceof JiraAttachmentError) { + logger.warn('Rejecting Jira task: attachment could not be safely processed', { + issue_key: issue.key, + jira_cloud_id: cloudId, + error: err.message, + }); + await safeReportIssueFailure( + issue.key, + cloudId, + `❌ ABCA couldn't safely process an attachment on this issue: ${err.message} Remove or fix the attachment and re-apply the trigger label.`, + ); + return; + } + throw err; + } + } + } + + const taskDescription = buildTaskDescription(issue, descriptionMarkdown, comments); const requestId = crypto.randomUUID(); const result = await createTaskCore( @@ -385,12 +474,14 @@ export async function handler(event: ProcessorEvent): Promise { // mapped repo, so it must not fall through the resolution ladder to the // repo-less default/agent-v1 (which never commits or opens a PR). #546 workflow_ref: CODING_WORKFLOW_ID, - ...(attachments.length > 0 && { attachments }), + ...(urlAttachments.length > 0 && { attachments: urlAttachments }), }, { userId: platformUserId, channelSource: 'jira', channelMetadata, + taskId, + ...(preScreenedAttachments.length > 0 && { preScreenedAttachments }), }, requestId, ); @@ -500,6 +591,7 @@ function buildCreateTaskFailureMessage(statusCode: number, rawBody: string): str function buildTaskDescription( issue: NonNullable, descriptionMarkdown: string, + comments: readonly RenderedComment[] = [], ): string { const parts: string[] = []; const summary = issue.fields?.summary?.trim(); @@ -512,133 +604,48 @@ function buildTaskDescription( parts.push(''); parts.push(descriptionMarkdown.trim()); } - return parts.join('\n'); + const core = parts.join('\n'); + + // Fold recent human comments in (oldest-first, already rendered to markdown) + // under a clear heading so the agent can tell them from the description + // (#577). Comments are ADVISORY and must stay fail-open: they must never grow + // the description past MAX_TASK_DESCRIPTION_LENGTH and turn createTaskCore's + // length check into a hard rejection. Only append what fits the remaining + // budget (reserving a small margin), truncating the section if needed. + if (comments.length === 0) return core; + const commentSection = renderCommentSection(comments); + const separator = '\n'; + const budget = MAX_TASK_DESCRIPTION_LENGTH - core.length - separator.length; + if (budget <= 0) return core; // description already fills the budget — drop comments + const fitted = commentSection.length <= budget + ? commentSection + : truncateCommentSection(commentSection, budget); + return fitted ? core + separator + fitted : core; } -/** - * Convert a Jira ADF (Atlassian Document Format) document into best-effort - * markdown. Intentionally minimal — extract paragraphs, headings, and - * list items as plain text. Anything else (panels, tables, embeds) is - * collapsed to its textual content. - * - * The full ADF spec has dozens of node types; rolling a complete converter - * here would dwarf the rest of the integration and add a new dependency - * surface. The agent gets the issue title + a coherent text rendering of - * the description; richer rendering (tables, mentions, attachments) can - * land in a follow-up. - */ -function extractDescriptionMarkdown(description: unknown): string { - if (!description) return ''; - if (typeof description === 'string') return description; - if (typeof description !== 'object') return ''; - - const lines: string[] = []; - walkAdf(description as AdfNode, lines, 0); - return lines.join('\n').replace(/\n{3,}/g, '\n\n').trim(); -} - -interface AdfNode { - readonly type?: string; - readonly text?: string; - readonly attrs?: { - readonly level?: number; - /** `media` node: `"external"` carries a direct `url`; `"file"`/`"link"` - * carry an attachment `id` that needs a Jira API call to resolve. */ - readonly type?: string; - readonly url?: string; - readonly alt?: string; - }; - readonly content?: AdfNode[]; -} +/** Notice appended when the comment section is truncated to fit the budget. */ +const COMMENT_TRUNCATION_NOTICE = '\n\n_(recent comments truncated)_'; -function walkAdf(node: AdfNode | undefined, out: string[], depth: number): void { - if (!node) return; - switch (node.type) { - case 'doc': - (node.content ?? []).forEach((c) => walkAdf(c, out, depth)); - return; - case 'paragraph': { - const text = (node.content ?? []).map(textOf).join(''); - if (text) { - out.push(text); - out.push(''); - } - return; - } - case 'heading': { - const level = node.attrs?.level ?? 1; - const prefix = '#'.repeat(Math.max(1, Math.min(MAX_MARKDOWN_HEADING_LEVEL, level))); - const text = (node.content ?? []).map(textOf).join(''); - if (text) { - out.push(`${prefix} ${text}`); - out.push(''); - } - return; - } - case 'bulletList': - case 'orderedList': { - (node.content ?? []).forEach((item, idx) => { - const itemText = (item.content ?? []) - .flatMap((sub) => collectInlineLines(sub)) - .join(' ') - .trim(); - if (!itemText) return; - const bullet = node.type === 'orderedList' ? `${idx + 1}.` : '-'; - out.push(`${' '.repeat(depth * 2)}${bullet} ${itemText}`); - }); - out.push(''); - return; - } - case 'codeBlock': { - const text = (node.content ?? []).map(textOf).join(''); - out.push('```'); - out.push(text); - out.push('```'); - out.push(''); - return; - } - case 'mediaSingle': - case 'mediaGroup': - // Container nodes — descend to the `media` children below. - (node.content ?? []).forEach((c) => walkAdf(c, out, depth)); - return; - case 'media': { - // Jira embeds images as `media` nodes (not markdown image text). Only - // `external` media carry a directly-usable URL; `file`/`link` media - // reference an attachment `id` that needs a Jira API round-trip to - // resolve — out of scope for this minimal converter, so we skip those. - const url = node.attrs?.url; - if (node.attrs?.type === 'external' && typeof url === 'string' && url.startsWith('https://')) { - const alt = node.attrs?.alt ?? ''; - out.push(`![${alt}](${url})`); - out.push(''); - } - return; - } - case 'text': - if (node.text) out.push(node.text); - return; - default: - // Unknown node — descend into its content if any so embedded text - // (e.g. inside a panel or quote) isn't lost. - (node.content ?? []).forEach((c) => walkAdf(c, out, depth)); +function renderCommentSection(comments: readonly RenderedComment[]): string { + const lines: string[] = ['', '## Recent comments']; + for (const c of comments) { + lines.push(''); + const attribution = c.createdAt ? `**${c.author}** (${c.createdAt}):` : `**${c.author}**:`; + lines.push(attribution); + lines.push(c.markdown); } + return lines.join('\n'); } -function textOf(node: AdfNode): string { - if (node.type === 'text' && node.text) return node.text; - if (node.content) return node.content.map(textOf).join(''); - return ''; -} - -function collectInlineLines(node: AdfNode): string[] { - if (node.type === 'paragraph') { - return [(node.content ?? []).map(textOf).join('')]; - } - if (node.type === 'text' && node.text) { - return [node.text]; - } - return []; +/** + * Trim a rendered comment section to at most `budget` characters, leaving room + * for a truncation notice. Returns '' if even the heading + notice can't fit, + * so the caller cleanly drops the section. + */ +function truncateCommentSection(section: string, budget: number): string { + const room = budget - COMMENT_TRUNCATION_NOTICE.length; + if (room <= 0) return ''; + return section.slice(0, room) + COMMENT_TRUNCATION_NOTICE; } /** diff --git a/cdk/src/handlers/shared/create-task-core.ts b/cdk/src/handlers/shared/create-task-core.ts index a208fcc8..95c725e8 100644 --- a/cdk/src/handlers/shared/create-task-core.ts +++ b/cdk/src/handlers/shared/create-task-core.ts @@ -66,6 +66,28 @@ export interface TaskCreationContext { readonly channelSource: ChannelSource; readonly channelMetadata: Record; readonly idempotencyKey?: string; + /** + * Task ID to use instead of minting one. Only trusted server-side callers + * set this — a webhook processor that downloads + screens + uploads + * attachments to S3 *before* calling createTaskCore needs the task ID up + * front so the S3 object keys match the eventual task record (issue #577). + */ + readonly taskId?: string; + /** + * Attachment records the caller has already downloaded, screened (status + * `passed`), and uploaded to S3 — e.g. Jira `media` file attachments a + * trusted webhook processor fetched authenticated and ran through the same + * Bedrock Guardrail pipeline (issue #577). Merged verbatim into the + * persisted attachments and NOT re-screened here. Every record MUST be a + * passed record with storage fields populated; a non-`passed` record is a + * caller contract violation and fails the request closed. + * + * These bypass the wire `Attachment`/`validateAttachments` path (which caps + * inline at 500 KB and can't authenticate a Jira download), so the caller is + * responsible for enforcing the per-file / total-size / count limits before + * upload. + */ + readonly preScreenedAttachments?: readonly AttachmentRecord[]; } const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({})); @@ -386,8 +408,11 @@ export async function createTaskCore( } } - // Generate task ID early so attachment S3 keys use the correct task ID - const taskId = ulid(); + // Generate task ID early so attachment S3 keys use the correct task ID. + // A trusted server-side caller (e.g. the Jira webhook processor, #577) may + // supply the ID so the S3 keys of attachments it uploaded before this call + // match the eventual task record. + const taskId = context.taskId ?? ulid(); // 2b. Process inline attachments: screen (with retry + EXIF strip), upload to S3, build records. // Presigned attachments are deferred to confirm-uploads; URL attachments are resolved during hydration. @@ -537,6 +562,31 @@ export async function createTaskCore( } } + // Pre-screened attachments: records a trusted server-side caller already + // downloaded, screened (passed), and uploaded to S3 — e.g. Jira `media` + // attachments fetched authenticated by the webhook processor (#577). They + // bypass the wire `Attachment` path, so they are merged verbatim and never + // re-screened. Fail closed on a caller contract violation: a non-`passed` + // record must never reach the agent. + if (context.preScreenedAttachments && context.preScreenedAttachments.length > 0) { + for (const rec of context.preScreenedAttachments) { + if (rec.screening.status !== 'passed') { + logger.error('Pre-screened attachment is not in passed state (fail-closed)', { + attachment_filename: rec.filename, + screening_status: rec.screening.status, + request_id: requestId, + metric_type: 'prescreened_attachment_invalid', + }); + // Roll back any inline uploads this call made (empty on the Jira + // webhook path, which supplies no wire attachments) before failing. + if (s3Client) await cleanupOrphanedAttachments(s3Client, uploadedS3Keys); + return errorResponse(500, ErrorCode.INTERNAL_ERROR, + 'A pre-screened attachment was not in a passed state.', requestId); + } + } + attachmentRecords.push(...context.preScreenedAttachments); + } + // 3. Check idempotency key if (context.idempotencyKey !== undefined && context.idempotencyKey !== null) { if (!isValidIdempotencyKey(context.idempotencyKey)) { diff --git a/cdk/src/handlers/shared/jira-adf.ts b/cdk/src/handlers/shared/jira-adf.ts new file mode 100644 index 00000000..e624f318 --- /dev/null +++ b/cdk/src/handlers/shared/jira-adf.ts @@ -0,0 +1,157 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +/** + * Jira ADF (Atlassian Document Format) → best-effort markdown conversion. + * + * Extracted from the webhook processor so both the processor (issue + * description) and the context-enrichment helper (issue comments, #577) can + * render ADF bodies without a circular import. Intentionally minimal — extract + * paragraphs, headings, and list items as plain text. Anything else (panels, + * tables, embeds) is collapsed to its textual content. + * + * The full ADF spec has dozens of node types; rolling a complete converter + * here would dwarf the rest of the integration and add a new dependency + * surface. The agent gets a coherent text rendering; richer rendering (tables, + * mentions) can land in a follow-up. + * + * Tests: cdk/test/handlers/jira-webhook-processor.test.ts (via the processor) + * and cdk/test/handlers/shared/jira-attachments.test.ts (comment rendering). + */ + +/** Deepest markdown heading level (`######`) ADF heading nodes are clamped to. */ +export const MAX_MARKDOWN_HEADING_LEVEL = 6; + +export interface AdfNode { + readonly type?: string; + readonly text?: string; + readonly attrs?: { + readonly level?: number; + /** `media` node: `"external"` carries a direct `url`; `"file"`/`"link"` + * carry an attachment `id` that needs a Jira API call to resolve. */ + readonly type?: string; + readonly url?: string; + readonly alt?: string; + }; + readonly content?: AdfNode[]; +} + +/** + * Convert a Jira ADF document (or a raw string) into best-effort markdown. + * Non-object, empty, or null inputs return an empty string. + */ +export function extractDescriptionMarkdown(description: unknown): string { + if (!description) return ''; + if (typeof description === 'string') return description; + if (typeof description !== 'object') return ''; + + const lines: string[] = []; + walkAdf(description as AdfNode, lines, 0); + return lines.join('\n').replace(/\n{3,}/g, '\n\n').trim(); +} + +function walkAdf(node: AdfNode | undefined, out: string[], depth: number): void { + if (!node) return; + switch (node.type) { + case 'doc': + (node.content ?? []).forEach((c) => walkAdf(c, out, depth)); + return; + case 'paragraph': { + const text = (node.content ?? []).map(textOf).join(''); + if (text) { + out.push(text); + out.push(''); + } + return; + } + case 'heading': { + const level = node.attrs?.level ?? 1; + const prefix = '#'.repeat(Math.max(1, Math.min(MAX_MARKDOWN_HEADING_LEVEL, level))); + const text = (node.content ?? []).map(textOf).join(''); + if (text) { + out.push(`${prefix} ${text}`); + out.push(''); + } + return; + } + case 'bulletList': + case 'orderedList': { + (node.content ?? []).forEach((item, idx) => { + const itemText = (item.content ?? []) + .flatMap((sub) => collectInlineLines(sub)) + .join(' ') + .trim(); + if (!itemText) return; + const bullet = node.type === 'orderedList' ? `${idx + 1}.` : '-'; + out.push(`${' '.repeat(depth * 2)}${bullet} ${itemText}`); + }); + out.push(''); + return; + } + case 'codeBlock': { + const text = (node.content ?? []).map(textOf).join(''); + out.push('```'); + out.push(text); + out.push('```'); + out.push(''); + return; + } + case 'mediaSingle': + case 'mediaGroup': + // Container nodes — descend to the `media` children below. + (node.content ?? []).forEach((c) => walkAdf(c, out, depth)); + return; + case 'media': { + // Jira embeds images as `media` nodes (not markdown image text). Only + // `external` media carry a directly-usable URL; `file`/`link` media + // reference an attachment `id` resolved separately (#577 fetches those + // authenticated via the Jira REST attachment API), so we skip them here. + const url = node.attrs?.url; + if (node.attrs?.type === 'external' && typeof url === 'string' && url.startsWith('https://')) { + const alt = node.attrs?.alt ?? ''; + out.push(`![${alt}](${url})`); + out.push(''); + } + return; + } + case 'text': + if (node.text) out.push(node.text); + return; + default: + // Unknown node — descend into its content if any so embedded text + // (e.g. inside a panel or quote) isn't lost. + (node.content ?? []).forEach((c) => walkAdf(c, out, depth)); + } +} + +function textOf(node: AdfNode): string { + if (node.type === 'text' && node.text) return node.text; + if (node.content) return node.content.map(textOf).join(''); + return ''; +} + +function collectInlineLines(node: AdfNode): string[] { + if (node.type === 'paragraph') { + return [(node.content ?? []).map(textOf).join('')]; + } + if (node.type === 'text' && node.text) { + return [node.text]; + } + return []; +} diff --git a/cdk/src/handlers/shared/jira-attachments.ts b/cdk/src/handlers/shared/jira-attachments.ts new file mode 100644 index 00000000..9d540aa4 --- /dev/null +++ b/cdk/src/handlers/shared/jira-attachments.ts @@ -0,0 +1,583 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +/** + * Authenticated Jira context enrichment at task-admission time (issue #577). + * + * The webhook processor calls these helpers after Jira OAuth is resolved to + * pull the same practical context a Linear-origin task can use, which the + * headless agent cannot fetch itself (the Atlassian Remote MCP only supports + * interactive OAuth — see the parent tracker #580 and ADR-015): + * + * - `downloadScreenAndStoreJiraAttachments` — resolve Jira `media` file + * attachments through the REST API (NOT the unauthenticated `content` + * URL), run each through the existing Bedrock Guardrail screening + * pipeline, upload the cleaned bytes to S3, and return `passed` + * AttachmentRecords for `createTaskCore` to persist verbatim. Fail-closed: + * a selected attachment that cannot be safely fetched/screened throws + * {@link JiraAttachmentError} so the caller rejects the whole task. + * - `fetchRecentHumanComments` — fetch recent human-authored comments and + * render them to markdown. Fail-open: any failure yields an empty list so + * the task still proceeds. + * + * Both reuse the auth + refresh-and-retry-once pattern established by + * `jira-feedback.ts` and the screen → upload → record pattern established by + * `resolve-url-attachments.ts`. + * + * Tests: cdk/test/handlers/shared/jira-attachments.test.ts + */ + +import { PutObjectCommand, type S3Client } from '@aws-sdk/client-s3'; +import { screenImage, screenTextFile, AttachmentScreeningError, type ScreeningConfig } from './attachment-screening'; +import { estimateImageTokensFromBuffer } from './image-tokens'; +import { extractDescriptionMarkdown } from './jira-adf'; +import { resolveJiraOauthToken } from './jira-oauth-resolver'; +import { logger } from './logger'; +import { createAttachmentRecord, type PassedAttachmentRecord } from './types'; +import { isAllowedMimeType, isValidFilename, validateMagicBytes, MAX_ATTACHMENT_SIZE_BYTES, MAX_TOTAL_ATTACHMENT_SIZE_BYTES, MAX_ATTACHMENTS_PER_TASK } from './validation'; +import { ATTACHMENT_OBJECT_KEY_PREFIX } from '../../constructs/attachments-bucket'; + +/** + * Safe Jira attachment id: digits/letters/`-`/`_` only. Jira attachment ids are + * numeric, but the payload is (in principle) attacker-shaped, and the id + * becomes both an S3 key segment and the agent-side on-disk directory name + * (`attachments_dir / attachment_id`). Reject anything that could traverse. + */ +const SAFE_ATTACHMENT_ID = /^[A-Za-z0-9_-]+$/; + +/** + * Atlassian cross-region REST gateway base. The per-tenant OAuth token is + * minted with `audience=api.atlassian.com`, so it is only valid against this + * gateway host scoped by `{cloudId}` — NOT against the raw `*.atlassian.net` + * site host (which 401s such a token). Matches `jira-feedback.ts` and + * `agent/src/jira_reactions.py`. + */ +const JIRA_API_BASE = 'https://api.atlassian.com/ex/jira'; + +/** Per-request timeout for the binary attachment download. */ +const ATTACHMENT_FETCH_TIMEOUT_MS = 10_000; + +/** Per-request timeout for the (small, JSON) comment fetch. */ +const COMMENT_FETCH_TIMEOUT_MS = 5_000; + +/** Default cap on recent comments folded into the task context. */ +export const DEFAULT_MAX_COMMENTS = 10; + +/** + * Tenant-scoped context for a Jira REST call, resolved once per task by the + * caller and threaded through so the OAuth resolver runs once, not per API + * call. Mirrors {@link import('./jira-feedback').JiraFeedbackContext}. + */ +export interface JiraTenantContext { + /** Atlassian tenant identifier (`cloudId`) — registry key. */ + readonly cloudId: string; + /** Name of JiraWorkspaceRegistryTable, from CDK stack output. */ + readonly registryTableName: string; +} + +/** S3 + screening dependencies for the attachment download path. */ +export interface JiraAttachmentStorage { + readonly s3Client: S3Client; + readonly bucketName: string; + readonly screeningConfig: ScreeningConfig; + /** Platform user the task is attributed to — part of the S3 key. */ + readonly userId: string; + /** Task ID minted by the caller — part of the S3 key. */ + readonly taskId: string; +} + +/** + * Thrown when a Jira attachment that was SELECTED for inclusion cannot be + * safely fetched, validated, or screened. The caller treats this as a + * fail-closed signal: reject the whole task rather than let the agent run + * with missing or unscreened context. (Attachments filtered out *before* + * download — unsupported MIME, oversized — are silently skipped instead, and + * never raise this.) + */ +export class JiraAttachmentError extends Error { + constructor(message: string, options?: { cause?: unknown }) { + super(message, options); + this.name = 'JiraAttachmentError'; + } +} + +/** + * Subset of a Jira `issue.fields.attachment[]` entry we depend on. Extra + * fields are tolerated. + */ +interface RawJiraAttachment { + readonly id?: string | number; + readonly filename?: string; + readonly mimeType?: string; + readonly size?: number; + readonly content?: string; +} + +/** A candidate attachment that passed the pre-download filter. */ +interface SelectedAttachment { + readonly id: string; + /** Sanitized, path-traversal-safe filename (see {@link safeFilename}). */ + readonly filename: string; + readonly mimeType: string; + readonly size: number; + readonly isImage: boolean; +} + +/** File extension to fall back to when a filename can't be made safe. */ +const MIME_FALLBACK_EXTENSION: Record = { + 'image/png': 'png', + 'image/jpeg': 'jpg', + 'text/plain': 'txt', + 'text/csv': 'csv', + 'text/markdown': 'md', + 'application/json': 'json', + 'application/pdf': 'pdf', + 'text/x-log': 'log', +}; + +/** + * Produce a path-traversal-safe filename for a Jira attachment. Jira filenames + * are arbitrary user uploads (spaces, parens, unicode), and the value becomes + * an S3 key segment AND the agent-side on-disk filename, so an unsanitized + * `../../evil` would be an arbitrary-write primitive. Mirror the URL path + * (`validation.filenameFromUrl`): replace unsafe characters, then fall back to + * a generated name keyed on the MIME type if the result still isn't valid. + * Never rejects — a legitimate attachment with an awkward name keeps its bytes + * under a safe name rather than being dropped. + */ +function safeFilename(rawFilename: string, mimeType: string, index: number): string { + const sanitized = rawFilename.replace(/[^a-zA-Z0-9._-]/g, '_'); + if (isValidFilename(sanitized)) return sanitized; + const ext = MIME_FALLBACK_EXTENSION[mimeType] ?? 'bin'; + return `attachment_${index}.${ext}`; +} + +/** + * Filter raw Jira attachments down to the set we'll fetch, enforcing the same + * limits the wire path enforces: + * - supported MIME types only (PNG/JPEG images; text/csv/md/json/pdf/log) + * - per-file size <= MAX_ATTACHMENT_SIZE_BYTES (10 MB) + * - combined count (with slots already used by URL image attachments) <= + * MAX_ATTACHMENTS_PER_TASK (10) + * - running total size <= MAX_TOTAL_ATTACHMENT_SIZE_BYTES (50 MB) + * + * Unsupported / oversized / over-cap attachments are DROPPED (logged), not + * errored: they simply never reach the agent, satisfying the fail-closed + * contract without failing the task. The download step (which can error) only + * ever sees this filtered, safe-by-metadata list. + */ +function selectAttachments( + raw: readonly RawJiraAttachment[], + remainingSlots: number, +): { selected: SelectedAttachment[]; skipped: number } { + const selected: SelectedAttachment[] = []; + let skipped = 0; + let runningTotal = 0; + const slotCap = Math.max(0, Math.min(remainingSlots, MAX_ATTACHMENTS_PER_TASK)); + + let index = 0; + for (const att of raw) { + if (selected.length >= slotCap) { + skipped++; + continue; + } + const id = att.id !== undefined && att.id !== null ? String(att.id) : ''; + const rawFilename = typeof att.filename === 'string' ? att.filename : ''; + const mimeType = typeof att.mimeType === 'string' ? att.mimeType : ''; + const size = typeof att.size === 'number' ? att.size : 0; + if (!id || !rawFilename || !mimeType) { + skipped++; + continue; + } + // The id becomes an S3 key segment AND the agent's on-disk directory name. + // A Jira id is numeric, but the payload is attacker-shaped in principle, so + // reject anything that isn't a safe token (path traversal / injection). + if (!SAFE_ATTACHMENT_ID.test(id)) { + skipped++; + continue; + } + const isImage = mimeType.startsWith('image/'); + const attachmentType = isImage ? 'image' : 'file'; + if (!isAllowedMimeType(mimeType, attachmentType)) { + skipped++; + continue; + } + if (size > MAX_ATTACHMENT_SIZE_BYTES) { + skipped++; + continue; + } + if (runningTotal + size > MAX_TOTAL_ATTACHMENT_SIZE_BYTES) { + skipped++; + continue; + } + runningTotal += size; + selected.push({ id, filename: safeFilename(rawFilename, mimeType, index++), mimeType, size, isImage }); + } + + return { selected, skipped }; +} + +/** + * GET the raw bytes of one attachment through the gateway attachment-content + * endpoint, enforcing the size cap while reading. The 3LO token is only valid + * against the gateway base (see {@link JIRA_API_BASE}); the raw site-host + * `content` URL in the webhook payload would 401 this token, so we address the + * attachment by id instead. Returns the outcome kind so the caller can force a + * token refresh on a 401/403. + */ +async function fetchAttachmentBytes( + accessToken: string, + cloudId: string, + attachmentId: string, +): Promise< + | { readonly kind: 'ok'; readonly content: Buffer } + | { readonly kind: 'auth' } + | { readonly kind: 'error'; readonly message: string; readonly tooLarge?: boolean } +> { + const url = + `${JIRA_API_BASE}/${encodeURIComponent(cloudId)}` + + `/rest/api/3/attachment/content/${encodeURIComponent(attachmentId)}`; + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), ATTACHMENT_FETCH_TIMEOUT_MS); + try { + // `redirect: 'follow'` (the default) lets the gateway hand off to the + // backing media store; the Authorization header is dropped on cross-origin + // redirects by fetch, which is the desired behaviour — the redirect target + // carries its own short-lived signed URL. + const resp = await fetch(url, { + method: 'GET', + headers: { + Authorization: `Bearer ${accessToken}`, + // The attachment-content endpoint returns the file's own content type + // (image/jpeg, application/pdf, …) and responds 406 Not Acceptable to a + // narrow `Accept: application/octet-stream`. Accept anything so the + // gateway can serve the real media type. + Accept: '*/*', + }, + signal: controller.signal, + }); + if (resp.status === 401 || resp.status === 403) { + return { kind: 'auth' }; + } + if (!resp.ok) { + return { kind: 'error', message: `HTTP ${resp.status}` }; + } + // Enforce the size cap while reading so a mislabeled/huge body can't blow + // the Lambda's memory. `size` metadata was already checked, but the body + // is authoritative. + const reader = resp.body?.getReader(); + if (!reader) { + return { kind: 'error', message: 'empty response body' }; + } + const chunks: Buffer[] = []; + let total = 0; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + total += value.length; + if (total > MAX_ATTACHMENT_SIZE_BYTES) { + await reader.cancel(); + return { kind: 'error', message: 'exceeds size limit', tooLarge: true }; + } + chunks.push(Buffer.from(value)); + } + return { kind: 'ok', content: Buffer.concat(chunks) }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + return { kind: 'error', message }; + } finally { + clearTimeout(timer); + } +} + +/** + * Fetch, screen, and store the selected Jira `media` attachments, returning + * `passed` AttachmentRecords for `createTaskCore` to persist verbatim. + * + * @param rawAttachments `issue.fields.attachment` (unknown/untrusted shape). + * @param remainingSlots attachment slots still free after URL image + * extraction (so the combined total respects the + * per-task cap of 10). + * @param ctx tenant context + S3/screening storage deps. + * @throws JiraAttachmentError if a SELECTED attachment cannot be safely + * fetched, validated, or screened (fail-closed — reject the task). + */ +export async function downloadScreenAndStoreJiraAttachments( + rawAttachments: unknown, + remainingSlots: number, + ctx: JiraTenantContext & JiraAttachmentStorage, +): Promise { + const raw = Array.isArray(rawAttachments) ? (rawAttachments as RawJiraAttachment[]) : []; + if (raw.length === 0 || remainingSlots <= 0) return []; + + const { selected, skipped } = selectAttachments(raw, remainingSlots); + if (skipped > 0) { + logger.info('Skipped unsupported/oversized/over-cap Jira attachments', { + jira_cloud_id: ctx.cloudId, + skipped, + selected: selected.length, + total: raw.length, + }); + } + if (selected.length === 0) return []; + + // Resolve the token once for the batch; refresh reactively on a 401/403. + let token = await resolveJiraOauthToken(ctx.cloudId, ctx.registryTableName); + if (!token) { + throw new JiraAttachmentError( + 'Could not resolve a Jira OAuth token to download issue attachments.', + ); + } + + const records: PassedAttachmentRecord[] = []; + + for (const att of selected) { + let outcome = await fetchAttachmentBytes(token.accessToken, ctx.cloudId, att.id); + + // Reactive refresh-and-retry-once on auth rejection, mirroring + // jira-feedback.postCommentWithResult: the stored token may be dead + // despite a not-yet-reached expiry (server-side revocation). + if (outcome.kind === 'auth') { + logger.info('Jira attachment download got auth rejection — forcing token refresh and retrying once', { + jira_cloud_id: ctx.cloudId, + attachment_filename: att.filename, + }); + const refreshed = await resolveJiraOauthToken(ctx.cloudId, ctx.registryTableName, { forceRefresh: true }); + if (!refreshed || refreshed.accessToken === token.accessToken) { + throw new JiraAttachmentError( + `Attachment '${att.filename}' could not be downloaded: Jira rejected the credential.`, + ); + } + token = refreshed; + outcome = await fetchAttachmentBytes(token.accessToken, ctx.cloudId, att.id); + } + + if (outcome.kind === 'auth') { + throw new JiraAttachmentError( + `Attachment '${att.filename}' could not be downloaded: Jira rejected the credential.`, + ); + } + if (outcome.kind === 'error') { + throw new JiraAttachmentError( + `Attachment '${att.filename}' could not be downloaded (${outcome.message}).`, + ); + } + + const content = outcome.content; + + // The declared metadata MIME already passed the allowlist; confirm the + // bytes actually match it (blocks a masquerading/polyglot payload). + if (!validateMagicBytes(content, att.mimeType)) { + throw new JiraAttachmentError( + `Attachment '${att.filename}' content does not match its declared type '${att.mimeType}'.`, + ); + } + + // Screen through the same Bedrock Guardrail pipeline as inline/URL + // attachments. Any block or screening failure is fail-closed. + let screenResult; + try { + screenResult = att.isImage + ? await screenImage(content, att.mimeType, att.filename, ctx.screeningConfig) + : await screenTextFile(content, att.mimeType, att.filename, ctx.screeningConfig); + } catch (err) { + if (err instanceof AttachmentScreeningError) { + throw new JiraAttachmentError( + `Attachment '${att.filename}' was blocked by content screening: ${err.message}`, + { cause: err }, + ); + } + throw new JiraAttachmentError( + `Attachment '${att.filename}' could not be screened: ${err instanceof Error ? err.message : String(err)}`, + { cause: err }, + ); + } + + if (screenResult.screening.status === 'blocked') { + throw new JiraAttachmentError( + `Attachment '${att.filename}' was blocked by content policy: ${screenResult.screening.categories.join(', ')}`, + ); + } + + // Upload the cleaned bytes under the same key layout as the inline/URL + // paths so downstream hydration/authorization is uniform. + const attachmentId = att.id; + const s3Key = `${ATTACHMENT_OBJECT_KEY_PREFIX}${ctx.userId}/${ctx.taskId}/${attachmentId}/${att.filename}`; + let putResult; + try { + putResult = await ctx.s3Client.send(new PutObjectCommand({ + Bucket: ctx.bucketName, + Key: s3Key, + Body: screenResult.content, + ContentType: att.mimeType, + })); + } catch (s3Err) { + logger.error('S3 upload failed for Jira attachment', { + jira_cloud_id: ctx.cloudId, + attachment_filename: att.filename, + s3_key: s3Key, + error: s3Err instanceof Error ? s3Err.message : String(s3Err), + metric_type: 'jira_attachment_upload_failure', + }); + throw new JiraAttachmentError( + `Attachment '${att.filename}' could not be stored.`, + { cause: s3Err }, + ); + } + + const tokenEstimate = att.isImage + ? estimateImageTokensFromBuffer(screenResult.content, att.mimeType) + : undefined; + + records.push(createAttachmentRecord({ + attachment_id: attachmentId, + type: att.isImage ? 'image' : 'file', + content_type: att.mimeType, + filename: att.filename, + s3_key: s3Key, + s3_version_id: putResult.VersionId ?? 'unversioned', + size_bytes: screenResult.content.length, + screening: { status: 'passed', screened_at: new Date().toISOString() }, + checksum_sha256: screenResult.checksum, + ...(tokenEstimate !== undefined && { token_estimate: tokenEstimate }), + }) as PassedAttachmentRecord); + + logger.info('Jira attachment downloaded, screened, and stored', { + jira_cloud_id: ctx.cloudId, + attachment_filename: att.filename, + s3_key: s3Key, + }); + } + + return records; +} + +/** A rendered issue comment folded into the task context. */ +export interface RenderedComment { + readonly author: string; + readonly createdAt: string; + readonly markdown: string; +} + +/** Subset of a Jira comment object we depend on. Extra fields tolerated. */ +interface RawJiraComment { + readonly author?: { + readonly displayName?: string; + readonly accountType?: string; + }; + readonly body?: unknown; // ADF document + readonly created?: string; +} + +interface JiraCommentPage { + readonly comments?: RawJiraComment[]; +} + +/** + * Fetch the most recent human-authored comments on an issue and render them to + * markdown, oldest-first. Best-effort / fail-open: any failure (auth, REST + * error, malformed body) logs a WARN and returns `[]` so the task still + * proceeds — comments are advisory context, not a gate. + * + * "Human" = author `accountType === 'atlassian'`. Atlassian marks app/bot + * authors with `accountType: 'app'`, so this drops ABCA's own REST-posted + * progress/failure comments (and other bots) without needing to know the app's + * own accountId. + */ +export async function fetchRecentHumanComments( + ctx: JiraTenantContext, + issueIdOrKey: string, + maxComments: number = DEFAULT_MAX_COMMENTS, +): Promise { + try { + const token = await resolveJiraOauthToken(ctx.cloudId, ctx.registryTableName); + if (!token) { + logger.warn('Skipping Jira comment fetch: could not resolve OAuth token', { + jira_cloud_id: ctx.cloudId, + issue_id_or_key: issueIdOrKey, + }); + return []; + } + + // Newest-first so `maxResults` keeps the most recent; we reverse to + // oldest-first for rendering. + const url = + `${JIRA_API_BASE}/${encodeURIComponent(ctx.cloudId)}` + + `/rest/api/3/issue/${encodeURIComponent(issueIdOrKey)}/comment` + + `?orderBy=-created&maxResults=${encodeURIComponent(String(maxComments))}`; + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), COMMENT_FETCH_TIMEOUT_MS); + let page: JiraCommentPage; + try { + const resp = await fetch(url, { + method: 'GET', + headers: { + Authorization: `Bearer ${token.accessToken}`, + Accept: 'application/json', + }, + signal: controller.signal, + }); + if (!resp.ok) { + logger.warn('Jira comment fetch non-2xx (proceeding without comments)', { + jira_cloud_id: ctx.cloudId, + issue_id_or_key: issueIdOrKey, + status: resp.status, + }); + return []; + } + page = (await resp.json()) as JiraCommentPage; + } finally { + clearTimeout(timer); + } + + const comments = Array.isArray(page.comments) ? page.comments : []; + const rendered: RenderedComment[] = []; + for (const c of comments) { + // Human authors only. Missing accountType is treated as non-human + // (conservative: better to omit an ambiguous author than surface a bot). + if (c.author?.accountType !== 'atlassian') continue; + const markdown = extractDescriptionMarkdown(c.body); + if (!markdown.trim()) continue; + rendered.push({ + author: c.author?.displayName?.trim() || 'Unknown', + createdAt: typeof c.created === 'string' ? c.created : '', + markdown: markdown.trim(), + }); + } + + // API returned newest-first; render oldest-first so the thread reads + // naturally. + rendered.reverse(); + if (rendered.length > 0) { + logger.info('Fetched recent human Jira comments for task context', { + jira_cloud_id: ctx.cloudId, + issue_id_or_key: issueIdOrKey, + count: rendered.length, + }); + } + return rendered; + } catch (err) { + logger.warn('Jira comment fetch failed (proceeding without comments)', { + jira_cloud_id: ctx.cloudId, + issue_id_or_key: issueIdOrKey, + error: err instanceof Error ? err.message : String(err), + }); + return []; // nosemgrep: ts-silent-success-masking -- issue #577 mandates comments are advisory: on any fetch failure the task proceeds without them (fail-open), warning logged above + } +} diff --git a/cdk/src/stacks/agent.ts b/cdk/src/stacks/agent.ts index 1382dce4..9c0c27b6 100644 --- a/cdk/src/stacks/agent.ts +++ b/cdk/src/stacks/agent.ts @@ -848,6 +848,9 @@ export class AgentStack extends Stack { orchestratorFunctionArn: orchestrator.alias.functionArn, guardrailId: inputGuardrail.guardrailId, guardrailVersion: inputGuardrail.guardrailVersion, + // Lets the processor fetch, screen, and store Jira media attachments at + // task-admission time (#577). Same bucket the orchestrator hydrates from. + attachmentsBucket: attachmentsBucket.bucket, }); // Agent runtime reads the per-tenant Jira OAuth token directly from diff --git a/cdk/test/constructs/task-api.test.ts b/cdk/test/constructs/task-api.test.ts index 7c20de3e..79fd7fa9 100644 --- a/cdk/test/constructs/task-api.test.ts +++ b/cdk/test/constructs/task-api.test.ts @@ -204,6 +204,48 @@ describe('TaskApi construct', () => { }); }); + test('allows large HMAC-verified Jira webhook bodies through the WAF common rule set', () => { + const webAcls = baseTemplate.findResources('AWS::WAFv2::WebACL'); + const webAcl = Object.values(webAcls)[0] as any; + const rules = webAcl.Properties.Rules as any[]; + const largeBodyRule = rules.find( + rule => rule.Name === 'AWSManagedRulesCommonRuleSet-TaskPaths', + ); + const fullCommonRule = rules.find( + rule => rule.Name === 'AWSManagedRulesCommonRuleSet', + ); + + expect( + largeBodyRule.Statement.ManagedRuleGroupStatement.ExcludedRules, + ).toEqual([{ Name: 'SizeRestrictions_BODY' }]); + expect( + largeBodyRule.Statement.ManagedRuleGroupStatement.ScopeDownStatement + .OrStatement.Statements, + ).toEqual(expect.arrayContaining([ + expect.objectContaining({ + ByteMatchStatement: expect.objectContaining({ + PositionalConstraint: 'EXACTLY', + SearchString: '/v1/jira/webhook', + }), + }), + ])); + expect( + fullCommonRule.Statement.ManagedRuleGroupStatement.ScopeDownStatement + .AndStatement.Statements, + ).toEqual(expect.arrayContaining([ + expect.objectContaining({ + NotStatement: expect.objectContaining({ + Statement: expect.objectContaining({ + ByteMatchStatement: expect.objectContaining({ + PositionalConstraint: 'EXACTLY', + SearchString: '/v1/jira/webhook', + }), + }), + }), + }), + ])); + }); + test('associates WAF with the API Gateway stage', () => { baseTemplate.resourceCountIs('AWS::WAFv2::WebACLAssociation', 1); }); diff --git a/cdk/test/handlers/jira-webhook-processor.test.ts b/cdk/test/handlers/jira-webhook-processor.test.ts index d808ac81..43ee2564 100644 --- a/cdk/test/handlers/jira-webhook-processor.test.ts +++ b/cdk/test/handlers/jira-webhook-processor.test.ts @@ -40,11 +40,34 @@ jest.mock('../../src/handlers/shared/jira-oauth-resolver', () => ({ resolveJiraOauthToken: (...args: unknown[]) => resolveJiraOauthTokenMock(...args), })); +// #577 context enrichment. The download/comment helpers are unit-tested in +// jira-attachments.test.ts; here we mock them to test the processor wiring +// (folding comments into the description, fail-closed attachment rejection) +// without real REST/S3. `JiraAttachmentError` is kept real so the processor's +// `instanceof` reject branch is exercised. +const fetchRecentHumanCommentsMock = jest.fn(); +const downloadJiraAttachmentsMock = jest.fn(); +jest.mock('../../src/handlers/shared/jira-attachments', () => { + const actual = jest.requireActual('../../src/handlers/shared/jira-attachments'); + return { + ...actual, + fetchRecentHumanComments: (...args: unknown[]) => fetchRecentHumanCommentsMock(...args), + downloadScreenAndStoreJiraAttachments: (...args: unknown[]) => downloadJiraAttachmentsMock(...args), + }; +}); + process.env.JIRA_PROJECT_MAPPING_TABLE_NAME = 'JiraProjects'; process.env.JIRA_USER_MAPPING_TABLE_NAME = 'JiraUsers'; process.env.JIRA_WORKSPACE_REGISTRY_TABLE_NAME = 'JiraWorkspaceRegistry'; +// Attachment enrichment needs a bucket + guardrail configured (#577); with +// these set the processor initializes S3/Bedrock clients (cheap, no network at +// construction) and screens attachments through the mocked helper. +process.env.ATTACHMENTS_BUCKET_NAME = 'attachments-bucket'; +process.env.GUARDRAIL_ID = 'gr-1'; +process.env.GUARDRAIL_VERSION = '1'; import { handler } from '../../src/handlers/jira-webhook-processor'; +import { JiraAttachmentError } from '../../src/handlers/shared/jira-attachments'; function eventWith(payload: Record): { raw_body: string } { return { raw_body: JSON.stringify(payload) }; @@ -94,6 +117,11 @@ describe('jira-webhook-processor handler', () => { siteUrl: 'https://acme.atlassian.net', oauthSecretArn: 'arn:aws:secretsmanager:us-east-1:123:secret:bgagent-jira-oauth-cloud-1', }); + // Default (#577): no comments, no attachments. Per-case tests override. + fetchRecentHumanCommentsMock.mockReset(); + fetchRecentHumanCommentsMock.mockResolvedValue([]); + downloadJiraAttachmentsMock.mockReset(); + downloadJiraAttachmentsMock.mockResolvedValue([]); }); test('skips missing raw_body', async () => { @@ -723,4 +751,184 @@ describe('jira-webhook-processor handler', () => { expect(reqBody.attachments[0]).toEqual({ type: 'url', url: 'https://cdn.example.com/mockup.png' }); }); }); + + // ─── #577: recent comments folded into the task context ───────────────────── + + describe('recent comments in task context', () => { + beforeEach(() => { + ddbSend + .mockResolvedValueOnce({ Item: { repo: 'org/repo', status: 'active' } }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + createTaskCoreMock.mockResolvedValueOnce({ statusCode: 201, body: JSON.stringify({ data: { task_id: 'T1' } }) }); + }); + + test('folds fetched human comments into the description (oldest-first, attributed)', async () => { + fetchRecentHumanCommentsMock.mockResolvedValueOnce([ + { author: 'Ada', createdAt: '2026-07-01T00:00:00Z', markdown: 'Use the attached log.' }, + { author: 'Grace', createdAt: '2026-07-02T00:00:00Z', markdown: 'Repro on staging only.' }, + ]); + + await handler(eventWith(issue())); + + const [reqBody] = createTaskCoreMock.mock.calls[0]; + expect(reqBody.task_description).toContain('## Recent comments'); + expect(reqBody.task_description).toContain('**Ada** (2026-07-01T00:00:00Z):'); + expect(reqBody.task_description).toContain('Use the attached log.'); + expect(reqBody.task_description).toContain('**Grace** (2026-07-02T00:00:00Z):'); + // Oldest-first: Ada's comment appears before Grace's. + expect(reqBody.task_description.indexOf('Ada')).toBeLessThan(reqBody.task_description.indexOf('Grace')); + }); + + test('no comments → no Recent comments section', async () => { + fetchRecentHumanCommentsMock.mockResolvedValueOnce([]); + + await handler(eventWith(issue())); + + const [reqBody] = createTaskCoreMock.mock.calls[0]; + expect(reqBody.task_description).not.toContain('## Recent comments'); + }); + + test('comment fetch failure is fail-open: task still created, no failure comment', async () => { + // The helper itself is fail-open (returns []), but assert the processor + // does not gate on it even if it somehow rejects. + fetchRecentHumanCommentsMock.mockResolvedValueOnce([]); + + await handler(eventWith(issue())); + + expect(createTaskCoreMock).toHaveBeenCalled(); + expect(reportIssueFailureMock).not.toHaveBeenCalled(); + }); + + test('a huge comment thread is truncated, keeping task_description within the 10k limit (fail-open)', async () => { + // A single very long comment would otherwise push the description past + // MAX_TASK_DESCRIPTION_LENGTH and cause createTaskCore to 400 — turning + // advisory comments into a hard gate. The processor must truncate instead. + fetchRecentHumanCommentsMock.mockResolvedValueOnce([ + { author: 'Ada', createdAt: '2026-07-01T00:00:00Z', markdown: 'x'.repeat(50_000) }, + ]); + + await handler(eventWith(issue())); + + expect(createTaskCoreMock).toHaveBeenCalled(); + const [reqBody] = createTaskCoreMock.mock.calls[0]; + expect(reqBody.task_description.length).toBeLessThanOrEqual(10_000); + expect(reqBody.task_description).toContain('recent comments truncated'); + }); + }); + + // ─── #577: Jira media attachments (fetch → screen → pre-screened records) ─── + + describe('Jira media attachment enrichment', () => { + const passedRecord = { + attachment_id: 'att-1', + type: 'file' as const, + content_type: 'text/plain', + filename: 'error.log', + s3_key: 'attachments/cognito-user-1/T1/att-1/error.log', + s3_version_id: 'v1', + size_bytes: 42, + checksum_sha256: 'abc', + screening: { status: 'passed' as const, screened_at: '2026-07-14T00:00:00Z' }, + }; + + function issueWithAttachment(): Record { + const payload = issue(); + (payload.issue as { fields: Record }).fields.attachment = [ + { id: 'att-1', filename: 'error.log', mimeType: 'text/plain', size: 42, content: 'https://acme.atlassian.net/secure/attachment/att-1/error.log' }, + ]; + return payload; + } + + test('passes screened attachments to createTaskCore as preScreenedAttachments', async () => { + ddbSend + .mockResolvedValueOnce({ Item: { repo: 'org/repo', status: 'active' } }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + createTaskCoreMock.mockResolvedValueOnce({ statusCode: 201, body: JSON.stringify({ data: { task_id: 'T1' } }) }); + downloadJiraAttachmentsMock.mockResolvedValueOnce([passedRecord]); + + await handler(eventWith(issueWithAttachment())); + + expect(createTaskCoreMock).toHaveBeenCalled(); + const [, ctx] = createTaskCoreMock.mock.calls[0]; + expect(ctx.preScreenedAttachments).toHaveLength(1); + expect(ctx.preScreenedAttachments[0].screening.status).toBe('passed'); + // taskId is minted in the processor and threaded through so S3 keys match. + expect(ctx.taskId).toBeDefined(); + }); + + test('passes remaining slots (10 - url image count) to the downloader', async () => { + ddbSend + .mockResolvedValueOnce({ Item: { repo: 'org/repo', status: 'active' } }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + createTaskCoreMock.mockResolvedValueOnce({ statusCode: 201, body: JSON.stringify({ data: { task_id: 'T1' } }) }); + downloadJiraAttachmentsMock.mockResolvedValueOnce([]); + + const payload = issueWithAttachment(); + // Two embedded image URLs consume two slots. + (payload.issue as { fields: Record }).fields.description = + '![a](https://cdn.example.com/a.png) ![b](https://cdn.example.com/b.png)'; + + await handler(eventWith(payload)); + + const [, remainingSlots] = downloadJiraAttachmentsMock.mock.calls[0]; + expect(remainingSlots).toBe(8); + }); + + test('fail-closed: JiraAttachmentError rejects the task with a Jira comment', async () => { + ddbSend + .mockResolvedValueOnce({ Item: { repo: 'org/repo', status: 'active' } }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + downloadJiraAttachmentsMock.mockRejectedValueOnce( + new JiraAttachmentError("Attachment 'error.log' was blocked by content policy: PROMPT_ATTACK"), + ); + + await handler(eventWith(issueWithAttachment())); + + expect(createTaskCoreMock).not.toHaveBeenCalled(); + expect(reportIssueFailureMock).toHaveBeenCalledTimes(1); + const msg = reportIssueFailureMock.mock.calls[0][2] as string; + expect(msg).toContain("couldn't safely process an attachment"); + }); + + test('fail-closed: attachments present but screening unconfigured rejects the task', async () => { + const savedBucket = process.env.ATTACHMENTS_BUCKET_NAME; + const savedGuardrail = process.env.GUARDRAIL_ID; + // The processor reads screening config at module load, so simulate the + // unconfigured state by re-importing with the env cleared. + jest.resetModules(); + delete process.env.ATTACHMENTS_BUCKET_NAME; + delete process.env.GUARDRAIL_ID; + + const freshModule = await import('../../src/handlers/jira-webhook-processor'); + const freshHandler = freshModule.handler; + + ddbSend + .mockResolvedValueOnce({ Item: { repo: 'org/repo', status: 'active' } }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + + await freshHandler(eventWith(issueWithAttachment())); + + expect(createTaskCoreMock).not.toHaveBeenCalled(); + expect(reportIssueFailureMock).toHaveBeenCalledTimes(1); + const msg = reportIssueFailureMock.mock.calls[0][2] as string; + expect(msg).toContain('attachment screening is not configured'); + + process.env.ATTACHMENTS_BUCKET_NAME = savedBucket; + process.env.GUARDRAIL_ID = savedGuardrail; + jest.resetModules(); + }); + + test('no attachments → downloader not called, task created normally', async () => { + ddbSend + .mockResolvedValueOnce({ Item: { repo: 'org/repo', status: 'active' } }) + .mockResolvedValueOnce({ Item: { platform_user_id: 'cognito-user-1', status: 'active' } }); + createTaskCoreMock.mockResolvedValueOnce({ statusCode: 201, body: JSON.stringify({ data: { task_id: 'T1' } }) }); + + await handler(eventWith(issue())); + + expect(downloadJiraAttachmentsMock).not.toHaveBeenCalled(); + const [, ctx] = createTaskCoreMock.mock.calls[0]; + expect(ctx.preScreenedAttachments).toBeUndefined(); + }); + }); }); diff --git a/cdk/test/handlers/shared/create-task-core.test.ts b/cdk/test/handlers/shared/create-task-core.test.ts index 992e4438..0c49aa20 100644 --- a/cdk/test/handlers/shared/create-task-core.test.ts +++ b/cdk/test/handlers/shared/create-task-core.test.ts @@ -898,4 +898,83 @@ describe('createTaskCore', () => { expect(mockLookupRepo).toHaveBeenCalledTimes(1); expect(mockLookupRepo).toHaveBeenCalledWith('org/repo'); }); + + // --- #577: pre-screened attachments + caller-supplied taskId --- + + describe('preScreenedAttachments and taskId (#577)', () => { + function persistedTaskItem() { + const putCall = mockSend.mock.calls.find( + (c: any) => c[0]?._type === 'Put' && c[0]?.input?.TableName === 'Tasks', + ); + return putCall?.[0]?.input?.Item; + } + + const passedRecord = { + attachment_id: 'att-jira-1', + type: 'file' as const, + content_type: 'text/plain', + filename: 'error.log', + s3_key: 'attachments/user-123/T-577/att-jira-1/error.log', + s3_version_id: 'v1', + size_bytes: 128, + checksum_sha256: 'sum', + screening: { status: 'passed' as const, screened_at: '2026-07-14T00:00:00Z' }, + }; + + test('merges pre-screened records onto the task without re-screening', async () => { + const result = await createTaskCore( + { repo: 'org/repo', task_description: 'Fix', workflow_ref: 'coding/new-task-v1' }, + makeContext({ preScreenedAttachments: [passedRecord] }), + 'req-577-a', + ); + + expect(result.statusCode).toBe(201); + const item = persistedTaskItem(); + expect(item.attachments).toHaveLength(1); + expect(item.attachments[0].attachment_id).toBe('att-jira-1'); + expect(item.attachments[0].screening.status).toBe('passed'); + // The pre-screened path must NOT re-run the guardrail on the attachment + // bytes. createTaskCore still screens the task DESCRIPTION text, so the + // guardrail may be called — but only with the description, never with any + // attachment content (the record carries no bytes to screen). + const guardrailContents = mockBedrockSend.mock.calls.map( + (c: any) => JSON.stringify(c[0]?.input?.content ?? []), + ); + for (const content of guardrailContents) { + expect(content).not.toContain('error.log'); + expect(content).toContain('Fix'); // only the description was screened + } + }); + + test('honors a caller-supplied taskId', async () => { + const result = await createTaskCore( + { repo: 'org/repo', task_description: 'Fix' }, + makeContext({ taskId: 'T-explicit-577' }), + 'req-577-b', + ); + + expect(result.statusCode).toBe(201); + expect(persistedTaskItem().task_id).toBe('T-explicit-577'); + expect(JSON.parse(result.body).data.task_id).toBe('T-explicit-577'); + }); + + test('fails closed if a pre-screened record is not in passed state', async () => { + const badRecord = { + ...passedRecord, + screening: { status: 'pending' as const }, + }; + const result = await createTaskCore( + { repo: 'org/repo', task_description: 'Fix' }, + // Cast: this is a contract violation we deliberately construct to prove + // the fail-closed guard rejects it rather than persisting it. + makeContext({ preScreenedAttachments: [badRecord as never] }), + 'req-577-c', + ); + + expect(result.statusCode).toBe(500); + expect(JSON.parse(result.body).error.code).toBe('INTERNAL_ERROR'); + // No task persisted. + expect(persistedTaskItem()).toBeUndefined(); + }); + }); }); diff --git a/cdk/test/handlers/shared/jira-attachments.test.ts b/cdk/test/handlers/shared/jira-attachments.test.ts new file mode 100644 index 00000000..38773418 --- /dev/null +++ b/cdk/test/handlers/shared/jira-attachments.test.ts @@ -0,0 +1,399 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +const resolveJiraOauthTokenMock = jest.fn(); +jest.mock('../../../src/handlers/shared/jira-oauth-resolver', () => ({ + resolveJiraOauthToken: (...args: unknown[]) => resolveJiraOauthTokenMock(...args), +})); + +const screenImageMock = jest.fn(); +const screenTextFileMock = jest.fn(); +jest.mock('../../../src/handlers/shared/attachment-screening', () => { + const actual = jest.requireActual('../../../src/handlers/shared/attachment-screening'); + return { + ...actual, + screenImage: (...args: unknown[]) => screenImageMock(...args), + screenTextFile: (...args: unknown[]) => screenTextFileMock(...args), + }; +}); + +import { AttachmentScreeningError, type ScreeningConfig } from '../../../src/handlers/shared/attachment-screening'; +import { + downloadScreenAndStoreJiraAttachments, + fetchRecentHumanComments, + JiraAttachmentError, +} from '../../../src/handlers/shared/jira-attachments'; +import { MAX_ATTACHMENT_SIZE_BYTES } from '../../../src/handlers/shared/validation'; + +// Plain text validates on absence of null bytes; a leading PNG signature for +// image cases. +const TEXT_BYTES = Buffer.from('error: boom\nstacktrace...'); +const PNG_BYTES = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00]); + +const putSendMock = jest.fn(); +const s3Client = { send: putSendMock } as unknown as import('@aws-sdk/client-s3').S3Client; + +const screeningConfig: ScreeningConfig = { + guardrailId: 'gr-1', + guardrailVersion: '1', + bedrockClient: {} as never, +}; + +function storageCtx() { + return { + cloudId: 'cloud-1', + registryTableName: 'JiraRegistry', + s3Client, + bucketName: 'attachments-bucket', + screeningConfig, + userId: 'user-1', + taskId: 'task-1', + }; +} + +/** A fetch Response-like object whose body streams the given buffer once. */ +function bytesResponse(buf: Buffer, status = 200): Response { + let sent = false; + return { + ok: status >= 200 && status < 300, + status, + body: { + getReader() { + return { + read() { + if (sent) return Promise.resolve({ done: true, value: undefined }); + sent = true; + return Promise.resolve({ done: false, value: new Uint8Array(buf) }); + }, + cancel() { return Promise.resolve(); }, + }; + }, + }, + } as unknown as Response; +} + +function jsonResponse(body: unknown, status = 200): Response { + return { + ok: status >= 200 && status < 300, + status, + json: () => Promise.resolve(body), + } as unknown as Response; +} + +const goodToken = { + accessToken: 'at-1', + scope: 'read:jira-work', + siteUrl: 'https://acme.atlassian.net', + oauthSecretArn: 'arn:secret', +}; + +beforeEach(() => { + resolveJiraOauthTokenMock.mockReset(); + resolveJiraOauthTokenMock.mockResolvedValue(goodToken); + screenImageMock.mockReset(); + screenTextFileMock.mockReset(); + putSendMock.mockReset(); + putSendMock.mockResolvedValue({ VersionId: 'v1' }); + (global.fetch as unknown) = jest.fn(); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe('downloadScreenAndStoreJiraAttachments', () => { + test('happy path: downloads, screens, uploads, returns passed records', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce(bytesResponse(TEXT_BYTES)); + screenTextFileMock.mockResolvedValueOnce({ + content: TEXT_BYTES, + contentType: 'text/plain', + checksum: 'sum', + screening: { status: 'passed' }, + }); + + const records = await downloadScreenAndStoreJiraAttachments( + [{ id: 'att-1', filename: 'error.log', mimeType: 'text/plain', size: TEXT_BYTES.length }], + 10, + storageCtx(), + ); + + expect(records).toHaveLength(1); + expect(records[0].screening.status).toBe('passed'); + expect(records[0].s3_key).toBe('attachments/user-1/task-1/att-1/error.log'); + expect(records[0].type).toBe('file'); + expect(putSendMock).toHaveBeenCalledTimes(1); + // GET was addressed by attachment id on the gateway base, not the raw content URL. + const fetchUrl = (global.fetch as jest.Mock).mock.calls[0][0] as string; + expect(fetchUrl).toContain('api.atlassian.com/ex/jira/cloud-1/rest/api/3/attachment/content/att-1'); + // Accept must be permissive: the content endpoint 406s on + // `application/octet-stream` and serves the file's own media type. + const fetchInit = (global.fetch as jest.Mock).mock.calls[0][1] as { headers: Record }; + expect(fetchInit.headers.Accept).toBe('*/*'); + }); + + test('routes images through screenImage and marks type image', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce(bytesResponse(PNG_BYTES)); + screenImageMock.mockResolvedValueOnce({ + content: PNG_BYTES, + contentType: 'image/png', + checksum: 'sum', + screening: { status: 'passed' }, + }); + + const records = await downloadScreenAndStoreJiraAttachments( + [{ id: 'img-1', filename: 'shot.png', mimeType: 'image/png', size: PNG_BYTES.length }], + 10, + storageCtx(), + ); + + expect(screenImageMock).toHaveBeenCalled(); + expect(records[0].type).toBe('image'); + }); + + test('filters unsupported MIME and oversized before download (silently skipped)', async () => { + const records = await downloadScreenAndStoreJiraAttachments( + [ + { id: 'a', filename: 'clip.gif', mimeType: 'image/gif', size: 10 }, + { id: 'b', filename: 'huge.log', mimeType: 'text/plain', size: MAX_ATTACHMENT_SIZE_BYTES + 1 }, + ], + 10, + storageCtx(), + ); + + expect(records).toHaveLength(0); + expect(global.fetch).not.toHaveBeenCalled(); + // No token needed when nothing survives the filter. + expect(resolveJiraOauthTokenMock).not.toHaveBeenCalled(); + }); + + test('respects remainingSlots (combined 10-attachment cap)', async () => { + (global.fetch as jest.Mock).mockResolvedValue(bytesResponse(TEXT_BYTES)); + screenTextFileMock.mockResolvedValue({ + content: TEXT_BYTES, contentType: 'text/plain', checksum: 's', screening: { status: 'passed' }, + }); + + const raw = Array.from({ length: 5 }, (_, i) => ({ + id: `att-${i}`, filename: `f${i}.log`, mimeType: 'text/plain', size: TEXT_BYTES.length, + })); + const records = await downloadScreenAndStoreJiraAttachments(raw, 2, storageCtx()); + + expect(records).toHaveLength(2); + }); + + test('respects the 50MB running total cap', async () => { + (global.fetch as jest.Mock).mockResolvedValue(bytesResponse(TEXT_BYTES)); + screenTextFileMock.mockResolvedValue({ + content: TEXT_BYTES, contentType: 'text/plain', checksum: 's', screening: { status: 'passed' }, + }); + const nineMb = 9 * 1024 * 1024; + // 6 x 9MB = 54MB > 50MB total; only the first 5 (45MB) fit. + const raw = Array.from({ length: 6 }, (_, i) => ({ + id: `att-${i}`, filename: `f${i}.log`, mimeType: 'text/plain', size: nineMb, + })); + const records = await downloadScreenAndStoreJiraAttachments(raw, 10, storageCtx()); + expect(records).toHaveLength(5); + }); + + test('magic-byte mismatch throws JiraAttachmentError (fail-closed)', async () => { + // Declared text/plain but bytes contain a null → validateMagicBytes fails. + (global.fetch as jest.Mock).mockResolvedValueOnce(bytesResponse(Buffer.from([0x00, 0x01, 0x02]))); + + await expect(downloadScreenAndStoreJiraAttachments( + [{ id: 'att-1', filename: 'error.log', mimeType: 'text/plain', size: 3 }], + 10, + storageCtx(), + )).rejects.toBeInstanceOf(JiraAttachmentError); + expect(putSendMock).not.toHaveBeenCalled(); + }); + + test('screening block throws JiraAttachmentError (fail-closed)', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce(bytesResponse(TEXT_BYTES)); + screenTextFileMock.mockRejectedValueOnce(new AttachmentScreeningError('PROMPT_ATTACK')); + + await expect(downloadScreenAndStoreJiraAttachments( + [{ id: 'att-1', filename: 'error.log', mimeType: 'text/plain', size: TEXT_BYTES.length }], + 10, + storageCtx(), + )).rejects.toThrow(/blocked by content screening/); + }); + + test('screening returns blocked status → JiraAttachmentError', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce(bytesResponse(TEXT_BYTES)); + screenTextFileMock.mockResolvedValueOnce({ + content: TEXT_BYTES, + contentType: 'text/plain', + checksum: 's', + screening: { status: 'blocked', categories: ['HATE'] }, + }); + + await expect(downloadScreenAndStoreJiraAttachments( + [{ id: 'att-1', filename: 'error.log', mimeType: 'text/plain', size: TEXT_BYTES.length }], + 10, + storageCtx(), + )).rejects.toThrow(/blocked by content policy/); + }); + + test('401 on download forces token refresh and retries once', async () => { + (global.fetch as jest.Mock) + .mockResolvedValueOnce(bytesResponse(Buffer.alloc(0), 401)) + .mockResolvedValueOnce(bytesResponse(TEXT_BYTES)); + resolveJiraOauthTokenMock + .mockResolvedValueOnce(goodToken) // initial + .mockResolvedValueOnce({ ...goodToken, accessToken: 'at-2' }); // forced refresh + screenTextFileMock.mockResolvedValueOnce({ + content: TEXT_BYTES, contentType: 'text/plain', checksum: 's', screening: { status: 'passed' }, + }); + + const records = await downloadScreenAndStoreJiraAttachments( + [{ id: 'att-1', filename: 'error.log', mimeType: 'text/plain', size: TEXT_BYTES.length }], + 10, + storageCtx(), + ); + + expect(records).toHaveLength(1); + expect(resolveJiraOauthTokenMock).toHaveBeenLastCalledWith('cloud-1', 'JiraRegistry', { forceRefresh: true }); + expect((global.fetch as jest.Mock)).toHaveBeenCalledTimes(2); + }); + + test('persistent 401 (refresh returns same token) → JiraAttachmentError, no second GET', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce(bytesResponse(Buffer.alloc(0), 401)); + resolveJiraOauthTokenMock.mockResolvedValue(goodToken); // refresh yields same token + + await expect(downloadScreenAndStoreJiraAttachments( + [{ id: 'att-1', filename: 'error.log', mimeType: 'text/plain', size: 5 }], + 10, + storageCtx(), + )).rejects.toThrow(/rejected the credential/); + expect((global.fetch as jest.Mock)).toHaveBeenCalledTimes(1); + }); + + test('body exceeding size limit while streaming → JiraAttachmentError', async () => { + const tooBig = Buffer.alloc(MAX_ATTACHMENT_SIZE_BYTES + 10, 0x61); + (global.fetch as jest.Mock).mockResolvedValueOnce(bytesResponse(tooBig)); + + await expect(downloadScreenAndStoreJiraAttachments( + // metadata claims small; the body is authoritative and overshoots. + [{ id: 'att-1', filename: 'error.log', mimeType: 'text/plain', size: 5 }], + 10, + storageCtx(), + )).rejects.toThrow(/could not be downloaded/); + }); + + test('unresolvable token throws JiraAttachmentError when attachments are selected', async () => { + resolveJiraOauthTokenMock.mockResolvedValueOnce(null); + await expect(downloadScreenAndStoreJiraAttachments( + [{ id: 'att-1', filename: 'error.log', mimeType: 'text/plain', size: 5 }], + 10, + storageCtx(), + )).rejects.toThrow(/Could not resolve a Jira OAuth token/); + }); + + test('empty input returns [] without resolving a token', async () => { + expect(await downloadScreenAndStoreJiraAttachments([], 10, storageCtx())).toEqual([]); + expect(await downloadScreenAndStoreJiraAttachments('not-an-array', 10, storageCtx())).toEqual([]); + expect(resolveJiraOauthTokenMock).not.toHaveBeenCalled(); + }); + + test('sanitizes an unsafe filename into a path-traversal-safe S3 key', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce(bytesResponse(TEXT_BYTES)); + screenTextFileMock.mockResolvedValueOnce({ + content: TEXT_BYTES, contentType: 'text/plain', checksum: 's', screening: { status: 'passed' }, + }); + + const records = await downloadScreenAndStoreJiraAttachments( + [{ id: 'att-1', filename: '../../../etc/evil', mimeType: 'text/plain', size: TEXT_BYTES.length }], + 10, + storageCtx(), + ); + + // No path traversal survives: the key stays under the per-task prefix and + // the filename segment carries no slashes or `..`. + expect(records).toHaveLength(1); + expect(records[0].s3_key.startsWith('attachments/user-1/task-1/att-1/')).toBe(true); + expect(records[0].filename).not.toContain('/'); + expect(records[0].filename).not.toContain('..'); + }); + + test('rejects (skips) an attachment whose id is not a safe token', async () => { + const records = await downloadScreenAndStoreJiraAttachments( + [{ id: '../evil', filename: 'ok.log', mimeType: 'text/plain', size: 5 }], + 10, + storageCtx(), + ); + // Unsafe id → dropped before any download; nothing selected. + expect(records).toHaveLength(0); + expect(global.fetch).not.toHaveBeenCalled(); + }); +}); + +describe('fetchRecentHumanComments', () => { + const ctx = { cloudId: 'cloud-1', registryTableName: 'JiraRegistry' }; + + function adf(text: string) { + return { type: 'doc', version: 1, content: [{ type: 'paragraph', content: [{ type: 'text', text }] }] }; + } + + test('keeps human (atlassian) authors, drops app/bot authors, renders oldest-first', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce(jsonResponse({ + comments: [ + // API returns newest-first. + { author: { displayName: 'Grace', accountType: 'atlassian' }, body: adf('Latest human note'), created: '2026-07-03T00:00:00Z' }, + { author: { displayName: 'ABCA Bot', accountType: 'app' }, body: adf('Starting…'), created: '2026-07-02T00:00:00Z' }, + { author: { displayName: 'Ada', accountType: 'atlassian' }, body: adf('First human note'), created: '2026-07-01T00:00:00Z' }, + ], + })); + + const comments = await fetchRecentHumanComments(ctx, 'ENG-1'); + + expect(comments).toHaveLength(2); + // Reversed to oldest-first. + expect(comments[0].author).toBe('Ada'); + expect(comments[0].markdown).toBe('First human note'); + expect(comments[1].author).toBe('Grace'); + // URL requests newest-first, capped by maxResults. + const url = (global.fetch as jest.Mock).mock.calls[0][0] as string; + expect(url).toContain('orderBy=-created'); + expect(url).toContain('maxResults=10'); + }); + + test('skips comments with empty rendered body', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce(jsonResponse({ + comments: [ + { author: { displayName: 'Ada', accountType: 'atlassian' }, body: adf(''), created: '2026-07-01T00:00:00Z' }, + ], + })); + expect(await fetchRecentHumanComments(ctx, 'ENG-1')).toEqual([]); + }); + + test('fail-open: non-2xx returns []', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce(jsonResponse({}, 500)); + expect(await fetchRecentHumanComments(ctx, 'ENG-1')).toEqual([]); + }); + + test('fail-open: fetch throws returns []', async () => { + (global.fetch as jest.Mock).mockRejectedValueOnce(new Error('network')); + expect(await fetchRecentHumanComments(ctx, 'ENG-1')).toEqual([]); + }); + + test('fail-open: unresolvable token returns []', async () => { + resolveJiraOauthTokenMock.mockResolvedValueOnce(null); + expect(await fetchRecentHumanComments(ctx, 'ENG-1')).toEqual([]); + expect(global.fetch).not.toHaveBeenCalled(); + }); +}); diff --git a/docs/guides/JIRA_SETUP_GUIDE.md b/docs/guides/JIRA_SETUP_GUIDE.md index 2abc03dd..80c82ce6 100644 --- a/docs/guides/JIRA_SETUP_GUIDE.md +++ b/docs/guides/JIRA_SETUP_GUIDE.md @@ -13,7 +13,7 @@ Set up the ABCA Jira Cloud integration so that adding a label to a Jira issue tr ## How it works -A Jira-site admin creates an Atlassian OAuth 2.0 (3LO) app and authorizes it on the site. The OAuth token bundle is stored in a per-tenant Secrets Manager secret (`bgagent-jira-oauth-`). When a user adds the trigger label to a Jira issue, Jira fires a webhook to ABCA; the receiver verifies the `X-Hub-Signature` HMAC, dedupes, and async-invokes the processor, which resolves the tenant, looks up the project→repo mapping, and creates a task. Jira-triggered tasks always run the `coding/new-task-v1` workflow (the processor pins `workflow_ref` explicitly, since a label-triggered task always targets a mapped repo). The agent clones the repo, opens a PR, and posts a "started" comment on the Jira issue via the Jira REST v3 API (using the same stored OAuth token). When the task reaches a terminal state, the platform's fan-out plane posts a single final status comment — outcome, cost, turns, duration, and PR link — via the same REST v3 endpoint (see [How it works](#how-it-works) below). +A Jira-site admin creates an Atlassian OAuth 2.0 (3LO) app and authorizes it on the site. The OAuth token bundle is stored in a per-tenant Secrets Manager secret (`bgagent-jira-oauth-`). When a user adds the trigger label to a Jira issue, Jira fires a webhook to ABCA; the receiver verifies the `X-Hub-Signature` HMAC, dedupes, and async-invokes the processor, which resolves the tenant, looks up the project→repo mapping, enriches the task with the issue's recent comments and screened file attachments (see [Issue context](#issue-context-attachments-and-comments)), and creates a task. Jira-triggered tasks always run the `coding/new-task-v1` workflow (the processor pins `workflow_ref` explicitly, since a label-triggered task always targets a mapped repo). The agent clones the repo, opens a PR, and posts a "started" comment on the Jira issue via the Jira REST v3 API (using the same stored OAuth token). When the task reaches a terminal state, the platform's fan-out plane posts a single final status comment — outcome, cost, turns, duration, and PR link — via the same REST v3 endpoint (see [How it works](#how-it-works) below). **Tenant key.** Everything is indexed on `cloudId` — the Atlassian tenant UUID, *not* the site domain or name. Webhook payloads and the OAuth flow both surface `cloudId`; it is the join key across the project-mapping, user-mapping, and workspace-registry tables. @@ -178,7 +178,7 @@ The teammate needs their own ABCA account first (Cognito user + configured CLI). ### 6. Test -Add the trigger label (`bgagent` by default) to a Jira issue in a mapped project. The agent should start within ~30 seconds, comment on the issue as it works, and post a PR link when ready. The issue **summary** plus the **description** (converted from Atlassian Document Format to markdown) becomes the task description. +Add the trigger label (`bgagent` by default) to a Jira issue in a mapped project. The agent should start within ~30 seconds, comment on the issue as it works, and post a PR link when ready. The issue **summary** plus the **description** (converted from Atlassian Document Format to markdown), the issue's **recent comments**, and any supported **file attachments** become the task context — see [Issue context: attachments and comments](#issue-context-attachments-and-comments). ## How webhook signature verification works @@ -196,6 +196,24 @@ The body must be verified as the *raw unparsed bytes* — never parsed-and-restr - **`jira:issue_updated`** — triggers only if the label was **newly added** in this update. Jira reports label changes in `changelog.items[]` (`field: "labels"`, with `fromString` / `toString`), *not* by re-sending the full label list. The processor diffs the changelog rather than inspecting `issue.fields.labels`, so re-saving an issue that already has the label does not re-trigger. - All other event types get a silent `200`. +## Issue context: attachments and comments + +Beyond the summary and description, the processor enriches the task with the practical context a Jira ticket usually carries — attached files and recent clarifications — so the agent isn't left guessing at "see the attached log" or an acceptance detail buried in a comment. Both are fetched **authenticated at task-admission time** using the tenant's existing `read:jira-work` scope (**no new OAuth scopes, no re-authorization**), because a headless agent can't fetch them itself. + +### File attachments + +Jira-hosted `media` attachments are downloaded through the Jira REST API, run through the **same Bedrock Guardrail content screening** as every other ABCA attachment, and stored for the agent — only after they pass. + +- **Supported types** — images `image/png`, `image/jpeg`; files `text/plain`, `text/csv`, `text/markdown`, `application/json`, `application/pdf`, `text/x-log`. +- **Limits** — at most **10 attachments per task** (shared with any images embedded in the description), **10 MB per file**, **50 MB total**. +- **Unsupported or oversized attachments are skipped silently** — they simply don't reach the agent; the task still runs with the rest of the context. +- **Fail-closed on unsafe content** — if a *selected* attachment can't be safely downloaded or screened (blocked by the guardrail, a content/type mismatch, a download/auth failure, or missing screening configuration), the task is **rejected** with a ❌ comment on the issue rather than run with missing context. Fix or remove the attachment and re-apply the trigger label. +- Embedded HTTPS image URLs in the description continue to work exactly as before. + +### Recent comments + +The most recent **human** comments (up to 10, oldest-first) are folded into the task description under a **Recent comments** heading, each attributed to its author. ABCA's own progress/final-status comments and other app/bot comments are excluded (filtered by Atlassian `accountType`). Comment enrichment is **best-effort / fail-open**: if the fetch fails, the task proceeds without comments (a warning is logged) — comments are advisory context, never a gate. Long comment histories are not fetched in full; only the recent window is included. + ## Board transitions As a Jira-triggered task progresses, the agent moves the originating issue across its workflow so the board reflects reality — the same at-a-glance signal Linear-origin tasks already give: diff --git a/docs/src/content/docs/using/Jira-setup-guide.md b/docs/src/content/docs/using/Jira-setup-guide.md index fb670a6c..6c5643c6 100644 --- a/docs/src/content/docs/using/Jira-setup-guide.md +++ b/docs/src/content/docs/using/Jira-setup-guide.md @@ -17,7 +17,7 @@ Set up the ABCA Jira Cloud integration so that adding a label to a Jira issue tr ## How it works -A Jira-site admin creates an Atlassian OAuth 2.0 (3LO) app and authorizes it on the site. The OAuth token bundle is stored in a per-tenant Secrets Manager secret (`bgagent-jira-oauth-`). When a user adds the trigger label to a Jira issue, Jira fires a webhook to ABCA; the receiver verifies the `X-Hub-Signature` HMAC, dedupes, and async-invokes the processor, which resolves the tenant, looks up the project→repo mapping, and creates a task. Jira-triggered tasks always run the `coding/new-task-v1` workflow (the processor pins `workflow_ref` explicitly, since a label-triggered task always targets a mapped repo). The agent clones the repo, opens a PR, and posts a "started" comment on the Jira issue via the Jira REST v3 API (using the same stored OAuth token). When the task reaches a terminal state, the platform's fan-out plane posts a single final status comment — outcome, cost, turns, duration, and PR link — via the same REST v3 endpoint (see [How it works](#how-it-works) below). +A Jira-site admin creates an Atlassian OAuth 2.0 (3LO) app and authorizes it on the site. The OAuth token bundle is stored in a per-tenant Secrets Manager secret (`bgagent-jira-oauth-`). When a user adds the trigger label to a Jira issue, Jira fires a webhook to ABCA; the receiver verifies the `X-Hub-Signature` HMAC, dedupes, and async-invokes the processor, which resolves the tenant, looks up the project→repo mapping, enriches the task with the issue's recent comments and screened file attachments (see [Issue context](#issue-context-attachments-and-comments)), and creates a task. Jira-triggered tasks always run the `coding/new-task-v1` workflow (the processor pins `workflow_ref` explicitly, since a label-triggered task always targets a mapped repo). The agent clones the repo, opens a PR, and posts a "started" comment on the Jira issue via the Jira REST v3 API (using the same stored OAuth token). When the task reaches a terminal state, the platform's fan-out plane posts a single final status comment — outcome, cost, turns, duration, and PR link — via the same REST v3 endpoint (see [How it works](#how-it-works) below). **Tenant key.** Everything is indexed on `cloudId` — the Atlassian tenant UUID, *not* the site domain or name. Webhook payloads and the OAuth flow both surface `cloudId`; it is the join key across the project-mapping, user-mapping, and workspace-registry tables. @@ -182,7 +182,7 @@ The teammate needs their own ABCA account first (Cognito user + configured CLI). ### 6. Test -Add the trigger label (`bgagent` by default) to a Jira issue in a mapped project. The agent should start within ~30 seconds, comment on the issue as it works, and post a PR link when ready. The issue **summary** plus the **description** (converted from Atlassian Document Format to markdown) becomes the task description. +Add the trigger label (`bgagent` by default) to a Jira issue in a mapped project. The agent should start within ~30 seconds, comment on the issue as it works, and post a PR link when ready. The issue **summary** plus the **description** (converted from Atlassian Document Format to markdown), the issue's **recent comments**, and any supported **file attachments** become the task context — see [Issue context: attachments and comments](#issue-context-attachments-and-comments). ## How webhook signature verification works @@ -200,6 +200,24 @@ The body must be verified as the *raw unparsed bytes* — never parsed-and-restr - **`jira:issue_updated`** — triggers only if the label was **newly added** in this update. Jira reports label changes in `changelog.items[]` (`field: "labels"`, with `fromString` / `toString`), *not* by re-sending the full label list. The processor diffs the changelog rather than inspecting `issue.fields.labels`, so re-saving an issue that already has the label does not re-trigger. - All other event types get a silent `200`. +## Issue context: attachments and comments + +Beyond the summary and description, the processor enriches the task with the practical context a Jira ticket usually carries — attached files and recent clarifications — so the agent isn't left guessing at "see the attached log" or an acceptance detail buried in a comment. Both are fetched **authenticated at task-admission time** using the tenant's existing `read:jira-work` scope (**no new OAuth scopes, no re-authorization**), because a headless agent can't fetch them itself. + +### File attachments + +Jira-hosted `media` attachments are downloaded through the Jira REST API, run through the **same Bedrock Guardrail content screening** as every other ABCA attachment, and stored for the agent — only after they pass. + +- **Supported types** — images `image/png`, `image/jpeg`; files `text/plain`, `text/csv`, `text/markdown`, `application/json`, `application/pdf`, `text/x-log`. +- **Limits** — at most **10 attachments per task** (shared with any images embedded in the description), **10 MB per file**, **50 MB total**. +- **Unsupported or oversized attachments are skipped silently** — they simply don't reach the agent; the task still runs with the rest of the context. +- **Fail-closed on unsafe content** — if a *selected* attachment can't be safely downloaded or screened (blocked by the guardrail, a content/type mismatch, a download/auth failure, or missing screening configuration), the task is **rejected** with a ❌ comment on the issue rather than run with missing context. Fix or remove the attachment and re-apply the trigger label. +- Embedded HTTPS image URLs in the description continue to work exactly as before. + +### Recent comments + +The most recent **human** comments (up to 10, oldest-first) are folded into the task description under a **Recent comments** heading, each attributed to its author. ABCA's own progress/final-status comments and other app/bot comments are excluded (filtered by Atlassian `accountType`). Comment enrichment is **best-effort / fail-open**: if the fetch fails, the task proceeds without comments (a warning is logged) — comments are advisory context, never a gate. Long comment histories are not fetched in full; only the recent window is included. + ## Board transitions As a Jira-triggered task progresses, the agent moves the originating issue across its workflow so the board reflects reality — the same at-a-glance signal Linear-origin tasks already give: