diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ea143042..c9342a830 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/): - PATCH version when you make backwards compatible bug fixes. # Next version +- [security fix] _agent_: The Teamscale access token was logged in clear text in DEBUG-level logs (e.g., when `debug=true` was set) and in the WARN-level log emitted when multiple `-javaagent` arguments are present. The token is now obfuscated in those logs as well, matching INFO-level behavior. # 36.5.1 - [fix] _agent_: The profiled application no longer crashes when the profiler configuration is invalid (e.g., missing `teamscale-user`). Instead, the application starts normally without coverage collection. diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java index a5bc35139..f2f14ad77 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java @@ -184,7 +184,7 @@ private static Pair> getAndApplyAgentOptions(Strin if (!differentAgents.isEmpty()) { delayedLogger.warn( "Using multiple java agents could interfere with coverage recording: " + - String.join(", ", differentAgents)); + AgentOptions.obfuscateAccessToken(String.join(", ", differentAgents))); } if (!javaAgents.get(0).contains("teamscale-jacoco-agent.jar")) { delayedLogger.warn("For best results consider registering the Teamscale Java Profiler first."); diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java b/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java index 4a19f3e3f..ae3ed937e 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java @@ -256,20 +256,35 @@ public String getOriginalOptionsString() { * "config-file=jacocoagent.properties,teamscale-access-token=************mNHn" */ public String getObfuscatedOptionsString() { - if (getOriginalOptionsString() == null) { + return obfuscateAccessToken(getOriginalOptionsString()); + } + + /** + * Obfuscates any "*-access-token=..." value in the given options string for safe logging. Keeps only the last 4 + * characters of each token. Returns an empty string for {@code null} input and the original string when no token + * pattern matches. + */ + public static String obfuscateAccessToken(String optionsString) { + if (optionsString == null) { return ""; } - Pattern pattern = Pattern.compile("(.*-access-token=)([^,]+)(.*)"); - Matcher match = pattern.matcher(getOriginalOptionsString()); - if (match.find()) { + Pattern pattern = Pattern.compile("(-access-token=)([^,\\n\\r]+)"); + Matcher match = pattern.matcher(optionsString); + StringBuffer obfuscated = new StringBuffer(); + boolean foundAny = false; + while (match.find()) { + foundAny = true; String apiKey = match.group(2); String obfuscatedApiKey = String.format("************%s", apiKey.substring(Math.max(0, apiKey.length() - 4))); - return String.format("%s%s%s", match.group(1), obfuscatedApiKey, match.group(3)); + match.appendReplacement(obfuscated, Matcher.quoteReplacement(match.group(1) + obfuscatedApiKey)); } - - return getOriginalOptionsString(); + if (!foundAny) { + return optionsString; + } + match.appendTail(obfuscated); + return obfuscated.toString(); } /** diff --git a/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java b/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java index 92fc1a5ad..6ad506881 100644 --- a/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java +++ b/agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java @@ -122,7 +122,7 @@ public void throwOnCollectedErrors() throws Exception { if (optionsString == null) { optionsString = ""; } - logger.debug("Parsing options: " + optionsString); + logger.debug("Parsing options: " + AgentOptions.obfuscateAccessToken(optionsString)); AgentOptions options = new AgentOptions(logger); options.originalOptionsString = optionsString; @@ -432,7 +432,8 @@ private void readConfigFromTeamscale( options.teamscaleServer.userAccessToken); options.configurationViaTeamscale = configuration; logger.debug( - "Received the following options from Teamscale: " + configuration.getProfilerConfiguration().configurationOptions); + "Received the following options from Teamscale: " + AgentOptions.obfuscateAccessToken( + configuration.getProfilerConfiguration().configurationOptions)); readConfigFromString(options, configuration.getProfilerConfiguration().configurationOptions); } diff --git a/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java b/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java index 649ce6939..669d171ab 100644 --- a/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java +++ b/agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsTest.java @@ -510,6 +510,50 @@ public void configIdWithAllMissingOptionsReportsAll() { .hasMessageContaining("teamscale-access-token"); } + /** Tests that {@link AgentOptions#obfuscateAccessToken(String)} hides the token in a comma-separated options string. */ + @Test + public void obfuscateAccessTokenHidesTokenInOptionsString() { + String input = "config-file=jacocoagent.properties,teamscale-access-token=unlYgehaYYYhbPAegNWV3WgjOzxkmNHn,teamscale-partition=p"; + assertThat(AgentOptions.obfuscateAccessToken(input)).isEqualTo( + "config-file=jacocoagent.properties,teamscale-access-token=************mNHn,teamscale-partition=p"); + } + + /** Tests that obfuscation also covers tokens in newline-separated input (the format Teamscale returns for config-id). */ + @Test + public void obfuscateAccessTokenHidesTokenInNewlineSeparatedString() { + String input = "teamscale-access-token=unlYgehaYYYhbPAegNWV3WgjOzxkmNHn\nteamscale-partition=p"; + assertThat(AgentOptions.obfuscateAccessToken(input)).isEqualTo( + "teamscale-access-token=************mNHn\nteamscale-partition=p"); + } + + /** Tests that obfuscation hides every {@code *-access-token=} occurrence, not just the last one. */ + @Test + public void obfuscateAccessTokenHidesMultipleTokens() { + String input = "teamscale-access-token=unlYgehaYYYhbPAegNWV3WgjOzxkmNHn,artifactory-access-token=anotherSecretAbcd"; + assertThat(AgentOptions.obfuscateAccessToken(input)).isEqualTo( + "teamscale-access-token=************mNHn,artifactory-access-token=************Abcd"); + } + + /** Tests that strings without an access token are returned unchanged. */ + @Test + public void obfuscateAccessTokenReturnsInputUnchangedWhenNoTokenPresent() { + String input = "config-file=jacocoagent.properties,teamscale-partition=p"; + assertThat(AgentOptions.obfuscateAccessToken(input)).isEqualTo(input); + } + + /** Tests the null-input contract used by {@link AgentOptions#getObfuscatedOptionsString()}. */ + @Test + public void obfuscateAccessTokenReturnsEmptyStringForNullInput() { + assertThat(AgentOptions.obfuscateAccessToken(null)).isEmpty(); + } + + /** Tests that a token shorter than 4 characters does not throw and is fully obfuscated. */ + @Test + public void obfuscateAccessTokenHandlesShortToken() { + assertThat(AgentOptions.obfuscateAccessToken("teamscale-access-token=abc")).isEqualTo( + "teamscale-access-token=************abc"); + } + /** * Delete created coverage folders */ diff --git a/common-system-test/src/main/kotlin/com/teamscale/test/commons/TeamscaleMockServer.kt b/common-system-test/src/main/kotlin/com/teamscale/test/commons/TeamscaleMockServer.kt index ef737fadd..54eab04c3 100644 --- a/common-system-test/src/main/kotlin/com/teamscale/test/commons/TeamscaleMockServer.kt +++ b/common-system-test/src/main/kotlin/com/teamscale/test/commons/TeamscaleMockServer.kt @@ -43,6 +43,9 @@ class TeamscaleMockServer(val port: Int) { private val service = Service.ignite() private var impactedTests = listOf() val profilerEvents = mutableListOf() + + /** All log entries that the profiler forwarded to this server via {@code POST /profilers/:profilerId/logs}. */ + val collectedLogMessages = mutableListOf() private var profilerConfiguration: ProfilerConfiguration? = null private var username: String? = null private var accessToken: String? = null @@ -70,6 +73,7 @@ class TeamscaleMockServer(val port: Int) { sessions.clear() allAvailableTests.clear() profilerEvents.clear() + collectedLogMessages.clear() impactedTestCommits.clear() baselines.clear() } @@ -229,6 +233,11 @@ class TeamscaleMockServer(val port: Int) { request.collectUserAgent() profilerEvents.add("Profiler " + request.params(":profilerId") + " sent logs") + try { + collectedLogMessages.addAll(deserializeList(request.body())) + } catch (e: JsonProcessingException) { + // Body wasn't a list of ProfilerLogEntry — ignore so we don't break tests that don't care about log bodies. + } return "" } diff --git a/system-tests/teamscale-profiler-configuration-test/src/test/kotlin/com/teamscale/client/TeamscaleProfilerConfigurationSystemTest.kt b/system-tests/teamscale-profiler-configuration-test/src/test/kotlin/com/teamscale/client/TeamscaleProfilerConfigurationSystemTest.kt index f1ae10b33..e9dc9280c 100644 --- a/system-tests/teamscale-profiler-configuration-test/src/test/kotlin/com/teamscale/client/TeamscaleProfilerConfigurationSystemTest.kt +++ b/system-tests/teamscale-profiler-configuration-test/src/test/kotlin/com/teamscale/client/TeamscaleProfilerConfigurationSystemTest.kt @@ -56,8 +56,50 @@ class TeamscaleProfilerConfigurationSystemTest { assertThat(result.stdout).contains("Production code") } + /** + * Reproduces the customer-reported scenario (dmTech) where enabling debug logging caused the + * `teamscale-access-token` to appear in clear text in the debug logs that are forwarded back to Teamscale via + * [com.teamscale.jacoco.agent.logging.LogToTeamscaleAppender]. Asserts that the raw token never reaches the + * server and that the obfuscated form does, proving the obfuscation is wired up at the parser-level debug logs. + */ + @Test + @Throws(Exception::class) + fun systemTestDebugLogsObfuscateAccessToken() { + val profilerConfiguration = ProfilerConfiguration().apply { + configurationId = "my-config" + configurationOptions = "teamscale-partition=foo\nteamscale-project=p\nteamscale-commit=master:12345" + } + val teamscaleMockServer = TeamscaleMockServer(FAKE_TEAMSCALE_PORT).acceptingReportUploads() + .withProfilerConfiguration(profilerConfiguration) + + val agentJar = System.getenv("AGENT_JAR") + val sampleJar = System.getenv("SYSTEM_UNDER_TEST_JAR") + val agentOptions = "config-id=my-config,teamscale-server-url=http://localhost:$FAKE_TEAMSCALE_PORT," + + "teamscale-user=fake,teamscale-access-token=$SECRET_ACCESS_TOKEN,debug=true" + val result = ProcessUtils.execute("java", "-javaagent:$agentJar=$agentOptions", "-jar", sampleJar) + println(result.stderr) + println(result.stdout) + assertThat(result.exitCode).isEqualTo(0) + + teamscaleMockServer.shutdown() + + val forwardedLogs = teamscaleMockServer.collectedLogMessages.joinToString("\n") { "${it.severity} ${it.message}" } + assertThat(forwardedLogs) + .`as`("Sanity check: the parser-level debug log must be forwarded for this test to be meaningful") + .contains("Parsing options:") + assertThat(forwardedLogs) + .`as`("Forwarded debug logs must not contain the raw access token in clear text") + .doesNotContain(SECRET_ACCESS_TOKEN) + assertThat(forwardedLogs) + .`as`("Forwarded debug logs must show the access token in obfuscated form") + .contains("************" + SECRET_ACCESS_TOKEN.takeLast(4)) + } + companion object { /** These ports must match what is configured for the -javaagent line in this project's build.gradle. */ private const val FAKE_TEAMSCALE_PORT = 64100 + + /** A clearly-marked secret used only by the obfuscation test so any leak in the assertion output is obvious. */ + private const val SECRET_ACCESS_TOKEN = "topSecretToken12345" } } diff --git a/teamscale-client/src/main/kotlin/com/teamscale/client/ProfilerLogEntry.kt b/teamscale-client/src/main/kotlin/com/teamscale/client/ProfilerLogEntry.kt index 2955276a7..f47522d5a 100644 --- a/teamscale-client/src/main/kotlin/com/teamscale/client/ProfilerLogEntry.kt +++ b/teamscale-client/src/main/kotlin/com/teamscale/client/ProfilerLogEntry.kt @@ -1,16 +1,19 @@ package com.teamscale.client +import com.fasterxml.jackson.annotation.JsonCreator +import com.fasterxml.jackson.annotation.JsonProperty + /** A log entry to be sent to Teamscale */ -class ProfilerLogEntry( +class ProfilerLogEntry @JsonCreator constructor( /** The time of the event */ - var timestamp: Long, + @param:JsonProperty("timestamp") var timestamp: Long, /** Log message */ - var message: String, + @param:JsonProperty("message") var message: String, /** Details, for example, the stack trace */ - var details: String?, + @param:JsonProperty("details") var details: String?, /** Event severity */ - var severity: String -) \ No newline at end of file + @param:JsonProperty("severity") var severity: String +)