- ❌ Key rotation CLI may OOM on large extension_storage tables.
- ⚠️ Operators may be unable to retire old encryption keys.
- ⚠️ Encrypted extension data may remain unrotated indefinitely.

1. Start a Superset instance with the extension storage model defined at
`superset/extension_storage/models.py:40-83` (the `ExtensionStorage` table with `value =
Column(LargeBinary(EXTENSION_STORAGE_MAX_SIZE))` and `EXTENSION_STORAGE_MAX_SIZE` set to
~16 MB at line 36).

2. Populate many encrypted rows in `extension_storage` by calling
`ExtensionStorageDAO.set(..., is_encrypted=True)` from
`superset/extension_storage/daos.py:142-188` in a loop, so that the combined size of
`value` blobs for encrypted rows is large relative to available RAM (for example,
thousands of rows with multi‑MB payloads).

3. Configure key rotation by setting `EXTENSION_STORAGE_ENCRYPTION_KEYS` in the Superset
config (used by `_fernet()` at `superset/extension_storage/daos.py:45-57`), then run the
CLI command `superset rotate-extension-storage-keys`, which invokes
`rotate_extension_storage_keys()` defined at `superset/cli/update.py:104-149`.

4. During execution, `rotate_extension_storage_keys()` issues
`db.session.query(ExtensionStorage).filter(ExtensionStorage.is_encrypted.is_(True)).all()`
at `superset/cli/update.py:121-124`, materializing every encrypted row (including all
large `value` blobs) into the `entries` list at once; on a sufficiently large table this
can cause excessive memory usage or `MemoryError`/process OOM, preventing the rotation
from completing.

This is a comment left during a code review.

**Path:** superset/cli/update.py
**Line:** 121:140
**Comment:**
	*Possible Bug: Loading every encrypted row with `.all()` materializes the full table (including large binary payloads) in memory at once, which can exhaust memory on real datasets. Iterate in batches (and avoid retaining all ORM instances) so key rotation remains safe for large tables.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

- ⚠️ Extensions storage API exposed when ENABLE_EXTENSIONS is False.
- ⚠️ Feature flag semantics for /api/v1/extensions become inconsistent.

1. Configure and start Superset with the default feature flags, where
`"ENABLE_EXTENSIONS": False` as defined in `superset/config.py:576-13` (comment at 576-10
and value at 576-12).

2. During app startup, `SupersetAppInitializer.init_views` in
`superset/initialization/__init__.py:1-55` executes the block that unconditionally imports
and registers `ExtensionStorageRestApi` via `from superset.extension_storage.api import
ExtensionStorageRestApi` and `appbuilder.add_api(ExtensionStorageRestApi)` at
`superset/initialization/__init__.py:37-39` (corresponding to PR diff lines 276–278).

3. In the same `init_views` method, the main extensions API is only registered when the
feature flag is enabled: `if
feature_flag_manager.is_feature_enabled("ENABLE_EXTENSIONS"):` followed by importing and
adding `ExtensionsRestApi` at `superset/initialization/__init__.py:41-44`, so with the
flag disabled only `ExtensionStorageRestApi` remains mounted under `/api/v1/extensions`.

4. With the server running and extensions feature flag still disabled, send an
authenticated request (session or JWT protected by `@protect()`) to `GET
/api/v1/extensions/<publisher>/<name>/storage/persistent/global/`, which is exposed by
`ExtensionStorageRestApi.list_global` at `superset/extension_storage/api.py:93-103`;
observe that the endpoint is fully functional even though `"ENABLE_EXTENSIONS"` is False,
demonstrating that extension-specific storage endpoints bypass the feature flag gating
applied to other extension APIs.

This is a comment left during a code review.

**Path:** superset/initialization/__init__.py
**Line:** 276:276
**Comment:**
	*Logic Error: The extension storage API is registered unconditionally, which bypasses the `ENABLE_EXTENSIONS` feature flag and exposes extension endpoints even when extensions are configured as disabled. Register this API only when the extensions feature flag is enabled so runtime behavior matches the documented security/feature contract.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

- ❌ PUT /storage/persistent/global/<key> can 500 on bad JSON.
- ❌ PUT /storage/persistent/user/<key> can 500 on bad JSON.
- ❌ PUT /storage/persistent/resources/... can 500 on bad JSON.
- ⚠️ Clients receive 500 instead of clear 4xx validation errors.

1. Start Superset with this PR code so that `ExtensionStorageRestApi` in
`superset/extension_storage/api.py` is registered (class definition at lines 93–99).

2. Perform an HTTP PUT to the global storage endpoint
`/api/v1/extensions/<publisher>/<name>/storage/persistent/global/<key>` (handler
`set_global` at lines 188–241) with `Content-Type: application/json` and a JSON array
body, e.g. `["not-a-dict"]`.

3. Inside `set_global`, `body = request.get_json(force=True) or {}` (line 229) will set
`body` to the list `["not-a-dict"]` because `request.get_json(force=True)` returns that
list and the `or {}` fallback is not used.

4. The next line `raw_value: str | bytes = body.get("value", "")` (line 230) calls `.get`
on a list, raising `AttributeError: 'list' object has no attribute 'get'`, which Flask
propagates as a 500 Internal Server Error instead of a 4xx client error. The same pattern
exists in `set_user` (lines 417–419) and `set_resource` (lines 651–653), so the issue
reproduces identically on `/user/<key>` and
`/resources/<resource_type>/<resource_uuid>/<key>` PUT endpoints.

This is a comment left during a code review.

**Path:** superset/extension_storage/api.py
**Line:** 229:231
**Comment:**
	*Possible Bug: `request.get_json(force=True)` can return a non-dict JSON value (for example, a list), and the next `.get(...)` calls will raise `AttributeError`, producing a 500 instead of a client error. Validate that the parsed body is a JSON object before reading fields.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

- ❌ PUT global storage can fail on explicit null MIME type.
- ❌ PUT user storage can fail on explicit null MIME type.
- ❌ PUT resource storage can fail on explicit null MIME type.
- ⚠️ Clients see server error instead of default MIME type.

1. Start Superset with this PR code so `ExtensionStorageRestApi` in
`superset/extension_storage/api.py` is active and `ExtensionStorageDAO.set` is callable
from the API.

2. Send an HTTP PUT to
`/api/v1/extensions/<publisher>/<name>/storage/persistent/global/<key>` (handler
`set_global` at lines 188–241) with JSON body `{"value": "abc", "value_type": null}` and
`Content-Type: application/json`.

3. In `set_global`, `body = request.get_json(force=True) or {}` (line 229) yields
`{"value": "abc", "value_type": None}`; `value_type=body.get("value_type", _MIME_DEFAULT)`
(line 236) assigns `None` to `value_type` because `get` finds the key and returns its
`None` value instead of the default.

4. That `None` is passed into `ExtensionStorageDAO.set(..., value_type=None, ...)` (lines
232–240); when the DAO persists to the `ExtensionStorage` model (whose `value_type` column
is declared as non-nullable in the new table in this PR), the ORM/database layer raises an
integrity or type error at commit time, causing the request to fail with a 500 instead of
cleanly defaulting to the fallback MIME type.

This is a comment left during a code review.

**Path:** superset/extension_storage/api.py
**Line:** 235:240
**Comment:**
	*Logic Error: `value_type` can be explicitly sent as `null`, and `body.get("value_type", _MIME_DEFAULT)` will return `None`, causing a DB integrity failure on a non-nullable column. Use a fallback that also handles `None`.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

- ❌ Global storage PUT may create duplicate logical keys.
- ❌ User storage PUT may return stale values intermittently.
- ❌ Resource storage GET may read unpredictable version of value.
- ⚠️ Delete endpoints may leave undeletable ghost entries behind.
- ⚠️ Extension key-value semantics broken under concurrent writes.

1. Apply migration
`superset/migrations/versions/2026-03-20_12-00_e5f6a7b8c9d0_add_extension_storage_table.py`,
which creates table `extension_storage` and a non-unique composite index
`ix_ext_storage_lookup` on `(extension_id, user_fk, resource_type, resource_uuid, key)` at
lines 45–49.

2. Start Superset so the REST API in `superset/extension_storage/api.py` is registered;
note that `set_user` at lines 193–191 and `set_global` at lines 99–152 call
`ExtensionStorageDAO.set(...)` inside a `@transaction()`-wrapped request handler, creating
separate DB transactions per HTTP PUT to
`/api/v1/extensions/<publisher>/<name>/storage/persistent/user/<key>` or `/global/<key>`.

3. Trigger two concurrent PUT requests for the same logical scoped key (for example, same
`publisher`, `name`, and `key` to `set_global`, or same `publisher`, `name`, `key`, and
authenticated user to `set_user`), so that both requests enter `ExtensionStorageDAO.set`
in `superset/extension_storage/daos.py` lines 142–190; each transaction executes a SELECT
with `.first()` at lines 158–167, sees no existing row, and then independently executes an
INSERT and `db.session.flush()`, resulting in two rows with identical `(extension_id,
user_fk, resource_type, resource_uuid, key)` but different `id`.

4. Later reads and deletes for that key become nondeterministic: `ExtensionStorageDAO.get`
and `get_value` at lines 100–138 and `delete` at lines 195–218 all use `.first()` without
`ORDER BY`, so GET endpoints (`get_global`, `get_user`, `get_resource` in
`superset/extension_storage/api.py` lines 62–97, 242–260, 38–95) may return either the
stale or the latest value arbitrarily, and DELETE endpoints only remove one of the
duplicate rows, leaving stale data that continues to appear in list endpoints
(`list_global`, `list_user`, `list_resource`), even though the extension believes the key
was uniquely upserted.

This is a comment left during a code review.

**Path:** superset/migrations/versions/2026-03-20_12-00_e5f6a7b8c9d0_add_extension_storage_table.py
**Line:** 78:78
**Comment:**
	*Race Condition: The scope lookup index is non-unique, so concurrent writes can insert duplicate rows for the same logical key. Since reads and deletes use `.first()`, duplicate rows make results nondeterministic and can leave stale duplicates behind. Make the lookup index unique while normalizing nullable scope fields so `NULL` values don't bypass uniqueness.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

- ❌ Encrypted extension storage reads 500 with misconfigured key type.
- ❌ Key rotation CLI fails decrypting data after config correction.
- ⚠️ Single-character key strings drastically reduce encryption key entropy.

1. Configure Superset so that `current_app.config["EXTENSION_STORAGE_ENCRYPTION_KEYS"]` is
a single string instead of a list (e.g. set it to `"mysupersecret"` in your deployment
config, despite the list type hint in `superset/config.py:242`).

2. Start Superset and call the encrypted global storage endpoint `PUT
/api/v1/extensions/<publisher>/<name>/storage/persistent/global/<key>` implemented in
`superset/extension_storage/api.py:188-241`, with JSON body
`{"value":"secret","is_encrypted":true}` so that `ExtensionStorageDAO.set(...,
is_encrypted=True)` at `superset/extension_storage/daos.py:142-156` is invoked.

3. During this request `_fernet()` at `superset/extension_storage/daos.py:45-57` reads the
string config value into `raw_keys`, iterates over it character-by-character in `[
_key_to_fernet(k) for k in raw_keys ]`, and encrypts the stored value with a Fernet key
derived only from the first single-character "key", producing ciphertext that can only be
decrypted with that unintended character-derived key.

4. Later, correct the configuration to a proper list (e.g.
`EXTENSION_STORAGE_ENCRYPTION_KEYS = ["mysupersecret"]`), restart Superset, and call `GET
/api/v1/extensions/<publisher>/<name>/storage/persistent/global/<key>` handled by
`get_global()` in `superset/extension_storage/api.py:151-187`;
`ExtensionStorageDAO.get_value()` at `superset/extension_storage/daos.py:122-138` now
builds a different `MultiFernet` without the old character-derived key and
`decrypt(entry.value)` raises `InvalidToken`, causing a 500 error when reading previously
written encrypted entries and similarly breaking the `rotate_extension_storage_keys` CLI
in `superset/cli/update.py:15-31`.

This is a comment left during a code review.

**Path:** superset/extension_storage/daos.py
**Line:** 54:56
**Comment:**
	*Security: `EXTENSION_STORAGE_ENCRYPTION_KEYS` is assumed to be a list, but if operators configure it as a single string, the current code iterates character-by-character and derives one Fernet key per character. That silently encrypts data with the first character-derived key, weakening security and causing decryption/rotation surprises when configuration is later corrected. Normalize string/bytes config values to a single-item list before building `MultiFernet`.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

- ❌ Duplicate extension_storage rows cause inconsistent GET /storage/persistent responses.
- ⚠️ DELETE endpoints may leave duplicate ghost rows for same key.
- ⚠️ Extension authors see nondeterministic state when writing concurrently.

1. Apply and run the Alembic migration
`superset/migrations/versions/2026-03-20_12-00_e5f6a7b8c9d0_add_extension_storage_table.py`
which creates the `extension_storage` table and only non-unique indexes
`ix_ext_storage_extension_id` and `ix_ext_storage_lookup` (lines 69–77), then start the
Superset webserver so that `ExtensionStorageRestApi` is registered in
`superset/initialization/__init__.py:260-29`.

2. Authenticate as a user and, following the documented frontend usage in
`docs/developer_docs/extensions/extension-points/persistent-storage.md:118-150`, issue two
concurrent HTTP `PUT` requests to the same user-scoped key, e.g. `PUT
/api/v1/extensions/acme/my-ext/storage/persistent/user/prefs`, from two separate
processes/threads, each with a different `"value"` in the JSON body.

3. Both requests are handled by `ExtensionStorageRestApi.set_user` in
`superset/extension_storage/api.py:372-71`, which is wrapped in `@transaction()` and calls
`ExtensionStorageDAO.set(...)` (API file lines 54–70) with identical `(extension_id, key,
user_fk)` arguments derived from `_extension_id()` and `get_user_id()`.

4. Inside `ExtensionStorageDAO.set` (`superset/extension_storage/daos.py:142-190`), each
transaction builds the same scope filter via `_scope_filter` (`daos.py:60-84`) and
executes `db.session.query(ExtensionStorage).filter(and_(...)).first()` (lines 158-167);
without any unique constraint on `(extension_id, user_fk, resource_type, resource_uuid,
key)` in the model (`superset/extension_storage/models.py:91-101`) or migration, both
concurrent SELECTs can see "no rows", each takes the `entry is None` branch, constructs a
new `ExtensionStorage` row (lines 176-187), and flushes, resulting in two rows with
identical logical key.

5. Subsequent reads via `GET /api/v1/extensions/acme/my-ext/storage/persistent/user/prefs`
hit `ExtensionStorageRestApi.get_user` (`api.py:331-369`), which calls
`ExtensionStorageDAO.get` (`daos.py:100-120`) and uses `.first()` without ordering; with
duplicate rows present, the database may return either row, making the returned value
nondeterministic, and `ExtensionStorageDAO.delete` (`daos.py:195-218`) will delete only
one of the duplicates, leaving "ghost" entries that continue to appear in `list_user`
(`daos.py:238-251`, exposed at `api.py:282-329`).

This is a comment left during a code review.

**Path:** superset/extension_storage/models.py
**Line:** 92:92
**Comment:**
	*Race Condition: The model does not enforce the documented per-scope key uniqueness at the database level, so concurrent `set` operations can insert duplicate rows for the same logical key and make reads/deletes nondeterministic (`.first()` may return any duplicate). Add unique partial indexes per scope so each logical key can exist only once.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

- ❌ Unit test test_key_rotation_roundtrip fails when encryption keys configured.
- ⚠️ Misaligned key precedence between DAO and unit tests.

1. Ensure the unit-test Flask app fixture defined in `tests/unit_tests/conftest.py:5-38`
loads `superset.config` (which defines `EXTENSION_STORAGE_ENCRYPTION_KEYS` at
`superset/config.py:5-11`) and permits overriding config keys via `request.param` (lines
26-38).

2. Run `pytest
tests/unit_tests/daos/test_extension_storage_dao.py::test_key_rotation_roundtrip -k '' -q`
while parametrizing the `app` fixture (per the pattern in
`tests/unit_tests/conftest.py:30-35`) so that
`app.config["EXTENSION_STORAGE_ENCRYPTION_KEYS"] = [b"custom-key-A", b"custom-key-B"]`,
making this list non-empty and different from `SECRET_KEY`.

3. Inside `test_key_rotation_roundtrip` at
`tests/unit_tests/daos/test_extension_storage_dao.py:73-88`, the call
`ExtensionStorageDAO.set(EXT_A, "rot_key", payload, is_encrypted=True)` delegates to
`ExtensionStorageDAO.set` in `superset/extension_storage/daos.py:63-77`, which computes
`stored_value = _fernet().encrypt(value)` using `_fernet()` at
`superset/extension_storage/daos.py:14-26`; `_fernet()` builds `raw_keys =
current_app.config.get("EXTENSION_STORAGE_ENCRYPTION_KEYS") or
[current_app.config["SECRET_KEY"]]` (lines 23-25), so with the overridden config the
ciphertext is encrypted with `raw_keys[0]` (the first element of
`EXTENSION_STORAGE_ENCRYPTION_KEYS`), not with `SECRET_KEY`.

4. Later in the same test, at `tests/unit_tests/daos/test_extension_storage_dao.py:89-97`,
the code constructs `old_fernet` from `old_raw = current_app.config["SECRET_KEY"]` (lines
92-95) and then calls `entry.value = multi.rotate(old_ciphertext)` at line 100 where
`multi = MultiFernet([new_fernet, old_fernet])`. Because `old_ciphertext` was not produced
by `old_fernet` (it was produced from `EXTENSION_STORAGE_ENCRYPTION_KEYS[0]`),
`MultiFernet.rotate(old_ciphertext)` cannot decrypt with either `new_fernet` or
`old_fernet` and raises `cryptography.fernet.InvalidToken`, causing
`test_key_rotation_roundtrip` to fail. Updating the test to derive `old_raw` via the same
precedence as `_fernet()` (first key from `EXTENSION_STORAGE_ENCRYPTION_KEYS` or
`SECRET_KEY` when the list is empty) aligns the test with production behavior and prevents
this spurious failure.

This is a comment left during a code review.

**Path:** tests/unit_tests/daos/test_extension_storage_dao.py
**Line:** 351:351
**Comment:**
	*Logic Error: This test derives the old encryption key from `SECRET_KEY`, but `ExtensionStorageDAO.set(..., is_encrypted=True)` uses the first key from `EXTENSION_STORAGE_ENCRYPTION_KEYS` when configured. In environments where that list is set, `multi.rotate(old_ciphertext)` will be built with the wrong old key and fail with `InvalidToken`. Derive the old key using the same config precedence as the DAO.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

- ❌ test_multi_fernet_decrypts_old_key fails when encryption keys configured.
- ⚠️ Tests no longer reflect DAO's real key precedence.

1. Use the same unit-test Flask app fixture from `tests/unit_tests/conftest.py:5-38` and
override its configuration (via the documented parametrization pattern at lines 26-38) so
that `app.config["EXTENSION_STORAGE_ENCRYPTION_KEYS"] = [b\"custom-key-A\",
b\"custom-key-B\"]`, making the extension encryption key list non-empty and not matching
`SECRET_KEY` from `superset/config.py:1-3`.

2. Run `pytest
tests/unit_tests/daos/test_extension_storage_dao.py::test_multi_fernet_decrypts_old_key -k
'' -q` so that `test_multi_fernet_decrypts_old_key` in
`tests/unit_tests/daos/test_extension_storage_dao.py:109-135` executes under this
configuration.

3. Within that test, the call `ExtensionStorageDAO.set(EXT_A, "multi_key", payload,
is_encrypted=True)` at line 119 invokes `ExtensionStorageDAO.set` in
`superset/extension_storage/daos.py:63-77`, which encrypts using `_fernet()` defined at
`superset/extension_storage/daos.py:14-26`; `_fernet()` builds a `MultiFernet` from
`raw_keys = current_app.config.get("EXTENSION_STORAGE_ENCRYPTION_KEYS") or
[current_app.config["SECRET_KEY"]]` (lines 23-25), so with the overridden config the
ciphertext in `entry.value` at line 121 is produced using the first configured extension
key, not `SECRET_KEY`.

4. Later in the same test at
`tests/unit_tests/daos/test_extension_storage_dao.py:124-132`, the code sets `old_raw =
current_app.config["SECRET_KEY"]` (lines 127-129) and derives `old_fernet` from that, then
constructs `multi = MultiFernet([new_fernet, old_fernet])` and calls
`multi.decrypt(entry.value)` at line 135. Because `entry.value` was not encrypted with
either `new_raw` or `SECRET_KEY` but with `EXTENSION_STORAGE_ENCRYPTION_KEYS[0]`,
`MultiFernet.decrypt` exhausts both keys and raises `cryptography.fernet.InvalidToken`,
causing `test_multi_fernet_decrypts_old_key` to fail despite the DAO working correctly.
Adjusting the test to obtain `old_raw` from `EXTENSION_STORAGE_ENCRYPTION_KEYS` (falling
back to `SECRET_KEY` when the list is empty) makes the test's key-selection logic
consistent with `_fernet()` and removes this spurious failure.

This is a comment left during a code review.

**Path:** tests/unit_tests/daos/test_extension_storage_dao.py
**Line:** 386:386
**Comment:**
	*Logic Error: This test also assumes the original ciphertext was encrypted with `SECRET_KEY`, but the DAO encrypts with the first key in `EXTENSION_STORAGE_ENCRYPTION_KEYS` when present. That mismatch makes the MultiFernet setup incorrect and can cause decryption failures in configured environments.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.