From 339e24bd87f7f32143084b91a90aff3c63bfe51b Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Wed, 25 Oct 2023 15:35:12 +0200 Subject: [PATCH] Change redacted values in snapshot & log templates in log template use {redacted} instead of the evaluation error message for snapshot indicated the by which part it was redacted: - redactedIdent for keyword redaction - redactedType for type redaction --- .../bootstrap/debugger/util/Redaction.java | 2 +- .../debugger/el/RedactedException.java | 7 +++++ .../el/expressions/ExpressionHelper.java | 4 +-- .../agent/LogMessageTemplateBuilder.java | 6 ++++- .../debugger/util/MoshiSnapshotHelper.java | 13 ++++++--- .../debugger/util/SerializerWithLimits.java | 13 ++++++--- .../debugger/util/StringTokenWriter.java | 10 +++++-- .../debugger/agent/CapturedSnapshotTest.java | 27 +++++++++++++++---- 8 files changed, 65 insertions(+), 17 deletions(-) create mode 100644 dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/RedactedException.java diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/util/Redaction.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/util/Redaction.java index 2d746395f68..41383d3c50a 100644 --- a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/util/Redaction.java +++ b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/util/Redaction.java @@ -13,7 +13,7 @@ public class Redaction { // Need to be a unique instance (new String) for reference equality (==) and // avoid internalization (intern) by the JVM because it's a string constant - public static final String REDACTED_VALUE = new String("REDACTED".toCharArray()); + public static final String REDACTED_VALUE = new String("redacted".toCharArray()); private static final Pattern COMMA_PATTERN = Pattern.compile(","); private static final List PREDEFINED_KEYWORDS = diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/RedactedException.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/RedactedException.java new file mode 100644 index 00000000000..a2ec6cdb02d --- /dev/null +++ b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/RedactedException.java @@ -0,0 +1,7 @@ +package com.datadog.debugger.el; + +public class RedactedException extends EvaluationException { + public RedactedException(String message, String expr) { + super(message, expr); + } +} diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ExpressionHelper.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ExpressionHelper.java index e77f17286dd..43d5930dcb6 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ExpressionHelper.java +++ b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ExpressionHelper.java @@ -1,13 +1,13 @@ package com.datadog.debugger.el.expressions; -import com.datadog.debugger.el.EvaluationException; import com.datadog.debugger.el.Expression; import com.datadog.debugger.el.PrettyPrintVisitor; +import com.datadog.debugger.el.RedactedException; public class ExpressionHelper { public static void throwRedactedException(Expression expr) { String strExpr = PrettyPrintVisitor.print(expr); - throw new EvaluationException( + throw new RedactedException( "Could not evaluate the expression because '" + strExpr + "' was redacted", strExpr); } } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/LogMessageTemplateBuilder.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/LogMessageTemplateBuilder.java index 4554630e8eb..6b314725870 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/LogMessageTemplateBuilder.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/LogMessageTemplateBuilder.java @@ -3,11 +3,13 @@ import static com.datadog.debugger.util.ValueScriptHelper.serializeValue; import com.datadog.debugger.el.EvaluationException; +import com.datadog.debugger.el.RedactedException; import com.datadog.debugger.el.Value; import com.datadog.debugger.el.ValueScript; import com.datadog.debugger.probe.LogProbe; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.EvaluationError; +import datadog.trace.bootstrap.debugger.util.Redaction; import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,7 +51,9 @@ public String evaluate(CapturedContext context, LogProbe.LogStatus status) { LOGGER.debug("Evaluation error: ", ex); status.addError(new EvaluationError(ex.getExpr(), ex.getMessage())); status.setLogTemplateErrors(true); - sb.append("{").append(ex.getMessage()).append("}"); + String msg = + ex instanceof RedactedException ? Redaction.REDACTED_VALUE : ex.getMessage(); + sb.append("{").append(msg).append("}"); } } } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/MoshiSnapshotHelper.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/MoshiSnapshotHelper.java index 7cec03f035c..5f5a9e63ee4 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/MoshiSnapshotHelper.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/MoshiSnapshotHelper.java @@ -44,7 +44,8 @@ public class MoshiSnapshotHelper { public static final String COLLECTION_SIZE_REASON = "collectionSize"; public static final String TIMEOUT_REASON = "timeout"; public static final String DEPTH_REASON = "depth"; - public static final String REDACTED_REASON = "redacted"; + public static final String REDACTED_IDENT_REASON = "redactedIdent"; + public static final String REDACTED_TYPE_REASON = "redactedType"; public static final String TYPE = "type"; public static final String VALUE = "value"; public static final String FIELDS = "fields"; @@ -462,10 +463,16 @@ public void notCaptured(SerializerWithLimits.NotCapturedReason reason) throws Ex jsonWriter.value(TIMEOUT_REASON); break; } - case REDACTED: + case REDACTED_IDENT: { jsonWriter.name(NOT_CAPTURED_REASON); - jsonWriter.value(REDACTED_REASON); + jsonWriter.value(REDACTED_IDENT_REASON); + break; + } + case REDACTED_TYPE: + { + jsonWriter.name(NOT_CAPTURED_REASON); + jsonWriter.value(REDACTED_TYPE_REASON); break; } default: diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/SerializerWithLimits.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/SerializerWithLimits.java index 68a7530a238..5d2da23f805 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/SerializerWithLimits.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/SerializerWithLimits.java @@ -50,7 +50,8 @@ enum NotCapturedReason { MAX_DEPTH, FIELD_COUNT, TIMEOUT, - REDACTED + REDACTED_IDENT, + REDACTED_TYPE } public interface TokenWriter { @@ -120,8 +121,14 @@ public void serialize(Object value, String type, Limits limits) throws Exception throw new IllegalArgumentException("Type is required for serialization"); } tokenWriter.prologue(value, type); - if (value == REDACTED_VALUE || Redaction.isRedactedType(type)) { - tokenWriter.notCaptured(NotCapturedReason.REDACTED); + NotCapturedReason reason = null; + if (value == REDACTED_VALUE) { + reason = NotCapturedReason.REDACTED_IDENT; + } else if (Redaction.isRedactedType(type)) { + reason = NotCapturedReason.REDACTED_TYPE; + } + if (reason != null) { + tokenWriter.notCaptured(reason); tokenWriter.epilogue(value); return; } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/StringTokenWriter.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/StringTokenWriter.java index 838f1c45491..fcd89a87dc1 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/StringTokenWriter.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/StringTokenWriter.java @@ -1,5 +1,8 @@ package com.datadog.debugger.util; +import static com.datadog.debugger.util.MoshiSnapshotHelper.REDACTED_IDENT_REASON; +import static com.datadog.debugger.util.MoshiSnapshotHelper.REDACTED_TYPE_REASON; + import com.datadog.debugger.el.Value; import datadog.trace.bootstrap.debugger.EvaluationError; import java.lang.reflect.Field; @@ -155,8 +158,11 @@ public void notCaptured(SerializerWithLimits.NotCapturedReason reason) { case FIELD_COUNT: sb.append(", ..."); break; - case REDACTED: - sb.append("{redacted}"); + case REDACTED_IDENT: + sb.append("{").append(REDACTED_IDENT_REASON).append("}"); + break; + case REDACTED_TYPE: + sb.append("{").append(REDACTED_TYPE_REASON).append("}"); break; default: throw new RuntimeException("Unsupported NotCapturedReason: " + reason); diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java index 5e9f98277d8..1b8ec443bce 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java @@ -4,7 +4,10 @@ import static com.datadog.debugger.util.MoshiSnapshotHelper.DEPTH_REASON; import static com.datadog.debugger.util.MoshiSnapshotHelper.FIELD_COUNT_REASON; import static com.datadog.debugger.util.MoshiSnapshotHelper.NOT_CAPTURED_REASON; +import static com.datadog.debugger.util.MoshiSnapshotHelper.REDACTED_IDENT_REASON; +import static com.datadog.debugger.util.MoshiSnapshotHelper.REDACTED_TYPE_REASON; import static com.datadog.debugger.util.TestHelper.setFieldInConfig; +import static datadog.trace.bootstrap.debugger.util.Redaction.REDACTED_VALUE; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -1627,17 +1630,23 @@ public void keywordRedaction() throws IOException, URISyntaxException { Assertions.assertEquals(42, result); Snapshot snapshot = assertOneSnapshot(listener); assertEquals( - "arg=secret123 secret={Could not evaluate the expression because 'secret' was redacted} password={Could not evaluate the expression because 'this.password' was redacted} fromMap={Could not evaluate the expression because 'strMap[\"password\"]' was redacted}", + "arg=secret123 secret={" + + REDACTED_VALUE + + "} password={" + + REDACTED_VALUE + + "} fromMap={" + + REDACTED_VALUE + + "}", snapshot.getMessage()); CapturedContext.CapturedValue secretLocalVar = snapshot.getCaptures().getReturn().getLocals().get("secret"); CapturedContext.CapturedValue secretValued = VALUE_ADAPTER.fromJson(secretLocalVar.getStrValue()); - assertEquals("redacted", secretValued.getNotCapturedReason()); + assertEquals(REDACTED_IDENT_REASON, secretValued.getNotCapturedReason()); Map thisFields = getFields(snapshot.getCaptures().getReturn().getArguments().get("this")); CapturedContext.CapturedValue passwordField = thisFields.get("password"); - assertEquals("redacted", passwordField.getNotCapturedReason()); + assertEquals(REDACTED_IDENT_REASON, passwordField.getNotCapturedReason()); Map strMap = (Map) thisFields.get("strMap").getValue(); assertNull(strMap.get("password")); } @@ -1737,12 +1746,20 @@ public void typeRedactionSnapshot() throws IOException, URISyntaxException { Assertions.assertEquals(42, result); Snapshot snapshot = assertOneSnapshot(listener); assertEquals( - "arg=secret123 credentials={Could not evaluate the expression because 'creds' was redacted} user={Could not evaluate the expression because 'this.creds' was redacted} code={Could not evaluate the expression because 'creds' was redacted} dave={Could not evaluate the expression because 'credMap[\"dave\"]' was redacted}", + "arg=secret123 credentials={" + + REDACTED_VALUE + + "} user={" + + REDACTED_VALUE + + "} code={" + + REDACTED_VALUE + + "} dave={" + + REDACTED_VALUE + + "}", snapshot.getMessage()); Map thisFields = getFields(snapshot.getCaptures().getReturn().getArguments().get("this")); CapturedContext.CapturedValue credsField = thisFields.get("creds"); - assertEquals("redacted", credsField.getNotCapturedReason()); + assertEquals(REDACTED_TYPE_REASON, credsField.getNotCapturedReason()); Map credMap = (Map) thisFields.get("credMap").getValue(); assertNull(credMap.get("dave")); }