Merge pull request #890 from cqse/ts/46179_access_key_log

TS-46179 Obfuscate access token in DEBUG and WARN logs

Author: stahlbauer

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.

agent/src/main/java/com/teamscale/jacoco/agent/PreMain.java

@@ -184,7 +184,7 @@ private static Pair<AgentOptions, List<Exception>> 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.");

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();
 	}
 
 	/**

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);
 	}
 

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
 	 */

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<String>()
 	val profilerEvents = mutableListOf<String>()
+
+	/** All log entries that the profiler forwarded to this server via {@code POST /profilers/:profilerId/logs}. */
+	val collectedLogMessages = mutableListOf<ProfilerLogEntry>()
 	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<ProfilerLogEntry>(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 ""
 	}
 

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"
 	}
 }

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
-)
+	@param:JsonProperty("severity") var severity: String
+)