aws / aws/aws-cdk

(aws-logs): convertArnPrincipalToAccountId in aws-logs causes false positive CloudFormation stack drift

Open
#37,797 5 comments 0 reactions 0 assignees View on GitHub
@aws-cdk/aws-logs bug effort/small good first issue p2
Dominant language
TypeScript
Stars
12.9k
Forks
4.6k
Avg merge
1d 19h
Merged PRs (30d)
74

Description

### Describe the bug

The convertArnPrincipalToAccountId() method in AWSSecureCDK_Peru (secure-cdk-lib/aws-cdk/packages/aws-cdk-lib/aws-logs/lib/log-group.ts) strips IAM principals down to just the AWS account ID when generating CloudWatch Logs resource policies via .grantRead().

For example, CDK generates:

{"AWS": "211125612616"}

But CloudFormation/AWS normalizes this to:

{"AWS": "arn:aws:iam::211125612616:root"}

These are semantically identical, but CloudFormation drift detection treats them as different, causing every stack that uses logGroup.grantRead() with a cross-account role to be flagged as drifted on every evaluation cycle.

This is generating recurring false-positive SEV3 tickets for our team (IMDbPro-DISCO) across multiple stacks and environments:

V2139077722 — IMDbListGraphQLStack (Prod)
V2114817343 — IMDbListGraphQLStack (Gamma)
V2096862319 — Fleet-PodMonitoringStack (Prod)
V2000840049 — NotificationProcessorStack (Prod)

Suggested fix: Update convertArnPrincipalToAccountId() to output the full ARN format (arn:aws:iam::ACCOUNT_ID:root) instead of just the account ID, so the CDK template matches what CloudFormation actually creates.

Impact: Any team using logGroup.grantRead() with cross-account principals will hit this same false drift issue. There is no workaround on the consumer side — the alarm architecture cannot distinguish this false positive from real drift.

### Regression Issue

- [ ] Select this option if this issue appears to be a regression.

### Last Known Working CDK Library Version

_No response_

### Expected Behavior

When `LogGroup.grantRead()` (or any code path reaching `addToResourcePolicy`) is called with a cross-account IAM principal, CDK should synthesize the resource policy principal in a form that matches what CloudFormation actually stores after deployment, so that CloudFormation drift detection does not report a persistent false positive.

Specifically, the synthesized principal should be the full root ARN:

```json
{ "AWS": "arn:aws:iam::211125612616:root" }
```

This is the form CloudFormation normalizes to on the server side, and it is what drift detection compares against.

### Current Behavior

`LogGroup.addToResourcePolicy` routes every principal through `convertArnPrincipalToAccountId`, which replaces the principal with a bare account ID string wrapped in `iam.ArnPrincipal`:

https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-logs/lib/log-group.ts#L308-L324

That emits the following into the synthesized CloudWatch Logs `ResourcePolicyDocument`:

```json
{ "AWS": "211125612616" }
```

CloudFormation accepts this, but on the deployed resource it stores the canonical form:

```json
{ "AWS": "arn:aws:iam::211125612616:root" }
```

CloudFormation drift detection performs a literal string comparison between the template principal (`"211125612616"`) and the deployed principal (`"arn:aws:iam::211125612616:root"`) and reports the stack as `DRIFTED` on every evaluation cycle. There is no way to suppress this from the consumer side because any `addToResourcePolicy` / `grantRead` call is rewritten by `convertArnPrincipalToAccountId` before synthesis.

Observed impact: every stack using `logGroup.grantRead(crossAccountRole)` is permanently marked drifted, generating recurring drift alarms across multiple production stacks.

No stack trace — this is a template correctness / drift-detection issue, not a runtime error.

### Reproduction Steps

Minimal SSCCE:

```ts
import { App, Stack } from 'aws-cdk-lib';
import { LogGroup } from 'aws-cdk-lib/aws-logs';
import { ArnPrincipal } from 'aws-cdk-lib/aws-iam';

const app = new App();
const stack = new Stack(app, 'DriftRepro', {
env: { account: '111111111111', region: 'us-east-1' },
});

const lg = new LogGroup(stack, 'LG');

// Cross-account role in another account
lg.grantRead(new ArnPrincipal('arn:aws:iam::211125612616:role/Reader'));

app.synth();
```

Synthesized `AWS::Logs::ResourcePolicy` contains:

```json
{
"PolicyDocument": {
"Statement": [
{
"Principal": { "AWS": "211125612616" }
}
]
}
}
```

Deploy the stack, then run CloudFormation drift detection (`aws cloudformation detect-stack-drift` → `describe-stack-resource-drifts`). The `AWS::Logs::ResourcePolicy` resource reports `MODIFIED` with:

- Expected: `"AWS": "211125612616"`
- Actual: `"AWS": "arn:aws:iam::211125612616:root"`

This persists forever — redeploying does not clear it because CDK keeps emitting the short form.

### Possible Solution

Change `convertArnPrincipalToAccountId` in `packages/aws-cdk-lib/aws-logs/lib/log-group.ts` so that when it reduces a principal to an account, it emits the canonical root ARN rather than the bare account ID:

```ts
private convertArnPrincipalToAccountId(principal: iam.IPrincipal) {
if (principal.principalAccount) {
return new iam.ArnPrincipal(`arn:${Aws.PARTITION}:iam::${principal.principalAccount}:root`);
}
if (principal instanceof iam.ArnPrincipal && principal.arn !== '*') {
const parsedArn = Arn.split(principal.arn, ArnFormat.SLASH_RESOURCE_NAME);
if (parsedArn.account) {
return new iam.ArnPrincipal(`arn:${parsedArn.partition ?? Aws.PARTITION}:iam::${parsedArn.account}:root`);
}
}
return principal;
}
```

This produces `{ "AWS": "arn:aws:iam:::root" }`, which is the form CloudFormation stores, so drift detection becomes a no-op. CloudWatch Logs Resource Policies accept this form (IAM/STS treats bare `""` and `"arn:aws:iam:::root"` as equivalent references to the account root).

Existing snapshot tests under `aws-logs` would need regenerating, and an integration test that asserts no drift on a deployed cross-account resource policy would guard against regression.

### Additional Information/Context

_No response_

### AWS CDK Library version (aws-cdk-lib)

`2.221.0` (declared range `^2.173.2`)

### AWS CDK CLI version

`2.1023.0 (build 45ceb89)`

### Node.js Version

`v22.22.2`

### OS

macOS (darwin)

### Language

TypeScript

### Language Version

_No response_

### Other information

_No response_

Contributor guide

Open the contributing guide

Research direction

Start in packages/aws-cdk-lib/aws-logs/lib/log-group.ts, focusing on convertArnPrincipalToAccountId and the addToResourcePolicy path. Run the existing aws-logs snapshot tests and inspect their expected principals. Done means cross-account principals synthesize as canonical IAM root ARNs, snapshots are updated, and an integration test covers the CloudFormation drift case.

Written by the indexing model from the issue text.

Assessment

Tech stack
aws, typescript
Domain
cloud, infrastructure
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.