diff --git a/docs/open_api_core.yaml b/docs/open_api_core.yaml index 40e64b22c..2203f77f7 100644 --- a/docs/open_api_core.yaml +++ b/docs/open_api_core.yaml @@ -16134,6 +16134,7 @@ components: key: type: string writeOnly: true + description: "Write-only secret, required on create. On PUT update an omitted key is preserved from the stored entity; an explicit null is rejected with 400." project: type: string role: @@ -17738,11 +17739,15 @@ components: key: type: string writeOnly: true + nullable: true + description: "Write-only secret. On PUT update: omitted - preserved from the stored entity; explicit null - erased; a value (including the empty string) - replaces the secret." responsesEndpoint: type: string secretExtraData: type: string writeOnly: true + nullable: true + description: "Write-only secret. On PUT update: omitted - preserved from the stored entity; explicit null - erased; a value (including the empty string) - replaces the secret." tier: type: integer weight: @@ -17761,9 +17766,13 @@ components: key: type: string writeOnly: true + nullable: true + description: "Write-only secret. On PUT update: omitted - preserved from the stored entity; explicit null - erased; a value (including the empty string) - replaces the secret." secretExtraData: type: string writeOnly: true + nullable: true + description: "Write-only secret. On PUT update: omitted - preserved from the stored entity; explicit null - erased; a value (including the empty string) - replaces the secret." UserInfoResponse: type: object properties: diff --git a/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java b/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java index 02a4b7109..7df7a3a52 100644 --- a/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java +++ b/server/src/main/java/com/epam/aidial/core/server/config/SecretFieldProcessor.java @@ -106,9 +106,9 @@ private static void applyStrip(ObjectNode target, Class entityClass) { } } - public ObjectNode mergePreservingOmittedSecrets(JsonNode existingBlobNode, - JsonNode requestNode, - Class entityClass) { + public ObjectNode mergeUpdateSecrets(JsonNode existingBlobNode, + JsonNode requestNode, + Class entityClass) { if (!(requestNode instanceof ObjectNode)) { throw new IllegalArgumentException("requestNode must be an object"); } @@ -125,11 +125,10 @@ private void mergeInto(ObjectNode target, JsonNode source, Class entityClass) String name = field.getName(); if (field.isAnnotationPresent(EncryptedField.class)) { JsonNode current = target.get(name); - // Preserve-on-omit: a null or absent secret in the request body keeps the prior - // ciphertext from the stored blob. Without the retired "***" mask sentinel, only - // null / missing signals "omitted" — a literal string in the request is treated as - // a real value and re-encrypted. - if (current == null || current.isNull()) { + // Update intents: an absent field preserves the prior ciphertext from the stored + // blob; explicit null erases the secret (null flows into the entity and the blob + // omits the field); a literal string — empty string included — is the new value. + if (current == null) { JsonNode existing = source.get(name); if (existing != null && !existing.isNull()) { target.set(name, existing.deepCopy()); 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 56c0eae76..4e222a2a6 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 @@ -1578,8 +1578,9 @@ private Future handlePut() { } JsonNode source; if (spec.hasEncryptedFields() && existingBody != null) { - // Update arm with secret fields — preserve omitted/sentinel-masked - // ciphertext from the prior blob (see SecretFieldProcessor). + // Update arm with secret fields — an omitted secret keeps the prior + // ciphertext from the blob, explicit null erases it, a literal value + // replaces it (see SecretFieldProcessor). JsonNode existingBlobNode; try { existingBlobNode = BLOB_MAPPER.readTree(existingBody); @@ -1603,7 +1604,7 @@ private Future handlePut() { + "proceeding with new secret as authoritative", descriptor.getUrl()); } } - source = secretFieldProcessor.mergePreservingOmittedSecrets( + source = secretFieldProcessor.mergeUpdateSecrets( existingBlobNode, requestNode, spec.entityClass()); } else { // Create arm (no prior blob) or no encrypted fields — use the request 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 abdeff2fe..82429d55e 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 @@ -494,6 +494,34 @@ void testKeyPut200PreserveKeyOnOmit() { verify(put, 200); } + @Test + void testKeyPutExplicitNullKeyRejected400() { + String body = """ + { + "key": "secret-erase", + "project": "projA", + "roles": ["admin"] + } + """; + verify(send(HttpMethod.PUT, "/v1/keys/platform/test-key-erase", null, + body, "authorization", "admin", "If-None-Match", "*"), 200); + + // Explicit null is the erase signal for upstream secrets, but a Key without its secret is + // meaningless: the merge leaves Key.key null and the explicit-key validation rejects it. + String eraseKey = """ + { + "key": null, + "project": "projB", + "roles": ["admin"] + } + """; + Response put = send(HttpMethod.PUT, "/v1/keys/platform/test-key-erase", null, + eraseKey, "authorization", "admin"); + verify(put, 400); + assertTrue(put.body().contains("must be provided explicitly"), + () -> "Expected explicit-key rejection: " + put.body()); + } + @Test void testKeyPutBareUpsertCreatesOnMissing() { // Bare PUT against missing — upsert creates (was 404 pre-U.0). diff --git a/server/src/test/java/com/epam/aidial/core/server/ModelWriteApiTest.java b/server/src/test/java/com/epam/aidial/core/server/ModelWriteApiTest.java index 7358eafda..103aca1e0 100644 --- a/server/src/test/java/com/epam/aidial/core/server/ModelWriteApiTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/ModelWriteApiTest.java @@ -1,10 +1,15 @@ package com.epam.aidial.core.server; import io.vertx.core.http.HttpMethod; +import okhttp3.mockwebserver.RecordedRequest; import org.junit.jupiter.api.Test; +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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -15,8 +20,8 @@ * *

Slice U.4 (2026-05-25) retired the {@code ?reveal_secrets=true} reveal flow, the * {@code security-admin} role, and the {@code "***"} mask sentinel. Secret fields drop from - * GET responses via {@code @JsonProperty(WRITE_ONLY)}; preserve-on-omit signals are - * null/absent only. + * GET responses via {@code @JsonProperty(WRITE_ONLY)}; on update, an omitted secret is + * preserved, an explicit null erases it, a literal value replaces it. * *

Slice 2S.14: write controllers call {@code MergedConfigStore.rebuildNow()} on the writer pod, * making post-write GETs immediately consistent — no polling helpers needed. @@ -164,6 +169,57 @@ void testPutPreservesOmittedSecret() { () -> "Upstream key must be absent on GET: " + get.body()); } + @Test + void testPutExplicitNullUpstreamKeyErasesSecret() { + // The model endpoint targets the local TestWebServer ("adapter" position); the upstream key + // reaches it as X-UPSTREAM-KEY, so the header's presence/absence is the direct runtime + // observable of preserve vs erase — no GET surface can show it (secrets drop everywhere). + String answer = "{\"id\":\"chatcmpl-1\",\"object\":\"chat.completion\",\"created\":1,\"model\":\"m\"," + + "\"choices\":[{\"index\":0,\"finish_reason\":\"stop\",\"message\":{\"role\":\"assistant\",\"content\":\"hi\"}}]}"; + String chatBody = "{\"model\":\"test-model-erase\",\"messages\":[{\"role\":\"user\",\"content\":\"hi\"}]}"; + String withSecret = """ + { + "type": "chat", + "endpoint": "http://localhost:4849/chat/completions", + "upstreams": [ + {"endpoint": "http://vendor.example/v1/chat/completions", "key": "real-secret"} + ] + } + """; + String eraseKey = """ + { + "type": "chat", + "endpoint": "http://localhost:4849/chat/completions", + "upstreams": [ + {"endpoint": "http://vendor.example/v1/chat/completions", "key": null} + ] + } + """; + AtomicReference captured = new AtomicReference<>(); + try (TestWebServer server = new TestWebServer(4849)) { + server.map(HttpMethod.POST, "/chat/completions", request -> { + captured.set(request); + return TestWebServer.createResponse(200, answer, "Content-Type", "application/json"); + }); + + verify(send(HttpMethod.PUT, "/v1/models/platform/test-model-erase", null, withSecret, + "authorization", "admin", "If-None-Match", "*"), 200); + + // Positive control: the create-time secret rides the header before the erase. + verify(send(HttpMethod.POST, "/openai/deployments/test-model-erase/chat/completions", null, + chatBody, "content-type", "application/json"), 200); + assertEquals("real-secret", captured.get().getHeader("X-UPSTREAM-KEY")); + + verify(send(HttpMethod.PUT, "/v1/models/platform/test-model-erase", null, eraseKey, + "authorization", "admin"), 200); + + verify(send(HttpMethod.POST, "/openai/deployments/test-model-erase/chat/completions", null, + chatBody, "content-type", "application/json"), 200); + assertNull(captured.get().getHeader("X-UPSTREAM-KEY"), + "Erased secret must not reach the upstream"); + } + } + @Test void testExtraDataVisibleOnGet() { String body = """ diff --git a/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorTest.java b/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorTest.java index 979454afd..f17cec170 100644 --- a/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/config/SecretFieldProcessorTest.java @@ -27,6 +27,7 @@ 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; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; @@ -200,7 +201,7 @@ void stripEncryptedFields_dropsSecretsNestedInUpstreamInterfaces() { } @Test - void mergePreservingOmittedSecrets_preservesSecretsNestedInUpstreamInterfaces() { + void mergeUpdateSecrets_preservesSecretsNestedInUpstreamInterfaces() { ObjectNode existing = ProxyUtil.MAPPER.createObjectNode(); ObjectNode existingUpstream = existing.putArray("upstreams").addObject(); existingUpstream.put("endpoint", "http://provider"); @@ -212,7 +213,7 @@ void mergePreservingOmittedSecrets_preservesSecretsNestedInUpstreamInterfaces() requestUpstream.putObject("interfaces").putObject("anthropicMessages") .put("endpoint", "http://anthropic/v1/messages"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); assertEquals("ENC[prior]", merged.get("upstreams").get(0) .get("interfaces").get("anthropicMessages").get("key").asText()); @@ -237,38 +238,104 @@ void resolveSecret_secretReferenceUnchanged() { } @Test - void mergePreservingOmittedSecrets_copiesCiphertextWhenAbsent() throws Exception { + void mergeUpdateSecrets_copiesCiphertextWhenAbsent() throws Exception { ObjectNode existing = (ObjectNode) M.readTree("{\"key\": \"ENC[abc]\", \"role\": \"r\"}"); ObjectNode request = (ObjectNode) M.readTree("{\"role\": \"r2\"}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Key.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Key.class); assertEquals("ENC[abc]", merged.get("key").asText()); assertEquals("r2", merged.get("role").asText()); } @Test - void mergePreservingOmittedSecrets_treatsMaskAsLiteralValue() throws Exception { + void mergeUpdateSecrets_treatsMaskAsLiteralValue() throws Exception { // Slice U.4: the "***" sentinel was retired. A textual "***" in the request body is a real - // value (re-encrypted on write), not a signal to preserve. Only null / missing preserves. + // value (re-encrypted on write), not a signal to preserve. Only missing preserves; + // explicit null erases. ObjectNode existing = (ObjectNode) M.readTree("{\"key\": \"ENC[abc]\"}"); ObjectNode request = (ObjectNode) M.readTree("{\"key\": \"***\"}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Key.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Key.class); assertEquals("***", merged.get("key").asText()); } @Test - void mergePreservingOmittedSecrets_keepsExplicitNewSecret() throws Exception { + void mergeUpdateSecrets_keepsExplicitNewSecret() throws Exception { ObjectNode existing = (ObjectNode) M.readTree("{\"key\": \"ENC[abc]\"}"); ObjectNode request = (ObjectNode) M.readTree("{\"key\": \"new-plain\"}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Key.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Key.class); assertEquals("new-plain", merged.get("key").asText()); } + @Test + void mergeUpdateSecrets_explicitNullErasesPriorSecret() throws Exception { + ObjectNode existing = (ObjectNode) M.readTree("{\"key\": \"ENC[abc]\", \"role\": \"r\"}"); + ObjectNode request = (ObjectNode) M.readTree("{\"key\": null, \"role\": \"r2\"}"); + + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Key.class); + + assertTrue(merged.get("key").isNull(), () -> "prior ciphertext must not survive: " + merged); + assertEquals("r2", merged.get("role").asText()); + } + + @Test + void mergeUpdateSecrets_explicitNullInUpstreamArrayElementErases() throws Exception { + ObjectNode existing = (ObjectNode) M.readTree( + "{\"upstreams\":[{\"endpoint\":\"A\",\"key\":\"ENC[a]\"},{\"endpoint\":\"B\",\"key\":\"ENC[b]\"}]}"); + ObjectNode request = (ObjectNode) M.readTree( + "{\"upstreams\":[{\"endpoint\":\"A\",\"key\":null},{\"endpoint\":\"B\"}]}"); + + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); + + assertTrue(merged.get("upstreams").get(0).get("key").isNull(), + () -> "element A's prior ciphertext must not survive: " + merged.get("upstreams")); + assertEquals("ENC[b]", merged.get("upstreams").get(1).get("key").asText()); + } + + @Test + void mergeUpdateSecrets_explicitNullInInterfaceMapEntryErases() { + ObjectNode existing = ProxyUtil.MAPPER.createObjectNode(); + ObjectNode existingUpstream = existing.putArray("upstreams").addObject(); + existingUpstream.put("endpoint", "http://provider"); + existingUpstream.putObject("interfaces").putObject("anthropicMessages").put("key", "ENC[prior]"); + + ObjectNode request = ProxyUtil.MAPPER.createObjectNode(); + ObjectNode requestUpstream = request.putArray("upstreams").addObject(); + requestUpstream.put("endpoint", "http://provider"); + requestUpstream.putObject("interfaces").putObject("anthropicMessages").putNull("key"); + + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); + + assertTrue(merged.get("upstreams").get(0) + .get("interfaces").get("anthropicMessages").get("key").isNull()); + } + + @Test + void mergeUpdateSecrets_emptyStringStaysLiteralValue() throws Exception { + ObjectNode existing = (ObjectNode) M.readTree("{\"key\": \"ENC[abc]\"}"); + ObjectNode request = (ObjectNode) M.readTree("{\"key\": \"\"}"); + + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Key.class); + + // "" is a literal value, not an erase or preserve signal. + assertEquals("", merged.get("key").asText()); + } + + @Test + void mergeUpdateSecrets_nullWithNoPriorValueIsNoop() throws Exception { + ObjectNode existing = (ObjectNode) M.readTree("{\"role\": \"r\"}"); + ObjectNode request = (ObjectNode) M.readTree("{\"key\": null, \"role\": \"r2\"}"); + + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Key.class); + + assertTrue(merged.get("key").isNull()); + assertEquals("r2", merged.get("role").asText()); + } + @Test void encryptFields_isNullSafe() { Key key = new Key(); @@ -315,7 +382,7 @@ void reorderPreservesCorrectSecretPerEndpoint() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{\"endpoint\":\"B\"},{\"endpoint\":\"A\"}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); assertEquals("B", merged.get("upstreams").get(0).get("endpoint").asText()); assertEquals("ENC[b]", merged.get("upstreams").get(0).get("key").asText()); @@ -330,7 +397,7 @@ void insertAtIndexZeroGetsNoSecret() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{\"endpoint\":\"C\"},{\"endpoint\":\"A\"}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); ObjectNode c = (ObjectNode) merged.get("upstreams").get(0); assertEquals("C", c.get("endpoint").asText()); @@ -346,7 +413,7 @@ void removalLeavesRemainingCorrect() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{\"endpoint\":\"A\"},{\"endpoint\":\"C\"}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); assertEquals("ENC[a]", merged.get("upstreams").get(0).get("key").asText()); assertEquals("ENC[c]", merged.get("upstreams").get(1).get("key").asText()); @@ -360,7 +427,7 @@ void duplicateEndpointsMatchInRelativeOrder() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{\"endpoint\":\"A\"},{\"endpoint\":\"A\"}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); assertEquals("ENC[a0]", merged.get("upstreams").get(0).get("key").asText()); assertEquals("ENC[a1]", merged.get("upstreams").get(1).get("key").asText()); @@ -373,7 +440,7 @@ void newUpstreamWithExplicitKeyKept() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{\"endpoint\":\"D\",\"key\":\"new-plain\"},{\"endpoint\":\"A\"}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); assertEquals("new-plain", merged.get("upstreams").get(0).get("key").asText()); assertEquals("ENC[a]", merged.get("upstreams").get(1).get("key").asText()); @@ -387,7 +454,7 @@ void secretExtraDataAlsoPreservedByEndpoint() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{\"endpoint\":\"B\"},{\"endpoint\":\"A\"}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); assertEquals("ENC[xb]", merged.get("upstreams").get(0).get("secretExtraData").asText()); assertEquals("ENC[xa]", merged.get("upstreams").get(1).get("secretExtraData").asText()); @@ -404,7 +471,7 @@ void mixedKeyedUnkeyedLosesSecretOnConsumedIndexSlot() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{\"endpoint\":\"B\"},{}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); assertEquals("ENC[b]", merged.get("upstreams").get(0).get("key").asText()); ObjectNode element1 = (ObjectNode) merged.get("upstreams").get(1); @@ -420,7 +487,7 @@ void allUnkeyedElementsPairByIndex() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{},{},{}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); assertEquals("ENC[a]", merged.get("upstreams").get(0).get("key").asText()); assertEquals("ENC[b]", merged.get("upstreams").get(1).get("key").asText()); @@ -434,7 +501,7 @@ void unkeyedElementBeyondSourceBoundsGetsNothing() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{},{\"key\":\"new-plain\"}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); // element0 index-pairs with slot 0; element1 index 1 is out of source bounds (size 1) → no // preservation, its explicit value survives (re-encrypted on write). @@ -449,7 +516,7 @@ void unkeyedFirstThenEndpointMatchOnSameSlot() throws Exception { ObjectNode request = (ObjectNode) M.readTree( "{\"upstreams\":[{},{\"endpoint\":\"A\"}]}"); - ObjectNode merged = processor.mergePreservingOmittedSecrets(existing, request, Model.class); + ObjectNode merged = processor.mergeUpdateSecrets(existing, request, Model.class); // element0 (no endpoint) strict-index-pairs slot 0 → preserves ENC[a], consumes slot 0. // element1 endpoint=A then finds slot 0 consumed and no other A source → preserves nothing.