codeant-ai-for-open-source[bot] commented on code in PR #41550:
URL: https://github.com/apache/superset/pull/41550#discussion_r3691343034
##########
superset-frontend/packages/superset-ui-core/src/components/DeleteModal/index.tsx:
##########
@@ -74,32 +81,34 @@ export function DeleteModal({
return (
<Modal
- disablePrimaryButton={disableChange}
+ disablePrimaryButton={showConfirmationInput ? disableChange : false}
Review Comment:
**Suggestion:** The confirmation state is not reset when the modal closes or
when its mode changes. After a user successfully types DELETE, `disableChange`
remains false, so reopening the same mounted modal allows a permanent deletion
without re-entering the confirmation text; switching from recoverable mode back
to confirmation mode can expose the same stale enabled state. Reset
`disableChange` whenever the modal is hidden or when confirmation mode is
re-entered. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Permanent deletion can bypass the type-to-confirm safeguard.
- ❌ Repeated modal use can cause accidental irreversible deletion.
- ⚠️ Affects archived-item purge and other mounted delete dialogs.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4a5ddcf350244fddb99261d7161c9aaf&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4a5ddcf350244fddb99261d7161c9aaf&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/DeleteModal/index.tsx
**Line:** 84:84
**Comment:**
*Logic Error: The confirmation state is not reset when the modal closes
or when its mode changes. After a user successfully types DELETE,
`disableChange` remains false, so reopening the same mounted modal allows a
permanent deletion without re-entering the confirmation text; switching from
recoverable mode back to confirmation mode can expose the same stale enabled
state. Reset `disableChange` whenever the modal is hidden or when confirmation
mode is re-entered.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=1f27d3cbabfecbe5ce8d122780a2cce009dd9b93730a17c48d7097262c9a0d69&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=1f27d3cbabfecbe5ce8d122780a2cce009dd9b93730a17c48d7097262c9a0d69&reaction=dislike'>👎</a>
##########
superset/commands/purge.py:
##########
@@ -0,0 +1,177 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Owner/admin-gated permanent delete (force-purge) of a soft-deleted entity.
+
+This is the RBAC-enforced REST surface anticipated by the deletion-retention
+force-purge contract: it verifies ownership (owners/admins, mirroring restore)
+on the soft-deleted row, then delegates the irreversible cascade removal to
+``ForcePurgeCommand`` (which handles dependents, M:N rows, version history,
+audit, and the commit). Restricted to *soft-deleted* rows so it only operates
on
+items the user already sees in the archive.
+"""
+
+import logging
+from dataclasses import dataclass
+from typing import Any
+
+from sqlalchemy.exc import SQLAlchemyError
+
+from superset import is_feature_enabled, security_manager
+from superset.commands.base import BaseCommand
+from superset.commands.deletion_retention.force_purge import ForcePurgeCommand
+from superset.daos.base import BaseDAO
+from superset.daos.exceptions import DAODeleteFailedError
+from superset.exceptions import SupersetSecurityException
+from superset.models.helpers import SoftDeleteMixin
+from superset.tasks.utils import get_current_user
+
+#: Recorded when the audit trail cannot name the acting user. The purge routes
+#: are ``@protect()``-ed, so this should be unreachable; it exists so an
+#: anomaly is visible as one rather than disguised as a plausible username.
+logger = logging.getLogger(__name__)
+
+UNKNOWN_ACTOR = "unknown"
+
+
+@dataclass(frozen=True)
+class SoftDeleteBinding:
+ """Entity-specific bindings for the soft-delete purge command.
+
+ Lets one command serve every soft-delete type without a subclass per
+ entity. The REST route supplies the binding for its entity (see each
+ ``*RestApi``).
+ """
+
+ dao: type[BaseDAO[Any]]
+ not_found: type[Exception]
+ forbidden: type[Exception]
+ delete_failed: type[Exception]
+
+
+class PurgeArchivedCommand(BaseCommand):
+ """Permanently delete a single soft-deleted entity, by UUID."""
+
+ def __init__(self, model_uuid: str, binding: SoftDeleteBinding) -> None:
+ self._model_uuid = model_uuid
+ self._binding = binding
+ #: The authorized entity, resolved by ``validate()``. ``BaseCommand``
+ #: fixes ``validate()``'s return type as ``None``, so the model is
+ #: handed to ``run()`` here rather than returned.
+ self._model: SoftDeleteMixin | None = None
+
+ def run(self) -> None:
+ self.validate()
+ model = self._model
+ if model is None: # pragma: no cover — validate() raises or sets it
+ raise self._binding.not_found(f"No row with
uuid={self._model_uuid!r}")
+ try:
+ # ForcePurgeCommand owns the cascade + commit + audit.
+ #
+ # model_cls pins resolution to the type this route authorized:
+ # UUIDs are unique per table but not across them, so an
+ # unconstrained search could purge a different entity than the one
+ # validate() checked the caller against.
+ #
+ # require_archived re-asserts soft-deleted state at resolution
+ # time, closing the window between authorization and purge in which
+ # a concurrent restore would otherwise expose a live row.
+ result = ForcePurgeCommand(
+ self._model_uuid,
+ actor=get_current_user() or UNKNOWN_ACTOR,
+ model_cls=type(model),
+ require_archived=True,
+ # An end user's irreversible purge must never run unaudited;
+ # the CLI's fail-open default is an operator-trust decision
+ # that does not extend to REST principals.
+ require_audit=True,
Review Comment:
**Suggestion:** The caller's editorship is checked only in `validate()`, but
`ForcePurgeCommand` resolves the row again after the audit write and then
performs the destructive cascade without rechecking authorization. If the
user's ownership or admin role is revoked after validation succeeds, this
request can still permanently purge the archived object. Tie the authorization
check to the final row resolution or pass an authorization callback that is
evaluated immediately before the cascade. [security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Revoked editors can still permanently delete archived datasets during an
in-flight request.
- ❌ Purge removes the entity and dependents through `cascade_hard_delete()`.
- ⚠️ The audit record does not establish that authorization remained valid.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9e6ddb386aa64516acf3ed8ddbf7ff74&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9e6ddb386aa64516acf3ed8ddbf7ff74&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/purge.py
**Line:** 92:100
**Comment:**
*Security: The caller's editorship is checked only in `validate()`, but
`ForcePurgeCommand` resolves the row again after the audit write and then
performs the destructive cascade without rechecking authorization. If the
user's ownership or admin role is revoked after validation succeeds, this
request can still permanently purge the archived object. Tie the authorization
check to the final row resolution or pass an authorization callback that is
evaluated immediately before the cascade.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=5dd5c6c81ff4206a66028e9dce18e7b46ef62658a93de1f2763edcec94097cb6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=5dd5c6c81ff4206a66028e9dce18e7b46ef62658a93de1f2763edcec94097cb6&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
