fix(bb-file-bucket): only enable autoDeleteObjects in sandbox mode

[pjkroker]

Problem

Issue #, if available: N/A

An explicit removalPolicy: 'destroy' previously enabled autoDeleteObjects
on any stack, including prod. CDK's L2 s3.Bucket implements auto-delete
via a hidden Custom::S3AutoDeleteObjects Lambda whose delete behavior a
stack-level retention Aspect (RemovalPolicies.of(stack).retain()) cannot
override. As a result, a bucket that operators believed was "retained" in prod
was silently emptied on cdk destroy — a silent data-loss footgun.

Changes

• Gate autoDeleteObjects on sandbox mode: it is now only enabled when
  isSandbox && destroy (previously enabled whenever destroy was set).
• In non-sandbox (prod) stacks, removalPolicy: 'destroy' still sets
  RemovalPolicy.DESTROY but provisions no auto-empty Lambda, so
  stack-level retention Aspects remain effective.
• Update the removalPolicy JSDoc to document the sandbox-gated behavior and
  the Aspect-override footgun.
• Add 4 CDK synth regression tests covering sandbox vs. non-sandbox behavior.

The invariant autoDeleteObjects => destroy is preserved, so CDK's
construct-time validation never trips.

⚠️ Behavior change

removalPolicy: 'destroy' now has context-dependent meaning:

• Sandbox: unchanged — bucket is auto-emptied and fully torn down on
  cdk destroy.
• Non-sandbox (prod): DESTROY policy is set, but the bucket is not
  auto-emptied. cdk destroy issues DeleteBucket against a non-empty bucket,
  S3 rejects it, and the stack lands in DELETE_FAILED, requiring manual
  cleanup.

This is intentional and safer (no silent data loss), but anyone relying on
'destroy' for full teardown of a non-empty prod bucket will now see failed
stack deletes instead of a clean destroy. There is currently no opt-in escape
hatch for a legitimately-ephemeral prod bucket — see discussion in the review
thread.

Validation

• cdk synth regression tests pass (4 new tests: sandbox destroy emits the
  auto-delete Lambda; non-sandbox destroy does not; retention Aspects remain
  effective in prod; invariant autoDeleteObjects => destroy holds).
• All CI checks green.

Checklist

• PR description included
• Tests are changed or added
• Relevant documentation is changed or added (and PR referenced)

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

[pranavosu]

Review

Solid, focused fix. The core reasoning is correct: autoDeleteObjects provisions a Custom::S3AutoDeleteObjects Lambda whose delete behavior a stack-level RemovalPolicies.of(stack).retain() Aspect cannot override, so keeping it out of prod stacks is the only way to keep those Aspects effective. The invariant holds cleanly too — autoDeleteObjects can only be true when destroy is true, so CDK's construct-time validation never trips. All checks pass, and the change ships tests + changeset + doc updates.

A few things worth addressing before this comes out of draft.

1. removalPolicy: 'destroy' now has a context-dependent meaning

Previously 'destroy' meant "bucket is fully torn down on cdk destroy." After this change, in a non-sandbox stack it means "DESTROY policy, but no auto-empty" — so cdk destroy issues DeleteBucket against a non-empty bucket, S3 rejects it, and the stack lands in DELETE_FAILED requiring manual cleanup. That is intentional and safer (no silent data loss), but it is a surprising semantic shift for the same option value.

This is fine, but I'd suggest calling it out explicitly in the PR description / changeset as a behavior change, since anyone relying on 'destroy' for full teardown in a prod stack will now see failed stack deletes.

2. No escape hatch for a legitimately-ephemeral prod bucket

By tying auto-delete to isSandbox rather than to an explicit opt-in, a prod user with an intentionally throwaway bucket (and no retention Aspects) can no longer get empty-then-destroy behavior at all. Was an explicit opt-in considered? A couple of alternatives:

Option A — explicit boolean opt-in, defaulting to sandbox behavior:

// autoDeleteObjects defaults to sandbox-only, but a customer can force it on
// for a genuinely ephemeral prod bucket. Still gated on DESTROY.
const autoDeleteObjects =
  destroy && (options?.autoDeleteObjects ?? isSandbox);

with a documented autoDeleteObjects?: boolean on FileBucketOptions. This preserves the safe default while leaving a deliberate, visible way out. The JSDoc can carry the same Aspect-override warning so the footgun is opt-in rather than default.

Option B — a distinct removal value (e.g. removalPolicy: 'destroy-and-empty') so the auto-empty intent is explicit in the value itself rather than inferred from sandbox mode. More explicit, but a bigger API surface change.

If the team's stance is "prod should never auto-empty, full stop," then the current approach is the right call — just capture that decision so it does not look like an oversight later.

3. Minor: weak assertion in the last test

In the non-sandbox default test:

const buckets = template.findResources('AWS::S3::Bucket', { DeletionPolicy: 'Delete' });
assert.strictEqual(Object.keys(buckets).length, 0);

This passes whether or not the matcher inspects the top-level DeletionPolicy, so it does not strongly prove "not DESTROY." The Custom::S3AutoDeleteObjects count is doing the real work. Consider asserting the positive default instead, e.g. template.hasResource('AWS::S3::Bucket', { DeletionPolicy: Match.absent() }) (or UpdateReplacePolicy: 'Retain'), to make the intent load-bearing.

4. Draft housekeeping

PR body is still the unfilled template (Problem / Changes / Validation empty) and the checklist is unchecked. Worth filling in before merge, especially the behavior-change note from (1).

---

None of these block correctness — the fix itself is right. Mainly want a conscious decision on the escape hatch (2) and the behavior-change callout (1).

[svidgen]

LGTM. The fix is correct and minimal — destroy && (options?.autoDeleteObjects ?? isSandbox) preserves CDK's invariant, defaults safe, and the escape hatch addresses @pranavosu's feedback cleanly. Tests cover all scenarios (sandbox default, prod destroy without auto-empty, retain, and the opt-in/opt-out paths). JSDoc and changeset document the behavior change well.

One minor nit: the test "autoDeleteObjects:true without destroy" silently ignores the explicit opt-in. Consider logging a warning so users know their config is a no-op — but not a blocker.