Repository: cdklabs/cdk-verified-permissions (language: python)

Issue:
addPoliciesFromPath() should custom descriptions rather than file path
Currently if you lever `addPoliciesFromPath()` from the Policy store the description on each policy is defaulted to the absolute file path. This is ugly and also opens up information about the system that deploys the policies.

Possibly a cedar annotation in the policy itself or even just the file name instead of the path might be better


Discussion / hints:
Thanks @ncino-esselman, taking a look
Thanks. There are a few options and I might try exploring targeting a comments annotation in the file but honestly the easiest would just be the file name minus the extension instead of the full path. Happy to contribute as well if that works

Fix patch:
diff --git a/src/policy.ts b/src/policy.ts
index fc9d8df..498ee68 100644
--- a/src/policy.ts
+++ b/src/policy.ts
@@ -1,4 +1,5 @@
 import * as fs from 'fs';
+import * as path from 'path';
 import { CfnPolicy } from 'aws-cdk-lib/aws-verifiedpermissions';
 import { IResource, Resource } from 'aws-cdk-lib/core';
 import { Construct } from 'constructs';
@@ -6,6 +7,7 @@ import { checkParsePolicy } from './cedar-helpers';
 import { IPolicyStore } from './policy-store';
 import { IPolicyTemplate } from './policy-template';
 
+export const POLICY_DESC_SUFFIX_FROM_FILE = ' - Created by CDK';
 export interface EntityIdentifierProperty {
   /**
    * The identifier of an entity.
@@ -194,11 +196,12 @@ export class Policy extends PolicyBase {
   ): Policy {
     const policyFileContents = fs.readFileSync(props.path).toString();
     checkParsePolicy(policyFileContents);
+    let relativePath = path.basename(props.path);
     return new Policy(scope, id, {
       definition: {
         static: {
           statement: policyFileContents,
-          description: props.description || `${props.path} - Created by CDK`,
+          description: props.description || `${relativePath}${POLICY_DESC_SUFFIX_FROM_FILE}`,
         },
       },
       policyStore: props.policyStore,


Test patch:
diff --git a/test/policy.test.ts b/test/policy.test.ts
index f2a7216..893d1a2 100644
--- a/test/policy.test.ts
+++ b/test/policy.test.ts
@@ -1,3 +1,4 @@
+import fs from 'fs';
 import path from 'path';
 import { Stack } from 'aws-cdk-lib';
 import { Template } from 'aws-cdk-lib/assertions';
@@ -6,7 +7,7 @@ import {
   CfnPolicyTemplate,
 } from 'aws-cdk-lib/aws-verifiedpermissions';
 import { getResourceLogicalId } from './utils';
-import { Policy, PolicyDefinitionProperty, PolicyType } from '../src/policy';
+import { Policy, PolicyDefinitionProperty, PolicyType, POLICY_DESC_SUFFIX_FROM_FILE } from '../src/policy';
 import { PolicyStore, ValidationSettingsMode } from '../src/policy-store';
 import { PolicyTemplate } from '../src/policy-template';
 
@@ -344,13 +345,15 @@ when { true };`;
           mode: ValidationSettingsMode.OFF,
         },
       });
-
+      let basePath = 'policy1.cedar';
+      let fullPath = path.join(__dirname, 'test-policies', 'all-valid', basePath);
+      let fileContents = fs.readFileSync(fullPath, 'utf8');
       // WHEN
       const policy = Policy.fromFile(
         stack,
         'ImportedPolicy',
         {
-          path: path.join(__dirname, 'test-policies', 'all-valid', 'policy1.cedar'),
+          path: fullPath,
           policyStore,
         },
       );
@@ -358,8 +361,26 @@ when { true };`;
       // THEN
       expect(policy.policyId).toBeDefined();
       expect(policy.policyType).toEqual(PolicyType.STATIC);
-      const policyStatement = policy.definition.static?.statement;
-      expect(policyStatement).toEqual('permit(principal, action, resource);');
+      expect(policy.definition.static?.description).toContain(basePath);
+      expect(policy.definition.static?.statement).toEqual(fileContents);
+      // Validate Cfn properties
+      Template.fromStack(stack).hasResourceProperties(
+        'AWS::VerifiedPermissions::Policy',
+        {
+          Definition: {
+            Static: {
+              Description: basePath + POLICY_DESC_SUFFIX_FROM_FILE,
+              Statement: fileContents,
+            },
+          },
+          PolicyStoreId: {
+            'Fn::GetAtt': [
+              getResourceLogicalId(policyStore, CfnPolicyStore),
+              'PolicyStoreId',
+            ],
+          },
+        },
+      );
     });
 
     test('Importing a syntactically invalid policy from a file should fail', () => {