Skip to content

Commit a381d1b

Browse files
committed
fix(bb-file-bucket): only enable autoDeleteObjects in sandbox mode
An explicit removalPolicy: 'destroy' previously enabled autoDeleteObjects in any stack, including prod. CDK's L2 s3.Bucket implements auto-delete via a hidden Custom::S3AutoDeleteObjects Lambda whose delete behavior stack-level retention Aspects (RemovalPolicies.of(stack).retain()) cannot override, so a "retained" prod bucket was silently emptied on cdk destroy. autoDeleteObjects is now gated on sandbox mode (isSandbox && destroy). In prod, 'destroy' still sets RemovalPolicy.DESTROY but no auto-empty Lambda, so S3 rejects deleting a non-empty bucket and retention Aspects stay effective. Updates the removalPolicy JSDoc and adds 4 CDK synth regression tests.
1 parent 190ac92 commit a381d1b

4 files changed

Lines changed: 78 additions & 9 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@aws-blocks/bb-file-bucket": patch
3+
---
4+
5+
Fix `FileBucket` silently dropping bucket contents in non-sandbox stacks. Previously an explicit `removalPolicy: 'destroy'` enabled `autoDeleteObjects` in any stack, which provisions a hidden `Custom::S3AutoDeleteObjects` Lambda whose delete behavior cannot be overridden by stack-level retention Aspects (`RemovalPolicies.of(stack).retain()`). `autoDeleteObjects` is now enabled only in sandbox mode, so prod buckets honor retention.

packages/bb-file-bucket/src/index.cdk.test.ts

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,12 @@ class StubBlocksStack extends cdk.Stack {
3131
}
3232
}
3333

34-
function setup(): { stack: StubBlocksStack; parent: Scope } {
35-
const app = new cdk.App();
34+
function setup(opts?: { sandbox?: boolean }): { stack: StubBlocksStack; parent: Scope } {
35+
// Sandbox mode is read via `stack.node.tryGetContext('sandboxMode')`, so
36+
// seed it on the app context when requested.
37+
const app = new cdk.App(
38+
opts?.sandbox ? { context: { sandboxMode: 'true' } } : undefined,
39+
);
3640
// S3 bucket names must be lowercase. The default-mode FileBucket derives
3741
// its bucket name from the scope chain, so keep ids lowercase.
3842
const stack = new StubBlocksStack(app, 'teststack');
@@ -82,3 +86,48 @@ test('CDK: fromExisting skips derived-name validation even when the chain is ove
8286
}),
8387
);
8488
});
89+
90+
// ── removalPolicy / autoDeleteObjects (R6 regression) ────────────────────────
91+
//
92+
// autoDeleteObjects provisions a hidden `Custom::S3AutoDeleteObjects` Lambda
93+
// whose delete behavior stack-level retention Aspects cannot override. It must
94+
// be attached ONLY in sandbox mode — never to a prod bucket, even with an
95+
// explicit `removalPolicy: 'destroy'` — or a "retained" prod bucket would be
96+
// silently emptied on `cdk destroy`.
97+
98+
test('CDK: sandbox default enables DESTROY + auto-delete Lambda', () => {
99+
const { stack, parent } = setup({ sandbox: true });
100+
new FileBucket(parent, 'uploads');
101+
const template = Template.fromStack(stack);
102+
// Sandbox cleanup ergonomics preserved: bucket is dropped and auto-emptied.
103+
template.hasResource('AWS::S3::Bucket', { DeletionPolicy: 'Delete' });
104+
template.resourceCountIs('Custom::S3AutoDeleteObjects', 1);
105+
});
106+
107+
test('CDK: non-sandbox explicit destroy sets DESTROY but NO auto-delete Lambda (regression)', () => {
108+
const { stack, parent } = setup();
109+
new FileBucket(parent, 'uploads', { removalPolicy: 'destroy' });
110+
const template = Template.fromStack(stack);
111+
// Bucket is marked for deletion, but the un-overridable auto-delete Lambda
112+
// must NOT be present in a prod stack — this is the core fix.
113+
template.hasResource('AWS::S3::Bucket', { DeletionPolicy: 'Delete' });
114+
template.resourceCountIs('Custom::S3AutoDeleteObjects', 0);
115+
});
116+
117+
test('CDK: non-sandbox explicit retain sets RETAIN and no auto-delete Lambda', () => {
118+
const { stack, parent } = setup();
119+
new FileBucket(parent, 'uploads', { removalPolicy: 'retain' });
120+
const template = Template.fromStack(stack);
121+
template.hasResource('AWS::S3::Bucket', { DeletionPolicy: 'Retain' });
122+
template.resourceCountIs('Custom::S3AutoDeleteObjects', 0);
123+
});
124+
125+
test('CDK: non-sandbox default omits DESTROY (Aspect-overridable RETAIN) and no auto-delete Lambda', () => {
126+
const { stack, parent } = setup();
127+
new FileBucket(parent, 'uploads');
128+
const template = Template.fromStack(stack);
129+
// No explicit removalPolicy → CDK default RETAIN, driven by Aspects.
130+
const buckets = template.findResources('AWS::S3::Bucket', { DeletionPolicy: 'Delete' });
131+
assert.strictEqual(Object.keys(buckets).length, 0);
132+
template.resourceCountIs('Custom::S3AutoDeleteObjects', 0);
133+
});

packages/bb-file-bucket/src/index.cdk.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,20 @@ export class FileBucket<O extends FileBucketOptions = FileBucketOptions> extends
4646
// In sandbox mode, default to DESTROY + autoDeleteObjects so
4747
// `cdk destroy` can fully clean up without manual bucket emptying.
4848
// Explicit `removalPolicy` from the customer takes precedence.
49-
// `autoDeleteObjects: true` is only valid paired with DESTROY (CDK
50-
// validates this at construct time), so we tie the two together.
5149
const isSandbox = cdk.Stack.of(this).node.tryGetContext('sandboxMode') === 'true';
5250
const destroy = options?.removalPolicy === 'destroy' || (isSandbox && options?.removalPolicy === undefined);
5351

52+
// `autoDeleteObjects` is enabled ONLY in sandbox mode — never in a prod
53+
// stack, even with an explicit `removalPolicy: 'destroy'`. CDK's L2
54+
// `s3.Bucket` implements auto-delete via a hidden
55+
// `Custom::S3AutoDeleteObjects` Lambda with a hardcoded delete behavior
56+
// that stack-level retention Aspects (`RemovalPolicies.of(stack).retain()`)
57+
// cannot override. Attaching it to a prod bucket would silently empty the
58+
// bucket on `cdk destroy` even when the stack intends to retain it. In
59+
// prod, `'destroy'` sets DESTROY without auto-empty; S3 rejects deleting a
60+
// non-empty bucket, which is the correct, safe behavior.
61+
const autoDeleteObjects = isSandbox && destroy;
62+
5463
// Bucket name is derived from the scope chain. Validate against S3's
5564
// naming rules at synth so an invalid name fails here rather than at
5665
// `cdk deploy` (where CloudFormation rejects it with a cryptic error).
@@ -66,7 +75,7 @@ export class FileBucket<O extends FileBucketOptions = FileBucketOptions> extends
6675
: options?.removalPolicy === 'retain'
6776
? RemovalPolicy.RETAIN
6877
: undefined,
69-
autoDeleteObjects: destroy,
78+
autoDeleteObjects,
7079
cors: options?.corsRules?.map((rule: CorsRule) => ({
7180
allowedOrigins: rule.allowedOrigins,
7281
allowedMethods: rule.allowedMethods.map(m => httpMethodMap[m]),

packages/bb-file-bucket/src/types.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,16 @@ export interface FileBucketOptions {
2323
* CDK's default applies (RETAIN — the bucket and its contents are
2424
* preserved on `cdk destroy`).
2525
*
26-
* Pass `'destroy'` for sandbox / ephemeral stacks where the bucket
27-
* should be dropped on teardown. This also enables `autoDeleteObjects`
28-
* so CloudFormation can empty the bucket before deletion — required
29-
* since S3 rejects DELETE on a non-empty bucket.
26+
* Pass `'destroy'` to drop the bucket on teardown (`RemovalPolicy.DESTROY`).
27+
* `autoDeleteObjects` (which empties the bucket before deletion) is enabled
28+
* **only in sandbox mode**. In a prod stack, `'destroy'` sets DESTROY but
29+
* does NOT auto-empty — S3 rejects deleting a non-empty bucket, so a
30+
* populated prod bucket is preserved rather than silently wiped.
31+
*
32+
* Why sandbox-only: CDK implements `autoDeleteObjects` via a hidden
33+
* `Custom::S3AutoDeleteObjects` Lambda whose delete behavior stack-level
34+
* retention Aspects (`RemovalPolicies.of(stack).retain()`) cannot override.
35+
* Keeping it out of prod stacks ensures those Aspects remain effective.
3036
*
3137
* Pass `'retain'` to set the policy explicitly (identical to omitting
3238
* it today, but robust against stack-layer policy overrides).

0 commit comments

Comments
 (0)