bucket: catch multiple policies added to a bucket

sst · Feb 28, 2024 · 07bb377 · 07bb377
1 parent 74dd225
commit 07bb377
Show file tree

Hide file tree

Showing 3 changed files with 104 additions and 103 deletions.
diff --git a/pkg/platform/src/auto/run.ts b/pkg/platform/src/auto/run.ts
@@ -15,6 +15,36 @@ import { VisibleError } from "../components/error";
 export async function run(program: automation.PulumiFn) {
   process.chdir($cli.paths.root);
 
+  addTransformationToRetainResourcesOnDelete();
+  addTransformationToEnsureUniqueComponentNames();
+  addTransformationToCheckBucketsHaveMultiplePolicies();
+
+  Link.makeLinkable(aws.dynamodb.Table, function () {
+    return {
+      properties: { tableName: this.name },
+    };
+  });
+  Link.AWS.makeLinkable(aws.dynamodb.Table, function () {
+    return [
+      {
+        actions: ["dynamodb:*"],
+        resources: [this.arn, interpolate`${this.arn}/*`],
+      },
+    ];
+  });
+
+  Hint.reset();
+  Link.reset();
+  Warp.reset();
+  const outputs = (await program()) || {};
+  outputs._links = Link.list();
+  outputs._hints = Hint.list();
+  outputs._warps = Warp.list();
+  outputs._receivers = Link.Receiver.list();
+  return outputs;
+}
+
+function addTransformationToRetainResourcesOnDelete() {
   runtime.registerStackTransformation((args: ResourceTransformationArgs) => {
     if (
       $app.removalPolicy === "retain-all" ||
@@ -32,7 +62,9 @@ export async function run(program: automation.PulumiFn) {
     }
     return undefined;
   });
+}
 
+function addTransformationToEnsureUniqueComponentNames() {
   const componentNames = new Set<string>();
   runtime.registerStackTransformation((args: ResourceTransformationArgs) => {
     if (args.type.startsWith("pulumi")) {
@@ -54,28 +86,22 @@ export async function run(program: automation.PulumiFn) {
 
     return undefined;
   });
+}
 
-  Link.makeLinkable(aws.dynamodb.Table, function () {
-    return {
-      properties: { tableName: this.name },
-    };
-  });
-  Link.AWS.makeLinkable(aws.dynamodb.Table, function () {
-    return [
-      {
-        actions: ["dynamodb:*"],
-        resources: [this.arn, interpolate`${this.arn}/*`],
-      },
-    ];
-  });
+function addTransformationToCheckBucketsHaveMultiplePolicies() {
+  const bucketsWithPolicy: Record<string, string> = {};
+  runtime.registerStackTransformation((args: ResourceTransformationArgs) => {
+    if (args.type !== "aws:s3/bucketPolicy:BucketPolicy") return;
 
-  Hint.reset();
-  Link.reset();
-  Warp.reset();
-  const outputs = (await program()) || {};
-  outputs._links = Link.list();
-  outputs._hints = Hint.list();
-  outputs._warps = Warp.list();
-  outputs._receivers = Link.Receiver.list();
-  return outputs;
+    args.props.bucket.apply((bucket: string) => {
+      if (bucketsWithPolicy[bucket])
+        throw new VisibleError(
+          `Cannot add bucket policy "${args.name}" to the AWS S3 Bucket "${bucket}". The bucket already has a policy attached "${bucketsWithPolicy[bucket]}".`,
+        );
+
+      bucketsWithPolicy[bucket] = args.name;
+    });
+
+    return undefined;
+  });
 }
diff --git a/pkg/platform/src/components/aws/bucket.ts b/pkg/platform/src/components/aws/bucket.ts
@@ -3,6 +3,7 @@ import {
   output,
   interpolate,
   all,
+  Output,
 } from "@pulumi/pulumi";
 import * as aws from "@pulumi/aws";
 import { RandomId } from "@pulumi/random";
@@ -29,17 +30,6 @@ export interface BucketArgs {
    * ```
    */
   public?: Input<boolean>;
-  /**
-   * Enforces SSL for all requests.
-   * @default `true`
-   * @example
-   * ```js
-   * {
-   *   enforceSsl: false
-   * }
-   * ```
-   */
-  enforceSsl?: Input<boolean>;
   /**
    * [Transform](/docs/components#transform/) how this component creates its underlying
    * resources.
@@ -193,7 +183,7 @@ export class Bucket
   implements Link.Linkable, Link.AWS.Linkable
 {
   private constructorName: string;
-  private bucket: aws.s3.BucketV2;
+  private bucket: Output<aws.s3.BucketV2>;
 
   constructor(
     name: string,
@@ -204,14 +194,17 @@ export class Bucket
 
     const parent = this;
     const publicAccess = normalizePublicAccess();
-    const enforceSsl = normalizeEnforceSsl();
 
     const bucket = createBucket();
     const publicAccessBlock = createPublicAccess();
-    createBucketPolicy();
+    const policy = createBucketPolicy();
 
     this.constructorName = name;
-    this.bucket = bucket;
+    // Ensure the policy is created when the bucket is used in another component
+    // (ie. bucket.name). Also, a bucket can only have one policy. We want to ensure
+    // the policy created here is created first. And SST will throw an error if
+    // another policy is created after this one.
+    this.bucket = policy.apply(() => bucket);
 
     function createBucket() {
       const input = transform(args?.transform?.bucket, {
@@ -252,56 +245,46 @@ export class Bucket
     }
 
     function createBucketPolicy() {
-      return all([publicAccess, enforceSsl]).apply(
-        ([publicAccess, enforceSsl]) => {
-          const statements = [];
-          if (publicAccess) {
-            statements.push({
-              principals: [{ type: "*", identifiers: ["*"] }],
-              actions: ["s3:GetObject"],
-              resources: [interpolate`${bucket.arn}/*`],
-            });
-          }
-          if (enforceSsl) {
-            statements.push({
-              effect: "Deny",
-              principals: [{ type: "*", identifiers: ["*"] }],
-              actions: ["s3:*"],
-              resources: [bucket.arn, interpolate`${bucket.arn}/*`],
-              conditions: [
-                {
-                  test: "Bool",
-                  variable: "aws:SecureTransport",
-                  values: ["false"],
-                },
-              ],
-            });
-          }
-
-          if (statements.length === 0) return;
-
-          new aws.s3.BucketPolicy(
-            `${name}Policy`,
-            transform(args?.transform?.policy, {
-              bucket: bucket.bucket,
-              policy: aws.iam.getPolicyDocumentOutput({ statements }).json,
-            }),
+      return publicAccess.apply((publicAccess) => {
+        const statements = [];
+        if (publicAccess) {
+          statements.push({
+            principals: [{ type: "*", identifiers: ["*"] }],
+            actions: ["s3:GetObject"],
+            resources: [interpolate`${bucket.arn}/*`],
+          });
+        }
+        statements.push({
+          effect: "Deny",
+          principals: [{ type: "*", identifiers: ["*"] }],
+          actions: ["s3:*"],
+          resources: [bucket.arn, interpolate`${bucket.arn}/*`],
+          conditions: [
             {
-              parent,
-              dependsOn: publicAccessBlock,
+              test: "Bool",
+              variable: "aws:SecureTransport",
+              values: ["false"],
             },
-          );
-        },
-      );
+          ],
+        });
+
+        return new aws.s3.BucketPolicy(
+          `${name}Policy`,
+          transform(args?.transform?.policy, {
+            bucket: bucket.bucket,
+            policy: aws.iam.getPolicyDocumentOutput({ statements }).json,
+          }),
+          {
+            parent,
+            dependsOn: publicAccessBlock,
+          },
+        );
+      });
     }
 
     function normalizePublicAccess() {
       return output(args?.public).apply((v) => v ?? false);
     }
-
-    function normalizeEnforceSsl() {
-      return output(args?.enforceSsl).apply((v) => v ?? true);
-    }
   }
 
   /**

diff --git a/www/src/content/docs/docs/component/aws/bucket.mdx b/www/src/content/docs/docs/component/aws/bucket.mdx
@@ -178,7 +178,7 @@ The underlying [resources](/docs/components/#nodes) this component creates.
 <Segment>
 <Section type="parameters">
 <InlineSection>
-**Type** [<code class="type">BucketV2</code>](https://www.pulumi.com/registry/packages/aws/api-docs/s3/bucketv2/)
+**Type** <code class="primitive">Output</code><code class="symbol">&lt;</code>[<code class="type">BucketV2</code>](https://www.pulumi.com/registry/packages/aws/api-docs/s3/bucketv2/)<code class="symbol">&gt;</code>
 </InlineSection>
 </Section>
 The Amazon S3 bucket.
@@ -199,25 +199,6 @@ The generated name of the S3 Bucket.
 </Segment>
 
 ## BucketArgs
-### enforceSsl?
-<Segment>
-<Section type="parameters">
-<InlineSection>
-**Type** <code class="primitive">Input</code><code class="symbol">&lt;</code><code class="primitive">boolean</code><code class="symbol">&gt;</code>
-</InlineSection>
-</Section>
-
-<InlineSection>
-**Default** <code class="primitive">true</code>
-</InlineSection>
-Enforces SSL for all requests.
-
-```js
-{
-  enforceSsl: false
-}
-```
-</Segment>
 ### public?
 <Segment>
 <Section type="parameters">
@@ -244,7 +225,8 @@ Enable public read access for all the files in the bucket.
 **Type** <code class="primitive">Object</code>
 </InlineSection>
 - <p>[<code class="key">bucket?</code>](#transform-bucket)</p>
-- <p>[<code class="key">bucketPolicy?</code>](#transform-bucketpolicy)</p>
+- <p>[<code class="key">policy?</code>](#transform-policy)</p>
+- <p>[<code class="key">publicAccessBlock?</code>](#transform-publicaccessblock)</p>
 </Section>
 [Transform](/docs/components#transform/) how this component creates its underlying
 resources.
@@ -260,14 +242,24 @@ resources.
 Transform the S3 Bucket resource.
 
 </Segment>
-<NestedTitle id="transform-bucketpolicy" Tag="h4" parent="transform.">bucketPolicy?</NestedTitle>
+<NestedTitle id="transform-policy" Tag="h4" parent="transform.">policy?</NestedTitle>
 <Segment>
 <Section type="parameters">
 <InlineSection>
 **Type** [<code class="type">BucketPolicyArgs</code>](https://www.pulumi.com/registry/packages/aws/api-docs/s3/bucketpolicy/#inputs)<code class="symbol"> | </code><code class="symbol">(</code><code class="primitive">args</code><code class="symbol">: </code>[<code class="type">BucketPolicyArgs</code>](https://www.pulumi.com/registry/packages/aws/api-docs/s3/bucketpolicy/#inputs)<code class="symbol"> => </code>[<code class="type">BucketPolicyArgs</code>](https://www.pulumi.com/registry/packages/aws/api-docs/s3/bucketpolicy/#inputs)<code class="symbol"> | </code><code class="primitive">void</code><code class="symbol">)</code>
 </InlineSection>
 </Section>
-Transform the IAM Policy that's attached to the Bucket.
+Transform the S3 Bucket Policy resource.
+
+</Segment>
+<NestedTitle id="transform-publicaccessblock" Tag="h4" parent="transform.">publicAccessBlock?</NestedTitle>
+<Segment>
+<Section type="parameters">
+<InlineSection>
+**Type** [<code class="type">BucketPublicAccessBlockArgs</code>](https://www.pulumi.com/registry/packages/aws/api-docs/s3/bucketpublicaccessblock/#inputs)<code class="symbol"> | </code><code class="symbol">(</code><code class="primitive">args</code><code class="symbol">: </code>[<code class="type">BucketPublicAccessBlockArgs</code>](https://www.pulumi.com/registry/packages/aws/api-docs/s3/bucketpublicaccessblock/#inputs)<code class="symbol"> => </code>[<code class="type">BucketPublicAccessBlockArgs</code>](https://www.pulumi.com/registry/packages/aws/api-docs/s3/bucketpublicaccessblock/#inputs)<code class="symbol"> | </code><code class="primitive">void</code><code class="symbol">)</code>
+</InlineSection>
+</Section>
+Transform the public access block resource that's attached to the Bucket.
 
 </Segment>