Skip to content

Commit 7965edd

Browse files
Flossyclaude
andcommitted
fix: resolve approved issues - batch verified by adversarial consensus
Issues resolved: - #277: Fix ClusteredApplicationManager anonymous listener memory leak - #278: Add HTTP method validation to NettyApiServer routes - #280: Configure EtcdServiceRegistry ObjectMapper for security - #281: Fix EtcdServiceRegistry.registerService() silent failure - #285: Add shutdown timeout to NettyApiServer.stop() - #286: Fix EtcdServiceRegistry.renewLeases() concurrent modification - #385: Add CASE_INSENSITIVE flag to CredentialMasker BASIC_AUTH_PATTERN - #386: Fix CredentialMasker ENV_VAR_PATTERN no-op - #387: Preserve original format in CredentialMasker replacements - #388: Replace System.exit calls with proper exception handling - #389: Shut down ApplicationManager.listenerExecutor on shutdown - #390: Add per-application lock to ApplicationManager.forceKill() - #391: Fix missing ExecutorService shutdown in ApplicationManager - #393: Fix IndexOutOfBoundsException in RedisClusterManager JSON parsing - #394: Add null safety to ContainerLauncher.readContainerId() - #396: Sanitize container name/properties to prevent command injection - #399: Make ApplicationResourceMonitor.history thread-safe - #400: Fix TOCTOU race in ClusteredApplicationManager deployment - #401: Replace non-thread-safe HashMap in DependencyResolver - #403: Fix ApplicationReloader versionHistory thread safety - #404: Fix ClassLoaderCleanupUtil ThreadLocal filtering - #407: Make HealthChecker.consecutiveFailures atomic - #412: Fix string index out of bounds in env var resolution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b5b1e5b commit 7965edd

22 files changed

Lines changed: 875 additions & 178 deletions

File tree

‎platform-api/src/main/java/org/flossware/platform/api/ApplicationDescriptor.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,9 @@ public Optional<String> resolveProperty(String propertyKey) {
214214
}
215215

216216
// Check if this looks like a secret reference: ${VARNAME}
217-
if (value.startsWith("${") && value.endsWith("}")) {
217+
// Minimum length of 4 ensures a non-empty variable name (e.g., "${X}")
218+
// and prevents StringIndexOutOfBoundsException from malformed patterns
219+
if (value.length() > 3 && value.startsWith("${") && value.endsWith("}")) {
218220
String varName = value.substring(2, value.length() - 1);
219221
String resolved = System.getenv(varName);
220222
return Optional.ofNullable(resolved);

‎platform-api/src/main/java/org/flossware/platform/api/CredentialMasker.java‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,19 @@ public final class CredentialMasker {
4545
private static final String MASKED_VALUE = "***MASKED***";
4646

4747
// Regex patterns for common credential patterns
48+
// Group 1 captures the prefix (key + delimiter) to preserve original format in replacements
4849
private static final Pattern PASSWORD_PATTERN =
49-
Pattern.compile("password[\"']?\\s*[:=]\\s*[\"']?([^\"'\\s,}]+)", Pattern.CASE_INSENSITIVE);
50+
Pattern.compile(
51+
"(password[\"']?\\s*[:=]\\s*[\"']?)([^\"'\\s,}]+)", Pattern.CASE_INSENSITIVE);
5052
private static final Pattern TOKEN_PATTERN =
51-
Pattern.compile("token[\"']?\\s*[:=]\\s*[\"']?([^\"'\\s,}]+)", Pattern.CASE_INSENSITIVE);
53+
Pattern.compile("(token[\"']?\\s*[:=]\\s*[\"']?)([^\"'\\s,}]+)", Pattern.CASE_INSENSITIVE);
5254
private static final Pattern API_KEY_PATTERN =
5355
Pattern.compile(
54-
"api[_\\s-]?key[\"']?\\s*[:=]\\s*[\"']?([^\"'\\s,}]+)", Pattern.CASE_INSENSITIVE);
56+
"(api[_\\s-]?key[\"']?\\s*[:=]\\s*[\"']?)([^\"'\\s,}]+)", Pattern.CASE_INSENSITIVE);
5557
private static final Pattern BEARER_PATTERN =
5658
Pattern.compile("Bearer\\s+([^\\s]+)", Pattern.CASE_INSENSITIVE);
57-
private static final Pattern BASIC_AUTH_PATTERN = Pattern.compile("Basic\\s+([^\\s]+)");
59+
private static final Pattern BASIC_AUTH_PATTERN =
60+
Pattern.compile("Basic\\s+([^\\s]+)", Pattern.CASE_INSENSITIVE);
5861
private static final Pattern ENV_VAR_PATTERN = Pattern.compile("\\$\\{([A-Z0-9_]+)\\}");
5962

6063
private CredentialMasker() {
@@ -74,23 +77,23 @@ public static String mask(String message) {
7477

7578
String masked = message;
7679

77-
// Mask password values
78-
masked = PASSWORD_PATTERN.matcher(masked).replaceAll("password=" + MASKED_VALUE);
80+
// Mask password values (preserve original prefix format via $1 backreference)
81+
masked = PASSWORD_PATTERN.matcher(masked).replaceAll("$1" + MASKED_VALUE);
7982

80-
// Mask token values
81-
masked = TOKEN_PATTERN.matcher(masked).replaceAll("token=" + MASKED_VALUE);
83+
// Mask token values (preserve original prefix format via $1 backreference)
84+
masked = TOKEN_PATTERN.matcher(masked).replaceAll("$1" + MASKED_VALUE);
8285

83-
// Mask API key values
84-
masked = API_KEY_PATTERN.matcher(masked).replaceAll("api_key=" + MASKED_VALUE);
86+
// Mask API key values (preserve original prefix format via $1 backreference)
87+
masked = API_KEY_PATTERN.matcher(masked).replaceAll("$1" + MASKED_VALUE);
8588

8689
// Mask Bearer tokens
8790
masked = BEARER_PATTERN.matcher(masked).replaceAll("Bearer " + MASKED_VALUE);
8891

8992
// Mask Basic auth
9093
masked = BASIC_AUTH_PATTERN.matcher(masked).replaceAll("Basic " + MASKED_VALUE);
9194

92-
// Mask environment variable references (but not the variable name)
93-
masked = ENV_VAR_PATTERN.matcher(masked).replaceAll("\\$\\{$1\\}");
95+
// Mask environment variable references (hide variable name)
96+
masked = ENV_VAR_PATTERN.matcher(masked).replaceAll("\\${" + MASKED_VALUE + "}");
9497

9598
return masked;
9699
}

‎platform-api/src/test/java/org/flossware/platform/api/ApplicationDescriptorTest.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,36 @@ void testResolvePropertyNonExistent() {
243243
assertFalse(resolved.isPresent());
244244
}
245245

246+
@Test
247+
void testResolvePropertyEmptyEnvVarPattern() {
248+
// "${}" (length 3) should be treated as a plain value, not an env var reference
249+
ApplicationDescriptor descriptor =
250+
ApplicationDescriptor.builder()
251+
.applicationId("edge-test")
252+
.mainClass("com.example.App")
253+
.property("empty.var", "${}")
254+
.build();
255+
256+
Optional<String> resolved = descriptor.resolveProperty("empty.var");
257+
assertTrue(resolved.isPresent());
258+
assertEquals("${}", resolved.get());
259+
}
260+
261+
@Test
262+
void testResolvePropertyMalformedEnvVarPattern() {
263+
// "${" (length 2) should be treated as a plain value
264+
ApplicationDescriptor descriptor =
265+
ApplicationDescriptor.builder()
266+
.applicationId("edge-test2")
267+
.mainClass("com.example.App")
268+
.property("malformed.var", "${")
269+
.build();
270+
271+
Optional<String> resolved = descriptor.resolveProperty("malformed.var");
272+
assertTrue(resolved.isPresent());
273+
assertEquals("${", resolved.get());
274+
}
275+
246276
@Test
247277
void testResolvePropertyWithEnvVarReference() {
248278
// Environment variable ${PATH} should be resolvable on any system

‎platform-api/src/test/java/org/flossware/platform/api/CredentialMaskerTest.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,63 @@ void testMaskCaseInsensitive() {
8282
assertFalse(masked.contains("xyz789"));
8383
}
8484

85+
@Test
86+
void testMaskEnvVarReference() {
87+
String input = "Connection host: ${DB_HOST}";
88+
String masked = CredentialMasker.mask(input);
89+
assertFalse(masked.contains("DB_HOST"), "Variable name should be masked");
90+
assertTrue(masked.contains("${***MASKED***}"), "Should contain masked env var syntax");
91+
assertEquals("Connection host: ${***MASKED***}", masked);
92+
}
93+
94+
@Test
95+
void testMaskMultipleEnvVarReferences() {
96+
String input = "Using ${SECRET_KEY} and ${APP_CONFIG}";
97+
String masked = CredentialMasker.mask(input);
98+
assertFalse(masked.contains("SECRET_KEY"));
99+
assertFalse(masked.contains("APP_CONFIG"));
100+
assertEquals("Using ${***MASKED***} and ${***MASKED***}", masked);
101+
}
102+
85103
@Test
86104
void testMaskNonCredentialContent() {
87105
String input = "Deploying application version 1.0.0";
88106
String masked = CredentialMasker.mask(input);
89107
assertEquals(input, masked);
90108
}
91109

110+
@Test
111+
void testMaskPreservesJsonFormat() {
112+
String input = "\"password\":\"secret123\"";
113+
String masked = CredentialMasker.mask(input);
114+
assertFalse(masked.contains("secret123"));
115+
assertTrue(masked.contains("\"password\":\"***MASKED***\""));
116+
}
117+
118+
@Test
119+
void testMaskPreservesColonDelimiter() {
120+
String input = "password: secret123";
121+
String masked = CredentialMasker.mask(input);
122+
assertFalse(masked.contains("secret123"));
123+
assertTrue(masked.contains("password: ***MASKED***"));
124+
}
125+
126+
@Test
127+
void testMaskPreservesHyphenInApiKey() {
128+
String input = "api-key=secret123";
129+
String masked = CredentialMasker.mask(input);
130+
assertFalse(masked.contains("secret123"));
131+
assertTrue(masked.contains("api-key=***MASKED***"));
132+
}
133+
134+
@Test
135+
void testMaskPreservesTokenColonFormat() {
136+
String input = "\"token\":\"abc456\"";
137+
String masked = CredentialMasker.mask(input);
138+
assertFalse(masked.contains("abc456"));
139+
assertTrue(masked.contains("\"token\":\"***MASKED***\""));
140+
}
141+
92142
@Test
93143
void testMaskNull() {
94144
assertNull(CredentialMasker.mask(null));

‎platform-classloader/src/main/java/org/flossware/platform/classloader/ClassLoaderCleanupUtil.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,13 @@ private boolean clearThreadLocals(Thread thread) {
173173
Object value = valueField.get(entry);
174174

175175
if (value != null && isLoadedByTargetClassLoader(value.getClass().getClassLoader())) {
176-
table[i] = null; // Clear the entry
176+
// Clear the value to release the application object reference
177+
valueField.set(entry, null);
178+
// Mark entry as stale by clearing the WeakReference key,
179+
// so ThreadLocalMap.expungeStaleEntries() can properly reclaim the slot
180+
if (entry instanceof WeakReference) {
181+
((WeakReference<?>) entry).clear();
182+
}
177183
}
178184
}
179185
}
@@ -201,7 +207,13 @@ private boolean clearThreadLocals(Thread thread) {
201207
Object value = valueField.get(entry);
202208

203209
if (value != null && isLoadedByTargetClassLoader(value.getClass().getClassLoader())) {
204-
inheritableTable[i] = null;
210+
// Clear the value to release the application object reference
211+
valueField.set(entry, null);
212+
// Mark entry as stale by clearing the WeakReference key,
213+
// so ThreadLocalMap.expungeStaleEntries() can properly reclaim the slot
214+
if (entry instanceof WeakReference) {
215+
((WeakReference<?>) entry).clear();
216+
}
205217
}
206218
}
207219
}

‎platform-cluster-redis/src/main/java/org/flossware/platform/cluster/redis/RedisClusterManager.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,9 @@ private String extractJsonString(String json, String key) {
315315
}
316316
start += pattern.length();
317317
int end = json.indexOf("\"", start);
318+
if (end == -1) {
319+
throw new IllegalArgumentException("Malformed JSON: missing closing quote for key " + key);
320+
}
318321
return json.substring(start, end);
319322
}
320323

@@ -336,6 +339,9 @@ private int extractJsonInt(String json, String key) {
336339
if (end == -1) {
337340
end = json.indexOf("}", start);
338341
}
342+
if (end == -1) {
343+
throw new IllegalArgumentException("Malformed JSON: missing delimiter after key " + key);
344+
}
339345
return Integer.parseInt(json.substring(start, end).trim());
340346
}
341347

@@ -357,6 +363,9 @@ private long extractJsonLong(String json, String key) {
357363
if (end == -1) {
358364
end = json.indexOf("}", start);
359365
}
366+
if (end == -1) {
367+
throw new IllegalArgumentException("Malformed JSON: missing delimiter after key " + key);
368+
}
360369
return Long.parseLong(json.substring(start, end).trim());
361370
}
362371

‎platform-cluster/src/main/java/org/flossware/platform/cluster/ApplicationScheduler.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.flossware.platform.cluster;
1919

2020
import java.util.*;
21+
import java.util.concurrent.TimeUnit;
2122
import java.util.concurrent.atomic.AtomicInteger;
2223

2324
import org.flossware.platform.api.ClusterManager;
@@ -206,6 +207,39 @@ public boolean isAssignedToLocalNode(String applicationId) {
206207
return localNode != null && assignedNode.equals(localNode.getNodeId());
207208
}
208209

210+
/**
211+
* Acquires a distributed lock on the assignment entry for the specified application. This prevents
212+
* concurrent modification of the assignment while deployment is in progress, eliminating TOCTOU
213+
* race conditions between assignment checks and deployment actions.
214+
*
215+
* <p>Uses Hazelcast IMap per-entry locking to ensure only one node can check and act on an
216+
* assignment at a time.
217+
*
218+
* @param applicationId the application identifier
219+
* @param timeout maximum time to wait for the lock
220+
* @param unit time unit for the timeout
221+
* @return true if the lock was acquired within the timeout
222+
* @throws InterruptedException if the thread is interrupted while waiting
223+
* @throws NullPointerException if applicationId is null
224+
*/
225+
public boolean tryLockAssignment(String applicationId, long timeout, TimeUnit unit)
226+
throws InterruptedException {
227+
Objects.requireNonNull(applicationId, "applicationId cannot be null");
228+
return assignmentMap.tryLock(applicationId, timeout, unit);
229+
}
230+
231+
/**
232+
* Releases the distributed lock on the assignment entry for the specified application. Must be
233+
* called after {@link #tryLockAssignment} returns true, typically in a finally block.
234+
*
235+
* @param applicationId the application identifier
236+
* @throws NullPointerException if applicationId is null
237+
*/
238+
public void unlockAssignment(String applicationId) {
239+
Objects.requireNonNull(applicationId, "applicationId cannot be null");
240+
assignmentMap.unlock(applicationId);
241+
}
242+
209243
/**
210244
* Returns all application assignments in the cluster.
211245
*

0 commit comments

Comments
 (0)