Always treat dash-prefixed expression as index exclusion (elastic#138467)

ywangd · web-flow · commit 92750be0e6bd · 2025-12-08T06:09:01.000+01:00
Fixed a bug that dash prefix expressions, including both concrete names and wildcard patterns, are not always treated as index exclusion during index resolution process. The PR also updates relevant document to make the behaviour clear. Resolves: elastic#64752 Resolves: elastic#83435
diff --git a/docs/changelog/138467.yaml b/docs/changelog/138467.yaml
@@ -0,0 +1,9 @@
+pr: 138467
+summary: |
+  Fixed a bug where dash-prefixed expressions were not consistently excluded during index resolution.
+  This impacted both specific index names and wildcard patterns (example: `-index, -logs-*`).
+area: Security
+type: bug
+issues:
+  - 64752
+  - 83435
diff --git a/docs/reference/elasticsearch/rest-apis/api-conventions.md b/docs/reference/elasticsearch/rest-apis/api-conventions.md
@@ -282,7 +282,20 @@ Most APIs that accept a `<data-stream>`, `<index>`, or `<target>` request path p
 
 In multi-target syntax, you can use a comma-separated list to run a request on multiple resources, such as data streams, indices, or aliases: `test1,test2,test3`. You can also use [glob-like](https://en.wikipedia.org/wiki/Glob_(programming)) wildcard (`*`) expressions to target resources that match a pattern: `test*` or `*test` or `te*t` or `*test*`.
 
-You can exclude targets using the `-` character: `test*,-test3`.
+Targets can be excluded by prefixing with the `-` character. This applies to both concrete names and wildcard patterns.
+For example, `test*,-test3` resolves to all resources that start with `test` except for the resource named `test3`.
+It is possible for exclusion to exclude all resources. For example, `test*,-test*` resolves to an empty set.
+An exclusion affects targets listed _before_ it and has no impact on targets listed _after_ it.
+For example, `test3*,-test3,test*` resolves to all resources that start with `test`, including `test3`, because it is included
+by the last `test*` pattern.
+
+{applies_to}`stack: ga 9.3` A dash-prefixed (`-`) expression is always treated as an exclusion.
+
+In previous versions, dash-prefixed expressions were sometimes not treated as exclusions due to a bug. For example:
+- `test,-test` threw an `IndexNotFoundException` instead of excluding `test`
+- `test3,-test*` incorrectly resolved to `test3` instead of excluding matches to the wildcard pattern
+
+This bug is fixed in 9.3.
 
 ::::{important}
 Aliases are resolved after wildcard expressions. This can result in a request that targets an excluded alias. For example, if `test3` is an index alias, the pattern `test*,-test3` still targets the indices for `test3`. To avoid this, exclude the concrete indices for the alias instead.
diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java
@@ -143,7 +143,13 @@ public void testBasicWorkFlow() throws Exception {
             }
         }
 
-        final String[] indicesToSnapshot = { "test-idx-*", "-test-idx-3" };
+        final String[] indicesToSnapshot;
+        final boolean exclusionWithoutWildcard = randomBoolean();
+        if (exclusionWithoutWildcard) {
+            indicesToSnapshot = new String[] { "test-idx-1", "test-idx-2", "test-idx-3", "-test-idx-3" };
+        } else {
+            indicesToSnapshot = new String[] { "test-idx-*", "-test-idx-3" };
+        }
 
         logger.info("--> capturing history UUIDs");
         final Map<ShardId, String> historyUUIDs = new HashMap<>();
@@ -243,7 +249,13 @@ public void testBasicWorkFlow() throws Exception {
                 .getSetting("test-idx-1", MetadataIndexStateService.VERIFIED_BEFORE_CLOSE_SETTING.getKey())
         );
 
-        for (ShardStats shardStats : indicesAdmin().prepareStats(indicesToSnapshot).clear().get().getShards()) {
+        final String[] indicesToStats;
+        if (exclusionWithoutWildcard) {
+            indicesToStats = new String[] { "test-idx-1", "test-idx-3", "-test-idx-3" };
+        } else {
+            indicesToStats = indicesToSnapshot;
+        }
+        for (ShardStats shardStats : indicesAdmin().prepareStats(indicesToStats).clear().get().getShards()) {
             String historyUUID = shardStats.getCommitStats().getUserData().get(Engine.HISTORY_UUID_KEY);
             ShardId shardId = shardStats.getShardRouting().shardId();
             assertThat(shardStats.getShardRouting() + " doesn't have a history uuid", historyUUID, notNullValue());
diff --git a/server/src/main/java/org/elasticsearch/action/ResolvedIndexExpressions.java b/server/src/main/java/org/elasticsearch/action/ResolvedIndexExpressions.java
@@ -93,8 +93,14 @@ public void addRemoteExpressions(String original, Set<String> remoteExpressions)
         public void excludeFromLocalExpressions(Set<String> expressionsToExclude) {
             Objects.requireNonNull(expressionsToExclude);
             if (expressionsToExclude.isEmpty() == false) {
-                for (ResolvedIndexExpression prior : expressions) {
-                    final Set<String> localExpressions = prior.localExpressions().indices();
+                final var iter = expressions.iterator();
+                while (iter.hasNext()) {
+                    final ResolvedIndexExpression current = iter.next();
+                    if (expressionsToExclude.contains(current.original())) {
+                        iter.remove();
+                        continue;
+                    }
+                    final Set<String> localExpressions = current.localExpressions().indices();
                     if (localExpressions.isEmpty()) {
                         continue;
                     }
diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java
@@ -49,9 +49,8 @@ public ResolvedIndexExpressions resolveIndexAbstractions(
         final boolean includeDataStreams
     ) {
         final ResolvedIndexExpressions.Builder resolvedExpressionsBuilder = ResolvedIndexExpressions.builder();
-        boolean wildcardSeen = false;
         for (String originalIndexExpression : indices) {
-            wildcardSeen = resolveIndexAbstraction(
+            resolveIndexAbstraction(
                 resolvedExpressionsBuilder,
                 originalIndexExpression,
                 originalIndexExpression, // in the case of local resolution, the local expression is always the same as the original
@@ -60,8 +59,7 @@ public ResolvedIndexExpressions resolveIndexAbstractions(
                 allAuthorizedAndAvailableBySelector,
                 isAuthorized,
                 includeDataStreams,
-                Set.of(),
-                wildcardSeen
+                Set.of()
             );
         }
         return resolvedExpressionsBuilder.build();
@@ -86,7 +84,6 @@ public ResolvedIndexExpressions resolveIndexAbstractions(
         final String originProjectAlias = targetProjects.originProjectAlias();
         final Set<String> linkedProjectAliases = targetProjects.allProjectAliases();
         final ResolvedIndexExpressions.Builder resolvedExpressionsBuilder = ResolvedIndexExpressions.builder();
-        boolean wildcardSeen = false;
         for (String originalIndexExpression : indices) {
             final CrossProjectIndexExpressionsRewriter.IndexRewriteResult indexRewriteResult = CrossProjectIndexExpressionsRewriter
                 .rewriteIndexExpression(originalIndexExpression, originProjectAlias, linkedProjectAliases, projectRouting);
@@ -100,7 +97,7 @@ public ResolvedIndexExpressions resolveIndexAbstractions(
                 continue;
             }
 
-            wildcardSeen = resolveIndexAbstraction(
+            resolveIndexAbstraction(
                 resolvedExpressionsBuilder,
                 originalIndexExpression,
                 localIndexExpression,
@@ -109,14 +106,13 @@ public ResolvedIndexExpressions resolveIndexAbstractions(
                 allAuthorizedAndAvailableBySelector,
                 isAuthorized,
                 includeDataStreams,
-                indexRewriteResult.remoteExpressions(),
-                wildcardSeen
+                indexRewriteResult.remoteExpressions()
             );
         }
         return resolvedExpressionsBuilder.build();
     }
 
-    private boolean resolveIndexAbstraction(
+    private void resolveIndexAbstraction(
         final ResolvedIndexExpressions.Builder resolvedExpressionsBuilder,
         final String originalIndexExpression,
         final String localIndexExpression,
@@ -125,12 +121,11 @@ private boolean resolveIndexAbstraction(
         final Function<IndexComponentSelector, Set<String>> allAuthorizedAndAvailableBySelector,
         final BiPredicate<String, IndexComponentSelector> isAuthorized,
         final boolean includeDataStreams,
-        final Set<String> remoteExpressions,
-        boolean wildcardSeen
+        final Set<String> remoteExpressions
     ) {
         String indexAbstraction;
         boolean minus = false;
-        if (localIndexExpression.charAt(0) == '-' && wildcardSeen) {
+        if (localIndexExpression.charAt(0) == '-') {
             indexAbstraction = localIndexExpression.substring(1);
             minus = true;
         } else {
@@ -150,7 +145,6 @@ private boolean resolveIndexAbstraction(
         indexAbstraction = IndexNameExpressionResolver.resolveDateMathExpression(indexAbstraction);
 
         if (indicesOptions.expandWildcardExpressions() && Regex.isSimpleMatchPattern(indexAbstraction)) {
-            wildcardSeen = true;
             final HashSet<String> resolvedIndices = new HashSet<>();
             for (String authorizedIndex : allAuthorizedAndAvailableBySelector.apply(selector)) {
                 if (Regex.simpleMatch(indexAbstraction, authorizedIndex)
@@ -167,11 +161,14 @@ && isIndexVisible(
                 }
             }
             if (resolvedIndices.isEmpty()) {
-                // es core honours allow_no_indices for each wildcard expression, we do the same here by throwing index not found.
-                if (indicesOptions.allowNoIndices() == false) {
-                    throw new IndexNotFoundException(indexAbstraction);
+                // Ignore empty result for exclusions
+                if (minus == false) {
+                    // es core honours allow_no_indices for each wildcard expression, we do the same here by throwing index not found.
+                    if (indicesOptions.allowNoIndices() == false) {
+                        throw new IndexNotFoundException(indexAbstraction);
+                    }
+                    resolvedExpressionsBuilder.addExpressions(originalIndexExpression, new HashSet<>(), SUCCESS, remoteExpressions);
                 }
-                resolvedExpressionsBuilder.addExpressions(originalIndexExpression, new HashSet<>(), SUCCESS, remoteExpressions);
             } else {
                 if (minus) {
                     resolvedExpressionsBuilder.excludeFromLocalExpressions(resolvedIndices);
@@ -219,7 +216,6 @@ && isIndexVisible(
                 }
             }
         }
-        return wildcardSeen;
     }
 
     private static void resolveSelectorsAndCollect(
diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java
@@ -401,12 +401,11 @@ protected static Collection<ResolvedExpression> resolveExpressionsToResources(Co
         Collection<ResolvedExpression> resources = expandWildcards && WildcardExpressionResolver.hasWildcards(expressions)
             ? new LinkedHashSet<>()
             : new ArrayList<>(expressions.length);
-        boolean wildcardSeen = false;
         for (int i = 0, n = expressions.length; i < n; i++) {
             String originalExpression = expressions[i];
 
-            // Resolve exclusion, a `-` prefixed expression is an exclusion only if it succeeds a wildcard.
-            boolean isExclusion = wildcardSeen && originalExpression.startsWith("-");
+            // Resolve exclusion, a `-` prefixed expression is an exclusion
+            boolean isExclusion = originalExpression.startsWith("-");
             String baseExpression = isExclusion ? originalExpression.substring(1) : originalExpression;
 
             // Parse a potential selector from the expression
@@ -422,7 +421,6 @@ protected static Collection<ResolvedExpression> resolveExpressionsToResources(Co
 
             // Check if it's wildcard
             boolean isWildcard = expandWildcards && WildcardExpressionResolver.isWildcard(baseExpression);
-            wildcardSeen |= isWildcard;
 
             if (isWildcard) {
                 Set<ResolvedExpression> matchingResources = WildcardExpressionResolver.matchWildcardToResources(
@@ -756,17 +754,7 @@ private static IndexNotFoundException notFoundException(String... indexExpressio
             infe = new IndexNotFoundException("no indices exist", Metadata.ALL);
             infe.setResources("index_or_alias", Metadata.ALL);
         } else if (indexExpressions.length == 1) {
-            if (indexExpressions[0].startsWith("-")) {
-                // this can arise when multi-target syntax is used with an exclusion not in the "inclusion" list, such as
-                // "GET test1,test2,-test3"
-                // the caller should have put "GET test*,-test3"
-                infe = new IndexNotFoundException(
-                    "if you intended to exclude this index, ensure that you use wildcards that include it before explicitly excluding it",
-                    indexExpressions[0]
-                );
-            } else {
-                infe = new IndexNotFoundException(indexExpressions[0]);
-            }
+            infe = new IndexNotFoundException(indexExpressions[0]);
             infe.setResources("index_or_alias", indexExpressions[0]);
         } else {
             infe = new IndexNotFoundException((String) null);
diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java
@@ -451,18 +451,8 @@ public void testConcreteIndexNamesExpandWildcards() {
         );
         assertThat(infe.getResourceId().toString(), equalTo("[-*]"));
 
-        infe = expectThrows(
-            IndexNotFoundException.class,
-            // throws error because "-foobar" was not covered by a wildcard that included it
-            () -> indexNameExpressionResolver.concreteIndexNames(context2, "bar", "hidden", "-foobar")
-        );
-        assertThat(
-            infe.getMessage(),
-            containsString(
-                "if you intended to exclude this index, ensure that you use wildcards that include it " + "before explicitly excluding it"
-            )
-        );
-        assertThat(infe.getResourceId().toString(), equalTo("[-foobar]"));
+        results = indexNameExpressionResolver.concreteIndexNames(context2, "bar", "hidden", "-foobar");
+        assertThat(results, arrayContainingInAnyOrder("bar", "hidden"));
 
         // open and hidden
         options = IndicesOptions.fromOptions(false, true, true, false, true);
@@ -983,9 +973,6 @@ public void testConcreteIndicesWildcardWithNegation() {
             .put(indexBuilder("testXXX").state(State.OPEN))
             .put(indexBuilder("testXXY").state(State.OPEN))
             .put(indexBuilder("testXYY").state(State.OPEN))
-            .put(indexBuilder("-testXYZ").state(State.OPEN))
-            .put(indexBuilder("-testXZZ").state(State.OPEN))
-            .put(indexBuilder("-testYYY").state(State.OPEN))
             .put(indexBuilder("testYYY").state(State.OPEN))
             .put(indexBuilder("testYYX").state(State.OPEN))
             .build();
@@ -1005,19 +992,13 @@ public void testConcreteIndicesWildcardWithNegation() {
             equalTo(newHashSet("testYYY", "testYYX"))
         );
 
-        assertThat(
-            newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "-testX*")),
-            equalTo(newHashSet("-testXYZ", "-testXZZ"))
-        );
+        assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "-testX*")), empty());
 
-        assertThat(
-            newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testXXY", "-testX*")),
-            equalTo(newHashSet("testXXY", "-testXYZ", "-testXZZ"))
-        );
+        assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testXXY", "-testX*")), empty());
 
         assertThat(
-            newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "*", "--testX*")),
-            equalTo(newHashSet("testXXX", "testXXY", "testXYY", "testYYX", "testYYY", "-testYYY"))
+            newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "*", "-testX*")),
+            equalTo(newHashSet("testYYX", "testYYY"))
         );
 
         assertThat(
@@ -1032,7 +1013,7 @@ public void testConcreteIndicesWildcardWithNegation() {
 
         assertThat(
             newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "testXXX", "testXXY", "testYYY", "-testYYY")),
-            equalTo(newHashSet("testXXX", "testXXY", "testYYY", "-testYYY"))
+            equalTo(newHashSet("testXXX", "testXXY"))
         );
 
         assertThat(
@@ -1042,16 +1023,13 @@ public void testConcreteIndicesWildcardWithNegation() {
 
         assertThat(
             newHashSet(indexNameExpressionResolver.concreteIndexNames(context, "-testXXX", "*testY*", "-testYYY")),
-            equalTo(newHashSet("testYYX", "-testYYY"))
+            equalTo(newHashSet("testYYX"))
         );
 
         String[] indexNames = indexNameExpressionResolver.concreteIndexNames(project, IndicesOptions.lenientExpandOpen(), "-doesnotexist");
         assertEquals(0, indexNames.length);
 
-        assertThat(
-            newHashSet(indexNameExpressionResolver.concreteIndexNames(project, IndicesOptions.lenientExpandOpen(), "-*")),
-            equalTo(newHashSet("-testXYZ", "-testXZZ", "-testYYY"))
-        );
+        assertThat(newHashSet(indexNameExpressionResolver.concreteIndexNames(project, IndicesOptions.lenientExpandOpen(), "-*")), empty());
 
         assertThat(
             newHashSet(
@@ -1064,7 +1042,7 @@ public void testConcreteIndicesWildcardWithNegation() {
                     "-testXXY"
                 )
             ),
-            equalTo(newHashSet("testXXX", "testXYY", "testXXY"))
+            equalTo(newHashSet("testXXX", "testXYY"))
         );
 
         indexNames = indexNameExpressionResolver.concreteIndexNames(project, IndicesOptions.lenientExpandOpen(), "*", "-*");
diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authz/ReadActionsTests.java
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/IndicesAndAliasesResolverTests.java
