fix(search): guard active-index deletion and gate ES/OS delete cascade (#35640)

dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImpl.java

@@ -46,6 +46,7 @@
 import com.dotcms.variant.model.Variant;
 import com.dotmarketing.business.APILocator;
 import com.dotmarketing.business.CacheLocator;
+import com.dotmarketing.business.DotStateException;
 import com.dotmarketing.common.db.DotConnect;
 import com.dotmarketing.common.reindex.BulkProcessorListener;
 import com.dotmarketing.common.reindex.ReindexEntry;
@@ -1668,33 +1669,208 @@ private void logSwitchover(final IndiciesInfo oldInfo, final String luckyServer)
 
     }
 
+    /**
+     * When {@code true}, bypasses the guard that blocks deletion of an active
+     * (working/live) or building (reindex) index. Off by default: deleting an in-use
+     * index leaves the site with nothing to serve reads from or reindex into. Intended
+     * only for emergency/scripted maintenance.
+     */
+    public static final String FF_ALLOW_ACTIVE_INDEX_DELETE = "FEATURE_FLAG_ALLOW_ACTIVE_INDEX_DELETE";
+
+    /**
+     * Controls whether {@link #delete(String)} cascades to the index's twin in the other engine.
+     * On by default. The cascade is <strong>bidirectional</strong>: deleting a logical index by
+     * either its ES (bare) name or its OpenSearch ({@code .os}) name removes the index in every
+     * engine that holds it in the current phase (ES + OS twin during dual-write).
+     * <ul>
+     *   <li><strong>true (default)</strong> — delete both twins together, so neither engine is
+     *       left with an orphan copy.</li>
+     *   <li><strong>false</strong> — tag-dispatched, single engine only: a {@code .os} name
+     *       deletes OpenSearch, any other name deletes Elasticsearch; the twin is left intact
+     *       (issue #35640, TC-016).</li>
+     * </ul>
+     */
+    public static final String FF_INDEX_DELETE_CASCADE = "FEATURE_FLAG_INDEX_DELETE_CASCADE";
+
+    /**
+     * Rejects deletion of an index that is currently active (working/live) or being
+     * rebuilt (a reindex slot). The protected set is resolved phase-aware via
+     * {@link #getCurrentIndex()} and {@link #getNewIndex()} — the same sources the
+     * maintenance UI uses to flag an index <em>active</em>/<em>building</em> — so this
+     * server-side guard mirrors exactly what the UI already hides. The UI hid the Delete
+     * option for these indices; a direct REST/AJAX call previously bypassed that and could
+     * wipe the only index (see issue #35640, TC-018).
+     *
+     * <p>Bypass with the {@value #FF_ALLOW_ACTIVE_INDEX_DELETE} feature flag.</p>
+     *
+     * @throws DotStateException if the index is active/building (bypass off), or if the
+     *                           active set cannot be resolved (fail closed — a destructive
+     *                           op must not proceed on an unknown state).
+     */
+    private void assertIndexIsDeletable(final String indexName) {
+        if (Config.getBooleanProperty(FF_ALLOW_ACTIVE_INDEX_DELETE, false)) {
+            return;
+        }
+        // Compare on the LOGICAL (cluster-stripped, untagged) name on BOTH sides. The active
+        // set is bare in Phases 0-2 and .os-tagged in Phase 3, and the caller may pass either
+        // the bare or the .os name — normalizing both to the logical form makes the guard match
+        // regardless of phase or which twin's name was given (issue #35640).
+        final String requested = IndexTag.OS.untag(indexAPI.removeClusterIdFromName(indexName));
+        final Set<String> protectedIndices = new HashSet<>();
+        try {
+            for (final String n : getCurrentIndex()) { // active working + live
+                protectedIndices.add(IndexTag.OS.untag(n));
+            }
+            for (final String n : getNewIndex()) {     // building reindex slots
+                protectedIndices.add(IndexTag.OS.untag(n));
+            }
+        } catch (final DotDataException e) {
+            throw new DotStateException("Unable to verify whether index '" + indexName
+                    + "' is active before deletion; refusing to delete.", e);
+        }
+        if (protectedIndices.contains(requested)) {
+            throw new DotStateException("Index '" + indexName
+                    + "' is active or being rebuilt and cannot be deleted. Deactivate it first"
+                    + " (or set " + FF_ALLOW_ACTIVE_INDEX_DELETE + "=true to override).");
+        }
+    }
+
     public boolean delete(String indexName) {
-        // Mirror createContentIndex: resolve the per-provider physical name (ES → bare,
-        // OS → .os tag) via ops.toPhysicalName so a router-driven delete targets the REAL
-        // OS index. Routing the bare logical name straight to OSIndexAPIImpl.delete would
-        // hit an untagged name that does not exist and orphan the actual .os index.
-        // Track the primary provider's result; shadow failures in dual-write phases are
-        // fire-and-forget (the primary ES delete must not be undone by an OS shadow miss).
-        final ContentletIndexOperations primary = router.readProvider();
+        // Guard first: never delete an active/building index (issue #35640, TC-018).
+        assertIndexIsDeletable(indexName);
+
+        // Bidirectional cascade (default): deleting by either the ES (bare) or OS (.os) name
+        // removes the index in every engine that holds it. When disabled, only the engine that
+        // owns the given name is touched. See FF_INDEX_DELETE_CASCADE (issue #35640, TC-016).
+        final boolean cascade = Config.getBooleanProperty(FF_INDEX_DELETE_CASCADE, true);
+        final ContentletIndexOperations primary;
+        final List<ContentletIndexOperations> targets;
+        final String nameForTargets;
+        if (cascade) {
+            // Broadcast the LOGICAL (untagged) name to every write provider so each re-derives
+            // its OWN physical name (ES → bare, OS → .os) and both twins are removed regardless
+            // of which name the caller passed. Track the read provider's result as the primary.
+            primary = router.readProvider();
+            targets = router.writeProviders();
+            nameForTargets = IndexTag.OS.untag(indexName);
+        } else {
+            // Tag-dispatched, single engine: .os → OS, otherwise ES. The twin is left intact.
+            primary = IndexTag.OS.isTagged(indexName) ? router.osImpl() : router.esImpl();
+            targets = List.of(primary);
+            nameForTargets = indexName;
+        }
+
+        // The logical (cluster-stripped, untagged) name is used to find and clear DB pointers.
+        final String logicalName = IndexTag.OS.untag(indexAPI.removeClusterIdFromName(indexName));
+
         boolean primaryResult = false;
-        for (final ContentletIndexOperations ops : router.writeProviders()) {
-            final String physicalName = ops.toPhysicalName(indexName);
+        for (final ContentletIndexOperations ops : targets) {
+            // Resolve the per-provider physical name (ES → bare, OS → .os tag) via
+            // ops.toPhysicalName so the delete targets the REAL index. Routing a bare
+            // logical name straight to OSIndexAPIImpl.delete would hit an untagged name
+            // that does not exist and orphan the actual .os index.
+            final String physicalName = ops.toPhysicalName(nameForTargets);
+            // (1) Delete the cluster index. A failure here must NOT abort the operation —
+            // keep going so the remaining engine and the DB pointers are still handled.
             try {
                 final boolean r = ops.indexAPI().delete(physicalName);
                 if (ops == primary) {
                     primaryResult = r;
                 }
-            } catch (Exception e) {
+            } catch (final Exception e) {
                 Logger.error(this.getClass(), "Error while deleting index " + physicalName, e);
                 if (ops == primary) {
                     primaryResult = false;
                 }
                 // shadow failures are fire-and-forget in dual-write phases
             }
+            // (2) Always remove the indicies-table pointer for this engine, even if the cluster
+            // delete above failed, so no DB row is left dangling at a deleted index.
+            try {
+                clearStorePointer(ops, logicalName);
+            } catch (final Exception e) {
+                Logger.warn(this.getClass(),
+                        "Could not clear the indicies DB pointer for " + physicalName, e);
+            }
         }
         return primaryResult;
     }
 
+    /**
+     * Removes any {@code indicies} row that points at {@code logicalName} in the store that backs
+     * {@code ops}: the OpenSearch provider clears the versioned store, any other provider clears
+     * the legacy ES store. Best-effort — the caller wraps this so a failure never aborts the
+     * delete (issue #35640).
+     */
+    private void clearStorePointer(final ContentletIndexOperations ops, final String logicalName)
+            throws DotDataException {
+        if (ops == router.osImpl()) {
+            clearOsStorePointer(logicalName);
+        } else {
+            clearEsStorePointer(logicalName);
+        }
+    }
+
+    /**
+     * True when {@code storedName} resolves to the same logical (cluster-stripped, untagged) name
+     * as {@code logicalName}. Lets a DB slot be matched regardless of the prefix/tag form it was
+     * stored in.
+     */
+    private boolean matchesLogical(final String storedName, final String logicalName) {
+        return storedName != null
+                && logicalName.equals(IndexTag.OS.untag(indexAPI.removeClusterIdFromName(storedName)));
+    }
+
+    /** Clears any legacy ES-store ({@link IndiciesInfo}) slot pointing at {@code logicalName}. */
+    private void clearEsStorePointer(final String logicalName) throws DotDataException {
+        final IndiciesInfo info = legacyIndiciesAPI.loadIndicies();
+        final IndiciesInfo.Builder builder = IndiciesInfo.Builder.copy(info);
+        boolean changed = false;
+        if (matchesLogical(info.getWorking(), logicalName))        { builder.setWorking(null);        changed = true; }
+        if (matchesLogical(info.getLive(), logicalName))           { builder.setLive(null);           changed = true; }
+        if (matchesLogical(info.getReindexWorking(), logicalName)) { builder.setReindexWorking(null); changed = true; }
+        if (matchesLogical(info.getReindexLive(), logicalName))    { builder.setReindexLive(null);    changed = true; }
+        if (matchesLogical(info.getSiteSearch(), logicalName))     { builder.setSiteSearch(null);     changed = true; }
+        if (changed) {
+            legacyIndiciesAPI.point(builder.build());
+        }
+    }
+
+    /** Clears any OS versioned-store ({@link VersionedIndices}) slot pointing at {@code logicalName}. */
+    private void clearOsStorePointer(final String logicalName) throws DotDataException {
+        final Optional<VersionedIndices> existingOpt = versionedIndicesAPI.loadDefaultVersionedIndices();
+        if (existingOpt.isEmpty()) {
+            return;
+        }
+        final VersionedIndices existing = existingOpt.get();
+        final VersionedIndicesImpl.Builder builder = VersionedIndicesImpl.builder();
+        boolean changed = false;
+        // Re-add every slot except the one(s) that resolve to the deleted logical name.
+        if (existing.working().isPresent()) {
+            if (matchesLogical(existing.working().get(), logicalName)) { changed = true; }
+            else { builder.working(existing.working().get()); }
+        }
+        if (existing.live().isPresent()) {
+            if (matchesLogical(existing.live().get(), logicalName)) { changed = true; }
+            else { builder.live(existing.live().get()); }
+        }
+        if (existing.reindexWorking().isPresent()) {
+            if (matchesLogical(existing.reindexWorking().get(), logicalName)) { changed = true; }
+            else { builder.reindexWorking(existing.reindexWorking().get()); }
+        }
+        if (existing.reindexLive().isPresent()) {
+            if (matchesLogical(existing.reindexLive().get(), logicalName)) { changed = true; }
+            else { builder.reindexLive(existing.reindexLive().get()); }
+        }
+        if (existing.siteSearch().isPresent()) {
+            if (matchesLogical(existing.siteSearch().get(), logicalName)) { changed = true; }
+            else { builder.siteSearch(existing.siteSearch().get()); }
+        }
+        if (changed) {
+            versionedIndicesAPI.saveIndices(builder.build());
+        }
+    }
+
     public boolean optimize(List<String> indexNames) {
         return indexAPI.optimize(indexNames);
     }

dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java

@@ -39,13 +39,15 @@
 import com.dotcms.content.elasticsearch.util.ESReindexationProcessStatus;
 import com.dotcms.contenttype.model.type.ContentType;
 import com.google.common.annotations.VisibleForTesting;
+import com.dotcms.rest.ErrorEntity;
 import com.dotcms.rest.InitDataObject;
 import com.dotcms.rest.ResourceResponse;
 import com.dotcms.rest.ResponseEntityView;
 import com.dotcms.rest.WebResource;
 import com.dotcms.rest.annotation.NoCache;
 import com.dotcms.rest.api.v1.authentication.ResponseUtil;
 import com.dotmarketing.business.APILocator;
+import com.dotmarketing.business.DotStateException;
 import com.dotmarketing.business.LayoutAPI;
 import com.dotmarketing.business.Role;
 import com.dotmarketing.common.reindex.IndexResourceHelper;
@@ -386,13 +388,30 @@ public Response deleteIndex(@Context final HttpServletRequest request, @Context
 
         final InitDataObject init = auth(request, response);
 
-        if(indexExists(indexName) ){
-            return Response.status(404).build();
+        // Accept either the short name (as the UI sends it) or the full physical name with the
+        // cluster prefix (as `_cat/indices` reports it): normalize to the cluster-stripped form
+        // that listDotCMSIndices()/delete() expect, so a full physical name no longer 404s
+        // (issue #35640, TC-016).
+        final String resolvedName = indexAPI.removeClusterIdFromName(indexName);
+
+        if(indexDoesNotExist(resolvedName) ){
+            // Readable 404 body (no stack trace) so a mistyped/nonexistent name is clear
+            // (issue #35640, TC-017).
+            return Response.status(Response.Status.NOT_FOUND)
+                    .entity(new ResponseEntityView<>(List.of(new ErrorEntity("INDEX_NOT_FOUND", "Index not found: " + indexName)))).build();
+        }
+
+        try {
+            idxApi.delete(resolvedName);
+        } catch (final DotStateException e) {
+            // The index is active/building (or its state could not be verified): reject with a
+            // readable 400 instead of a stack trace. See issue #35640, TC-018.
+            Logger.warn(this, "Rejected deletion of index '" + resolvedName + "': " + e.getMessage());
+            return Response.status(Response.Status.BAD_REQUEST)
+                    .entity(new ResponseEntityView<>(List.of(new ErrorEntity("INDEX_NOT_DELETABLE", e.getMessage())))).build();
         }
-
-        idxApi.delete(indexName);
 
-        String message = "Index:" + indexName + " deleted";
+        String message = "Index:" + resolvedName + " deleted";
 
         sendAdminMessage(message, MessageSeverity.INFO,init.getUser(), 5000);
 
@@ -541,30 +560,37 @@ public Response modIndex(@Context final HttpServletRequest request, @Context fin
 
         final InitDataObject init = auth(request, response);
         final IndexAction indexAction = IndexAction.fromString(action);
-
-        if(indexExists(indexName) ){
-            return Response.status(404).build();
+
+        // Same normalization as deleteIndex: accept both the short name and the full physical
+        // name (with cluster prefix) so this endpoint stays consistent and does not 404 on the
+        // latter (issue #35640, TC-016).
+        final String resolvedName = indexAPI.removeClusterIdFromName(indexName);
+
+        if(indexDoesNotExist(resolvedName) ){
+            // Readable 404 body, consistent with deleteIndex (issue #35640, TC-017).
+            return Response.status(Response.Status.NOT_FOUND)
+                    .entity(new ResponseEntityView<>(List.of(new ErrorEntity("INDEX_NOT_FOUND", "Index not found: " + indexName)))).build();
         }
-        
-        
+
+
         switch(indexAction){
             case DEACTIVATE:
-                APILocator.getContentletIndexAPI().deactivateIndex(indexName);
+                APILocator.getContentletIndexAPI().deactivateIndex(resolvedName);
                 break;
             case CLEAR:
-                APILocator.getESIndexAPI().clearIndex(indexName);
+                APILocator.getESIndexAPI().clearIndex(resolvedName);
                 break;
             case OPEN:
-                APILocator.getESIndexAPI().openIndex(indexName);
+                APILocator.getESIndexAPI().openIndex(resolvedName);
                 break;
             case CLOSE:
-                APILocator.getESIndexAPI().closeIndex(indexName);
+                APILocator.getESIndexAPI().closeIndex(resolvedName);
                 break;
             default:
-                APILocator.getContentletIndexAPI().activateIndex(indexName);
-            
+                APILocator.getContentletIndexAPI().activateIndex(resolvedName);
+
         }
-        String message = indexAction.name().toLowerCase() + " " + indexName;
+        String message = indexAction.name().toLowerCase() + " " + resolvedName;
 
         sendAdminMessage(message, MessageSeverity.INFO,init.getUser(), 5000);
 
@@ -590,7 +616,12 @@ public Response getIndexStatus(@Context final HttpServletRequest request,
 
     }
 
-    private boolean indexExists(final String indexName) {
+    /**
+     * Returns {@code true} when the given index name is present in neither the open nor the
+     * closed dotCMS index list — i.e. the index does <strong>not</strong> exist. Named for the
+     * caller's guard ({@code if (indexDoesNotExist(...)) return 404}).
+     */
+    private boolean indexDoesNotExist(final String indexName) {
         return !idxApi.listDotCMSIndices().contains(indexName) && !idxApi.listDotCMSClosedIndices().contains(indexName) ;
     }
 }