Mitigate KQL injection in TopicToTableMapping configuration values#155
Mitigate KQL injection in TopicToTableMapping configuration values#155
Conversation
Validate db, table, mapping, and format fields against strict allowlist patterns to prevent KQL metacharacter injection (semicolons, pipes, single quotes, etc.) in management commands constructed via String.formatted() in KustoSinkTask. - db, table, mapping: Only allow alphanumeric, underscores, hyphens, spaces, and dots - format: Only allow alphanumeric characters - Add 7 new tests covering injection patterns and valid names Agent-Logs-Url: https://github.com/Azure/kafka-sink-azure-kusto/sessions/2be3571a-a8d8-49fc-a8d4-c8678f5b1872 Co-authored-by: tanmaya-panda1 <108695755+tanmaya-panda1@users.noreply.github.com>
…e name test Agent-Logs-Url: https://github.com/Azure/kafka-sink-azure-kusto/sessions/2be3571a-a8d8-49fc-a8d4-c8678f5b1872 Co-authored-by: tanmaya-panda1 <108695755+tanmaya-panda1@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Strengthens configuration validation to prevent KQL management-command injection by allowlisting safe characters in TopicToTableMapping fields that are interpolated into KQL, and adds unit tests to cover common metacharacter injection vectors.
Changes:
- Add allowlist regex validation for
db,table,mapping, andformatinTopicToTableMapping.validate(). - Add new unit tests asserting invalid characters (e.g.,
;,|,') are rejected and valid identifiers are accepted.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/main/java/com/microsoft/azure/kusto/kafka/connect/sink/TopicToTableMapping.java |
Introduces regex allowlist validation for Kusto identifiers and ingestion format to mitigate KQL injection. |
src/test/java/com/microsoft/azure/kusto/kafka/connect/sink/KustoSinkConnectorConfigTest.java |
Adds tests for rejection of injection-like inputs and acceptance of valid identifiers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| validateKustoIdentifier("db", db); | ||
| validateKustoIdentifier("table", table); | ||
| if (mapping != null && !mapping.isEmpty()) { | ||
| validateKustoIdentifier("mapping", mapping); | ||
| } | ||
| if (format != null && !format.isEmpty()) { | ||
| if (!VALID_FORMAT_PATTERN.matcher(format).matches()) { | ||
| throw new ConfigException( | ||
| String.format("'format' contains invalid characters: '%s'. Only alphanumeric characters are allowed.", format)); | ||
| } |
There was a problem hiding this comment.
validate() currently only validates format if it is non-null/non-empty. If the config omits format, this passes validation but later code paths call format.equalsIgnoreCase(...) (e.g., in table-access validation), which can throw a NullPointerException. Consider making format required here (throw ConfigException when missing/blank) or ensuring downstream callers handle null safely; add a test for missing/blank format to lock this in.
| validateKustoIdentifier("db", db); | |
| validateKustoIdentifier("table", table); | |
| if (mapping != null && !mapping.isEmpty()) { | |
| validateKustoIdentifier("mapping", mapping); | |
| } | |
| if (format != null && !format.isEmpty()) { | |
| if (!VALID_FORMAT_PATTERN.matcher(format).matches()) { | |
| throw new ConfigException( | |
| String.format("'format' contains invalid characters: '%s'. Only alphanumeric characters are allowed.", format)); | |
| } | |
| if (null == format || format.isEmpty()) { | |
| throw new ConfigException("'format' must be provided for each mapping"); | |
| } | |
| validateKustoIdentifier("db", db); | |
| validateKustoIdentifier("table", table); | |
| if (mapping != null && !mapping.isEmpty()) { | |
| validateKustoIdentifier("mapping", mapping); | |
| } | |
| if (!VALID_FORMAT_PATTERN.matcher(format).matches()) { | |
| throw new ConfigException( | |
| String.format("'format' contains invalid characters: '%s'. Only alphanumeric characters are allowed.", format)); |
| private static void validateKustoIdentifier(String fieldName, String value) { | ||
| if (!VALID_KUSTO_IDENTIFIER_PATTERN.matcher(value).matches()) { | ||
| throw new ConfigException( | ||
| String.format("'%s' contains invalid characters: '%s'. Only alphanumeric characters, underscores, hyphens, and dots are allowed.", | ||
| fieldName, value)); |
There was a problem hiding this comment.
validateKustoIdentifier(...) assumes value is non-null and will throw a NullPointerException if called with null. Since this helper is new and likely to be reused, it would be more robust to explicitly check for null/blank and throw a ConfigException with a clear message (or fold the required/optional logic into the helper).
| @Test | ||
| public void shouldRejectTableNameWithSemicolon() { | ||
| HashMap<String, String> settings = setupConfigs(); |
There was a problem hiding this comment.
PR description mentions adding 8 test cases for injection vectors, but this diff adds 7 new tests (5 rejection + 2 acceptance). Either add the missing test case or update the PR description/release notes to match what’s actually covered.
Pull Request Description
TopicToTableMapping.validate()only checked for null/empty ondb,table,mapping, andformatfields. These values are interpolated directly into KQL commands viaString.formatted()inKustoSinkTask, enabling injection of arbitrary management commands through KQL metacharacters (;,|,').Changes:
TopicToTableMapping.validate()for all fields interpolated into KQL:db,table,mapping:^[a-zA-Z0-9_.\-]+$format:^[a-zA-Z0-9]+$Future Release Comment
Breaking Changes:
db,table, andmappingvalues inkusto.tables.topics.mappingnow reject names containing characters outside[a-zA-Z0-9_.-].formatis restricted to[a-zA-Z0-9]. Configurations using spaces or other special characters in entity names will fail validation at startup.Features:
Fixes:
kusto.tables.topics.mappingfields to prevent KQL command injection (CWE-943)