From 5a951a2195c7dc27b6e3b1704ba7409bac85b8aa Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Mon, 15 Jun 2026 22:29:12 +0300 Subject: [PATCH 1/2] feat: redact secret key material in credential toString() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KeyCredential and NamedKeyCredential hold secret key material in plain fields. Any code path that stringified them — a log line, an exception message, or a debugger view — could surface the raw secret. Override toString() on both so the key is never rendered: KeyCredential emits `apiKey=***` while keeping the non-secret headerName and prefix, and NamedKeyCredential emits `key=***` while keeping the non-secret name. This matches the existing redaction on BearerToken. The override is toString-only: these are reference types with identity equality, and that behaviour is preserved (covered by tests). Adding an explicit toString() surfaces it as a declared public member, so the sdk-core API snapshot is regenerated accordingly. --- sdk-core/api/sdk-core.api | 2 + .../sdk/core/http/auth/KeyCredential.kt | 8 +++ .../sdk/core/http/auth/NamedKeyCredential.kt | 8 +++ .../sdk/core/http/auth/CredentialTest.kt | 50 +++++++++++++++++++ 4 files changed, 68 insertions(+) diff --git a/sdk-core/api/sdk-core.api b/sdk-core/api/sdk-core.api index de3a5d14..729e204b 100644 --- a/sdk-core/api/sdk-core.api +++ b/sdk-core/api/sdk-core.api @@ -201,12 +201,14 @@ public final class org/dexpace/sdk/core/http/auth/KeyCredential : org/dexpace/sd public final fun getApiKey ()Ljava/lang/String; public final fun getHeaderName ()Lorg/dexpace/sdk/core/http/common/HttpHeaderName; public final fun getPrefix ()Ljava/lang/String; + public fun toString ()Ljava/lang/String; } public final class org/dexpace/sdk/core/http/auth/NamedKeyCredential : org/dexpace/sdk/core/http/auth/Credential { public fun (Ljava/lang/String;Ljava/lang/String;)V public final fun getKey ()Ljava/lang/String; public final fun getName ()Ljava/lang/String; + public fun toString ()Ljava/lang/String; } public final class org/dexpace/sdk/core/http/common/CommonMediaTypes { diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/KeyCredential.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/KeyCredential.kt index 92513063..b5c9c1ba 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/KeyCredential.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/KeyCredential.kt @@ -34,4 +34,12 @@ public class KeyCredential init { require(apiKey.isNotBlank()) { "apiKey must not be blank" } } + + /** + * Redacts the secret [apiKey]. Without this override any log line, exception message, + * or debugger that stringifies the credential would expose the key; this emits + * `apiKey=***` instead while keeping the non-secret [headerName] and [prefix] for + * diagnostics. Identity equality is unaffected. + */ + override fun toString(): String = "KeyCredential(apiKey=***, headerName=$headerName, prefix=$prefix)" } diff --git a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/NamedKeyCredential.kt b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/NamedKeyCredential.kt index 2fb69f1b..e5e2be54 100644 --- a/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/NamedKeyCredential.kt +++ b/sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/auth/NamedKeyCredential.kt @@ -24,4 +24,12 @@ public class NamedKeyCredential(public val name: String, public val key: String) require(name.isNotBlank()) { "name must not be blank" } require(key.isNotBlank()) { "key must not be blank" } } + + /** + * Redacts the secret [key]. Without this override any log line, exception message, or + * debugger that stringifies the credential would expose the key; this emits `key=***` + * instead while keeping the non-secret [name] for diagnostics. Identity equality is + * unaffected. + */ + override fun toString(): String = "NamedKeyCredential(name=$name, key=***)" } diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/auth/CredentialTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/auth/CredentialTest.kt index 150d5fff..5bc0d74a 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/auth/CredentialTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/auth/CredentialTest.kt @@ -11,7 +11,10 @@ import org.dexpace.sdk.core.http.common.HttpHeaderName import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals import kotlin.test.assertNull +import kotlin.test.assertTrue class CredentialTest { // ----------------- KeyCredential ----------------- @@ -54,6 +57,27 @@ class CredentialTest { assertEquals("k", (cred as KeyCredential).apiKey) } + @Test + fun `KeyCredential toString redacts the apiKey but keeps non-secret fields`() { + val cred = KeyCredential("super-secret-key", prefix = "SharedAccessKey") + val rendered = cred.toString() + assertFalse(rendered.contains("super-secret-key"), "toString must not contain the raw apiKey") + assertTrue(rendered.contains("apiKey=***"), "toString must redact the apiKey") + assertTrue(rendered.contains("SharedAccessKey"), "toString should keep the non-secret prefix for diagnostics") + assertTrue( + rendered.contains(HttpHeaderName.AUTHORIZATION.toString()), + "toString should keep the non-secret headerName for diagnostics", + ) + } + + @Test + fun `KeyCredential toString redacts the apiKey with default fields`() { + val cred = KeyCredential("another-secret") + val rendered = cred.toString() + assertFalse(rendered.contains("another-secret"), "toString must not contain the raw apiKey") + assertTrue(rendered.contains("apiKey=***"), "toString must redact the apiKey") + } + // ----------------- NamedKeyCredential ----------------- @Test @@ -92,4 +116,30 @@ class CredentialTest { val cred: Credential = NamedKeyCredential("acct", "secret") assertEquals("acct", (cred as NamedKeyCredential).name) } + + @Test + fun `NamedKeyCredential toString redacts the key but keeps the name`() { + val cred = NamedKeyCredential("acct-name", "super-secret-key") + val rendered = cred.toString() + assertFalse(rendered.contains("super-secret-key"), "toString must not contain the raw key") + assertTrue(rendered.contains("key=***"), "toString must redact the key") + assertTrue(rendered.contains("acct-name"), "toString should keep the non-secret name for diagnostics") + } + + @Test + fun `NamedKeyCredential keeps identity equality - redaction is toString-only`() { + // These are reference types (not data classes): equality is identity-based and the + // redacting toString override must not change that. Same instance equals itself; + // distinct instances with the same fields are not equal. + val cred = NamedKeyCredential("acct", "secret") + assertEquals(cred, cred) + assertNotEquals(cred, NamedKeyCredential("acct", "secret")) + } + + @Test + fun `KeyCredential keeps identity equality - redaction is toString-only`() { + val cred = KeyCredential("secret-key") + assertEquals(cred, cred) + assertNotEquals(cred, KeyCredential("secret-key")) + } } From bc10ab048efd73ae4c1c68cae66b6fa36e5cf497 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Tue, 16 Jun 2026 22:23:31 +0300 Subject: [PATCH 2/2] test: assert default-field KeyCredential toString keeps the headerName The default-fields redaction test only asserted the secret was masked. Add an assertion that the non-secret headerName (Authorization) is still rendered, so the test pins both halves of the contract like the explicit-fields test does. --- .../kotlin/org/dexpace/sdk/core/http/auth/CredentialTest.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/auth/CredentialTest.kt b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/auth/CredentialTest.kt index 5bc0d74a..cad9f7ac 100644 --- a/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/auth/CredentialTest.kt +++ b/sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/auth/CredentialTest.kt @@ -76,6 +76,10 @@ class CredentialTest { val rendered = cred.toString() assertFalse(rendered.contains("another-secret"), "toString must not contain the raw apiKey") assertTrue(rendered.contains("apiKey=***"), "toString must redact the apiKey") + assertTrue( + rendered.contains(HttpHeaderName.AUTHORIZATION.toString()), + "toString should keep the default headerName for diagnostics", + ) } // ----------------- NamedKeyCredential -----------------