Problem
storageDeleteHandler (apps/api/src/routes/workspace-settings.ts, previously in apps/api/src/routes/me.ts) rejects with byo_bucket_disabled whenever byoBucketAllowed(record) is false — including when a workspace already has BYO storage configured (accessKeyId/secretAccessKey, serving from the customer bucket) and the byoBucketEnabled flag is later turned off.
That leaves the workspace stranded: it's already on the customer bucket, but has no API path back to the shared bucket, because the same flag gate that (correctly) blocks new BYO attaches also blocks detach.
Proposed fix (from CodeRabbit review on PR #617)
Gate the byoBucketAllowed flag check on attach paths only, and allow detach whenever the record is currently configured as BYO:
- if (!byoBucketAllowed(record)) {
+ // Detach must stay available after the flag is revoked, otherwise the
+ // workspace is stranded on the customer bucket.
+ if (!byoBucketAllowed(record) && !record.storageConfiguredAt) {
throw new ForbiddenError("BYO storage is not enabled for this workspace", {
code: "byo_bucket_disabled",
});
}
Context
Confirmed pre-existing: this exact check exists verbatim on main in the pre-#613-consolidation apps/api/src/routes/me.ts (DELETE /workspaces/:name/storage), so it predates PR #617's refactor and is out of scope for that (behavior-preserving) PR. Flagged in CodeRabbit's review on PR #617.
Problem
storageDeleteHandler(apps/api/src/routes/workspace-settings.ts, previously inapps/api/src/routes/me.ts) rejects withbyo_bucket_disabledwheneverbyoBucketAllowed(record)is false — including when a workspace already has BYO storage configured (accessKeyId/secretAccessKey, serving from the customer bucket) and thebyoBucketEnabledflag is later turned off.That leaves the workspace stranded: it's already on the customer bucket, but has no API path back to the shared bucket, because the same flag gate that (correctly) blocks new BYO attaches also blocks detach.
Proposed fix (from CodeRabbit review on PR #617)
Gate the
byoBucketAllowedflag check on attach paths only, and allow detach whenever the record is currently configured as BYO:Context
Confirmed pre-existing: this exact check exists verbatim on
mainin the pre-#613-consolidationapps/api/src/routes/me.ts(DELETE /workspaces/:name/storage), so it predates PR #617's refactor and is out of scope for that (behavior-preserving) PR. Flagged in CodeRabbit's review on PR #617.