From 6606afb63673f464a0952817fef68f8c53798402 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Thu, 30 Oct 2025 15:48:03 +0530 Subject: [PATCH 1/4] Support hyphen in identifier names --- .../postgres/utils/BasicPostgresSecurityValidator.java | 2 +- .../postgres/utils/PostgresSecurityValidatorTest.java | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/BasicPostgresSecurityValidator.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/BasicPostgresSecurityValidator.java index e126bf98..a9f36e39 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/BasicPostgresSecurityValidator.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/BasicPostgresSecurityValidator.java @@ -43,7 +43,7 @@ public class BasicPostgresSecurityValidator implements PostgresSecurityValidator * Documentation */ private static final String DEFAULT_IDENTIFIER_PATTERN = - "^[a-zA-Z_][a-zA-Z0-9_]*(\\.[a-zA-Z_][a-zA-Z0-9_]*)*$"; + "^[a-zA-Z_][a-zA-Z0-9_-]*(\\.[a-zA-Z_][a-zA-Z0-9_-]*)*$"; /** * Default pattern for JSON field names within JSONB columns. diff --git a/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java b/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java index 83ae9ed9..19fa7d54 100644 --- a/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java +++ b/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java @@ -31,6 +31,11 @@ void testValidIdentifierWithNumbers() { assertDoesNotThrow(() -> validator.validateIdentifier("col_1")); } + @Test + void testValidIdentifierWithHyphens() { + assertDoesNotThrow(() -> validator.validateIdentifier("API.repo-url")); + } + @Test void testInvalidIdentifierNull() { SecurityException ex = From 87dfdb3991fbd1db008f39e5ac304c8efa38ea36 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Thu, 30 Oct 2025 15:59:09 +0530 Subject: [PATCH 2/4] Fix failing test --- .../postgres/utils/PostgresSecurityValidatorTest.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java b/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java index 19fa7d54..2cfb8bca 100644 --- a/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java +++ b/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java @@ -80,13 +80,6 @@ void testInvalidIdentifierSqlInjection_Semicolon() { assertTrue(ex.getMessage().contains("invalid")); } - @Test - void testInvalidIdentifierHyphen() { - SecurityException ex = - assertThrows(SecurityException.class, () -> validator.validateIdentifier("field-name")); - assertTrue(ex.getMessage().contains("invalid")); - } - @Test void testValidIdentifierWithDotNotation() { assertDoesNotThrow(() -> validator.validateIdentifier("field.name")); From c2324bccffcabc87da45e6bfb55664416e583388 Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Thu, 30 Oct 2025 16:04:19 +0530 Subject: [PATCH 3/4] Fix failing test --- .../JsonIdentifierExpressionSecurityTest.java | 65 +++++++------------ 1 file changed, 24 insertions(+), 41 deletions(-) diff --git a/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionSecurityTest.java b/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionSecurityTest.java index 7b8048f8..344f7cd3 100644 --- a/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionSecurityTest.java +++ b/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionSecurityTest.java @@ -7,48 +7,43 @@ import java.util.List; import org.junit.jupiter.api.Test; -/** Security tests for JsonIdentifierExpression to ensure SQL injection prevention. */ public class JsonIdentifierExpressionSecurityTest { - // ===== Valid Expressions ===== - @Test - void testValidExpression_SimpleField() { + void testValidExpressionSimpleField() { assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "brand")); } @Test - void testValidExpression_NestedField() { + void testValidExpressionNestedField() { assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "seller", "name")); } @Test - void testValidExpression_DeeplyNested() { + void testValidExpressionDeeplyNested() { assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "seller", "address", "city")); } @Test - void testValidExpression_WithNumbers() { + void testValidExpressionWithNumbers() { assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "field123")); assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "1st_choice")); } @Test - void testValidExpression_WithUnderscore() { + void testValidExpressionWithUnderscore() { assertDoesNotThrow(() -> JsonIdentifierExpression.of("_internal", "field")); assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "_private")); } @Test - void testValidExpression_UsingListConstructor() { + void testValidExpressionUsingListConstructor() { assertDoesNotThrow( () -> JsonIdentifierExpression.of("props", List.of("seller", "address", "city"))); } - // ===== Invalid Column Names ===== - @Test - void testInvalidExpression_ColumnName_DropTable() { + void testInvalidExpressionColumnNameDropTable() { SecurityException ex = assertThrows( SecurityException.class, @@ -57,7 +52,7 @@ void testInvalidExpression_ColumnName_DropTable() { } @Test - void testInvalidExpression_ColumnName_WithQuote() { + void testInvalidExpressionColumnNameWithQuote() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("props\"name", "brand")); @@ -65,7 +60,7 @@ void testInvalidExpression_ColumnName_WithQuote() { } @Test - void testInvalidExpression_ColumnName_WithSemicolon() { + void testInvalidExpressionColumnNameWithSemicolon() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("props;SELECT", "brand")); @@ -73,7 +68,7 @@ void testInvalidExpression_ColumnName_WithSemicolon() { } @Test - void testInvalidExpression_ColumnName_StartsWithNumber() { + void testInvalidExpressionColumnNameStartsWithNumber() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("123props", "brand")); @@ -81,25 +76,15 @@ void testInvalidExpression_ColumnName_StartsWithNumber() { } @Test - void testInvalidExpression_ColumnName_WithHyphen() { - SecurityException ex = - assertThrows( - SecurityException.class, () -> JsonIdentifierExpression.of("my-column", "brand")); - assertTrue(ex.getMessage().contains("invalid")); - } - - @Test - void testInvalidExpression_ColumnName_WithSpace() { + void testInvalidExpressionColumnNameWithSpace() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("my column", "brand")); assertTrue(ex.getMessage().contains("invalid")); } - // ===== Invalid JSON Paths ===== - @Test - void testInvalidExpression_JsonPath_WithQuote() { + void testInvalidExpressionJsonPathWithQuote() { SecurityException ex = assertThrows( SecurityException.class, @@ -108,7 +93,7 @@ void testInvalidExpression_JsonPath_WithQuote() { } @Test - void testInvalidExpression_JsonPath_WithDoubleQuote() { + void testInvalidExpressionJsonPathWithDoubleQuote() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("props", "name\"--")); @@ -116,7 +101,7 @@ void testInvalidExpression_JsonPath_WithDoubleQuote() { } @Test - void testInvalidExpression_JsonPath_WithSemicolon() { + void testInvalidExpressionJsonPathWithSemicolon() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("props", "field; DROP")); @@ -124,7 +109,7 @@ void testInvalidExpression_JsonPath_WithSemicolon() { } @Test - void testInvalidExpression_JsonPath_WithHyphen() { + void testInvalidExpressionJsonPathWithHyphen() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("props", "field-name")); @@ -132,7 +117,7 @@ void testInvalidExpression_JsonPath_WithHyphen() { } @Test - void testInvalidExpression_JsonPath_WithDot() { + void testInvalidExpressionJsonPathWithDot() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("props", "field.name")); @@ -140,7 +125,7 @@ void testInvalidExpression_JsonPath_WithDot() { } @Test - void testInvalidExpression_JsonPath_WithSpace() { + void testInvalidExpressionJsonPathWithSpace() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("props", "field name")); @@ -148,7 +133,7 @@ void testInvalidExpression_JsonPath_WithSpace() { } @Test - void testInvalidExpression_JsonPath_EmptyElement() { + void testInvalidExpression_sonPathEmptyElement() { SecurityException ex = assertThrows( SecurityException.class, @@ -157,7 +142,7 @@ void testInvalidExpression_JsonPath_EmptyElement() { } @Test - void testInvalidExpression_JsonPath_TooDeep() { + void testInvalidExpressionJsonPathTooDeep() { String[] deepPath = new String[11]; // Max is 10 for (int i = 0; i < 11; i++) { deepPath[i] = "level" + i; @@ -167,10 +152,8 @@ void testInvalidExpression_JsonPath_TooDeep() { assertTrue(ex.getMessage().contains("exceeds maximum depth")); } - // ===== Real-world Attack Scenarios ===== - @Test - void testAttackScenario_SqlCommentInjection() { + void testAttackScenarioSqlCommentInjection() { SecurityException ex = assertThrows( SecurityException.class, @@ -179,7 +162,7 @@ void testAttackScenario_SqlCommentInjection() { } @Test - void testAttackScenario_UnionSelect() { + void testAttackScenarioUnionSelect() { SecurityException ex = assertThrows( SecurityException.class, @@ -190,7 +173,7 @@ void testAttackScenario_UnionSelect() { } @Test - void testAttackScenario_OrTrueInjection() { + void testAttackScenarioOrTrueInjection() { SecurityException ex = assertThrows( SecurityException.class, @@ -199,7 +182,7 @@ void testAttackScenario_OrTrueInjection() { } @Test - void testAttackScenario_NestedInjection() { + void testAttackScenarioNestedInjection() { SecurityException ex = assertThrows( SecurityException.class, @@ -208,7 +191,7 @@ void testAttackScenario_NestedInjection() { } @Test - void testAttackScenario_SpecialCharacterCombination() { + void testAttackScenarioSpecialCharacterCombination() { SecurityException ex = assertThrows( SecurityException.class, () -> JsonIdentifierExpression.of("props", "field'\"`;DROP")); From be03d75452f8b2bb62df52540a6a9f3aead0069c Mon Sep 17 00:00:00 2001 From: Prashant Pandey Date: Thu, 30 Oct 2025 16:27:05 +0530 Subject: [PATCH 4/4] WIP --- .../utils/BasicPostgresSecurityValidator.java | 40 ++----------------- .../JsonIdentifierExpressionSecurityTest.java | 26 +++++++----- .../utils/PostgresSecurityValidatorTest.java | 28 +++++++------ 3 files changed, 34 insertions(+), 60 deletions(-) diff --git a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/BasicPostgresSecurityValidator.java b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/BasicPostgresSecurityValidator.java index a9f36e39..4f3178f7 100644 --- a/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/BasicPostgresSecurityValidator.java +++ b/document-store/src/main/java/org/hypertrace/core/documentstore/postgres/utils/BasicPostgresSecurityValidator.java @@ -45,48 +45,16 @@ public class BasicPostgresSecurityValidator implements PostgresSecurityValidator private static final String DEFAULT_IDENTIFIER_PATTERN = "^[a-zA-Z_][a-zA-Z0-9_-]*(\\.[a-zA-Z_][a-zA-Z0-9_-]*)*$"; - /** - * Default pattern for JSON field names within JSONB columns. - * - *

Pattern: {@code ^[a-zA-Z0-9_]+$} - * - *

Allowed: - * - *

    - *
  • Can start with: letter (a-z, A-Z), digit (0-9), or underscore (_) - *
  • Can contain: letters (a-z, A-Z), digits (0-9), underscores (_) - *
  • Examples: {@code "brand"}, {@code "1st_choice"}, {@code "field123"}, {@code "_private"} - *
- * - *

Not allowed: - * - *

    - *
  • Hyphens: {@code "field-name"} - *
  • Dots: {@code "field.name"} (use path segments instead: ["field", "name"]) - *
  • Spaces: {@code "my field"} - *
  • Special characters: {@code "field@name"}, {@code "field#name"} - *
  • Quotes: {@code "field\"name"}, {@code "field'name"} - *
  • Semicolons: {@code "field; DROP TABLE"} - *
  • SQL operators: {@code "field\" OR \"1\"=\"1"} - *
- * - *

Note: More permissive than identifier pattern to support flexible JSON schemas where - * field names may start with numbers. - */ - private static final String DEFAULT_JSON_FIELD_PATTERN = "^[a-zA-Z0-9_]+$"; - /** Default instance with hardcoded values for convenient static access. */ private static final BasicPostgresSecurityValidator DEFAULT = new BasicPostgresSecurityValidator( DEFAULT_MAX_IDENTIFIER_LENGTH, DEFAULT_MAX_JSON_FIELD_LENGTH, DEFAULT_MAX_JSON_PATH_DEPTH, - DEFAULT_IDENTIFIER_PATTERN, - DEFAULT_JSON_FIELD_PATTERN); + DEFAULT_IDENTIFIER_PATTERN); // Instance variables for configured limits private final Pattern validIdentifier; - private final Pattern validJsonField; private final int maxIdentifierLength; private final int maxJsonFieldLength; private final int maxJsonPathDepth; @@ -105,13 +73,11 @@ private BasicPostgresSecurityValidator( int maxIdentifierLength, int maxJsonFieldLength, int maxJsonPathDepth, - String identifierPattern, - String jsonFieldPattern) { + String identifierPattern) { this.maxIdentifierLength = maxIdentifierLength; this.maxJsonFieldLength = maxJsonFieldLength; this.maxJsonPathDepth = maxJsonPathDepth; this.validIdentifier = Pattern.compile(identifierPattern); - this.validJsonField = Pattern.compile(jsonFieldPattern); } @Override @@ -163,7 +129,7 @@ public void validateJsonPath(List path) { field, i, maxJsonFieldLength)); } - if (!validJsonField.matcher(field).matches()) { + if (!validIdentifier.matcher(field).matches()) { throw new SecurityException( String.format( "JSON field '%s' at index %d contains invalid characters. " diff --git a/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionSecurityTest.java b/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionSecurityTest.java index 344f7cd3..d2ab7831 100644 --- a/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionSecurityTest.java +++ b/document-store/src/test/java/org/hypertrace/core/documentstore/expression/impl/JsonIdentifierExpressionSecurityTest.java @@ -27,7 +27,15 @@ void testValidExpressionDeeplyNested() { @Test void testValidExpressionWithNumbers() { assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "field123")); - assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "1st_choice")); + assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "field_1")); + } + + @Test + void testInvalidExpressionJsonPathStartsWithNumber() { + SecurityException ex = + assertThrows( + SecurityException.class, () -> JsonIdentifierExpression.of("props", "1st_choice")); + assertTrue(ex.getMessage().contains("invalid characters")); } @Test @@ -109,19 +117,15 @@ void testInvalidExpressionJsonPathWithSemicolon() { } @Test - void testInvalidExpressionJsonPathWithHyphen() { - SecurityException ex = - assertThrows( - SecurityException.class, () -> JsonIdentifierExpression.of("props", "field-name")); - assertTrue(ex.getMessage().contains("invalid characters")); + void testValidExpressionJsonPathWithHyphen() { + assertDoesNotThrow(() -> JsonIdentifierExpression.of("customAttribute", "repo-url")); + assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "user-id")); } @Test - void testInvalidExpressionJsonPathWithDot() { - SecurityException ex = - assertThrows( - SecurityException.class, () -> JsonIdentifierExpression.of("props", "field.name")); - assertTrue(ex.getMessage().contains("invalid characters")); + void testValidExpressionJsonPathWithDot() { + assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "field.name")); + assertDoesNotThrow(() -> JsonIdentifierExpression.of("props", "user.address.city")); } @Test diff --git a/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java b/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java index 2cfb8bca..1abad456 100644 --- a/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java +++ b/document-store/src/test/java/org/hypertrace/core/documentstore/postgres/utils/PostgresSecurityValidatorTest.java @@ -33,7 +33,7 @@ void testValidIdentifierWithNumbers() { @Test void testValidIdentifierWithHyphens() { - assertDoesNotThrow(() -> validator.validateIdentifier("API.repo-url")); + assertDoesNotThrow(() -> validator.validateIdentifier("repo-url")); } @Test @@ -140,7 +140,15 @@ void testValidJsonPathMultiLevel() { @Test void testValidJsonPathWithNumbers() { assertDoesNotThrow(() -> validator.validateJsonPath(List.of("field123"))); - assertDoesNotThrow(() -> validator.validateJsonPath(List.of("1st_choice"))); + assertDoesNotThrow(() -> validator.validateJsonPath(List.of("field_1"))); + } + + @Test + void testInvalidJsonPathStartsWithNumber() { + SecurityException ex = + assertThrows( + SecurityException.class, () -> validator.validateJsonPath(List.of("1st_choice"))); + assertTrue(ex.getMessage().contains("invalid characters")); } @Test @@ -209,19 +217,15 @@ void testInvalidJsonPathSqlInjectionSemicolon() { } @Test - void testInvalidJsonPathHyphen() { - SecurityException ex = - assertThrows( - SecurityException.class, () -> validator.validateJsonPath(List.of("field-name"))); - assertTrue(ex.getMessage().contains("invalid characters")); + void testValidJsonPathWithHyphen() { + assertDoesNotThrow(() -> validator.validateJsonPath(List.of("field-name"))); + assertDoesNotThrow(() -> validator.validateJsonPath(List.of("user-id"))); } @Test - void testInvalidJsonPathDot() { - SecurityException ex = - assertThrows( - SecurityException.class, () -> validator.validateJsonPath(List.of("field.name"))); - assertTrue(ex.getMessage().contains("invalid characters")); + void testValidJsonPathWithDotNotation() { + assertDoesNotThrow(() -> validator.validateJsonPath(List.of("field.name"))); + assertDoesNotThrow(() -> validator.validateJsonPath(List.of("user.address.city"))); } @Test