Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.DISABLE_SESSION_FIELD;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.EMBEDDING_MODEL_ID_FIELD;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.EMBEDDING_MODEL_TYPE_FIELD;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.INDEX_PREFIX_INVALID_CHARACTERS_ERROR;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.INDEX_SETTINGS_FIELD;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.INVALID_EMBEDDING_MODEL_TYPE_ERROR;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.LLM_ID_FIELD;
Expand All @@ -36,6 +37,8 @@
import java.util.Map;
import java.util.UUID;

import org.opensearch.OpenSearchParseException;
import org.opensearch.cluster.metadata.MetadataCreateIndexService;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.common.io.stream.Writeable;
Expand Down Expand Up @@ -133,6 +136,14 @@ private String buildIndexPrefix(String indexPrefix, boolean useSystemIndex) {
? DEFAULT_MEMORY_INDEX_PREFIX
: UUID.randomUUID().toString().replace("-", "").substring(0, 8).toLowerCase();
}
if (indexPrefix.indexOf('\r') >= 0 || indexPrefix.indexOf('\n') >= 0 || indexPrefix.chars().anyMatch(ch -> ch < 32)) {
throw new OpenSearchParseException(INDEX_PREFIX_INVALID_CHARACTERS_ERROR);
}
MetadataCreateIndexService
.validateIndexOrAliasName(
indexPrefix,
(s1, s2) -> new OpenSearchParseException("missing or invalid index prefix [" + s1 + "] " + s2)
);
return indexPrefix;
}

Expand Down Expand Up @@ -615,11 +626,6 @@ public static String extractModelIdFromPipeline(Map<String, Object> pipelineSour
if (modelId instanceof String) {
return (String) modelId;
}
log
.warn(
"Pipeline text_embedding model_id is not a String: {}",
modelId != null ? modelId.getClass().getSimpleName() : "null"
);
}
}
// Check sparse_encoding processor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ public class MemoryContainerConstants {
public static final String INVALID_EMBEDDING_MODEL_TYPE_ERROR = "Embedding model type must be either TEXT_EMBEDDING or SPARSE_ENCODING";
public static final String MAX_INFER_SIZE_LIMIT_ERROR = "Maximum infer size cannot exceed 10";
public static final String FIELD_NOT_ALLOWED_SEMANTIC_DISABLED_ERROR = "Field %s is not allowed when semantic storage is disabled";
public static final String INDEX_PREFIX_INVALID_CHARACTERS_ERROR = "Index prefix must not contain any control characters";
public static final String BACKEND_ROLE_INVALID_LENGTH_ERROR = "Backend role exceeds maximum length of 128 characters: %s";
public static final String BACKEND_ROLE_INVALID_CHARACTERS_ERROR =
"Backend role contains invalid characters. Only alphanumeric characters and :+=,.@-_/ are allowed: %s";
public static final String BACKEND_ROLE_EMPTY_ERROR = "Backend role cannot be empty or blank";

// Model validation error messages
public static final String LLM_MODEL_NOT_FOUND_ERROR = "LLM model with ID %s not found";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,16 @@
import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken;
import static org.opensearch.ml.common.CommonValue.BACKEND_ROLES_FIELD;
import static org.opensearch.ml.common.CommonValue.TENANT_ID_FIELD;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.BACKEND_ROLE_EMPTY_ERROR;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.BACKEND_ROLE_INVALID_CHARACTERS_ERROR;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.BACKEND_ROLE_INVALID_LENGTH_ERROR;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.regex.Pattern;

import org.opensearch.OpenSearchParseException;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.common.io.stream.Writeable;
Expand Down Expand Up @@ -58,6 +63,7 @@ public MLCreateMemoryContainerInput(
this.configuration = configuration;
}
this.tenantId = tenantId;
validateBackendRoles(backendRoles);
this.backendRoles = backendRoles;
}

Expand Down Expand Up @@ -114,6 +120,42 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
return builder;
}

/**
* Validates backend roles according to security constraints.
* Each role must:
* - Be at most 128 Unicode characters long
* - Contain only alphanumeric characters and special symbols: :+=,.@-_/
* - Not be empty or blank
*
* @param backendRoles List of backend role strings to validate (null/empty list is allowed)
* @throws OpenSearchParseException if any role violates constraints
*/
public static void validateBackendRoles(List<String> backendRoles) {
if (backendRoles == null || backendRoles.isEmpty()) {
return; // null or empty list is allowed
}

// Regex pattern: Unicode letters/digits + allowed special chars: :+=,.@-_/
Pattern validPattern = Pattern.compile("^[\\p{L}\\p{N}:+=,.@\\-_/]+$");

for (String role : backendRoles) {
// Check for null or empty
if (role == null || role.isEmpty() || role.isBlank()) {
throw new OpenSearchParseException(BACKEND_ROLE_EMPTY_ERROR);
}

// Check length (Unicode character count)
if (role.length() > 128) {
throw new OpenSearchParseException(String.format(BACKEND_ROLE_INVALID_LENGTH_ERROR, role));
}

// Check allowed characters
if (!validPattern.matcher(role).matches()) {
throw new OpenSearchParseException(String.format(BACKEND_ROLE_INVALID_CHARACTERS_ERROR, role));
}
}
}

public static MLCreateMemoryContainerInput parse(XContentParser parser) throws IOException {
String name = null;
String description = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import static org.opensearch.ml.common.CommonValue.BACKEND_ROLES_FIELD;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.DESCRIPTION_FIELD;
import static org.opensearch.ml.common.memorycontainer.MemoryContainerConstants.NAME_FIELD;
import static org.opensearch.ml.common.transport.memorycontainer.MLCreateMemoryContainerInput.validateBackendRoles;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -40,6 +41,7 @@ public class MLUpdateMemoryContainerInput implements ToXContentObject, Writeable
public MLUpdateMemoryContainerInput(String name, String description, List<String> backendRoles, MemoryConfiguration configuration) {
this.name = name;
this.description = description;
validateBackendRoles(backendRoles);
this.backendRoles = backendRoles;
this.configuration = configuration;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

Expand All @@ -18,6 +19,7 @@
import java.util.Map;

import org.junit.Test;
import org.opensearch.OpenSearchParseException;
import org.opensearch.ml.common.FunctionName;
import org.opensearch.ml.common.memorycontainer.MemoryConfiguration.EmbeddingConfig;

Expand Down Expand Up @@ -993,4 +995,89 @@ public void testGetMemoryIndexMapping_EmptySettings() {
Map<String, Object> result = config.getMemoryIndexMapping("any-index");
assertNull(result);
}

// ==================== buildIndexPrefix CRLF Injection Tests ====================

@Test
public void testBuildIndexPrefix_RejectsCarriageReturn() {
OpenSearchParseException exception = assertThrows(
OpenSearchParseException.class,
() -> MemoryConfiguration.builder().indexPrefix("test\rinjection").build()
);
assertTrue(exception.getMessage().contains("control characters"));
}

@Test
public void testBuildIndexPrefix_RejectsLineFeed() {
OpenSearchParseException exception = assertThrows(
OpenSearchParseException.class,
() -> MemoryConfiguration.builder().indexPrefix("test\ninjection").build()
);
assertTrue(exception.getMessage().contains("control characters"));
}

@Test
public void testBuildIndexPrefix_RejectsCRLF() {
OpenSearchParseException exception = assertThrows(
OpenSearchParseException.class,
() -> MemoryConfiguration.builder().indexPrefix("test\r\ninjection").build()
);
assertTrue(exception.getMessage().contains("control characters"));
}

@Test
public void testBuildIndexPrefix_RejectsTab() {
OpenSearchParseException exception = assertThrows(
OpenSearchParseException.class,
() -> MemoryConfiguration.builder().indexPrefix("test\tinjection").build()
);
assertTrue(exception.getMessage().contains("control characters"));
}

@Test
public void testBuildIndexPrefix_RejectsNullCharacter() {
OpenSearchParseException exception = assertThrows(
OpenSearchParseException.class,
() -> MemoryConfiguration.builder().indexPrefix("test\u0000injection").build()
);
assertTrue(exception.getMessage().contains("control characters"));
}

@Test
public void testBuildIndexPrefix_RejectsOtherControlCharacters() {
// Test ASCII control character 1 (SOH - Start of Header)
OpenSearchParseException exception = assertThrows(
OpenSearchParseException.class,
() -> MemoryConfiguration.builder().indexPrefix("test\u0001injection").build()
);
assertTrue(exception.getMessage().contains("control characters"));
}

@Test
public void testBuildIndexPrefix_BackslashRejectedByMetadataCreateIndexService() {
// Verify that backslash is rejected by OpenSearch's built-in validation
// MetadataCreateIndexService.validateIndexOrAliasName handles this
OpenSearchParseException exception = assertThrows(
OpenSearchParseException.class,
() -> MemoryConfiguration.builder().indexPrefix("test\\backslash").build()
);
// The error message should come from MetadataCreateIndexService
assertTrue(
exception.getMessage().contains("invalid")
|| exception.getMessage().contains("index")
|| exception.getMessage().contains("prefix")
);
}

@Test
public void testBuildIndexPrefix_AcceptsValidPrefix() {
MemoryConfiguration config = MemoryConfiguration.builder().indexPrefix("valid-prefix-123").build();
assertEquals("valid-prefix-123", config.getIndexPrefix());
}

@Test
public void testBuildIndexPrefix_AcceptsHyphenAndUnderscore() {
MemoryConfiguration config = MemoryConfiguration.builder().indexPrefix("valid_prefix-with-chars").build();
assertEquals("valid_prefix-with-chars", config.getIndexPrefix());
}
}
Loading
Loading