From be70c822a1608cb43a4b4b17fc0430f3352797c6 Mon Sep 17 00:00:00 2001
From: Sumu Pitchayan <35242245+sumupitchayan@users.noreply.github.com>
Date: Wed, 2 Oct 2024 11:23:02 -0400
Subject: [PATCH] fix(core): `isTaggable` function can return undefined instead
 of false (#31600)

Closes #26495

### Reason for this change

The `isTaggable` function of the `TagManager` class is currently broken in Python, as it can return `undefined` instead of `true` or `false`.

### Description of changes

In JS/TS, the logical AND operator (`&&`) returns the first falsy value it encounters, even if that value is `undefined` instead of `false` - so the current implementation of `isTaggable` allows for `undefined` to be returned if `tags` is undefined:

```ts
public static isTaggable(construct: any): construct is ITaggable {
  const tags = (construct as any).tags;
  return tags && typeof tags === 'object' && (tags as any)[TAG_MANAGER_SYM];
}
```

The fix is simply changing the return line to the following to handle cases where tags is `null` or `undefined`:

```ts
return tags !== undefined && tags !== null && typeof tags === 'object' && (tags as any)[TAG_MANAGER_SYM];
```

### Description of how you validated changes

Added a unit test to assert that `isTaggable` returns `false`, not `undefined` for a non-taggable Construct (and still returns `true` for a taggable construct).

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 packages/aws-cdk-lib/core/lib/tag-manager.ts  |  2 +-
 .../aws-cdk-lib/core/test/tag-manager.test.ts | 37 +++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/packages/aws-cdk-lib/core/lib/tag-manager.ts b/packages/aws-cdk-lib/core/lib/tag-manager.ts
index 091a275d0a9ee..a18a26420ee5d 100644
--- a/packages/aws-cdk-lib/core/lib/tag-manager.ts
+++ b/packages/aws-cdk-lib/core/lib/tag-manager.ts
@@ -303,7 +303,7 @@ export class TagManager {
    */
   public static isTaggable(construct: any): construct is ITaggable {
     const tags = (construct as any).tags;
-    return tags && typeof tags === 'object' && (tags as any)[TAG_MANAGER_SYM];
+    return tags !== undefined && tags !== null && typeof tags === 'object' && (tags as any)[TAG_MANAGER_SYM];
   }
 
   /**
diff --git a/packages/aws-cdk-lib/core/test/tag-manager.test.ts b/packages/aws-cdk-lib/core/test/tag-manager.test.ts
index 86641a7a8af20..16a5c4858e019 100644
--- a/packages/aws-cdk-lib/core/test/tag-manager.test.ts
+++ b/packages/aws-cdk-lib/core/test/tag-manager.test.ts
@@ -1,3 +1,5 @@
+import { Construct } from 'constructs';
+import * as cdk from '../../../aws-cdk-lib';
 import { TagType } from '../lib/cfn-resource';
 import { TagManager } from '../lib/tag-manager';
 
@@ -225,4 +227,39 @@ describe('tag manager', () => {
     expect(true).toEqual(mgr.applyTagAspectHere(['AWS::Fake::Resource'], []));
     expect(false).toEqual(mgr.applyTagAspectHere(['AWS::Wrong::Resource'], []));
   });
+
+  test('isTaggable function works as expected', () => {
+    const app = new cdk.App();
+    const taggableConstruct = new TaggableConstruct(app, 'MyConstruct');
+    const nonTaggableConstruct = new NonTaggableConstruct(app, 'NonTaggableConstruct');
+
+    // Assert that isTaggable returns true for a taggable construct
+    expect(TagManager.isTaggable(taggableConstruct)).toEqual(true);
+
+    // Assert that isTaggable returns false (not undefined) for a non-taggable construct
+    expect(TagManager.isTaggable(nonTaggableConstruct)).not.toEqual(undefined);
+    expect(TagManager.isTaggable(nonTaggableConstruct)).toEqual(false);
+  });
 });
+
+// `Stack` extends ITaggable so this construct is Taggable by default
+class TaggableConstruct extends cdk.Stack {
+  constructor(scope: Construct, id: string) {
+    super(scope, id);
+    new cdk.CfnResource(this, 'Resource', {
+      type: 'Whatever::The::Type',
+      properties: {
+        Tags: this.tags.renderedTags,
+      },
+    });
+  }
+}
+
+// Simple Construct that does not extend ITaggable
+class NonTaggableConstruct extends Construct {
+  public readonly id: string;
+  constructor(scope: Construct, id: string) {
+    super(scope, id);
+    this.id = id;
+  }
+}
\ No newline at end of file