From cf4bd6101923ee5e224cd020bf3a5d4e8120c4d4 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:55:20 -0500 Subject: [PATCH 1/4] fix: cloudtrail diagnosis can display control-plane errors outside of the relevant stack --- .../toolkit-lib/lib/actions/diagnose/index.ts | 7 +++ .../diagnosing/cloudtrail-investigation.ts | 13 ++++- .../api/diagnosing/diagnosis-formatting.ts | 7 ++- .../lib/api/diagnosing/stack-diagnoser.ts | 31 +++++------ .../cloudtrail-investigation.test.ts | 54 +++++++++++++++++++ .../diagnosing/diagnosis-formatting.test.ts | 20 +++++++ .../api/diagnosing/stack-diagnoser.test.ts | 47 ++++++++++++++++ 7 files changed, 161 insertions(+), 18 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/diagnose/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/diagnose/index.ts index 7e1936c41..9dc6b4aaf 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/diagnose/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/diagnose/index.ts @@ -111,6 +111,13 @@ export interface AdditionalDiagnosticContext { */ readonly source: string; + /** + * An optional heading rendered above the messages. + * + * @default - no heading + */ + readonly heading?: string; + /** * The log lines or messages retrieved */ diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts index e45941e2c..5faf48525 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts @@ -392,6 +392,12 @@ interface ParsedCloudTrailEvent { * errored calls against stack resources to the stack. */ readonly callerIdentityStrings: string[]; + + /** + * ARN of the resource the caller's credentials were issued to + * (`userIdentity.inScopeOf.credentialsIssuedTo`, e.g. the Lambda function that made the call). + */ + readonly credentialsIssuedTo?: string; } interface SweepResult { @@ -504,10 +510,12 @@ function mostSpecificMatch(event: ParsedCloudTrailEvent, identities: StackIdenti .filter((s): s is string => !!s), ); + const callers = event.credentialsIssuedTo ? [event.credentialsIssuedTo] : event.callerIdentityStrings; + let best: StackIdentity | undefined; let bestRank = -1; for (const identity of identities) { - if (!event.callerIdentityStrings.some((s) => identityMatches(s, identity.key))) { + if (!callers.some((s) => identityMatches(s, identity.key))) { continue; } // Two-tier rank: session-name matches sort above everything else; key length breaks ties. @@ -579,8 +587,8 @@ function parseCloudTrailEvent(json: string | undefined): ParsedCloudTrailEvent | const callerIdentityStrings = [ obj.userIdentity?.arn, obj.userIdentity?.sessionContext?.sessionIssuer?.arn, - obj.userIdentity?.inScopeOf?.credentialsIssuedTo, ].filter((s: any): s is string => typeof s === 'string'); + const credentialsIssuedTo = obj.userIdentity?.inScopeOf?.credentialsIssuedTo; return { eventTime: typeof obj.eventTime === 'string' ? new Date(obj.eventTime) : undefined, eventSource: typeof obj.eventSource === 'string' ? obj.eventSource : undefined, @@ -588,6 +596,7 @@ function parseCloudTrailEvent(json: string | undefined): ParsedCloudTrailEvent | errorCode: typeof obj.errorCode === 'string' ? obj.errorCode : undefined, errorMessage: typeof obj.errorMessage === 'string' ? obj.errorMessage : undefined, callerIdentityStrings, + credentialsIssuedTo: typeof credentialsIssuedTo === 'string' ? credentialsIssuedTo : undefined, }; } catch { return undefined; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/diagnosis-formatting.ts b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/diagnosis-formatting.ts index 1b0e8df24..b19fcfa2b 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/diagnosis-formatting.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/diagnosis-formatting.ts @@ -155,8 +155,13 @@ function formatResourceErrorBlock(group: TracedResourceError[]): string { for (const e of group) { for (const ctx of e.additionalContext ?? []) { lines.push(''); + // Headed context is indented beneath its heading so it reads as one section. + const indent = ctx.heading ? `${CONTEXT_INDENT} ` : CONTEXT_INDENT; + if (ctx.heading) { + lines.push(`${CONTEXT_INDENT}${ctx.heading}`); + } for (const msg of ctx.messages) { - lines.push(`${CONTEXT_INDENT}${msg}`); + lines.push(`${indent}${msg}`); } if (ctx.link) { const leader = ctx.linkLabel ? `${ctx.linkLabel}: ` : ''; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts index e8a4ae3d1..5a54c2c50 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts @@ -16,6 +16,8 @@ import { fetchHookResultDetails, formatHookResultDetails, isFailedHookStatus, no import type { ResourceError, ResourceErrors } from '../stack-events/resource-errors'; import { StackStatus } from '../stack-events/stack-status'; +const CLOUDTRAIL_HEADING = 'CloudTrail control-plane errors:'; + export interface CloudFormationStackDiagnoserProps { readonly sdk: SDK; readonly envResources?: EnvironmentResources; @@ -204,35 +206,34 @@ export class CloudFormationStackDiagnoser { return problems; } - // Contexts whose resource is a reported problem attach to it; the rest are labeled with + // Messages whose resource is a reported problem attach to it; the rest are labeled with // their resource so they don't read as belonging to whatever problem carries them. - const attachable = new Map(); - const stackLevel: AdditionalDiagnosticContext[] = []; + const attachable = new Map(); + const stackLevel: string[] = []; for (const attributed of attributedCloudTrailContexts(investigation)) { const problem = problems.find((p) => p.logicalId === attributed.logicalId && p.stackArn === attributed.stackArn); if (problem) { - attachable.set(problem, [...(attachable.get(problem) ?? []), attributed.context]); + attachable.set(problem, [...(attachable.get(problem) ?? []), ...attributed.context.messages]); } else { - stackLevel.push({ - ...attributed.context, - messages: attributed.context.messages.map((m) => `${attributed.description}: ${m}`), - }); + stackLevel.push(...attributed.context.messages.map((m) => `${attributed.description}: ${m}`)); } } - if (investigation.notes.length > 0) { - stackLevel.push({ source: 'CloudTrail', messages: investigation.notes }); - } + stackLevel.push(...investigation.notes); return problems.map((p, i) => { - const additional = [ + const messages = [ ...(attachable.get(p) ?? []), // Stack-level findings have no problem of their own to live under; report them once, // on the first problem, labeled with the resource they belong to. ...(i === 0 ? stackLevel : []), ]; - return additional.length > 0 - ? { ...p, additionalContext: [...(p.additionalContext ?? []), ...additional] } - : p; + if (messages.length === 0) { + return p; + } + // One headed section per problem, so CloudTrail findings are not mistaken for the + // CloudFormation error above them. + const context: AdditionalDiagnosticContext = { source: 'CloudTrail Errors', heading: CLOUDTRAIL_HEADING, messages }; + return { ...p, additionalContext: [...(p.additionalContext ?? []), context] }; }); } diff --git a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts index 4b96ed2a6..cb592a17c 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts @@ -173,6 +173,60 @@ describe('correlation', () => { expect(result!.errors).toHaveLength(0); }); + test.each([ + ['imported by the stack', `arn:aws:iam::${ACCOUNT}:role/shared-role`, 'shared-role'], + ['created by the stack', { 'Fn::GetAtt': ['CrHandlerRole', 'Arn'] }, CR_ROLE_NAME], + ])('excludes calls by a Lambda outside the stack that shares a role %s', async (_, roleRef, roleName) => { + // Issue #2084: another stack's function runs with a role this stack also references. + // `inScopeOf` names that function as the caller, so the shared role must not claim it. + mockCloudFormationClient.on(GetTemplateCommand).resolves({ + TemplateBody: JSON.stringify({ + Resources: { CrHandler: { Type: 'AWS::Lambda::Function', Properties: { Role: roleRef } } }, + }), + }); + mockCloudTrailClient.on(LookupEventsCommand).resolves({ + Events: [{ + CloudTrailEvent: JSON.stringify({ + eventTime: '2026-06-19T11:59:00Z', + eventSource: 'dynamodb.amazonaws.com', + eventName: 'DescribeTable', + errorCode: 'AccessDenied', + userIdentity: { + arn: `arn:aws:sts::${ACCOUNT}:assumed-role/${roleName}/other-stack-fn`, + sessionContext: { sessionIssuer: { arn: `arn:aws:iam::${ACCOUNT}:role/${roleName}` } }, + inScopeOf: { credentialsIssuedTo: `arn:aws:lambda:us-east-1:${ACCOUNT}:function:other-stack-fn` }, + }, + }), + }], + }); + + const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); + + expect(result!.errors).toHaveLength(0); + }); + + test('attributes calls by a stack Lambda running with an imported role', async () => { + mockCloudFormationClient.on(GetTemplateCommand).resolves({ + TemplateBody: JSON.stringify({ + Resources: { CrHandler: { Type: 'AWS::Lambda::Function', Properties: { Role: `arn:aws:iam::${ACCOUNT}:role/shared-role` } } }, + }), + }); + mockCloudTrailClient.on(LookupEventsCommand).resolves({ + Events: [handlerEvent({ + userIdentity: { + arn: `arn:aws:sts::${ACCOUNT}:assumed-role/shared-role/${CR_FUNCTION_NAME}`, + sessionContext: { sessionIssuer: { arn: `arn:aws:iam::${ACCOUNT}:role/shared-role` } }, + inScopeOf: { credentialsIssuedTo: `arn:aws:lambda:us-east-1:${ACCOUNT}:function:${CR_FUNCTION_NAME}` }, + }, + })], + }); + + const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); + + expect(result!.errors).toHaveLength(1); + expect(result!.errors[0].logicalId).toEqual('CrHandler'); + }); + test('excludes errored calls made by the deploying principal (already in StatusReason)', async () => { // CloudFormation's own denied calls carry no stack identity in userIdentity (and often an // empty resources list); their message is already surfaced via the resource StatusReason. diff --git a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/diagnosis-formatting.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/diagnosis-formatting.test.ts index f1e1e4ebd..b75e50c84 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/diagnosis-formatting.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/diagnosis-formatting.test.ts @@ -98,6 +98,26 @@ describe('hostMessageFromDiagnosis', () => { expect(msg.message).not.toContain('CloudWatch Logs:'); }); + test('renders a context heading above its indented messages', () => { + const msg = hostMessageFromDiagnosis(diagnosedStack('MyStack', { + type: 'problem', + detectedBy: { type: 'deployment', stackStatus: 'CREATE_FAILED', statusReason: 'Resource creation failed' }, + problems: [ + tracedError({ + logicalId: 'MyFunc', + resourceType: 'AWS::Lambda::Function', + message: 'Handler error', + additionalContext: [{ + source: 'CloudTrail Errors', + heading: 'CloudTrail control-plane errors:', + messages: ['AccessDenied on s3.amazonaws.com:CreateBucket'], + }], + }), + ], + })); + expect(msg.message).toContain(' CloudTrail control-plane errors:\n AccessDenied on s3.amazonaws.com:CreateBucket'); + }); + test('deployment failure without resource errors', () => { const msg = hostMessageFromDiagnosis(diagnosedStack('MyStack', { type: 'problem', diff --git a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts index b3bd1b98a..128a470c3 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts @@ -1033,9 +1033,56 @@ describe('CloudFormationStackDiagnoser', () => { const problem = assertProblem(result); const context = problem.problems[0].additionalContext?.find((c) => c.source === 'CloudTrail Errors'); expect(context).toBeDefined(); + expect(context!.heading).toEqual('CloudTrail control-plane errors:'); expect(context!.messages[0]).toMatch(/AccessDenied on s3\.amazonaws\.com:CreateBucket/); }); + test('reports stack-level CloudTrail findings and notes under a single heading', async () => { + // The correlated resource (CrHandlerRole) is not itself a problem, and the failure is + // recent: the labeled finding and the delivery-latency note share one headed section. + jest.useFakeTimers().setSystemTime(FAILURE_TIME.valueOf() + 60 * 1000); + try { + fakeCfn.createStackSync({ StackName: 'MyStack', StackStatus: 'UPDATE_FAILED', StackId: STACK_ARN }); + mockCloudFormationClient.on(DescribeStackResourcesCommand).resolves({ + StackResources: [{ + LogicalResourceId: 'CrHandlerRole', + ResourceType: 'AWS::IAM::Role', + PhysicalResourceId: 'MyStack-CrHandlerRole-abc123', + ResourceStatus: ResourceStatus.CREATE_COMPLETE, + Timestamp: FAILURE_TIME, + }], + }); + mockCloudTrailClient.on(LookupEventsCommand).resolves({ + Events: [{ + CloudTrailEvent: JSON.stringify({ + eventTime: '2026-06-19T11:59:00Z', + eventSource: 'ec2.amazonaws.com', + eventName: 'CreateNetworkInterface', + errorCode: 'Client.UnauthorizedOperation', + userIdentity: { arn: 'arn:aws:sts::123456789012:assumed-role/MyStack-CrHandlerRole-abc123/awslambda_1' }, + }), + }], + }); + + const result = await makeExploringDiagnoser().diagnoseFromErrorCollection(makeErrors(), { + StackName: 'MyStack', + StackStatus: 'UPDATE_FAILED', + CreationTime: new Date(), + }); + + const problem = assertProblem(result); + const contexts = (problem.problems[0].additionalContext ?? []).filter((c) => c.source.startsWith('CloudTrail')); + expect(contexts).toHaveLength(1); + expect(contexts[0].heading).toEqual('CloudTrail control-plane errors:'); + expect(contexts[0].messages).toEqual([ + expect.stringMatching(/^CrHandlerRole \(AWS::IAM::Role\): Client\.UnauthorizedOperation/), + expect.stringMatching(/CloudTrail delivery can lag/), + ]); + } finally { + jest.useRealTimers(); + } + }); + test('does not run the investigation without an exploration SDK', async () => { const result = await makeDiagnoser().diagnoseFromErrorCollection(makeErrors(), { StackName: 'MyStack', From 77eb0db797fbcb51beae978503e7208e8e281aee Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:26:08 -0500 Subject: [PATCH 2/4] fix(toolkit-lib): do not treat roles the stack only references as cloudtrail identities Imported roles may be shared with resources outside the stack, whose calls would then be attributed to it. Roles the stack creates are still matched through their physical IDs, and stack resources running on an imported role still correlate through their own identity. --- .../diagnosing/cloudtrail-investigation.ts | 83 ++++++------------- .../cloudtrail-investigation.test.ts | 36 ++++++-- 2 files changed, 51 insertions(+), 68 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts index 5faf48525..1ae6c7cdc 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts @@ -26,10 +26,15 @@ import type { ResourceError } from '../stack-events/resource-errors'; * calls use unpredictable assumed-role session names, so there is no per-resource lookup key * we could compute. Instead we do one unfiltered sweep over the failure window and correlate * client-side: an event belongs to the stack when its *calling principal* carries a known - * stack identity (a physical ID, an IAM role referenced by resource properties, or a - * custom-resource backing function) as a whole ARN segment. Matching only the caller — never - * the resources a call touched — keeps unrelated principals' activity against stack resources - * out; a missed correlation degrades to "no events found", not a wrong attribution. + * stack identity (a physical ID, including the stack's own IAM roles, or a custom-resource + * backing function) as a whole ARN segment. Matching only the caller — never the resources a + * call touched — keeps unrelated principals' activity against stack resources out; a missed + * correlation degrades to "no events found", not a wrong attribution. + * + * Roles the stack merely references (imported by ARN or name) are deliberately not + * identities: they may be shared with resources outside the stack, whose calls would then be + * attributed to it. Calls made with such a role still correlate when the caller itself is a + * stack resource (e.g. a stack Lambda, by its session name). */ /** @@ -269,14 +274,10 @@ export function attributedCloudTrailContexts(investigation: CloudTrailInvestigat * Build the set of identities that tie CloudTrail events back to this stack: * * - physical IDs of all stack resources (a Lambda's role-session name is its function name; - * role physical IDs appear in assumed-role session ARNs); - * - IAM roles referenced by resource properties named `Role`/`RoleArn`/`ServiceToken` at any - * depth in the template (services acting on a resource's behalf assume these; the role - * *name* — not ARN — is embedded in the resulting session ARN, so bare names are indexed - * too). + * the stack's own role physical IDs appear in assumed-role session ARNs); + * - the backing functions of custom resources, from their top-level `ServiceToken`. * - * Best-effort: if the resource listing fails, template-derived identities (e.g. a literal - * ServiceToken ARN) are still collected. + * Best-effort: if the resource listing fails, a literal ServiceToken ARN is still collected. */ async function buildStackIdentitySet( cfn: ICloudFormationClient, @@ -305,26 +306,17 @@ async function buildStackIdentitySet( } } } catch (e: any) { - // Not fatal: template-derived identities below don't need the physical IDs (literal - // ARNs/names resolve on their own; only Ref/GetAtt references do). + // Not fatal: a literal ServiceToken below resolves without the physical IDs (only + // Ref/GetAtt references need them). await debug(`CloudTrail investigation: failed to list stack resources: ${e.message}`); } const template = await getStackTemplate(cfn, stackArn, debug); for (const [logicalId, resource] of Object.entries(template?.Resources ?? {})) { - for (const { propName, value } of collectPrincipalProperties(resource?.Properties)) { - const resolved = resolveToIdentity(value, physicalIdByLogicalId); - if (!resolved) { - continue; - } - const description = `${logicalId} (via ${propName})`; - if (propName === 'ServiceToken') { - // The backing function's role-session name is the function name. - add(functionNameFromArnOrName(resolved), logicalId, description); - } else { - add(resolved, logicalId, description); - add(nameSegment(resolved), logicalId, description); - } + const resolved = resolveToIdentity(resource?.Properties?.ServiceToken, physicalIdByLogicalId); + if (resolved) { + // The backing function's role-session name is the function name. + add(functionNameFromArnOrName(resolved), logicalId, `${logicalId} (via ServiceToken)`); } } @@ -332,29 +324,10 @@ async function buildStackIdentitySet( } /** - * Collect properties that reference a calling principal — `Role`/`RoleArn` (any depth, since - * many resources nest role references, e.g. Firehose destination configurations or - * EventBridge targets) and the top-level `ServiceToken` of custom resources. - */ -function collectPrincipalProperties(props: any, depth = 0): Array<{ propName: string; value: any }> { - if (!props || typeof props !== 'object' || depth > 10) { - return []; - } - const ret: Array<{ propName: string; value: any }> = []; - for (const [key, value] of Object.entries(props)) { - if (/Role(Arn)?$/i.test(key) || key === 'ServiceToken') { - ret.push({ propName: key, value }); - } - ret.push(...collectPrincipalProperties(value, depth + 1)); - } - return ret; -} - -/** - * Resolve a template property value to a matchable identity string: literals pass through, - * `Ref`/`Fn::GetAtt` resolve via the resource's physical ID, and an `Fn::Sub` role ARN - * yields its role name when that segment is literal. Unresolvable expressions (e.g. - * `Fn::ImportValue`) return `undefined` — a missed identity degrades to "no events found". + * Resolve a template property value to a matchable identity string: literals (and `Fn::Sub` + * without substitutions) pass through, `Ref`/`Fn::GetAtt` resolve via the resource's + * physical ID. Unresolvable expressions (e.g. `Fn::ImportValue`) return `undefined` — a + * missed identity degrades to "no events found". */ function resolveToIdentity(value: any, physicalIdByLogicalId: Map): string | undefined { if (typeof value === 'string') { @@ -365,16 +338,8 @@ function resolveToIdentity(value: any, physicalIdByLogicalId: Map' — the name segment - // is all we need for matching, and it is often free of substitutions. - const name = nameSegment(sub); - if (name && !name.includes('${')) { - return name; - } + if (typeof sub === 'string' && !sub.includes('${')) { + return sub; } return undefined; } diff --git a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts index cb592a17c..04044ec26 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts @@ -227,6 +227,27 @@ describe('correlation', () => { expect(result!.errors[0].logicalId).toEqual('CrHandler'); }); + test('attributes calls by a stack Lambda on an imported role via its session name, without inScopeOf', async () => { + mockCloudFormationClient.on(GetTemplateCommand).resolves({ + TemplateBody: JSON.stringify({ + Resources: { CrHandler: { Type: 'AWS::Lambda::Function', Properties: { Role: `arn:aws:iam::${ACCOUNT}:role/shared-role` } } }, + }), + }); + mockCloudTrailClient.on(LookupEventsCommand).resolves({ + Events: [handlerEvent({ + userIdentity: { + arn: `arn:aws:sts::${ACCOUNT}:assumed-role/shared-role/${CR_FUNCTION_NAME}`, + sessionContext: { sessionIssuer: { arn: `arn:aws:iam::${ACCOUNT}:role/shared-role` } }, + }, + })], + }); + + const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); + + expect(result!.errors).toHaveLength(1); + expect(result!.errors[0].logicalId).toEqual('CrHandler'); + }); + test('excludes errored calls made by the deploying principal (already in StatusReason)', async () => { // CloudFormation's own denied calls carry no stack identity in userIdentity (and often an // empty resources list); their message is already surfaced via the resource StatusReason. @@ -318,10 +339,9 @@ describe('correlation', () => { expect(result!.errors).toHaveLength(0); }); - test('correlates roles referenced by nested template properties', async () => { - // Role references are often nested (Firehose destination configs, EventBridge targets); - // the identity scan must find them at any depth. The role here is NOT a stack resource - // (imported by ARN), so only the template scan can index it. + test('does not correlate via a role the stack only imports, even when nested', async () => { + // An imported role may be shared with resources outside the stack (issue #2084), so a + // service session on it cannot be told apart from another stack's use of the same role. mockCloudFormationClient.on(GetTemplateCommand).resolves({ TemplateBody: JSON.stringify({ Resources: { @@ -354,11 +374,10 @@ describe('correlation', () => { const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); - expect(result!.errors).toHaveLength(1); - expect(result!.errors[0].logicalId).toEqual('DeliveryStream'); + expect(result!.errors).toHaveLength(0); }); - test('correlates a role expressed as Fn::Sub with a literal name segment', async () => { + test('does not correlate via an imported role expressed as Fn::Sub', async () => { mockCloudFormationClient.on(GetTemplateCommand).resolves({ TemplateBody: JSON.stringify({ Resources: { @@ -386,8 +405,7 @@ describe('correlation', () => { const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); - expect(result!.errors).toHaveLength(1); - expect(result!.errors[0].logicalId).toEqual('Project'); + expect(result!.errors).toHaveLength(0); }); test('still collects template-derived identities when DescribeStackResources fails', async () => { From eddc6b80a3f5af192151aa89cccaedcbeae83093 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:42:50 -0500 Subject: [PATCH 3/4] revert(toolkit-lib): drop the cloudtrail findings heading Keeps this change focused on attribution; the heading also changed the shape of the diagnosis result for programmatic consumers. --- .../toolkit-lib/lib/actions/diagnose/index.ts | 7 --- .../api/diagnosing/diagnosis-formatting.ts | 7 +-- .../lib/api/diagnosing/stack-diagnoser.ts | 31 ++++++------ .../diagnosing/diagnosis-formatting.test.ts | 20 -------- .../api/diagnosing/stack-diagnoser.test.ts | 47 ------------------- 5 files changed, 16 insertions(+), 96 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/actions/diagnose/index.ts b/packages/@aws-cdk/toolkit-lib/lib/actions/diagnose/index.ts index 9dc6b4aaf..7e1936c41 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/actions/diagnose/index.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/actions/diagnose/index.ts @@ -111,13 +111,6 @@ export interface AdditionalDiagnosticContext { */ readonly source: string; - /** - * An optional heading rendered above the messages. - * - * @default - no heading - */ - readonly heading?: string; - /** * The log lines or messages retrieved */ diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/diagnosis-formatting.ts b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/diagnosis-formatting.ts index b19fcfa2b..1b0e8df24 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/diagnosis-formatting.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/diagnosis-formatting.ts @@ -155,13 +155,8 @@ function formatResourceErrorBlock(group: TracedResourceError[]): string { for (const e of group) { for (const ctx of e.additionalContext ?? []) { lines.push(''); - // Headed context is indented beneath its heading so it reads as one section. - const indent = ctx.heading ? `${CONTEXT_INDENT} ` : CONTEXT_INDENT; - if (ctx.heading) { - lines.push(`${CONTEXT_INDENT}${ctx.heading}`); - } for (const msg of ctx.messages) { - lines.push(`${indent}${msg}`); + lines.push(`${CONTEXT_INDENT}${msg}`); } if (ctx.link) { const leader = ctx.linkLabel ? `${ctx.linkLabel}: ` : ''; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts index 5a54c2c50..e8a4ae3d1 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts @@ -16,8 +16,6 @@ import { fetchHookResultDetails, formatHookResultDetails, isFailedHookStatus, no import type { ResourceError, ResourceErrors } from '../stack-events/resource-errors'; import { StackStatus } from '../stack-events/stack-status'; -const CLOUDTRAIL_HEADING = 'CloudTrail control-plane errors:'; - export interface CloudFormationStackDiagnoserProps { readonly sdk: SDK; readonly envResources?: EnvironmentResources; @@ -206,34 +204,35 @@ export class CloudFormationStackDiagnoser { return problems; } - // Messages whose resource is a reported problem attach to it; the rest are labeled with + // Contexts whose resource is a reported problem attach to it; the rest are labeled with // their resource so they don't read as belonging to whatever problem carries them. - const attachable = new Map(); - const stackLevel: string[] = []; + const attachable = new Map(); + const stackLevel: AdditionalDiagnosticContext[] = []; for (const attributed of attributedCloudTrailContexts(investigation)) { const problem = problems.find((p) => p.logicalId === attributed.logicalId && p.stackArn === attributed.stackArn); if (problem) { - attachable.set(problem, [...(attachable.get(problem) ?? []), ...attributed.context.messages]); + attachable.set(problem, [...(attachable.get(problem) ?? []), attributed.context]); } else { - stackLevel.push(...attributed.context.messages.map((m) => `${attributed.description}: ${m}`)); + stackLevel.push({ + ...attributed.context, + messages: attributed.context.messages.map((m) => `${attributed.description}: ${m}`), + }); } } - stackLevel.push(...investigation.notes); + if (investigation.notes.length > 0) { + stackLevel.push({ source: 'CloudTrail', messages: investigation.notes }); + } return problems.map((p, i) => { - const messages = [ + const additional = [ ...(attachable.get(p) ?? []), // Stack-level findings have no problem of their own to live under; report them once, // on the first problem, labeled with the resource they belong to. ...(i === 0 ? stackLevel : []), ]; - if (messages.length === 0) { - return p; - } - // One headed section per problem, so CloudTrail findings are not mistaken for the - // CloudFormation error above them. - const context: AdditionalDiagnosticContext = { source: 'CloudTrail Errors', heading: CLOUDTRAIL_HEADING, messages }; - return { ...p, additionalContext: [...(p.additionalContext ?? []), context] }; + return additional.length > 0 + ? { ...p, additionalContext: [...(p.additionalContext ?? []), ...additional] } + : p; }); } diff --git a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/diagnosis-formatting.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/diagnosis-formatting.test.ts index b75e50c84..f1e1e4ebd 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/diagnosis-formatting.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/diagnosis-formatting.test.ts @@ -98,26 +98,6 @@ describe('hostMessageFromDiagnosis', () => { expect(msg.message).not.toContain('CloudWatch Logs:'); }); - test('renders a context heading above its indented messages', () => { - const msg = hostMessageFromDiagnosis(diagnosedStack('MyStack', { - type: 'problem', - detectedBy: { type: 'deployment', stackStatus: 'CREATE_FAILED', statusReason: 'Resource creation failed' }, - problems: [ - tracedError({ - logicalId: 'MyFunc', - resourceType: 'AWS::Lambda::Function', - message: 'Handler error', - additionalContext: [{ - source: 'CloudTrail Errors', - heading: 'CloudTrail control-plane errors:', - messages: ['AccessDenied on s3.amazonaws.com:CreateBucket'], - }], - }), - ], - })); - expect(msg.message).toContain(' CloudTrail control-plane errors:\n AccessDenied on s3.amazonaws.com:CreateBucket'); - }); - test('deployment failure without resource errors', () => { const msg = hostMessageFromDiagnosis(diagnosedStack('MyStack', { type: 'problem', diff --git a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts index 128a470c3..b3bd1b98a 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts @@ -1033,56 +1033,9 @@ describe('CloudFormationStackDiagnoser', () => { const problem = assertProblem(result); const context = problem.problems[0].additionalContext?.find((c) => c.source === 'CloudTrail Errors'); expect(context).toBeDefined(); - expect(context!.heading).toEqual('CloudTrail control-plane errors:'); expect(context!.messages[0]).toMatch(/AccessDenied on s3\.amazonaws\.com:CreateBucket/); }); - test('reports stack-level CloudTrail findings and notes under a single heading', async () => { - // The correlated resource (CrHandlerRole) is not itself a problem, and the failure is - // recent: the labeled finding and the delivery-latency note share one headed section. - jest.useFakeTimers().setSystemTime(FAILURE_TIME.valueOf() + 60 * 1000); - try { - fakeCfn.createStackSync({ StackName: 'MyStack', StackStatus: 'UPDATE_FAILED', StackId: STACK_ARN }); - mockCloudFormationClient.on(DescribeStackResourcesCommand).resolves({ - StackResources: [{ - LogicalResourceId: 'CrHandlerRole', - ResourceType: 'AWS::IAM::Role', - PhysicalResourceId: 'MyStack-CrHandlerRole-abc123', - ResourceStatus: ResourceStatus.CREATE_COMPLETE, - Timestamp: FAILURE_TIME, - }], - }); - mockCloudTrailClient.on(LookupEventsCommand).resolves({ - Events: [{ - CloudTrailEvent: JSON.stringify({ - eventTime: '2026-06-19T11:59:00Z', - eventSource: 'ec2.amazonaws.com', - eventName: 'CreateNetworkInterface', - errorCode: 'Client.UnauthorizedOperation', - userIdentity: { arn: 'arn:aws:sts::123456789012:assumed-role/MyStack-CrHandlerRole-abc123/awslambda_1' }, - }), - }], - }); - - const result = await makeExploringDiagnoser().diagnoseFromErrorCollection(makeErrors(), { - StackName: 'MyStack', - StackStatus: 'UPDATE_FAILED', - CreationTime: new Date(), - }); - - const problem = assertProblem(result); - const contexts = (problem.problems[0].additionalContext ?? []).filter((c) => c.source.startsWith('CloudTrail')); - expect(contexts).toHaveLength(1); - expect(contexts[0].heading).toEqual('CloudTrail control-plane errors:'); - expect(contexts[0].messages).toEqual([ - expect.stringMatching(/^CrHandlerRole \(AWS::IAM::Role\): Client\.UnauthorizedOperation/), - expect.stringMatching(/CloudTrail delivery can lag/), - ]); - } finally { - jest.useRealTimers(); - } - }); - test('does not run the investigation without an exploration SDK', async () => { const result = await makeDiagnoser().diagnoseFromErrorCollection(makeErrors(), { StackName: 'MyStack', From 7623ac06f5e0501d8c8cea37e09fe29103e420c8 Mon Sep 17 00:00:00 2001 From: Ian Hou <45278651+iankhou@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:47:22 -0500 Subject: [PATCH 4/4] fix(toolkit-lib): build cloudtrail identities from the stack and its events, not the template CloudTrail identities are now the roles and Lambda functions in the stack (DescribeStackResources) plus those seen in the operation's stack events, which covers resources that were created and then rolled back. Other resource types and the template are no longer used, so imported roles, shared ServiceToken providers and short physical IDs cannot claim unrelated events. The suggestion to re-run `cdk diagnose` is dropped once CloudTrail has delivered any event from after the sweep window. --- .../diagnosing/cloudtrail-investigation.ts | 115 ++++++------ .../lib/api/diagnosing/stack-diagnoser.ts | 6 +- .../lib/api/stack-events/resource-errors.ts | 56 ++++++ .../cloudtrail-investigation.test.ts | 170 +++++++++--------- .../api/diagnosing/stack-diagnoser.test.ts | 45 +++++ .../api/stack-events/resource-errors.test.ts | 37 ++++ 6 files changed, 289 insertions(+), 140 deletions(-) diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts index 1ae6c7cdc..ef718e4f0 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/cloudtrail-investigation.ts @@ -1,16 +1,10 @@ -import { - assumedRoleSessionName, - functionNameFromArnOrName, - identityMatches, - nameSegment, - serviceTokenReferencedLogicalId, -} from './resource-identifiers'; -import { getStackTemplate } from './stack-template'; +import { assumedRoleSessionName, identityMatches, nameSegment } from './resource-identifiers'; import type { AdditionalDiagnosticContext } from '../../actions/diagnose'; import type { ICloudFormationClient, ICloudTrailClient, SDK } from '../aws-auth/sdk'; import { isAccessDeniedError } from '../aws-auth/util'; import type { IoHelper } from '../io/private/io-helper'; -import type { ResourceError } from '../stack-events/resource-errors'; +import type { OperationPrincipal, ResourceError } from '../stack-events/resource-errors'; +import { PRINCIPAL_RESOURCE_TYPES } from '../stack-events/resource-errors'; /** * Stack-level CloudTrail investigation. @@ -26,8 +20,8 @@ import type { ResourceError } from '../stack-events/resource-errors'; * calls use unpredictable assumed-role session names, so there is no per-resource lookup key * we could compute. Instead we do one unfiltered sweep over the failure window and correlate * client-side: an event belongs to the stack when its *calling principal* carries a known - * stack identity (a physical ID, including the stack's own IAM roles, or a custom-resource - * backing function) as a whole ARN segment. Matching only the caller — never the resources a + * stack identity (the physical ID of a role or Lambda function the stack operation involved) + * as a whole ARN segment. Matching only the caller — never the resources a * call touched — keeps unrelated principals' activity against stack resources out; a missed * correlation degrades to "no events found", not a wrong attribution. * @@ -164,6 +158,7 @@ export async function investigateStackViaCloudTrail( sdk: SDK, errors: readonly ResourceError[], ioHelper: IoHelper, + operationPrincipals: readonly OperationPrincipal[] = [], ): Promise { const debug = (msg: string) => ioHelper.defaults.debug(msg); @@ -183,7 +178,7 @@ export async function investigateStackViaCloudTrail( const cfn = sdk.cloudFormation(); const identities: StackIdentity[] = []; for (const stackArn of new Set(errors.map((e) => e.stackArn).filter((s) => !!s))) { - identities.push(...await buildStackIdentitySet(cfn, stackArn, debug)); + identities.push(...await buildStackIdentitySet(cfn, stackArn, operationPrincipals, debug)); } if (identities.length === 0) { await debug('CloudTrail investigation: no stack identities to correlate against'); @@ -206,8 +201,9 @@ export async function investigateStackViaCloudTrail( notes.push(`(${omitted} more distinct CloudTrail error event(s) not shown)`); } // Delivery latency applies whether or not something was already found: the fatal event may - // still be in flight while an earlier, benign error has already been delivered. - if (Date.now() - Math.max(...timestamps) < DELIVERY_LATENCY_MS) { + // still be in flight while an earlier, benign error has already been delivered. Once + // CloudTrail has delivered any event from after the window, assume the window is complete. + if (Date.now() - Math.max(...timestamps) < DELIVERY_LATENCY_MS && !await deliveredPast(sdk.cloudTrail(), windowEnd, ioHelper)) { notes.push( 'CloudTrail delivery can lag up to 15 minutes behind the failure; ' + 're-run `cdk diagnose` in a few minutes to check for additional control-plane errors (e.g. AccessDenied).', @@ -271,79 +267,67 @@ export function attributedCloudTrailContexts(investigation: CloudTrailInvestigat } /** - * Build the set of identities that tie CloudTrail events back to this stack: + * Build the set of identities that tie CloudTrail events back to this stack: the physical + * IDs of the roles and Lambda functions involved in the stack operation. * - * - physical IDs of all stack resources (a Lambda's role-session name is its function name; - * the stack's own role physical IDs appear in assumed-role session ARNs); - * - the backing functions of custom resources, from their top-level `ServiceToken`. + * - A role appears in the session ARN of whoever assumes it (a resource's own code, or a + * service acting on the resource's behalf with a generated session name). + * - A Lambda function is the session name, and `inScopeOf` resource, of calls its code + * makes; this also covers custom-resource handlers in the stack. * - * Best-effort: if the resource listing fails, a literal ServiceToken ARN is still collected. + * The set is the union of the roles and functions currently in the stack and those seen in + * the operation's stack events. The latter include resources the operation created and then + * rolled back, which no longer show up in the resource listing. Together they are an upper + * bound on the stack's own principals during the operation. + * + * Roles the stack merely references (imported) never appear in either source, so they are + * not identities. Best-effort: if the resource listing fails, the event-derived principals + * are still used. */ async function buildStackIdentitySet( cfn: ICloudFormationClient, stackArn: string, + operationPrincipals: readonly OperationPrincipal[], debug: (msg: string) => Promise, ): Promise { const identities: StackIdentity[] = []; const seen = new Set(); - const add = (key: string | undefined, logicalId: string, description: string) => { + const add = (key: string | undefined, logicalId: string, resourceType: string) => { if (key && key.length >= MIN_IDENTITY_KEY_LENGTH && !seen.has(key)) { seen.add(key); - identities.push({ key, stackArn, logicalId, description }); + identities.push({ key, stackArn, logicalId, description: `${logicalId} (${resourceType})` }); + } + }; + const addPrincipal = (logicalId: string, resourceType: string, physicalId: string) => { + if (!PRINCIPAL_RESOURCE_TYPES.includes(resourceType)) { + return; } + add(physicalId, logicalId, resourceType); + // ARN-shaped physical IDs also get their trailing name segment indexed: session ARNs + // embed names, not ARNs. + add(nameSegment(physicalId), logicalId, resourceType); }; - const physicalIdByLogicalId = new Map(); try { const resp = await cfn.describeStackResources({ StackName: stackArn }); for (const r of resp.StackResources ?? []) { - if (r.LogicalResourceId && r.PhysicalResourceId) { - physicalIdByLogicalId.set(r.LogicalResourceId, r.PhysicalResourceId); - add(r.PhysicalResourceId, r.LogicalResourceId, `${r.LogicalResourceId} (${r.ResourceType})`); - // ARN-shaped physical IDs also get their trailing name segment indexed: session ARNs - // embed names, not ARNs. - add(nameSegment(r.PhysicalResourceId), r.LogicalResourceId, `${r.LogicalResourceId} (${r.ResourceType})`); + if (r.LogicalResourceId && r.PhysicalResourceId && r.ResourceType) { + addPrincipal(r.LogicalResourceId, r.ResourceType, r.PhysicalResourceId); } } } catch (e: any) { - // Not fatal: a literal ServiceToken below resolves without the physical IDs (only - // Ref/GetAtt references need them). await debug(`CloudTrail investigation: failed to list stack resources: ${e.message}`); } - const template = await getStackTemplate(cfn, stackArn, debug); - for (const [logicalId, resource] of Object.entries(template?.Resources ?? {})) { - const resolved = resolveToIdentity(resource?.Properties?.ServiceToken, physicalIdByLogicalId); - if (resolved) { - // The backing function's role-session name is the function name. - add(functionNameFromArnOrName(resolved), logicalId, `${logicalId} (via ServiceToken)`); + for (const p of operationPrincipals) { + if (p.stackArn === stackArn) { + addPrincipal(p.logicalId, p.resourceType, p.physicalId); } } return identities; } -/** - * Resolve a template property value to a matchable identity string: literals (and `Fn::Sub` - * without substitutions) pass through, `Ref`/`Fn::GetAtt` resolve via the resource's - * physical ID. Unresolvable expressions (e.g. `Fn::ImportValue`) return `undefined` — a - * missed identity degrades to "no events found". - */ -function resolveToIdentity(value: any, physicalIdByLogicalId: Map): string | undefined { - if (typeof value === 'string') { - return value; - } - const referenced = serviceTokenReferencedLogicalId(value); - if (referenced) { - return physicalIdByLogicalId.get(referenced); - } - const sub = value?.['Fn::Sub']; - if (typeof sub === 'string' && !sub.includes('${')) { - return sub; - } - return undefined; -} - interface ParsedCloudTrailEvent { readonly eventTime?: Date; readonly eventSource?: string; @@ -434,6 +418,23 @@ async function sweepCloudTrail( return { events, eventsChecked, truncated }; } +/** + * Whether CloudTrail has delivered any event that happened at or after `time`. + * + * CloudTrail does not deliver events in strict `eventTime` order, so this is a heuristic for + * "events up to `time` have been delivered", not a guarantee. A failed probe answers `false`: + * the hint to re-run is the safe default. + */ +async function deliveredPast(cloudTrail: ICloudTrailClient, time: Date, ioHelper: IoHelper): Promise { + try { + const resp = await cloudTrail.lookupEvents({ StartTime: time, EndTime: new Date(), MaxResults: 1 }); + return (resp.Events ?? []).length > 0; + } catch (e: any) { + await ioHelper.defaults.debug(`CloudTrail investigation: delivery probe failed: ${e.message}`); + return false; + } +} + /** * Attribute errored events to stack resources. * diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts index e8a4ae3d1..b56b88e5f 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/diagnosing/stack-diagnoser.ts @@ -172,7 +172,7 @@ export class CloudFormationStackDiagnoser { stackStatus: stack.StackStatus ?? '', statusReason: stack.StackStatusReason ?? '', }, - await this.addCloudTrailContext(await this.enhanceErrors(errors.all), errors.all), + await this.addCloudTrailContext(await this.enhanceErrors(errors.all), errors), ); } @@ -187,7 +187,7 @@ export class CloudFormationStackDiagnoser { * level. When run right after a deployment, the events may not be delivered yet, in which * case the output notifies the user. */ - private async addCloudTrailContext(problems: TracedResourceError[], errs: readonly ResourceError[]): Promise { + private async addCloudTrailContext(problems: TracedResourceError[], errors: ResourceErrors): Promise { const sdk = await this.additionalExplorationSdk(); if (!sdk || problems.length === 0) { return problems; @@ -195,7 +195,7 @@ export class CloudFormationStackDiagnoser { let investigation; try { - investigation = await investigateStackViaCloudTrail(sdk, errs, this.props.ioHelper); + investigation = await investigateStackViaCloudTrail(sdk, errors.all, this.props.ioHelper, errors.principals); } catch (e: any) { await this.props.ioHelper.defaults.debug(`CloudTrail investigation failed: ${e.message}`); return problems; diff --git a/packages/@aws-cdk/toolkit-lib/lib/api/stack-events/resource-errors.ts b/packages/@aws-cdk/toolkit-lib/lib/api/stack-events/resource-errors.ts index b7a807ff3..3ff879454 100644 --- a/packages/@aws-cdk/toolkit-lib/lib/api/stack-events/resource-errors.ts +++ b/packages/@aws-cdk/toolkit-lib/lib/api/stack-events/resource-errors.ts @@ -101,6 +101,38 @@ export interface ResourceError { readonly warnings?: string[]; } +/** + * A stack resource that can act as an AWS principal, seen during a stack operation. + */ +export interface OperationPrincipal { + /** + * The stack the resource belongs to (a stack ID, not a name) + */ + readonly stackArn: string; + + /** + * Logical ID of the resource + */ + readonly logicalId: string; + + /** + * Resource type + */ + readonly resourceType: string; + + /** + * Physical ID of the resource (a role or function name) + */ + readonly physicalId: string; +} + +/** + * Resource types whose physical IDs identify the caller of an AWS API call in CloudTrail: + * a role appears in the session ARN of whoever assumes it, and a Lambda function is the + * session name (and `inScopeOf` resource) of calls made by its code. + */ +export const PRINCIPAL_RESOURCE_TYPES = ['AWS::IAM::Role', 'AWS::Lambda::Function']; + /** * Class used to send stack event errors into, to come up with root causes. */ @@ -122,6 +154,12 @@ export class ResourceErrors { */ private readonly seenHookFailures = new Map(); + /** + * Principal-capable resources seen in any event of the operation, keyed by stack ARN + + * logical ID + physical ID. + */ + private readonly seenPrincipals = new Map(); + public isEmpty() { return this._errors.length === 0; } @@ -130,6 +168,15 @@ export class ResourceErrors { return this._errors; } + /** + * Every role and Lambda function that appeared in an event of the operation, including + * ones that were created and have since been rolled back (and so no longer appear in the + * stack's resource list), and ones replaced by the update. + */ + public get principals(): ReadonlyArray { + return [...this.seenPrincipals.values()]; + } + /** * Update the error collection with the given stack activity * @@ -140,6 +187,7 @@ export class ResourceErrors { public update(...events: ResourceEvent[]) { for (const event of events) { this.trackHookFailure(event.event); + this.trackPrincipal(event.event); if (isErrorEvent(event.event)) { // Cancelled is not an interesting failure reason, nor is the stack message (stack // message will just say something like "stack failed to update") @@ -150,6 +198,14 @@ export class ResourceErrors { } } + private trackPrincipal(event: StackEvent) { + const { StackId: stackArn, LogicalResourceId: logicalId, ResourceType: resourceType, PhysicalResourceId: physicalId } = event; + if (!stackArn || !logicalId || !physicalId || !resourceType || !PRINCIPAL_RESOURCE_TYPES.includes(resourceType)) { + return; + } + this.seenPrincipals.set(`${stackArn}|${logicalId}|${physicalId}`, { stackArn, logicalId, resourceType, physicalId }); + } + private trackHookFailure(event: StackEvent) { if (!isFailedHookStatus(event.HookStatus) || !event.HookType || !event.LogicalResourceId || !event.StackId) { return; diff --git a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts index 04044ec26..bc52c93cc 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/cloudtrail-investigation.test.ts @@ -1,8 +1,8 @@ import type { StackResource } from '@aws-sdk/client-cloudformation'; -import { DescribeStackResourcesCommand, GetTemplateCommand, ResourceStatus } from '@aws-sdk/client-cloudformation'; +import { DescribeStackResourcesCommand, ResourceStatus } from '@aws-sdk/client-cloudformation'; import { LookupEventsCommand } from '@aws-sdk/client-cloudtrail'; import { investigateStackViaCloudTrail } from '../../../lib/api/diagnosing/cloudtrail-investigation'; -import type { ResourceError } from '../../../lib/api/stack-events/resource-errors'; +import type { OperationPrincipal, ResourceError } from '../../../lib/api/stack-events/resource-errors'; import { mockCloudFormationClient, mockCloudTrailClient, @@ -42,18 +42,6 @@ beforeEach(() => { resource('MyCustomResource', 'Custom::MyThing', '2026/06/19/[$LATEST]abc123'), ], }); - mockCloudFormationClient.on(GetTemplateCommand).resolves({ - TemplateBody: JSON.stringify({ - Resources: { - CrHandler: { Type: 'AWS::Lambda::Function', Properties: { Role: { 'Fn::GetAtt': ['CrHandlerRole', 'Arn'] } } }, - VpcFn: { Type: 'AWS::Lambda::Function', Properties: { Role: { 'Fn::GetAtt': ['VpcFnRole', 'Arn'] } } }, - MyCustomResource: { - Type: 'Custom::MyThing', - Properties: { ServiceToken: { 'Fn::GetAtt': ['CrHandler', 'Arn'] } }, - }, - }, - }), - }); }); afterEach(() => { @@ -139,12 +127,13 @@ function unrelatedEvent() { } describe('correlation', () => { - test('attributes a handler AccessDenied to the custom resource via the role-session ARN', async () => { + test('attributes a custom-resource handler\'s AccessDenied to the handler function', async () => { mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [handlerEvent(), unrelatedEvent()] }); const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); expect(result!.errors).toHaveLength(1); + expect(result!.errors[0].logicalId).toEqual('CrHandler'); expect(result!.errors[0].message).toMatch(/AccessDenied on s3\.amazonaws\.com:CreateBucket/); expect(result!.errors[0].message).toMatch(/not authorized to perform: s3:CreateBucket/); }); @@ -174,16 +163,11 @@ describe('correlation', () => { }); test.each([ - ['imported by the stack', `arn:aws:iam::${ACCOUNT}:role/shared-role`, 'shared-role'], - ['created by the stack', { 'Fn::GetAtt': ['CrHandlerRole', 'Arn'] }, CR_ROLE_NAME], - ])('excludes calls by a Lambda outside the stack that shares a role %s', async (_, roleRef, roleName) => { - // Issue #2084: another stack's function runs with a role this stack also references. + ['imported by the stack', 'shared-role'], + ['created by the stack', CR_ROLE_NAME], + ])('excludes calls by a Lambda outside the stack that shares a role %s', async (_, roleName) => { + // Issue #2084: another stack's function runs with a role this stack also uses. // `inScopeOf` names that function as the caller, so the shared role must not claim it. - mockCloudFormationClient.on(GetTemplateCommand).resolves({ - TemplateBody: JSON.stringify({ - Resources: { CrHandler: { Type: 'AWS::Lambda::Function', Properties: { Role: roleRef } } }, - }), - }); mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [{ CloudTrailEvent: JSON.stringify({ @@ -206,11 +190,6 @@ describe('correlation', () => { }); test('attributes calls by a stack Lambda running with an imported role', async () => { - mockCloudFormationClient.on(GetTemplateCommand).resolves({ - TemplateBody: JSON.stringify({ - Resources: { CrHandler: { Type: 'AWS::Lambda::Function', Properties: { Role: `arn:aws:iam::${ACCOUNT}:role/shared-role` } } }, - }), - }); mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [handlerEvent({ userIdentity: { @@ -228,11 +207,6 @@ describe('correlation', () => { }); test('attributes calls by a stack Lambda on an imported role via its session name, without inScopeOf', async () => { - mockCloudFormationClient.on(GetTemplateCommand).resolves({ - TemplateBody: JSON.stringify({ - Resources: { CrHandler: { Type: 'AWS::Lambda::Function', Properties: { Role: `arn:aws:iam::${ACCOUNT}:role/shared-role` } } }, - }), - }); mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [handlerEvent({ userIdentity: { @@ -339,24 +313,9 @@ describe('correlation', () => { expect(result!.errors).toHaveLength(0); }); - test('does not correlate via a role the stack only imports, even when nested', async () => { + test('does not correlate via a role that is not a stack resource', async () => { // An imported role may be shared with resources outside the stack (issue #2084), so a // service session on it cannot be told apart from another stack's use of the same role. - mockCloudFormationClient.on(GetTemplateCommand).resolves({ - TemplateBody: JSON.stringify({ - Resources: { - DeliveryStream: { - Type: 'AWS::KinesisFirehose::DeliveryStream', - Properties: { - S3DestinationConfiguration: { - BucketARN: 'arn:aws:s3:::some-bucket', - RoleARN: `arn:aws:iam::${ACCOUNT}:role/imported-firehose-role`, - }, - }, - }, - }, - }), - }); mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [{ CloudTrailEvent: JSON.stringify({ @@ -377,27 +336,25 @@ describe('correlation', () => { expect(result!.errors).toHaveLength(0); }); - test('does not correlate via an imported role expressed as Fn::Sub', async () => { - mockCloudFormationClient.on(GetTemplateCommand).resolves({ - TemplateBody: JSON.stringify({ - Resources: { - Project: { - Type: 'AWS::CodeBuild::Project', - Properties: { ServiceRole: { 'Fn::Sub': 'arn:aws:iam::${AWS::AccountId}:role/my-codebuild-role' } }, - }, - }, - }), + test('does not use physical IDs of non-principal resources as identities', async () => { + // A short physical ID (e.g. an API Gateway stage named `prod`) can equal a segment of an + // unrelated caller's ARN; only roles and functions identify callers. + mockCloudFormationClient.on(DescribeStackResourcesCommand).resolves({ + StackResources: [ + resource('CrHandler', 'AWS::Lambda::Function', CR_FUNCTION_NAME), + resource('Stage', 'AWS::ApiGateway::Stage', 'prod'), + ], }); mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [{ CloudTrailEvent: JSON.stringify({ eventTime: '2026-06-19T11:59:00Z', - eventSource: 'ec2.amazonaws.com', - eventName: 'CreateNetworkInterface', - errorCode: 'Client.UnauthorizedOperation', + eventSource: 's3.amazonaws.com', + eventName: 'GetObject', + errorCode: 'AccessDenied', userIdentity: { - arn: `arn:aws:sts::${ACCOUNT}:assumed-role/my-codebuild-role/codebuild-session`, - sessionContext: { sessionIssuer: { arn: `arn:aws:iam::${ACCOUNT}:role/my-codebuild-role` } }, + arn: `arn:aws:sts::${ACCOUNT}:assumed-role/other-role/prod`, + sessionContext: { sessionIssuer: { arn: `arn:aws:iam::${ACCOUNT}:role/other-role` } }, }, }), }], @@ -408,28 +365,54 @@ describe('correlation', () => { expect(result!.errors).toHaveLength(0); }); - test('still collects template-derived identities when DescribeStackResources fails', async () => { - // A denied/throttled resource listing must not abort the investigation: a literal - // ServiceToken ARN in the template is resolvable without physical IDs. + test('correlates via a role the operation created and then rolled back', async () => { + // The role no longer appears in DescribeStackResources, but the stack events recorded it. + const rolledBackRole = 'MyStack-NewFnRole-QWERTY123456'; + const principals: OperationPrincipal[] = [ + { stackArn: STACK_ARN, logicalId: 'NewFnRole', resourceType: 'AWS::IAM::Role', physicalId: rolledBackRole }, + ]; + mockCloudTrailClient.on(LookupEventsCommand).resolves({ + Events: [serviceOnBehalfEvent({ + userIdentity: { + arn: `arn:aws:sts::${ACCOUNT}:assumed-role/${rolledBackRole}/awslambda_320_20260619115800000`, + sessionContext: { sessionIssuer: { arn: `arn:aws:iam::${ACCOUNT}:role/${rolledBackRole}` } }, + }, + })], + }); + + const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose'), principals); + + expect(result!.errors).toHaveLength(1); + expect(result!.errors[0].logicalId).toEqual('NewFnRole'); + }); + + test('ignores operation principals of other stacks', async () => { + const principals: OperationPrincipal[] = [{ + stackArn: `arn:aws:cloudformation:us-east-1:${ACCOUNT}:stack/OtherStack/def`, + logicalId: 'OtherRole', + resourceType: 'AWS::IAM::Role', + physicalId: 'other-stack-OtherRole-yyy', + }]; + mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [unrelatedEvent()] }); + + const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose'), principals); + + expect(result!.errors).toHaveLength(0); + }); + + test('still uses the operation\'s principals when DescribeStackResources fails', async () => { const denied = new Error('not authorized'); denied.name = 'AccessDeniedException'; mockCloudFormationClient.on(DescribeStackResourcesCommand).rejects(denied); - mockCloudFormationClient.on(GetTemplateCommand).resolves({ - TemplateBody: JSON.stringify({ - Resources: { - MyCustomResource: { - Type: 'Custom::MyThing', - Properties: { ServiceToken: `arn:aws:lambda:us-east-1:${ACCOUNT}:function:${CR_FUNCTION_NAME}` }, - }, - }, - }), - }); + const principals: OperationPrincipal[] = [ + { stackArn: STACK_ARN, logicalId: 'CrHandler', resourceType: 'AWS::Lambda::Function', physicalId: CR_FUNCTION_NAME }, + ]; mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [handlerEvent()] }); - const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); + const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose'), principals); expect(result!.errors).toHaveLength(1); - expect(result!.errors[0].logicalId).toEqual('MyCustomResource'); + expect(result!.errors[0].logicalId).toEqual('CrHandler'); }); }); @@ -568,6 +551,7 @@ describe('sweep mechanics', () => { // must not be suppressed by partial delivery. jest.setSystemTime(FAILURE_TIME.valueOf() + 3 * 60 * 1000); mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [handlerEvent()] }); + mockCloudTrailClient.on(LookupEventsCommand, { MaxResults: 1 }).resolves({ Events: [] }); const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); @@ -575,6 +559,32 @@ describe('sweep mechanics', () => { expect(result!.notes.join('\n')).toMatch(/can lag up to 15 minutes/); }); + test('drops the delivery-latency hint once CloudTrail has delivered events from after the window', async () => { + jest.setSystemTime(FAILURE_TIME.valueOf() + 10 * 60 * 1000); + mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [handlerEvent()] }); + mockCloudTrailClient.on(LookupEventsCommand, { MaxResults: 1 }).resolves({ Events: [unrelatedEvent()] }); + + const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); + + expect(result!.errors).toHaveLength(1); + expect(result!.notes).toHaveLength(0); + // The probe starts where the sweep window ends (2 minutes after the last failure). + expect(mockCloudTrailClient).toHaveReceivedCommandWith(LookupEventsCommand, { + StartTime: new Date(FAILURE_TIME.valueOf() + 2 * 60 * 1000), + MaxResults: 1, + }); + }); + + test('keeps the delivery-latency hint when the delivery probe fails', async () => { + jest.setSystemTime(FAILURE_TIME.valueOf() + 3 * 60 * 1000); + mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [] }); + mockCloudTrailClient.on(LookupEventsCommand, { MaxResults: 1 }).rejects(new Error('ThrottlingException: rate exceeded')); + + const result = await investigateStackViaCloudTrail(sdk, [resourceError()], ioHost.asHelper('diagnose')); + + expect(result!.notes.join('\n')).toMatch(/can lag up to 15 minutes/); + }); + test('stays quiet when nothing correlates and the failure is old', async () => { mockCloudTrailClient.on(LookupEventsCommand).resolves({ Events: [] }); diff --git a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts index b3bd1b98a..6be889eeb 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/diagnosing/stack-diagnoser.test.ts @@ -1036,6 +1036,51 @@ describe('CloudFormationStackDiagnoser', () => { expect(context!.messages[0]).toMatch(/AccessDenied on s3\.amazonaws\.com:CreateBucket/); }); + test('correlates via roles seen in the operation\'s stack events, even if no longer in the stack', async () => { + fakeCfn.createStackSync({ StackName: 'MyStack', StackStatus: 'UPDATE_ROLLBACK_COMPLETE', StackId: STACK_ARN }); + // The role was rolled back: the stack's resource list no longer has it. + mockCloudFormationClient.on(DescribeStackResourcesCommand).resolves({ StackResources: [] }); + mockCloudTrailClient.on(LookupEventsCommand).resolves({ + Events: [{ + CloudTrailEvent: JSON.stringify({ + eventTime: '2026-06-19T11:59:00Z', + eventSource: 'ec2.amazonaws.com', + eventName: 'CreateNetworkInterface', + errorCode: 'Client.UnauthorizedOperation', + userIdentity: { + arn: 'arn:aws:sts::123456789012:assumed-role/MyStack-NewRole-abc123def456/awslambda_1_2', + sessionContext: { sessionIssuer: { arn: 'arn:aws:iam::123456789012:role/MyStack-NewRole-abc123def456' } }, + }, + }), + }], + }); + const errors = makeErrors(); + errors.update({ + event: { + EventId: 'evt-0', + StackId: STACK_ARN, + StackName: 'MyStack', + LogicalResourceId: 'NewRole', + PhysicalResourceId: 'MyStack-NewRole-abc123def456', + ResourceType: 'AWS::IAM::Role', + ResourceStatus: 'CREATE_COMPLETE', + Timestamp: FAILURE_TIME, + }, + parentStackLogicalIds: [], + isRootStackEvent: false, + }); + + const result = await makeExploringDiagnoser().diagnoseFromErrorCollection(errors, { + StackName: 'MyStack', + StackStatus: 'UPDATE_ROLLBACK_COMPLETE', + CreationTime: new Date(), + }); + + const problem = assertProblem(result); + const context = problem.problems[0].additionalContext?.find((c) => c.source === 'CloudTrail Errors'); + expect(context!.messages[0]).toMatch(/^NewRole \(AWS::IAM::Role\): Client\.UnauthorizedOperation/); + }); + test('does not run the investigation without an exploration SDK', async () => { const result = await makeDiagnoser().diagnoseFromErrorCollection(makeErrors(), { StackName: 'MyStack', diff --git a/packages/@aws-cdk/toolkit-lib/test/api/stack-events/resource-errors.test.ts b/packages/@aws-cdk/toolkit-lib/test/api/stack-events/resource-errors.test.ts index df33295b0..36514192d 100644 --- a/packages/@aws-cdk/toolkit-lib/test/api/stack-events/resource-errors.test.ts +++ b/packages/@aws-cdk/toolkit-lib/test/api/stack-events/resource-errors.test.ts @@ -131,3 +131,40 @@ describe('hook failure correlation', () => { expect(errors.all[0].hookFailures).toBeUndefined(); }); }); + +describe('operation principals', () => { + test('records roles and functions seen in any event, including ones later rolled back', () => { + const errors = new ResourceErrors(); + errors.update( + // No physical ID yet on the first CREATE_IN_PROGRESS event + resourceEvent({ EventId: '1', LogicalResourceId: 'NewRole', ResourceType: 'AWS::IAM::Role', ResourceStatus: 'CREATE_IN_PROGRESS' }), + resourceEvent({ EventId: '2', LogicalResourceId: 'NewRole', ResourceType: 'AWS::IAM::Role', PhysicalResourceId: 'MyStack-NewRole-abc', ResourceStatus: 'CREATE_COMPLETE' }), + resourceEvent({ EventId: '3', LogicalResourceId: 'Fn', ResourceType: 'AWS::Lambda::Function', PhysicalResourceId: 'MyStack-Fn-def', ResourceStatus: 'CREATE_FAILED' }), + resourceEvent({ EventId: '4', LogicalResourceId: 'NewRole', ResourceType: 'AWS::IAM::Role', PhysicalResourceId: 'MyStack-NewRole-abc', ResourceStatus: 'DELETE_COMPLETE' }), + ); + + expect(errors.principals).toEqual([ + { stackArn: STACK_ARN, logicalId: 'NewRole', resourceType: 'AWS::IAM::Role', physicalId: 'MyStack-NewRole-abc' }, + { stackArn: STACK_ARN, logicalId: 'Fn', resourceType: 'AWS::Lambda::Function', physicalId: 'MyStack-Fn-def' }, + ]); + }); + + test('records both the old and the new physical ID of a replaced role', () => { + const errors = new ResourceErrors(); + errors.update( + resourceEvent({ EventId: '1', LogicalResourceId: 'Role', ResourceType: 'AWS::IAM::Role', PhysicalResourceId: 'old-role', ResourceStatus: 'UPDATE_IN_PROGRESS' }), + resourceEvent({ EventId: '2', LogicalResourceId: 'Role', ResourceType: 'AWS::IAM::Role', PhysicalResourceId: 'new-role', ResourceStatus: 'UPDATE_COMPLETE' }), + ); + + expect(errors.principals.map((p) => p.physicalId)).toEqual(['old-role', 'new-role']); + }); + + test('ignores resources that do not act as principals', () => { + const errors = new ResourceErrors(); + errors.update( + resourceEvent({ EventId: '1', LogicalResourceId: 'Bucket', ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'my-bucket', ResourceStatus: 'CREATE_COMPLETE' }), + ); + + expect(errors.principals).toEqual([]); + }); +});