Skip to content

Commit beef46a

Browse files
ayushtr-awsisadeks
andauthored
feat(jira): include Jira attachments and recent comments in task context (#577) (#619)
* feat(jira): include Jira attachments and recent comments in task context (#577) Jira-origin tasks previously saw only the issue summary + description (ADF→ markdown) plus embedded HTTPS image URLs. Jira-hosted `media` file attachments and issue comments were invisible to the agent, so a ticket that says "see the attached log" or carries acceptance clarifications in comments would run under-informed. Bring Jira to parity with Linear's on-demand context reads by fetching the context authenticated at task-admission time in the webhook processor (the Atlassian Remote MCP can't run headlessly — see #580). - agent-admission download path (cdk/src/handlers/shared/jira-attachments.ts): * downloadScreenAndStoreJiraAttachments — resolve Jira `media` attachments via the gateway attachment-content endpoint (api.atlassian.com/ex/jira/{cloudId}, the only host the 3LO token is valid against), refresh-and-retry-once on 401/403, enforce the size cap while streaming, validate magic bytes, screen through the existing Bedrock Guardrail, upload cleaned bytes to S3, and return `passed` AttachmentRecords. Fail-closed: a selected attachment that can't be safely fetched/screened throws JiraAttachmentError and the task is rejected with a Jira comment. Unsupported/oversized/over-cap attachments are silently skipped. Filenames are sanitized and ids validated to a safe token so neither can traverse the S3 key / agent on-disk path. * fetchRecentHumanComments — recent human (accountType `atlassian`) comments, ADF→markdown, oldest-first. Fail-open: any failure proceeds without them. - create-task-core: TaskCreationContext gains optional `taskId` (so processor- uploaded S3 keys match the eventual record) and `preScreenedAttachments` (merged verbatim, never re-screened; a non-`passed` record fails closed). - jira-webhook-processor: wire both in; comments fold under a "## Recent comments" heading, bounded so they never push task_description past the 10k limit (advisory context must not become a hard gate). - jira-adf.ts: extract the ADF→markdown walker so the processor and the comment helper share it without a circular import (behavior-preserving). - CDK: pass the attachments bucket to JiraIntegration; grant the processor ReadWrite + ATTACHMENTS_BUCKET_NAME env; bump the async processor timeout to 60s for serial download+screen. No new OAuth scopes (read:jira-work covers it). - docs: JIRA_SETUP_GUIDE "Issue context: attachments and comments" (supported types, limits, skip vs fail-closed reject, comment behavior) + Starlight mirror. Tests: new jira-attachments.test.ts (download/filter/screen/upload, sanitize, 401 retry, size cap, comment human/app filter + fail-open); processor tests for comment folding + truncation and fail-closed attachment rejection; create-task- core tests for preScreenedAttachments merge + taskId. `mise run build` green. * fix(jira): exempt /v1/jira/webhook from WAF SizeRestrictions_BODY (#577) Jira issue webhook payloads that carry attachment metadata exceed the AWS managed `SizeRestrictions_BODY` 8 KB limit, so WAF returned 403 at the edge and the webhook receiver Lambda never ran — the delivery vanished before ABCA saw it. This is exactly the case #577 depends on (an issue with file attachments), so without this exemption the feature can't work in any WAF-protected deploy. Mirror the existing `/v1/linear/webhook` and `/v1/github/webhook` handling — both rules must be updated together: - AWSManagedRulesCommonRuleSet-TaskPaths (excludes SizeRestrictions_BODY): add `/v1/jira/webhook` to the orStatement scope-down so large Jira bodies pass. - AWSManagedRulesCommonRuleSet (full CRS for other paths): add a NOT `/v1/jira/webhook` clause so the path isn't re-covered by the strict rule. The receiver still HMAC-verifies the raw body and the priority-4 rate-limit rule still applies, so relaxing the body-size check on this path is safe. Test: task-api.test.ts asserts the exclusion scope-down contains the Jira path and the full-CRS scope-down excludes it. 42 pass. * fix(jira): use Accept: */* when downloading attachment content (#577) The Jira attachment-content endpoint (`/rest/api/3/attachment/content/{id}`) serves the file's own media type and responds 406 Not Acceptable to a narrow `Accept: application/octet-stream`. That made every attachment download fail with HTTP 406, and the fail-closed path then rejected the whole task ("could not be downloaded (HTTP 406)"). Send `Accept: */*` so the gateway serves the real content type. Verified against the live endpoint: octet-stream → 406, */* → 200. Test asserts the download request sets Accept: */*. * fix(jira): address #619 review — orphan cleanup, real-byte cap, comment screening, idempotency (#577) Addresses isadeks's review on PR #619: 1. Zero-byte attachment → JiraAttachmentError (fail-closed). A 0-byte body passed magic-byte + screening then tripped createAttachmentRecord's size guard with a plain Error outside the fail-closed conversion (no ❌ comment). Now rejected explicitly with a Jira comment. 2. 50 MB total-attachment cap enforced on REAL downloaded bytes, not the attacker-declared `size` metadata (which can under-report). Cumulative real bytes tracked in the download loop; throws once over the ceiling. 3. S3 orphan cleanup. downloadScreenAndStoreJiraAttachments now tracks uploaded keys and best-effort-deletes them if the batch throws mid-way; the processor deletes the returned pre-screened objects when createTaskCore returns non-201 (and on a 200 idempotent replay, whose objects the replayed task doesn't reference). New exported cleanupPreScreenedAttachments helper. 4. Comment content screened SEPARATELY and dropped fail-open. Third-party comment text that trips the guardrail no longer fails the reporter's task — screenCommentsOrDrop screens the folded comment block and drops it (task proceeds) on GUARDRAIL_INTERVENED / screening outage / no guardrail. 5. Deterministic idempotency key (`jira-<issueKey>-<webhookTimestamp>`) so an async webhook re-delivery dedupes instead of minting a duplicate task and re-downloading every attachment. A 200 idempotent replay is handled as success (no ❌ comment), cleaning up the round's re-uploaded objects. 6. Webhook-processor timeout 60s → 300s. 10 serial 10s attachment fetches can sum past 60s; a mid-loop kill would orphan objects and force a retry. Tests: +5 (zero-byte reject, real-byte total cap, mid-batch orphan cleanup, comment-blocked fail-open drop, deterministic idempotency key + 200-replay). mise run build green (cdk 2362). --------- Co-authored-by: Sphia Sadek <isadeks@gmail.com>
1 parent 9549473 commit beef46a

13 files changed

Lines changed: 2010 additions & 130 deletions

File tree

cdk/src/constructs/jira-integration.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import * as dynamodb from 'aws-cdk-lib/aws-dynamodb';
2525
import * as iam from 'aws-cdk-lib/aws-iam';
2626
import { Runtime, Architecture } from 'aws-cdk-lib/aws-lambda';
2727
import * as lambda from 'aws-cdk-lib/aws-lambda-nodejs';
28+
import * as s3 from 'aws-cdk-lib/aws-s3';
2829
import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager';
2930
import { NagSuppressions } from 'cdk-nag';
3031
import { Construct } from 'constructs';
@@ -35,8 +36,17 @@ import { JiraWorkspaceRegistryTable } from './jira-workspace-registry-table';
3536
/** Default task-record retention used for TTL computation (days). */
3637
const DEFAULT_TASK_RETENTION_DAYS = 90;
3738

38-
/** Webhook-processor Lambda timeout (seconds). */
39-
const WEBHOOK_PROCESSOR_TIMEOUT_SECONDS = 30;
39+
/**
40+
* Webhook-processor Lambda timeout (seconds). The processor is invoked
41+
* asynchronously (not behind the API Gateway 30s deadline), so it can run
42+
* longer than the receiver. #577 added serial authenticated download +
43+
* Bedrock screening of up to 10 Jira attachments; the per-attachment fetch
44+
* timeout alone (10s) can sum past 60s across a full batch, and a mid-loop
45+
* kill would orphan objects and force an idempotent retry. 300s covers the
46+
* worst-case serial batch with headroom. (A future optimization could
47+
* parallelize the download/screen loop and lower this again.)
48+
*/
49+
const WEBHOOK_PROCESSOR_TIMEOUT_SECONDS = 300;
4050

4151
/**
4252
* Marker key embedded in the auto-generated stack-wide webhook-secret
@@ -87,6 +97,14 @@ export interface JiraIntegrationProps {
8797
/** Bedrock Guardrail version for input screening. */
8898
readonly guardrailVersion?: string;
8999

100+
/**
101+
* S3 bucket for task attachment storage. Required for the webhook processor
102+
* to fetch, screen, and store Jira `media` file attachments at task-admission
103+
* time (issue #577). When omitted, issues carrying supported file attachments
104+
* are rejected with a Jira comment rather than silently dropping them.
105+
*/
106+
readonly attachmentsBucket?: s3.IBucket;
107+
90108
/** Task retention in days for TTL computation. */
91109
readonly taskRetentionDays?: number;
92110

@@ -206,6 +224,9 @@ export class JiraIntegration extends Construct {
206224
createTaskEnv.GUARDRAIL_ID = props.guardrailId;
207225
createTaskEnv.GUARDRAIL_VERSION = props.guardrailVersion;
208226
}
227+
if (props.attachmentsBucket) {
228+
createTaskEnv.ATTACHMENTS_BUCKET_NAME = props.attachmentsBucket.bucketName;
229+
}
209230

210231
// --- Cognito Authorizer (for /jira/link) ---
211232
const cognitoAuthorizer = new apigw.CognitoUserPoolsAuthorizer(this, 'JiraCognitoAuthorizer', {
@@ -280,6 +301,13 @@ export class JiraIntegration extends Construct {
280301
],
281302
}));
282303
}
304+
// The processor downloads Jira `media` attachments, screens them, and
305+
// writes the cleaned bytes to the attachments bucket before creating the
306+
// task (#577). ReadWrite mirrors the confirm-uploads path (Put + Get for
307+
// multipart/versioned writes).
308+
if (props.attachmentsBucket) {
309+
props.attachmentsBucket.grantReadWrite(webhookProcessorFn);
310+
}
283311

284312
// --- Webhook receiver (verifies HMAC, dedups, invokes processor) ---
285313
const webhookFn = new lambda.NodejsFunction(this, 'WebhookFn', {

cdk/src/constructs/task-api.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,18 @@ export class TaskApi extends Construct {
326326
textTransformations: [{ priority: 0, type: 'NONE' }],
327327
},
328328
},
329+
{
330+
// Jira issue payloads include the full issue field set.
331+
// Attachment metadata can push them over 8 KB. The
332+
// receiver HMAC-verifies the raw body and the priority-4
333+
// rule below still rate-limits this route.
334+
byteMatchStatement: {
335+
fieldToMatch: { uriPath: {} },
336+
positionalConstraint: 'EXACTLY',
337+
searchString: '/v1/jira/webhook',
338+
textTransformations: [{ priority: 0, type: 'NONE' }],
339+
},
340+
},
329341
{
330342
// GitHub deployment_status webhook (preview-deploy
331343
// screenshot pipeline). The full payload (workflow run
@@ -376,6 +388,18 @@ export class TaskApi extends Construct {
376388
},
377389
},
378390
},
391+
{
392+
notStatement: {
393+
statement: {
394+
byteMatchStatement: {
395+
fieldToMatch: { uriPath: {} },
396+
positionalConstraint: 'EXACTLY',
397+
searchString: '/v1/jira/webhook',
398+
textTransformations: [{ priority: 0, type: 'NONE' }],
399+
},
400+
},
401+
},
402+
},
379403
{
380404
notStatement: {
381405
statement: {

0 commit comments

Comments
 (0)