Skip to content
Closed
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 @@ -44,8 +44,7 @@ public HikariDataSource replicaDataSource(Environment environment) {
initializationFailTimeout = Long.parseLong(initializationFailTimeoutValue);
} catch (NumberFormatException e) {
throw new IllegalStateException(
"Invalid value for REPLICA_HIKARI_INITIALIZATION_FAIL_TIMEOUT_MS: " + initializationFailTimeoutValue,
e
"Invalid value for REPLICA_HIKARI_INITIALIZATION_FAIL_TIMEOUT_MS"
);
}
config.setInitializationFailTimeout(initializationFailTimeout);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,78 @@

import org.springframework.lang.Nullable;

/**
* Validates bounded replica database configuration without republishing rejected values.
*
* <p>Successful values are returned in their trimmed canonical form. Failure messages identify
* the configuration key and validation class only, because rejected deployment values can contain
* connection coordinates, control characters, or other secret-adjacent diagnostic material.</p>
*/
public final class ValidationUtils {

private ValidationUtils() {}

/**
* Validates a decimal TCP port in the inclusive range 1 through 65535.
*
* @param value configured port text, possibly surrounded by whitespace
* @param key stable configuration key used in failure diagnostics
* @return the trimmed decimal port
* @throws IllegalStateException when the value is missing, non-decimal, or outside the port range
*/
public static String requireValidPort(@Nullable String value, String key) {
if (value == null || value.isBlank()) {
throw new IllegalStateException("Missing required port for " + key);
}
String trimmed = value.trim();
if (!trimmed.matches("^[0-9]+$")) {
throw new IllegalStateException("Invalid port for " + key + ": " + value);
throw new IllegalStateException("Invalid port for " + key);
}
try {
int port = Integer.parseInt(trimmed);
if (port < 1 || port > 65535) {
throw new IllegalStateException("Invalid port for " + key + ": " + value);
throw new IllegalStateException("Invalid port for " + key);
}
return trimmed;
} catch (NumberFormatException e) {
throw new IllegalStateException("Invalid port for " + key + ": " + value, e);
} catch (NumberFormatException exception) {
throw new IllegalStateException("Invalid port for " + key);
}
}

/**
* Validates a hostname or numeric host token accepted by the replica JDBC configuration.
*
* @param value configured host text, possibly surrounded by whitespace
* @param key stable configuration key used in failure diagnostics
* @return the trimmed host token
* @throws IllegalStateException when the value is missing or contains unsupported characters
*/
public static String requireValidHost(@Nullable String value, String key) {
if (value == null || value.isBlank()) {
throw new IllegalStateException("Missing required host for " + key);
}
String trimmed = value.trim();
if (!trimmed.matches("^[a-zA-Z0-9._-]+$")) {
throw new IllegalStateException("Invalid host for " + key + ": " + value);
throw new IllegalStateException("Invalid host for " + key);
}
return trimmed;
}

/**
* Validates a simple configuration identifier such as a replica database name.
*
* @param value configured identifier text, possibly surrounded by whitespace
* @param key stable configuration key used in failure diagnostics
* @return the trimmed identifier
* @throws IllegalStateException when the value is missing or contains unsupported characters
*/
public static String requireValidIdentifier(@Nullable String value, String key) {
if (value == null || value.isBlank()) {
throw new IllegalStateException("Missing required value for " + key);
}
String trimmed = value.trim();
if (!trimmed.matches("^[a-zA-Z0-9_-]+$")) {
throw new IllegalStateException("Invalid value for " + key + ": " + value);
throw new IllegalStateException("Invalid value for " + key);
}
return trimmed;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.concurrent.atomic.AtomicReference;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
Expand Down Expand Up @@ -82,20 +83,36 @@ void failsFastWhenRequiredReplicaEnvIsMissing() {
}

@Test
void failsWithHelpfulMessageWhenInitializationFailTimeoutIsNotANumber() {
void invalidInitializationFailTimeoutDoesNotRepublishRejectedValueOrParserDiagnostics() {
String sensitiveFragment = "password=timeout-secret-9431";

contextRunner
.withPropertyValues(
"xtrmetl.replica.enabled=true",
"REPLICA_PGHOST=replica-host",
"REPLICA_PGDATABASE=xtrmetl",
"REPLICA_PGUSER=xtrmetl_user",
"REPLICA_PGPASSWORD=xtrmetl_password",
"REPLICA_HIKARI_INITIALIZATION_FAIL_TIMEOUT_MS=not-a-number"
"REPLICA_HIKARI_INITIALIZATION_FAIL_TIMEOUT_MS=not-a-number?" + sensitiveFragment
)
.run(context -> {
Throwable failure = context.getStartupFailure();
assertNotNull(failure);
assertTrue(failure.getMessage().contains("REPLICA_HIKARI_INITIALIZATION_FAIL_TIMEOUT_MS"));

String diagnosticChain = diagnosticChain(failure);
assertTrue(diagnosticChain.contains("REPLICA_HIKARI_INITIALIZATION_FAIL_TIMEOUT_MS"));
assertFalse(diagnosticChain.contains(sensitiveFragment));
assertFalse(diagnosticChain.contains("NumberFormatException"));
});
}

private static String diagnosticChain(Throwable failure) {
StringBuilder diagnostics = new StringBuilder();
Throwable current = failure;
while (current != null) {
diagnostics.append(current.getClass().getName()).append(':').append(current.getMessage()).append('\n');
current = current.getCause();
}
return diagnostics.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

class ValidationUtilsTest {

Expand Down Expand Up @@ -54,4 +57,50 @@ void requireValidIdentifierThrowsWhenMissingOrInvalid() {
assertThrows(IllegalStateException.class, () -> ValidationUtils.requireValidIdentifier("xtrmetl?evil", "REPLICA_PGDATABASE"));
assertThrows(IllegalStateException.class, () -> ValidationUtils.requireValidIdentifier("xtrmetl/db", "REPLICA_PGDATABASE"));
}

@Test
void invalidConfigurationDiagnosticsDoNotRepublishRejectedValues() {
String sensitiveFragment = "password=secret-8472";

IllegalStateException hostFailure = assertThrows(
IllegalStateException.class,
() -> ValidationUtils.requireValidHost(
"replica-host?" + sensitiveFragment + "\r\nforged-log-line",
"REPLICA_PGHOST"
)
);
IllegalStateException portFailure = assertThrows(
IllegalStateException.class,
() -> ValidationUtils.requireValidPort("5432?" + sensitiveFragment, "REPLICA_PGPORT")
);
IllegalStateException oversizedPortFailure = assertThrows(
IllegalStateException.class,
() -> ValidationUtils.requireValidPort("999999999999999999999999999999999999", "REPLICA_PGPORT")
);
IllegalStateException identifierFailure = assertThrows(
IllegalStateException.class,
() -> ValidationUtils.requireValidIdentifier(
"customer_db?" + sensitiveFragment,
"REPLICA_PGDATABASE"
)
);

assertSafeDiagnostic(hostFailure, "REPLICA_PGHOST", sensitiveFragment);
assertSafeDiagnostic(portFailure, "REPLICA_PGPORT", sensitiveFragment);
assertSafeDiagnostic(oversizedPortFailure, "REPLICA_PGPORT", sensitiveFragment);
assertNull(oversizedPortFailure.getCause());
assertSafeDiagnostic(identifierFailure, "REPLICA_PGDATABASE", sensitiveFragment);
}

private static void assertSafeDiagnostic(
IllegalStateException failure,
String expectedKey,
String sensitiveFragment
) {
assertTrue(failure.getMessage().contains(expectedKey));
assertFalse(failure.getMessage().contains(sensitiveFragment));
assertFalse(failure.getMessage().contains("forged-log-line"));
assertFalse(failure.getMessage().contains("\r"));
assertFalse(failure.getMessage().contains("\n"));
Comment on lines +88 to +104

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

두 진단 테스트가 거부된 입력의 부분 재출력을 허용합니다.

두 헬퍼는 선택한 민감한 조각만 검사합니다. 거부된 입력의 다른 부분이 진단에 남아도 회귀가 통과할 수 있습니다. 전체 입력과 안전한 오류 메시지를 함께 검증해 주세요.

  • cdc-service/src/test/java/com/xtrmetl/cdc/util/ValidationUtilsTest.java#L88-L104: 각 검증 실패에 전체 거부 값을 전달하고, 전체 값과 안정적인 오류 메시지를 검사하세요.
  • cdc-service/src/test/java/com/xtrmetl/cdc/config/ReplicaJdbcTemplateConfigTest.java#L102-L105: 전체 타임아웃 값과 not-a-number? 부분을 검사하고, 안전한 IllegalStateException 메시지를 확인하세요.
📍 Affects 2 files
  • cdc-service/src/test/java/com/xtrmetl/cdc/util/ValidationUtilsTest.java#L88-L104 (this comment)
  • cdc-service/src/test/java/com/xtrmetl/cdc/config/ReplicaJdbcTemplateConfigTest.java#L102-L105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cdc-service/src/test/java/com/xtrmetl/cdc/util/ValidationUtilsTest.java`
around lines 88 - 104, Update ValidationUtilsTest.java at lines 88-104 so each
validation failure passes the complete rejected value to assertSafeDiagnostic,
and have the helper verify that the complete value is absent along with the
stable error-message safety checks. Update ReplicaJdbcTemplateConfigTest.java at
lines 102-105 to validate the complete timeout value and the “not-a-number?”
fragment, while confirming the resulting IllegalStateException message is safe.

}
Comment on lines +60 to +105

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

while IFS= read -r -d '' file; do
  printf '\n== %s ==\n' "$file"
  cat "$file"
done < <(fd --hidden --type f --glob 'AGENTS.md' --print0 .)

./mvnw -B test

Repository: ContextualWisdomLab/mightyETL

Length of output: 219


🏁 Script executed:

#!/usr/bin/env bash
set -u

printf '%s\n' '== AGENTS.md files =='
find .. -name AGENTS.md -type f -print 2>/dev/null | while IFS= read -r file; do
  printf '\n== %s ==\n' "$file"
  cat "$file"
done

printf '\n%s\n' '== Relevant test and implementation files =='
find . -type f \( -name 'ValidationUtilsTest.java' -o -name 'ValidationUtils.java' \) -print

printf '\n%s\n' '== Test file excerpt =='
test_file="$(find . -type f -name 'ValidationUtilsTest.java' -print -quit)"
if [ -n "$test_file" ]; then
  cat -n "$test_file" | sed -n '1,150p'
fi

printf '\n%s\n' '== Implementation excerpt =='
impl_file="$(find . -type f -name 'ValidationUtils.java' -print -quit)"
if [ -n "$impl_file" ]; then
  cat -n "$impl_file" | sed -n '1,220p'
fi

printf '\n%s\n' '== Maven wrapper =='
if [ -x ./mvnw ]; then
  ./mvnw -B test
else
  printf '%s\n' './mvnw is missing or is not executable'
  exit 2
fi

Repository: ContextualWisdomLab/mightyETL

Length of output: 50386


🏁 Script executed:

#!/usr/bin/env bash
set -u

log=/tmp/coderabbit-shell-logs/shell-output-Ttu25l
printf '%s\n' '== Maven failure summary =='
if [ -f "$log" ]; then
  grep -E '(^\[ERROR\]|BUILD |Tests run:|Failures:|Errors:|There are test failures|Failed to execute)' "$log" | tail -n 120
  printf '\n%s\n' '== Maven output tail =='
  tail -n 120 "$log"
else
  printf '%s\n' "Missing log: $log"
fi

printf '\n%s\n' '== Targeted ValidationUtilsTest =='
set -o pipefail
./mvnw -B -pl cdc-service -Dtest=ValidationUtilsTest test 2>&1 \
  | tee /tmp/validation-utils-test.log \
  | grep -E '(^\[INFO\] ---|Tests run:|(^\[ERROR\])|BUILD (SUCCESS|FAILURE)|Failed to execute|No tests were executed)' \
  | tail -n 160
status=${PIPESTATUS[0]}
printf '\nMaven exit status: %s\n' "$status"
exit "$status"

Repository: ContextualWisdomLab/mightyETL

Length of output: 15644


🏁 Script executed:

#!/usr/bin/env bash
set -u

printf '%s\n' '== Related diagnostic helpers and timeout tests =='
rg -n -C 4 'assertSafeDiagnostic|timeout|DoNotRepublish|republish|sensitiveFragment' \
  cdc-service/src/test/java cdc-service/src/main/java 2>/dev/null || true

printf '\n%s\n' '== Java runtime =='
java -version 2>&1 || true
javac -version 2>&1 || true

printf '\n%s\n' '== Targeted cdc-service test =='
log=/tmp/validation-utils-test-direct.log
./mvnw -B -pl cdc-service -Dtest=ValidationUtilsTest test >"$log" 2>&1
status=$?
grep -E '(^\[INFO\] ---|Tests run:|(^\[ERROR\])|BUILD (SUCCESS|FAILURE)|Failed to execute|No tests were executed|SKIPPED)' "$log" | tail -n 160
printf '\nMaven exit status: %s\n' "$status"
exit "$status"

Repository: ContextualWisdomLab/mightyETL

Length of output: 13117


🏁 Script executed:

#!/usr/bin/env bash
python3 - <<'PY'
def helper_passes(message, expected_key, sensitive_fragment):
    return (
        expected_key in message
        and sensitive_fragment not in message
        and "forged-log-line" not in message
        and "\r" not in message
        and "\n" not in message
    )

cases = [
    (
        "host",
        "Invalid host for REPLICA_PGHOST: replica-host?",
        "REPLICA_PGHOST",
        "password=secret-8472",
    ),
    (
        "port",
        "Invalid port for REPLICA_PGPORT: 5432?",
        "REPLICA_PGPORT",
        "password=secret-8472",
    ),
    (
        "identifier",
        "Invalid value for REPLICA_PGDATABASE: customer_db?",
        "REPLICA_PGDATABASE",
        "password=secret-8472",
    ),
]
for name, message, key, fragment in cases:
    print(f"{name}: {'passes' if helper_passes(message, key, fragment) else 'fails'}")
PY

Repository: ContextualWisdomLab/mightyETL

Length of output: 212


거부된 입력의 비노출 범위를 확대해 검사하세요.

assertSafeDiagnostic는 민감한 조각과 "forged-log-line"만 검사합니다. 따라서 "Invalid host for REPLICA_PGHOST: replica-host?"와 같은 부분 누출은 통과합니다. 각 거부 입력 전체와 예외 원인 체인에 입력 값이 없는지 검사하세요. oversizedPortFailure에는 sensitiveFragment가 전달되지 않으므로 별도의 입력값 검사가 필요합니다. 동일한 검사를 ReplicaJdbcTemplateConfigTest에도 적용하세요.

./mvnw -B test와 대상 테스트는 JDK 17이 Java release 25를 지원하지 않아 실행되지 않았습니다.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cdc-service/src/test/java/com/xtrmetl/cdc/util/ValidationUtilsTest.java`
around lines 60 - 105, Expand assertSafeDiagnostic in ValidationUtilsTest so
each failure message and entire exception-cause chain are checked for absence of
the complete rejected input, not only sensitiveFragment and forged-log-line.
Pass the full host, port, and identifier inputs to the helper, and add a
separate full-input assertion for oversizedPortFailure while preserving key and
newline checks. Apply the same diagnostic-safety assertions to
ReplicaJdbcTemplateConfigTest.

Source: Coding guidelines

}
Loading