From d0a45869826fab9475e7c8ed554a8eddb606b595 Mon Sep 17 00:00:00 2001 From: Kiryl_Kurnosenka Date: Mon, 14 Sep 2026 22:31:22 +0300 Subject: [PATCH] fix: reject key writes whose secret is already used by another key entity #1979 ApiKeyStore.keys is keyed by the plaintext secret, so two Key entities created with the same secret silently collapsed: auth, roles, rate-limit project, and author attribution flipped to whichever entity was written last. Reject such writes on all write paths (409 on PUT, FAILED per entity on apply/validate), mirroring the schema duplicate-$id guard; the config-file migration endpoint pre-removes the file twin from the scratch so it remains the supported handoff for file-defined keys. Updates that keep the entity's own prior secret stay allowed. Co-Authored-By: Claude Code --- .../server/config/ConfigPostProcessor.java | 20 +++ .../core/server/config/MergedConfigStore.java | 5 + .../ConfigFileMigrateController.java | 5 + .../controller/ConfigResourceController.java | 30 +++- .../service/config/ConfigApplyService.java | 9 +- .../config/ConfigValidationService.java | 10 ++ .../aidial/core/server/AdminApplyApiTest.java | 144 ++++++++++++++++++ .../core/server/AdminValidateApiTest.java | 134 ++++++++++++++++ .../core/server/ConfigEntityWriteApiTest.java | 99 +++++++++++- .../config/ConfigPostProcessorTest.java | 49 ++++++ 10 files changed, 498 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/com/epam/aidial/core/server/config/ConfigPostProcessor.java b/server/src/main/java/com/epam/aidial/core/server/config/ConfigPostProcessor.java index 007e0efdd..bda7215e3 100644 --- a/server/src/main/java/com/epam/aidial/core/server/config/ConfigPostProcessor.java +++ b/server/src/main/java/com/epam/aidial/core/server/config/ConfigPostProcessor.java @@ -38,6 +38,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.BiConsumer; import java.util.regex.Pattern; @@ -767,6 +768,25 @@ public static boolean isDeploymentIdTakenByAnotherDeploymentType(Config config, .anyMatch(entry -> entry.getKey() != type && entry.getValue().containsKey(shortName)); } + /** + * Returns {@code true} if {@code candidate}'s secret is already used by a different entry of + * the folded {@code Config.keys} map — file-sourced entries keyed by their raw secret, + * blob-sourced entries keyed by canonical id. The entry stored under {@code selfMapKey} is + * the candidate's own and never counts as a collision. + */ + public static boolean isKeySecretTakenByAnotherKey(Config config, String selfMapKey, Key candidate) { + String secret = candidate.getKey(); + if (secret == null || secret.isBlank()) { + return false; + } + return config.getKeys().entrySet().stream() + .filter(entry -> !entry.getKey().equals(selfMapKey)) + .map(Map.Entry::getValue) + .filter(Objects::nonNull) + .map(Key::getKey) + .anyMatch(secret::equals); + } + private static boolean isValidResourceKey(String resourceKey) { return RESOURCE_KEY_PATTERN.matcher(resourceKey).matches(); } diff --git a/server/src/main/java/com/epam/aidial/core/server/config/MergedConfigStore.java b/server/src/main/java/com/epam/aidial/core/server/config/MergedConfigStore.java index 392be5bfd..082c01b46 100644 --- a/server/src/main/java/com/epam/aidial/core/server/config/MergedConfigStore.java +++ b/server/src/main/java/com/epam/aidial/core/server/config/MergedConfigStore.java @@ -393,6 +393,11 @@ void applyReplicaEvent(ResourceDescriptor descriptor, ResourceEvent.Action actio if (oldSecret != null && !oldSecret.isBlank() && !oldSecret.equals(secret)) { apiKeyStore.removeKey(oldSecret); } + if (snapshot != null + && ConfigPostProcessor.isKeySecretTakenByAnotherKey(snapshot, mapKey, key)) { + log.warn("Replica key event carries a secret already used by a different key entity: {}", + descriptor.getUrl()); + } } applyEntityWrite(type, mapKey, entity); } finally { diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ConfigFileMigrateController.java b/server/src/main/java/com/epam/aidial/core/server/controller/ConfigFileMigrateController.java index e5338db3b..f7b951eaf 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/ConfigFileMigrateController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/ConfigFileMigrateController.java @@ -313,6 +313,11 @@ private void collectKeys(Config fileConfig, Config scratch, boolean dryRun, // serialization regardless of the Java object's field state — inject it directly. ObjectNode specNode = ProxyUtil.MAPPER.valueToTree(fileKey); specNode.put("key", secret); + // The duplicate-secret guard rejects a key write whose secret is already used by a + // different scratch entry. Since scratch starts as a copy of the merged/live config, + // the file-sourced key being migrated still sits under its raw secret there — remove + // the shadow so validation sees a genuinely new blob entry, not a collision with itself. + scratch.getKeys().remove(secret); collect(new AdminManifest("Key", canonicalId, specNode), null, scratch, dryRun, toApply, results); } } diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java b/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java index 44054e33f..9dd738aec 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/ConfigResourceController.java @@ -1625,7 +1625,8 @@ private Future handlePut() { } if (spec.isKey()) { keyEntity = (Key) entity; - validateKeyForApiWrite(keyEntity, "PUT"); + validateKeyForApiWrite(keyEntity); + rejectDuplicateKeySecret(keyEntity, oldSecret, requestNode, descriptor); } if (spec.hasEncryptedFields()) { secretFieldProcessor.encryptFields(entity, descriptor); @@ -1807,16 +1808,37 @@ private void rejectDuplicateDeploymentId(ResourceTypes type, String shortName) { } } + /** + * Rejects a key write whose secret is already used by a different key entity in the live + * merged Config — ApiKeyStore indexes keys by plaintext secret, so a duplicate would silently + * collapse the two entities' auth, roles and project attribution. Skipped when the request + * omits the secret (preserve-on-omit) or re-supplies the entity's own current secret. + */ + private void rejectDuplicateKeySecret(Key keyEntity, String oldSecret, JsonNode requestNode, + ResourceDescriptor descriptor) { + if (!requestNode.hasNonNull("key") || keyEntity.getKey().equals(oldSecret)) { + return; + } + Config snapshot = mergedConfigStore.get(); + if (snapshot == null) { + return; + } + if (ConfigPostProcessor.isKeySecretTakenByAnotherKey(snapshot, + MergedConfigStore.resolveMapKeyFor(descriptor), keyEntity)) { + throw new HttpException(HttpStatus.CONFLICT, + "Key secret is already used by a different key entity"); + } + } + private static ApiKeyData apiKeyData(Key key) { ApiKeyData data = new ApiKeyData(); data.setOriginalKey(key); return data; } - private static void validateKeyForApiWrite(Key key, String method) { + private static void validateKeyForApiWrite(Key key) { if (StringUtils.isBlank(key.getKey())) { - throw new HttpException(HttpStatus.BAD_REQUEST, - "Key.key must be provided explicitly on " + method); + throw new HttpException(HttpStatus.BAD_REQUEST, "Key.key must be provided explicitly on PUT"); } validateProjectKey(key); } diff --git a/server/src/main/java/com/epam/aidial/core/server/service/config/ConfigApplyService.java b/server/src/main/java/com/epam/aidial/core/server/service/config/ConfigApplyService.java index 50b87e8ca..ffbb0b640 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/config/ConfigApplyService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/config/ConfigApplyService.java @@ -147,7 +147,7 @@ private EntityResult applySingle(ConfigManifestSupport.ParsedManifest parsed, Co applyManagedEntity(role.spec(), id, parsed.name(), ResourceTypes.ROLE, scratch, pending); case AdminRouteManifest route -> applyManagedEntity(route.spec(), id, parsed.name(), ResourceTypes.ROUTE, scratch, pending); - case AdminKeyManifest key -> applyKey(key.spec(), id, parsed.name(), pending); + case AdminKeyManifest key -> applyKey(key.spec(), id, parsed.name(), scratch, pending); case AdminModelManifest model -> applyModel(model.spec(), id, parsed.name(), scratch, pending); case AdminToolSetManifest toolSet -> applyToolSet(toolSet.spec(), id, parsed.name(), scratch, pending); case AdminApplicationManifest application -> @@ -226,7 +226,7 @@ private EntityResult applyTranslator(Translator translator, String id, ParsedNam return new EntityResult(id, AdminApplyStatus.APPLIED, null); } - private EntityResult applyKey(Key key, String id, ParsedName parsed, List pending) { + private EntityResult applyKey(Key key, String id, ParsedName parsed, Config scratch, List pending) { if (StringUtils.isBlank(key.getKey())) { return new EntityResult(id, AdminApplyStatus.FAILED, "Key.key must be provided explicitly"); } @@ -256,6 +256,11 @@ private EntityResult applyKey(Key key, String id, ParsedName parsed, List { List warnings = new ArrayList<>(); diff --git a/server/src/test/java/com/epam/aidial/core/server/AdminApplyApiTest.java b/server/src/test/java/com/epam/aidial/core/server/AdminApplyApiTest.java index 37d1d2d79..0a4dc3935 100644 --- a/server/src/test/java/com/epam/aidial/core/server/AdminApplyApiTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/AdminApplyApiTest.java @@ -788,6 +788,150 @@ void batchKeyRotationRemovesOldSecret() { verify(send(HttpMethod.GET, "/v1/bucket", null, "", "Api-key", "apply-secret-new"), 200); } + @Test + @SneakyThrows + void applyKeyDuplicateSecretFailsSecondEntity() { + // ApiKeyStore indexes keys by plaintext secret, so two key entities sharing one secret + // would silently collapse their auth, roles and project attribution. The second entity + // in the batch must fail and leave nothing behind. + String body = """ + { + "precheck": false, + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/apply-dup-key-a", + "spec": {"key": "apply-dup-secret", "project": "projA", "roles": ["admin"]} + }, + { + "kind": "Key", + "name": "keys/platform/apply-dup-key-b", + "spec": {"key": "apply-dup-secret", "project": "projB", "roles": ["admin"]} + } + ] + } + """; + Response response = send(HttpMethod.POST, "/v1/admin/apply", null, body, "authorization", "admin"); + verify(response, 200); + JsonNode parsed = ProxyUtil.MAPPER.readTree(response.body()); + assertEquals(1, parsed.get("applied").asInt(), () -> "Body: " + response.body()); + assertEquals(1, parsed.get("failed").asInt(), () -> "Body: " + response.body()); + assertTrue(parsed.get("results").get(1).get("error").asText() + .contains("already used by a different key entity"), () -> "Body: " + response.body()); + assertFalse(response.body().contains("apply-dup-secret"), + () -> "Response must not echo the secret: " + response.body()); + verify(send(HttpMethod.GET, "/v1/keys/platform/apply-dup-key-a", null, "", + "authorization", "admin"), 200); + verify(send(HttpMethod.GET, "/v1/keys/platform/apply-dup-key-b", null, "", + "authorization", "admin"), 404); + verify(send(HttpMethod.GET, "/v1/bucket", null, "", "Api-key", "apply-dup-secret"), 200); + } + + @Test + @SneakyThrows + void applyKeyRotateToExistingSecretFailsEntity() { + String bodyA = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/apply-rotate-dup-key-a", + "spec": {"key": "apply-rotate-dup-a", "project": "projA", "roles": ["admin"]} + } + ] + } + """; + String bodyB = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/apply-rotate-dup-key-b", + "spec": {"key": "apply-rotate-dup-b", "project": "projB", "roles": ["admin"]} + } + ] + } + """; + String bodyRotate = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/apply-rotate-dup-key-a", + "spec": {"key": "apply-rotate-dup-b", "project": "projA", "roles": ["admin"]} + } + ] + } + """; + verify(send(HttpMethod.POST, "/v1/admin/apply", null, bodyA, "authorization", "admin"), 200); + verify(send(HttpMethod.POST, "/v1/admin/apply", null, bodyB, "authorization", "admin"), 200); + + Response response = send(HttpMethod.POST, "/v1/admin/apply", null, bodyRotate, "authorization", "admin"); + verify(response, 422); + JsonNode parsed = ProxyUtil.MAPPER.readTree(response.body()); + assertEquals(1, parsed.get("failed").asInt(), () -> "Body: " + response.body()); + + // Both original secrets still authenticate; the rotation was refused wholesale. + verify(send(HttpMethod.GET, "/v1/bucket", null, "", "Api-key", "apply-rotate-dup-a"), 200); + verify(send(HttpMethod.GET, "/v1/bucket", null, "", "Api-key", "apply-rotate-dup-b"), 200); + } + + @Test + @SneakyThrows + void applyKeyUpdateWithUnchangedSecretSucceeds() { + // An update re-supplying the entity's own current secret is not a collision with itself — + // the guard only polices new secrets. + String body = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/apply-unchanged-key", + "spec": {"key": "apply-unchanged-secret", "project": "projA", "roles": ["admin"]} + } + ] + } + """; + String bodyUpdated = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/apply-unchanged-key", + "spec": {"key": "apply-unchanged-secret", "project": "projB", "roles": ["default"]} + } + ] + } + """; + verify(send(HttpMethod.POST, "/v1/admin/apply", null, body, "authorization", "admin"), 200); + verify(send(HttpMethod.POST, "/v1/admin/apply", null, bodyUpdated, "authorization", "admin"), 200); + verify(send(HttpMethod.GET, "/v1/bucket", null, "", "Api-key", "apply-unchanged-secret"), 200); + } + + @Test + @SneakyThrows + void applyKeyWithFileKeySecretFails() { + // The file→blob handoff for keys goes through the migration endpoint; a direct apply + // claiming a file-sourced key's secret must fail. + String body = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/apply-file-secret-key", + "spec": {"key": "proxyKey1", "project": "someone-else", "roles": ["admin"]} + } + ] + } + """; + Response response = send(HttpMethod.POST, "/v1/admin/apply", null, body, "authorization", "admin"); + verify(response, 422); + assertFalse(response.body().contains("proxyKey1"), + () -> "Response must not echo the secret: " + response.body()); + verify(send(HttpMethod.GET, "/v1/keys/platform/apply-file-secret-key", null, "", + "authorization", "admin"), 404); + } + @Test @SneakyThrows void testApplyEmptyManifestsBatchOk() { diff --git a/server/src/test/java/com/epam/aidial/core/server/AdminValidateApiTest.java b/server/src/test/java/com/epam/aidial/core/server/AdminValidateApiTest.java index 7d03c8dc3..335321f16 100644 --- a/server/src/test/java/com/epam/aidial/core/server/AdminValidateApiTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/AdminValidateApiTest.java @@ -8,6 +8,7 @@ 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.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -757,6 +758,139 @@ void testValidateAndApplyCatalogSchemaDuplicateIdParity() { "authorization", "admin"), 404); } + @Test + @SneakyThrows + void testValidateKeyDuplicateSecretWithinBatchRejected() { + // Precheck-side counterpart of AdminApplyApiTest#applyKeyDuplicateSecretFailsSecondEntity: + // the second key in a batch must collide with the first one validated before it. + String body = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/validate-dup-key-a", + "spec": {"key": "validate-dup-secret", "project": "projA", "roles": ["admin"]} + }, + { + "kind": "Key", + "name": "keys/platform/validate-dup-key-b", + "spec": {"key": "validate-dup-secret", "project": "projB", "roles": ["admin"]} + } + ] + } + """; + Response response = send(HttpMethod.POST, "/v1/admin/validate", null, body, "authorization", "admin"); + verify(response, 422); + JsonNode parsed = ProxyUtil.MAPPER.readTree(response.body()); + assertEquals(0, parsed.get("valid").asInt(), () -> "Body: " + response.body()); + assertEquals(1, parsed.get("failed").asInt(), () -> "Body: " + response.body()); + assertEquals("SKIPPED", parsed.get("results").get(0).get("status").asText(), () -> "Body: " + response.body()); + assertEquals("FAILED", parsed.get("results").get(1).get("status").asText(), () -> "Body: " + response.body()); + assertTrue(parsed.get("results").get(1).get("error").asText() + .contains("already used by a different key entity"), () -> "Body: " + response.body()); + assertFalse(response.body().contains("validate-dup-secret"), + () -> "Response must not echo the secret: " + response.body()); + verify(send(HttpMethod.GET, "/v1/keys/platform/validate-dup-key-a", null, "", + "authorization", "admin"), 404); + verify(send(HttpMethod.GET, "/v1/keys/platform/validate-dup-key-b", null, "", + "authorization", "admin"), 404); + } + + @Test + @SneakyThrows + void testValidateKeyDuplicateSecretAgainstAppliedKeyRejected() { + String existingBody = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/validate-existing-key", + "spec": {"key": "validate-existing-secret", "project": "projA", "roles": ["admin"]} + } + ] + } + """; + verify(send(HttpMethod.POST, "/v1/admin/apply", null, existingBody, "authorization", "admin"), 200); + + String conflictBody = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/validate-conflict-key", + "spec": {"key": "validate-existing-secret", "project": "projB", "roles": ["admin"]} + } + ] + } + """; + Response response = send(HttpMethod.POST, "/v1/admin/validate", null, conflictBody, "authorization", "admin"); + verify(response, 422); + JsonNode parsed = ProxyUtil.MAPPER.readTree(response.body()); + assertEquals(1, parsed.get("failed").asInt(), () -> "Body: " + response.body()); + assertEquals("FAILED", parsed.get("results").get(0).get("status").asText(), () -> "Body: " + response.body()); + verify(send(HttpMethod.GET, "/v1/keys/platform/validate-conflict-key", null, "", + "authorization", "admin"), 404); + } + + @Test + @SneakyThrows + void testValidateKeyUnchangedSecretUpdateValid() { + // Re-supplying the entity's own current secret is an update, not a collision. + String existingBody = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/validate-unchanged-key", + "spec": {"key": "validate-unchanged-secret", "project": "projA", "roles": ["admin"]} + } + ] + } + """; + verify(send(HttpMethod.POST, "/v1/admin/apply", null, existingBody, "authorization", "admin"), 200); + + String updateBody = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/validate-unchanged-key", + "spec": {"key": "validate-unchanged-secret", "project": "projB", "roles": ["default"]} + } + ] + } + """; + Response response = send(HttpMethod.POST, "/v1/admin/validate", null, updateBody, "authorization", "admin"); + verify(response, 200); + JsonNode parsed = ProxyUtil.MAPPER.readTree(response.body()); + assertEquals(1, parsed.get("valid").asInt(), () -> "Body: " + response.body()); + assertEquals(0, parsed.get("failed").asInt()); + } + + @Test + @SneakyThrows + void testValidateKeyWithFileKeySecretRejected() { + // File-sourced keys occupy their raw secret in the folded config; the file→blob handoff + // goes through the migration endpoint. + String body = """ + { + "manifests": [ + { + "kind": "Key", + "name": "keys/platform/validate-file-secret-key", + "spec": {"key": "proxyKey1", "project": "someone-else", "roles": ["admin"]} + } + ] + } + """; + Response response = send(HttpMethod.POST, "/v1/admin/validate", null, body, "authorization", "admin"); + verify(response, 422); + assertFalse(response.body().contains("proxyKey1"), + () -> "Response must not echo the secret: " + response.body()); + verify(send(HttpMethod.GET, "/v1/keys/platform/validate-file-secret-key", null, "", + "authorization", "admin"), 404); + } + @Test @SneakyThrows void testV16SettingsDoesNotPersist() { diff --git a/server/src/test/java/com/epam/aidial/core/server/ConfigEntityWriteApiTest.java b/server/src/test/java/com/epam/aidial/core/server/ConfigEntityWriteApiTest.java index b652ef935..c26cb8b94 100644 --- a/server/src/test/java/com/epam/aidial/core/server/ConfigEntityWriteApiTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/ConfigEntityWriteApiTest.java @@ -3,6 +3,7 @@ import io.vertx.core.http.HttpMethod; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -500,11 +501,107 @@ void testKeyPut200PreserveKeyOnOmit() { "Api-key", "secret-preserve"), 200); } + @Test + void testKeyPutCreate409OnDuplicateSecret() { + String bodyA = """ + { + "key": "secret-dup-409", + "project": "projA", + "roles": ["admin"] + } + """; + String bodyB = """ + { + "key": "secret-dup-409", + "project": "projB", + "roles": ["admin"] + } + """; + verify(send(HttpMethod.PUT, "/v1/keys/platform/test-key-dup-a", null, + bodyA, "authorization", "admin", "If-None-Match", "*"), 200); + + Response put = send(HttpMethod.PUT, "/v1/keys/platform/test-key-dup-b", null, + bodyB, "authorization", "admin", "If-None-Match", "*"); + verify(put, 409); + assertFalse(put.body().contains("secret-dup-409"), + () -> "Response must not echo the secret: " + put.body()); + verify(send(HttpMethod.GET, "/v1/keys/platform/test-key-dup-b", null, "", + "authorization", "admin"), 404); + verify(send(HttpMethod.GET, "/v1/bucket", null, "", + "Api-key", "secret-dup-409"), 200); + } + + @Test + void testKeyPutRotateToExistingSecret409KeepsBoth() { + String bodyA = """ + { + "key": "secret-rotate-dup-a", + "project": "projA", + "roles": ["admin"] + } + """; + String bodyB = """ + { + "key": "secret-rotate-dup-b", + "project": "projB", + "roles": ["admin"] + } + """; + String bodyRotate = """ + { + "key": "secret-rotate-dup-b", + "project": "projA", + "roles": ["admin"] + } + """; + verify(send(HttpMethod.PUT, "/v1/keys/platform/test-key-rotate-dup-a", null, + bodyA, "authorization", "admin", "If-None-Match", "*"), 200); + verify(send(HttpMethod.PUT, "/v1/keys/platform/test-key-rotate-dup-b", null, + bodyB, "authorization", "admin", "If-None-Match", "*"), 200); + + Response put = send(HttpMethod.PUT, "/v1/keys/platform/test-key-rotate-dup-a", null, + bodyRotate, "authorization", "admin"); + verify(put, 409); + + verify(send(HttpMethod.GET, "/v1/bucket", null, "", + "Api-key", "secret-rotate-dup-a"), 200); + verify(send(HttpMethod.GET, "/v1/bucket", null, "", + "Api-key", "secret-rotate-dup-b"), 200); + } + + @Test + void testKeyPutCreate409OnFileKeySecret() { + // A file-sourced key occupies its raw secret in the folded config, so a blob key claiming + // the same secret would collapse with it in ApiKeyStore's secret-indexed map. The + // file→blob handoff goes through the migration endpoint, not a direct PUT. + String body = """ + { + "key": "proxyKey1", + "project": "someone-else", + "roles": ["admin"] + } + """; + Response put = send(HttpMethod.PUT, "/v1/keys/platform/test-key-file-secret", null, + body, "authorization", "admin", "If-None-Match", "*"); + verify(put, 409); + assertFalse(put.body().contains("proxyKey1"), + () -> "Response must not echo the secret: " + put.body()); + verify(send(HttpMethod.GET, "/v1/keys/platform/test-key-file-secret", null, "", + "authorization", "admin"), 404); + } + @Test void testKeyPutBareUpsertCreatesOnMissing() { // Bare PUT against missing — upsert creates (was 404 pre-U.0). + String body = """ + { + "key": "secret-bare-upsert", + "project": "projA", + "roles": ["admin"] + } + """; Response put = send(HttpMethod.PUT, "/v1/keys/platform/no-such-key-create", null, - KEY_BODY_PROJECT_A, "authorization", "admin"); + body, "authorization", "admin"); verify(put, 200); } diff --git a/server/src/test/java/com/epam/aidial/core/server/config/ConfigPostProcessorTest.java b/server/src/test/java/com/epam/aidial/core/server/config/ConfigPostProcessorTest.java index e5c5fa193..f8da6c7df 100644 --- a/server/src/test/java/com/epam/aidial/core/server/config/ConfigPostProcessorTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/config/ConfigPostProcessorTest.java @@ -5,6 +5,7 @@ import com.epam.aidial.core.config.DeploymentInterface; import com.epam.aidial.core.config.Interceptor; import com.epam.aidial.core.config.InterfaceMode; +import com.epam.aidial.core.config.Key; import com.epam.aidial.core.config.Model; import com.epam.aidial.core.config.Pricing; import com.epam.aidial.core.config.PricingRate; @@ -673,4 +674,52 @@ void testProcessOrchestratesStructuralThenSemanticInAbortMode() { assertEquals(List.of("good"), List.copyOf(config.getModels().keySet())); } + + private static Key keyWithSecret(String secret) { + Key key = new Key(); + key.setKey(secret); + key.setProject("proj"); + return key; + } + + @Test + void testIsKeySecretTakenByAnotherKeyBlobAgainstBlob() { + Config config = new Config(); + config.getKeys().put("keys/platform/other", keyWithSecret("shared-secret")); + config.getKeys().put("keys/platform/self", keyWithSecret("own-secret")); + + assertTrue(ConfigPostProcessor.isKeySecretTakenByAnotherKey( + config, "keys/platform/self", keyWithSecret("shared-secret"))); + assertFalse(ConfigPostProcessor.isKeySecretTakenByAnotherKey( + config, "keys/platform/self", keyWithSecret("own-secret"))); + assertFalse(ConfigPostProcessor.isKeySecretTakenByAnotherKey( + config, "keys/platform/self", keyWithSecret("fresh-secret"))); + } + + @Test + void testIsKeySecretTakenByAnotherKeyFileSourcedEntry() { + // File-sourced entries sit under their raw secret as the map key, with the secret + // back-filled into the value by ApiKeyStore before the merged config is served. + Config config = new Config(); + config.getKeys().put("file-secret", keyWithSecret("file-secret")); + + assertTrue(ConfigPostProcessor.isKeySecretTakenByAnotherKey( + config, "keys/platform/self", keyWithSecret("file-secret"))); + } + + @Test + void testIsKeySecretTakenByAnotherKeySkipsSelfBlankAndNull() { + Config config = new Config(); + config.getKeys().put("keys/platform/self", keyWithSecret("own-secret")); + config.getKeys().put("keys/platform/blank", keyWithSecret(null)); + config.getKeys().put("keys/platform/null-value", null); + + // Self-map-key skip, blank candidate, blank other, null other value. + assertFalse(ConfigPostProcessor.isKeySecretTakenByAnotherKey( + config, "keys/platform/self", keyWithSecret("own-secret"))); + assertFalse(ConfigPostProcessor.isKeySecretTakenByAnotherKey( + config, "keys/platform/self", keyWithSecret(" "))); + assertFalse(ConfigPostProcessor.isKeySecretTakenByAnotherKey( + config, "keys/platform/self", keyWithSecret("blank"))); + } }