fix(logs): emit account root ARN in resource policies to avoid drift false positive

[UTKARSH698]

Issue

Closes #37797

Reason for this change

LogGroup.addToResourcePolicy (reached via grantRead / grantWrite and direct calls) routed every principal through convertArnPrincipalToAccountId, which reduced cross-account ARN principals to a bare account id:

{ "AWS": "211125612616" }

CloudFormation accepts that, but on the deployed resource it stores the canonical account root ARN:

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

Drift detection does a literal string comparison between the two, so every stack that grants a cross-account principal access to a log group is reported as DRIFTED on every evaluation cycle. There is no consumer-side workaround. The maintainer reproduced and confirmed the root cause in #37797.

Description of changes

convertArnPrincipalToAccountId now emits the canonical account root ARN (arn:<partition>:iam::<account>:root) instead of a bare account id, via a small private accountRootArn(account, partition?) helper built on Stack.formatArn:

• Concrete ArnPrincipals preserve their parsed partition (gov/china safe) and produce a literal arn:aws:iam::<account>:root.
• Account-only principals (principalAccount, often a token) fall back to the stack partition pseudo parameter ({Ref: AWS::Partition}).
• Service principals and * (AnyPrincipal) are unchanged.

This makes the synthesized resource policy match the form CloudFormation stores, eliminating the persistent drift false positive while remaining semantically identical.

Describe any new or updated permissions being added

None.

Description of how you validated changes

Updated the two existing aws-logs resource-policy unit tests (concrete IAM user principal, and imported-role principal via CFN pseudo parameters) to expect the account root ARN, and added a dedicated regression test for the concrete cross-account case from the issue.

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

[aws-cdk-automation]

(This review is outdated)

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[UTKARSH698]

Heads-up for reviewers: the red cli-changes check here is not a code failure in this PR. The request-cli-integ-test workflow runs on pull_request_target, and GitHub refuses to check out fork PR code under that trigger:

Refusing to check out fork pull request code from a 'pull_request_target' workflow.

That is a security guard applied to all external fork PRs, so it can only be cleared maintainer-side. Nothing in this change touches the CLI — the diff is confined to aws-logs (log-group.ts and its tests/integ snapshot). All code-relevant checks (validate-pr, build, PR linter) are green. Happy to rebase if that helps re-trigger anything. Thanks for taking a look!

[UTKARSH698]

Friendly follow-up 👋 It’s been a couple of weeks, so flagging that this is still open and ready for review. The branch is a bit behind main now — happy to rebase whenever a reviewer picks it up. As noted above, the red cli-changes check is the known non-code integ gate, not a failure in this change. Thanks for taking a look!

[UTKARSH698]

Cross-linking for whoever triages this: #38330 targets the same issue (#37797) by changing the same convertArnPrincipalToAccountId method, so only one of the two needs to land. This PR predates it (opened 2026-06-24 vs 2026-07-16) and is green across the code-relevant checks.

One substantive difference worth a look when picking between them: this PR derives the account-root ARN through Stack.formatArn(...), so the partition follows the stack's own resolution — including the @aws-cdk/core:enablePartitionLiterals feature flag, which emits a literal (e.g. aws) instead of {"Ref": "AWS::Partition"} when the region is known. #38330 interpolates the Aws.PARTITION pseudo-parameter directly, so that flag has no effect there.

Happy to rebase onto main or fold in anything useful from the other PR if that's the faster path to closing #37797. Thanks for taking a look!

[UTKARSH698]

Heads-up for reviewers: I pushed one new commit (9a42cadf) on top of the branch update. It is a readability/consistency refactor of the same fix — no change to the synthesized output. Stacked on top of the existing history, nothing rewritten.

What changed

The principalAccount branch previously built the account root ARN by hand through a local accountRootArn() helper:

if (principal.principalAccount) {
  return new iam.ArnPrincipal(this.accountRootArn(principal.principalAccount));
}

It now uses the type the library already has for this:

if (principal.principalAccount) {
  return new iam.AccountPrincipal(principal.principalAccount);
}

AccountPrincipal is CDK's canonical representation of an account root, and it renders as arn:${stack.partition}:iam::${accountId}:root through a StackDependentToken (principals.ts#L486) — the same string the hand-rolled helper produced, resolved against the same stack partition. So this is the identical value expressed in the library's own vocabulary rather than a local reimplementation.

The raw ArnPrincipal branch is unchanged in behaviour: it still goes through Stack.formatArn with the partition parsed off the incoming ARN, so aws-cn and aws-us-gov principals keep their own partition instead of being rewritten to aws. That path can't use AccountPrincipal, since AccountPrincipal always resolves to the stack's partition — which would be wrong for an explicitly cross-partition ARN.

With the local helper no longer needed it is gone, and the private method is renamed convertArnPrincipalToAccountId -> convertArnPrincipalToAccountRootArn, since it returns a root ARN rather than an account id. The explanatory comment (and the #37797 reference) moved onto that method.

Test coverage

Added three unit cases that the original commit did not cover: an account-bearing principal, a non-default partition, and an ArnPrincipal that carries no account (must pass through untouched).

Still happy to drop this commit if you would rather review the minimal diff — the original two commits stand on their own, and this one is purely presentational.

[UTKARSH698]

@gjurova — thanks for pulling main in on Friday. Pushing 9a42cadf reset the first-time-contributor gate, so the five workflows that had gone green on 61402cc2 are now sitting at action_required on the new head and won't start on their own:

• PR Build
• Codebuild PR Build
• CodeQL
• Codecov Collect
• Check all requires files are in LFS

Could you hit Approve and run when you get a moment?

Since CI can't run the new commit yet, I verified the change locally so the ask isn't a blind one:

• PrincipalBase.principalAccount defaults to undefined (principals.ts#L152) and ArnPrincipal does not override it, so a raw ArnPrincipal falls through to the second branch as intended — the cross-partition path is reachable, not dead code.
• Synthesized each of the four principal shapes and compared the rendered AWS::Logs::ResourcePolicy document against the assertions:

principal	rendered Principal.AWS
AccountPrincipal('123456789012')	Fn::Join → arn:${AWS::Partition}:iam::123456789012:root
ArnPrincipal('arn:aws-cn:iam::123456789012:role/SomeRole')	arn:aws-cn:iam::123456789012:root
ArnPrincipal('arn:aws:s3:::some-bucket')	unchanged
ArnPrincipal('arn:aws:iam::211125612616:role/SomeRole') (pre-existing test)	arn:aws:iam::211125612616:root

The last row is the assertion that was already in the suite before this PR, and it is byte-identical — which is the main thing I wanted to confirm, since the refactor is meant to be output-neutral.

Happy to squash the two commits if you'd prefer a single one in the history.